Fixing a flaky Linux exploitability unittest.

BUG=https://code.google.com/p/chromium/issues/detail?id=584174
R=mmandlis@chromium.org

Review URL: https://codereview.chromium.org/1697963002 .

3 files changed, 81 insertions, 15 deletions

diff --git a/src/processor/exploitability_linux.cc b/src/processor/exploitability_linux.cc
index a196da79..c520059c 100644
--- a/src/processor/exploitability_linux.cc
+++ b/src/processor/exploitability_linux.cc
@@ -231,21 +231,10 @@ bool ExploitabilityLinux::EndedOnIllegalWrite(uint64_t instruction_ptr) {
                    MAX_OBJDUMP_BUFFER_LEN,
                    objdump_output_buffer);
 
-  // Put buffer data into stream to output line-by-line.
-  std::stringstream objdump_stream;
-  objdump_stream.str(string(objdump_output_buffer));
   string line;
-
-  // Pipe each output line into the string until the string contains
-  // the first instruction from objdump.
-  // Loop until the line shows the first instruction or there are no lines left.
-  do {
-    if (!getline(objdump_stream, line)) {
-      BPLOG(INFO) << "Objdump instructions not found";
-      return false;
-    }
-  } while (line.find("0:") == string::npos);
-  // This first instruction contains the above substring.
+  if (!GetObjdumpInstructionLine(objdump_output_buffer, &line)) {
+    return false;
+  }
 
   // Convert objdump instruction line into the operation and operands.
   string instruction = "";
@@ -399,6 +388,33 @@ bool ExploitabilityLinux::CalculateAddress(const string &address_expression,
   return true;
 }
 
+// static
+bool ExploitabilityLinux::GetObjdumpInstructionLine(
+    const char *objdump_output_buffer,
+    string *instruction_line) {
+  // Put buffer data into stream to output line-by-line.
+  std::stringstream objdump_stream;
+  objdump_stream.str(string(objdump_output_buffer));
+
+  // Pipe each output line into the string until the string contains the first
+  // instruction from objdump.  All lines before the "<.data>:" section are
+  // skipped.  Loop until the line shows the first instruction or there are no
+  // lines left.
+  bool data_section_seen = false;
+  do {
+    if (!getline(objdump_stream, *instruction_line)) {
+      BPLOG(INFO) << "Objdump instructions not found";
+      return false;
+    }
+    if (instruction_line->find("<.data>:") != string::npos) {
+      data_section_seen = true;
+    }
+  } while (!data_section_seen || instruction_line->find("0:") == string::npos);
+  // This first instruction contains the above substring.
+
+  return true;
+}
+
 bool ExploitabilityLinux::TokenizeObjdumpInstruction(const string &line,
                                                      string *operation,
                                                      string *dest,
diff --git a/src/processor/exploitability_linux.h b/src/processor/exploitability_linux.h
index 93c5082f..e3ff13b6 100644
--- a/src/processor/exploitability_linux.h
+++ b/src/processor/exploitability_linux.h
@@ -86,6 +86,13 @@ class ExploitabilityLinux : public Exploitability {
                                const unsigned int MAX_OBJDUMP_BUFFER_LEN,
                                char *objdump_output_buffer);
 
+  // Parses the objdump output given in |objdump_output_buffer| and extracts
+  // the line of the first instruction into |instruction_line|.  Returns true
+  // when the instruction line is successfully extracted.
+  static bool GetObjdumpInstructionLine(
+      const char *objdump_output_buffer,
+      string *instruction_line);
+
   // Tokenizes out the operation and operands from a line of instruction
   // disassembled by objdump. This method modifies the pointers to match the
   // tokens of the instruction, and returns if the tokenizing was a success.
diff --git a/src/processor/exploitability_unittest.cc b/src/processor/exploitability_unittest.cc
index ec845bf0..502edcc7 100644
--- a/src/processor/exploitability_unittest.cc
+++ b/src/processor/exploitability_unittest.cc
@@ -47,9 +47,10 @@ namespace google_breakpad {
 
 class ExploitabilityLinuxTest : public ExploitabilityLinux {
  public:
+  using ExploitabilityLinux::CalculateAddress;
   using ExploitabilityLinux::DisassembleBytes;
+  using ExploitabilityLinux::GetObjdumpInstructionLine;
   using ExploitabilityLinux::TokenizeObjdumpInstruction;
-  using ExploitabilityLinux::CalculateAddress;
 };
 
 class ExploitabilityLinuxTestMinidumpContext : public MinidumpContext {
@@ -200,6 +201,48 @@ TEST(ExploitabilityLinuxUtilsTest, DisassembleBytesTest) {
   ASSERT_EQ(line, "   0:\tc7 00 05 00 00 00    \tmov    DWORD PTR [rax],0x5");
 }
 
+TEST(ExploitabilityLinuxUtilsTest, GetObjdumpInstructionLine) {
+  string disassebly =
+      "\n"
+      "/tmp/breakpad_mem_region-raw_bytes-tMmMo0:     file format binary\n"
+      "// Trying to confuse the parser 0:\n"
+      "\n"
+      "Disassembly of section .data:\n"
+      "\n"
+      "0000000000000000 <.data>:\n"
+      "   0:\tc7 00 01 00 00 00    \tmov    DWORD PTR [rax],0x1\n"
+      "   6:\t5d                   \tpop    rbp\n"
+      "   7:\tc3                   \tret    \n"
+      "   8:\t55                   \tpush   rbp\n"
+      "   9:\t48 89 e5             \tmov    rbp,rsp\n"
+      "   c:\t53                   \tpush   rbx\n"
+      "   d:\t48                   \trex.W\n"
+      "   e:\t81                   \t.byte 0x81\n";
+  string line;
+  EXPECT_TRUE(ExploitabilityLinuxTest::GetObjdumpInstructionLine(
+      disassebly.c_str(), &line));
+  EXPECT_EQ("   0:\tc7 00 01 00 00 00    \tmov    DWORD PTR [rax],0x1", line);
+
+  // There is no "0:" after "<.data>:".  Expected to return false.
+  disassebly =
+      "\n"
+      "/tmp/breakpad_mem_region-raw_bytes-tMmMo0:     file format binary\n"
+      "// Trying to confuse the parser 0:\n"
+      "\n"
+      "Disassembly of section .data:\n"
+      "\n"
+      "   0:\tc7 00 01 00 00 00    \tmov    DWORD PTR [rax],0x1\n"
+      "   6:\t5d                   \tpop    rbp\n"
+      "   7:\tc3                   \tret    \n"
+      "   8:\t55                   \tpush   rbp\n"
+      "   9:\t48 89 e5             \tmov    rbp,rsp\n"
+      "   d:\t48                   \trex.W\n"
+      "0000000000000000 <.data>:\n"
+      "   c:\t53                   \tpush   rbx\n";
+  EXPECT_FALSE(ExploitabilityLinuxTest::GetObjdumpInstructionLine(
+      disassebly.c_str(), &line));
+}
+
 TEST(ExploitabilityLinuxUtilsTest, TokenizeObjdumpInstructionTest) {
   ASSERT_FALSE(ExploitabilityLinuxTest::TokenizeObjdumpInstruction("",
                                                                    NULL,