Fixing a flaky Linux exploitability unittest.

src/processor/exploitability_linux.cc

 // Copyright (c) 2013 Google Inc.
 // All rights reserved.
 //
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 // notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
@@ skipping 213 matching lines @@
     return false;
   }
 
   // Convert bytes into objdump output.
   char objdump_output_buffer[MAX_OBJDUMP_BUFFER_LEN] = {0};
   DisassembleBytes(architecture,
                    raw_memory + offset,
                    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 = "";
   string dest = "";
   string src = "";
   TokenizeObjdumpInstruction(line, &instruction, &dest, &src);
 
   // Check if the operation is a write to memory. First, the instruction
   // must one that can write to memory. Second, the write destination
   // must be a spot in memory rather than a register. Since there are no
@@ skipping 133 matching lines @@
   }
 
   // Add or subtract constant from write address (if applicable).
   *write_address =
       positive_add_constant ?
       *write_address + add_constant : *write_address - add_constant;
 
   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)) {

[Mark Mentovai, 2016/02/17 02:24:18]

std::getline

[ivanpe, 2016/02/17 06:27:41]

I'll handle this in a new CL.

+      BPLOG(INFO) << "Objdump instructions not found";
+      return false;
+    }
+    if (instruction_line->find("<.data>:") != string::npos) {
+      data_section_seen = true;
+    }

[Mark Mentovai, 2016/02/17 02:24:18]

At some point after seeing <.data>:, if you enter another section without having
fallen out of this loop, shouldn’t data_section_seen be set back to false?

[ivanpe, 2016/02/17 06:27:41]

Here, we are parsing the output of objdump.  objdump is dissasembling a buffer. 
The buffer length guarantees that there will be at least one instruction.  So
the output should contain exactly one <.data>: section followed by an
instruction line that contains '0:'.  IMO, there is no need to complicate this
function any further.

+  } 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,
                                                      string *src) {
   if (!operation || !dest || !src) {
     BPLOG(ERROR) << "Null parameters passed.";
     return false;
   }
 
   // Set all pointer values to empty strings.
@@ skipping 183 matching lines @@
     case MD_EXCEPTION_CODE_LIN_DUMP_REQUESTED:
       return true;
       break;
     default:
       return false;
       break;
   }
 }
 
 }  // namespace google_breakpad