[Courgette] Fix ELF reference sorting.

[Courgette] Fix ELF reference sorting.

This CL addresses 2 reference sorting issues in DisassemblerElf32:

(1) Bug fix: In ParseFile(), |abs32_locations_| (RVAs) is translated to
    |abs_offsets| (file offsets), but we sort |abs32_locations_|, which
    is redundant. Actually we should sort |abs_offsets|.
(2) Cleanup: |rel32_relocations_| stores rel32 references sorted by
    RVA, but in ParseFile() we re-sort these in offset order. Previously
    Disassemble() optimizes away redundant sorts, but this makes the
    code less robust. We de-optimize this a little potentially redundant
    sort-by-RVA, to assert that |rel32_locations_| is sorted by RVA
    outside of ParseFile().

This CL also makes Disassemble() more uniform, to prepare for
refactoring in a follow-up. Meanwhile, DisassemblerWin32 does not
experience issue since it assumes RVA order is same as file offset
order (this assumption has not has not caused problems so far).

BUG=660980
Review-Url: https://codereview.chromium.org/2744373004
Cr-Commit-Position: refs/heads/master@{#458650}
Committed: https://chromium.googlesource.com/chromium/src/+/c615c911eea856986f8daaab73b7f30860234009

[huangs, 2017-03-14 21:38:14]

Description was changed from

==========
[Courgette] Fix ELF reference sorting.

This CL addresses 2 reference sorting issues in DisassemblerElf32:

(1) Bug fix: In ParseFile(), |abs32_locations_| (RVAs) is translated to
    |abs_offsets| (file offsets), but we sort |abs32_locations_|, which
    is redundant. Actually we should sort |abs_offsets|.
(2) Cleanup: |rel32_relocations_| stores rel32 references sorted by
    RVA, but in ParseFile() we re-sort these in offset order. Previously
    Disassemble() optimizes away redundant sorts, but this makes the
    code less robust. We de-optimize a little this by performing more
    sort-by-RVA, to assert that |rel32_locations_| is sorted by
    RVA outside of ParseFile().

Meanwhile, DisassemblerWin32 does not experience issue since it assumes
RVA order is same as file offset order -- so far this has not caused
problem.
==========

to

==========
[Courgette] Fix ELF reference sorting.

This CL addresses 2 reference sorting issues in DisassemblerElf32:

(1) Bug fix: In ParseFile(), |abs32_locations_| (RVAs) is translated to
    |abs_offsets| (file offsets), but we sort |abs32_locations_|, which
    is redundant. Actually we should sort |abs_offsets|.
(2) Cleanup: |rel32_relocations_| stores rel32 references sorted by
    RVA, but in ParseFile() we re-sort these in offset order. Previously
    Disassemble() optimizes away redundant sorts, but this makes the
    code less robust. We de-optimize a little this by performing more
    sort-by-RVA, to assert that |rel32_locations_| is sorted by
    RVA outside of ParseFile().

Meanwhile, DisassemblerWin32 does not experience issue since it assumes
RVA order is same as file offset order -- so far this has not caused
problem.

BUG=660980
==========

[huangs, 2017-03-14 21:39:46]

Description was changed from

==========
[Courgette] Fix ELF reference sorting.

This CL addresses 2 reference sorting issues in DisassemblerElf32:

(1) Bug fix: In ParseFile(), |abs32_locations_| (RVAs) is translated to
    |abs_offsets| (file offsets), but we sort |abs32_locations_|, which
    is redundant. Actually we should sort |abs_offsets|.
(2) Cleanup: |rel32_relocations_| stores rel32 references sorted by
    RVA, but in ParseFile() we re-sort these in offset order. Previously
    Disassemble() optimizes away redundant sorts, but this makes the
    code less robust. We de-optimize a little this by performing more
    sort-by-RVA, to assert that |rel32_locations_| is sorted by
    RVA outside of ParseFile().

Meanwhile, DisassemblerWin32 does not experience issue since it assumes
RVA order is same as file offset order -- so far this has not caused
problem.

BUG=660980
==========

to

==========
[Courgette] Fix ELF reference sorting.

This CL addresses 2 reference sorting issues in DisassemblerElf32:

(1) Bug fix: In ParseFile(), |abs32_locations_| (RVAs) is translated to
    |abs_offsets| (file offsets), but we sort |abs32_locations_|, which
    is redundant. Actually we should sort |abs_offsets|.
(2) Cleanup: |rel32_relocations_| stores rel32 references sorted by
    RVA, but in ParseFile() we re-sort these in offset order. Previously
    Disassemble() optimizes away redundant sorts, but this makes the
    code less robust. We de-optimize a little this by performing more
    sort-by-RVA, to assert that |rel32_locations_| is sorted by
    RVA outside of ParseFile().

This CL also makes ParseFile() more uniform, to set up refactoring in a
follow-up. Meanwhile, DisassemblerWin32 does not experience issue since
it assumes RVA order is same as file offset order -- so far this has
not caused problem.

BUG=660980
==========

[huangs, 2017-03-14 21:42:41]

Description was changed from

==========
[Courgette] Fix ELF reference sorting.

This CL addresses 2 reference sorting issues in DisassemblerElf32:

(1) Bug fix: In ParseFile(), |abs32_locations_| (RVAs) is translated to
    |abs_offsets| (file offsets), but we sort |abs32_locations_|, which
    is redundant. Actually we should sort |abs_offsets|.
(2) Cleanup: |rel32_relocations_| stores rel32 references sorted by
    RVA, but in ParseFile() we re-sort these in offset order. Previously
    Disassemble() optimizes away redundant sorts, but this makes the
    code less robust. We de-optimize a little this by performing more
    sort-by-RVA, to assert that |rel32_locations_| is sorted by
    RVA outside of ParseFile().

This CL also makes ParseFile() more uniform, to set up refactoring in a
follow-up. Meanwhile, DisassemblerWin32 does not experience issue since
it assumes RVA order is same as file offset order -- so far this has
not caused problem.

BUG=660980
==========

to

==========
[Courgette] Fix ELF reference sorting.

This CL addresses 2 reference sorting issues in DisassemblerElf32:

(1) Bug fix: In ParseFile(), |abs32_locations_| (RVAs) is translated to
    |abs_offsets| (file offsets), but we sort |abs32_locations_|, which
    is redundant. Actually we should sort |abs_offsets|.
(2) Cleanup: |rel32_relocations_| stores rel32 references sorted by
    RVA, but in ParseFile() we re-sort these in offset order. Previously
    Disassemble() optimizes away redundant sorts, but this makes the
    code less robust. We de-optimize a little this by performing more
    sort-by-RVA, to assert that |rel32_locations_| is sorted by
    RVA outside of ParseFile().

This CL also makes Disassemble() more uniform, to prepare for
refactoring in a follow-up. Meanwhile, DisassemblerWin32 does not
experience issue since it assumes RVA order is same as file offset
order. So far this has not caused problems.

BUG=660980
==========

[huangs, 2017-03-14 21:42:41]

huangs@chromium.org changed reviewers:
+ chrisha@chromium.org, wfh@chromium.org

[huangs, 2017-03-14 21:43:07]

Bug fix before more refactoring CLs. PTAL. Thanks!

[chrisha, 2017-03-20 18:31:20]

lgtm

[huangs, 2017-03-20 18:56:23]

Description was changed from

==========
[Courgette] Fix ELF reference sorting.

This CL addresses 2 reference sorting issues in DisassemblerElf32:

(1) Bug fix: In ParseFile(), |abs32_locations_| (RVAs) is translated to
    |abs_offsets| (file offsets), but we sort |abs32_locations_|, which
    is redundant. Actually we should sort |abs_offsets|.
(2) Cleanup: |rel32_relocations_| stores rel32 references sorted by
    RVA, but in ParseFile() we re-sort these in offset order. Previously
    Disassemble() optimizes away redundant sorts, but this makes the
    code less robust. We de-optimize a little this by performing more
    sort-by-RVA, to assert that |rel32_locations_| is sorted by
    RVA outside of ParseFile().

This CL also makes Disassemble() more uniform, to prepare for
refactoring in a follow-up. Meanwhile, DisassemblerWin32 does not
experience issue since it assumes RVA order is same as file offset
order. So far this has not caused problems.

BUG=660980
==========

to

==========
[Courgette] Fix ELF reference sorting.

This CL addresses 2 reference sorting issues in DisassemblerElf32:

(1) Bug fix: In ParseFile(), |abs32_locations_| (RVAs) is translated to
    |abs_offsets| (file offsets), but we sort |abs32_locations_|, which
    is redundant. Actually we should sort |abs_offsets|.
(2) Cleanup: |rel32_relocations_| stores rel32 references sorted by
    RVA, but in ParseFile() we re-sort these in offset order. Previously
    Disassemble() optimizes away redundant sorts, but this makes the
    code less robust. We de-optimize this a little potentially redundant
    sort-by-RVA, to assert that |rel32_locations_| is sorted by RVA
    outside of ParseFile().

This CL also makes Disassemble() more uniform, to prepare for
refactoring in a follow-up. Meanwhile, DisassemblerWin32 does not
experience issue since it assumes RVA order is same as file offset
order (this assumption has not has not caused problems so far).

BUG=660980
==========

[huangs, 2017-03-21 23:19:32]

Ping wfh@, PTAL. Thanks@

[Will Harris, 2017-03-21 23:29:31]

I looked at this after chris did, and certainly lgtm but did not comment since
a) you are an owner of this code and are therefore trusted to make changes
autonomously b) you didn't ask specifically which parts you wanted me to look at
and which parts you wanted chrisha to look at and c) chrisha had already lgtmed.

[Will Harris, 2017-03-21 23:30:00]

when adding two or more reviewers please specify the expectations of each
reviewer. thanks.

[huangs, 2017-03-21 23:31:57]

Ah okay. Thanks!

[huangs, 2017-03-22 01:53:11]

The CQ bit was checked by huangs@chromium.org

[huangs, 2017-03-22 01:53:11]

The patchset sent to the CQ was uploaded after l-g-t-m from
chrisha@chromium.org, wfh@chromium.org
Link to the patchset: https://codereview.chromium.org/2744373004/#ps20001
(title: "Sync.")

[commit-bot: I haz the power, 2017-03-22 01:53:42]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-22 03:55:24]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-22 03:55:25]

Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_TIMED_OUT, build hasn't
started yet, builder probably lacks capacity)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_TIMED_OUT,
build hasn't started yet, builder probably lacks capacity)

[huangs, 2017-03-22 04:41:17]

The CQ bit was checked by huangs@chromium.org

[commit-bot: I haz the power, 2017-03-22 04:41:53]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-22 06:52:29]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1490157677335460,
"parent_rev": "ba7b76c799d7ed719aba76ced546935ddf3da9fd", "commit_rev":
"c615c911eea856986f8daaab73b7f30860234009"}

[commit-bot: I haz the power, 2017-03-22 06:53:34]

Description was changed from

==========
[Courgette] Fix ELF reference sorting.

This CL addresses 2 reference sorting issues in DisassemblerElf32:

(1) Bug fix: In ParseFile(), |abs32_locations_| (RVAs) is translated to
    |abs_offsets| (file offsets), but we sort |abs32_locations_|, which
    is redundant. Actually we should sort |abs_offsets|.
(2) Cleanup: |rel32_relocations_| stores rel32 references sorted by
    RVA, but in ParseFile() we re-sort these in offset order. Previously
    Disassemble() optimizes away redundant sorts, but this makes the
    code less robust. We de-optimize this a little potentially redundant
    sort-by-RVA, to assert that |rel32_locations_| is sorted by RVA
    outside of ParseFile().

This CL also makes Disassemble() more uniform, to prepare for
refactoring in a follow-up. Meanwhile, DisassemblerWin32 does not
experience issue since it assumes RVA order is same as file offset
order (this assumption has not has not caused problems so far).

BUG=660980
==========

to

==========
[Courgette] Fix ELF reference sorting.

This CL addresses 2 reference sorting issues in DisassemblerElf32:

(1) Bug fix: In ParseFile(), |abs32_locations_| (RVAs) is translated to
    |abs_offsets| (file offsets), but we sort |abs32_locations_|, which
    is redundant. Actually we should sort |abs_offsets|.
(2) Cleanup: |rel32_relocations_| stores rel32 references sorted by
    RVA, but in ParseFile() we re-sort these in offset order. Previously
    Disassemble() optimizes away redundant sorts, but this makes the
    code less robust. We de-optimize this a little potentially redundant
    sort-by-RVA, to assert that |rel32_locations_| is sorted by RVA
    outside of ParseFile().

This CL also makes Disassemble() more uniform, to prepare for
refactoring in a follow-up. Meanwhile, DisassemblerWin32 does not
experience issue since it assumes RVA order is same as file offset
order (this assumption has not has not caused problems so far).

BUG=660980

Review-Url: https://codereview.chromium.org/2744373004
Cr-Commit-Position: refs/heads/master@{#458650}
Committed:
https://chromium.googlesource.com/chromium/src/+/c615c911eea856986f8daaab73b7...
==========

[commit-bot: I haz the power, 2017-03-22 06:53:35]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/c615c911eea856986f8daaab73b7...