Fixing a flaky Linux exploitability unittest.

Fixing a flaky Linux exploitability unittest.

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

Committed: https://chromium.googlesource.com/breakpad/breakpad/+/12bd7fc52b66b9f89d38d861a5aed194df03390a

[ivanpe, 2016-02-15 02:59:30]

Description was changed from

==========
Fixing a flaky Linux exploitability unittest.

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

to

==========
Fixing a flaky Linux exploitability unittest.

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

[ivanpe, 2016-02-15 02:59:30]

ivanpe@chromium.org changed reviewers:
+ mark@chromium.org, mmandlis@chromium.org

[mmandlis, 2016-02-16 19:21:16]

lgtm

Please, address comments before submitting. Thanks!

https://codereview.chromium.org/1697963002/diff/1/src/processor/exploitabilit...
File src/processor/exploitability_linux.h (right):

https://codereview.chromium.org/1697963002/diff/1/src/processor/exploitabilit...
src/processor/exploitability_linux.h:90: // the line of the first instruction
into |instruction_line|.
Please, also comment on the return value.

https://codereview.chromium.org/1697963002/diff/1/src/processor/exploitabilit...
File src/processor/exploitability_unittest.cc (right):

https://codereview.chromium.org/1697963002/diff/1/src/processor/exploitabilit...
src/processor/exploitability_unittest.cc:213: "   0:\tc7 00 01 00 00 00    \tmov
   DWORD PTR [rax],0x1\n"
IIUC, we are after the line starting with 0: that follows the <.data> line.
Maybe it's worth putting another 0: line before the <.data> line, and make sure
the right one is returned. And maybe even an additional case for 0: preceding
the <.data>, so that false is returned.

[ivanpe, 2016-02-16 19:41:43]

https://codereview.chromium.org/1697963002/diff/1/src/processor/exploitabilit...
File src/processor/exploitability_linux.h (right):

https://codereview.chromium.org/1697963002/diff/1/src/processor/exploitabilit...
src/processor/exploitability_linux.h:90: // the line of the first instruction
into |instruction_line|.
On 2016/02/16 19:21:16, mmandlis wrote:
> Please, also comment on the return value.

Done.

https://codereview.chromium.org/1697963002/diff/1/src/processor/exploitabilit...
File src/processor/exploitability_unittest.cc (right):

https://codereview.chromium.org/1697963002/diff/1/src/processor/exploitabilit...
src/processor/exploitability_unittest.cc:213: "   0:\tc7 00 01 00 00 00    \tmov
   DWORD PTR [rax],0x1\n"
On 2016/02/16 19:21:16, mmandlis wrote:
> IIUC, we are after the line starting with 0: that follows the <.data> line.
> Maybe it's worth putting another 0: line before the <.data> line, and make
sure
> the right one is returned. And maybe even an additional case for 0: preceding
> the <.data>, so that false is returned.

Done.

[mmandlis, 2016-02-16 19:45:00]

lgtm

[ivanpe, 2016-02-16 19:46:06]

Description was changed from

==========
Fixing a flaky Linux exploitability unittest.

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

to

==========
Fixing a flaky Linux exploitability unittest.

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

Committed:
https://chromium.googlesource.com/breakpad/breakpad/+/12bd7fc52b66b9f89d38d86...
==========

[ivanpe, 2016-02-16 19:46:07]

Committed patchset #3 (id:40001) manually as
12bd7fc52b66b9f89d38d861a5aed194df03390a (presubmit successful).

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

https://codereview.chromium.org/1697963002/diff/40001/src/processor/exploitab...
File src/processor/exploitability_linux.cc (right):

https://codereview.chromium.org/1697963002/diff/40001/src/processor/exploitab...
src/processor/exploitability_linux.cc:405: if (!getline(objdump_stream,
*instruction_line)) {
std::getline

https://codereview.chromium.org/1697963002/diff/40001/src/processor/exploitab...
src/processor/exploitability_linux.cc:411: }
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]

https://codereview.chromium.org/1697963002/diff/40001/src/processor/exploitab...
File src/processor/exploitability_linux.cc (right):

https://codereview.chromium.org/1697963002/diff/40001/src/processor/exploitab...
src/processor/exploitability_linux.cc:405: if (!getline(objdump_stream,
*instruction_line)) {
On 2016/02/17 02:24:18, Mark Mentovai wrote:
> std::getline

I'll handle this in a new CL.

https://codereview.chromium.org/1697963002/diff/40001/src/processor/exploitab...
src/processor/exploitability_linux.cc:411: }
On 2016/02/17 02:24:18, Mark Mentovai wrote:
> 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?

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.