[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-Original-Commit-Position: refs/heads/master@{#458650}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: c615c911eea856986f8daaab73b7f30860234009
diff --git a/disassembler_elf_32.cc b/disassembler_elf_32.cc
index e305b4c..e35a3af 100644
--- a/disassembler_elf_32.cc
+++ b/disassembler_elf_32.cc
@@ -181,13 +181,6 @@
return false;
}
- // Finally sort rel32 locations.
- std::sort(rel32_locations_.begin(),
- rel32_locations_.end(),
- TypedRVA::IsLessThanByRVA);
- DCHECK(rel32_locations_.empty() ||
- rel32_locations_.back()->rva() != kUnassignedRVA);
-
program->DefaultAssignIndexes();
return true;
}
@@ -384,6 +377,11 @@
if (!found_rel32)
VLOG(1) << "Warning: Found no rel32 addresses. Missing .text section?";
+ std::sort(rel32_locations_.begin(), rel32_locations_.end(),
+ TypedRVA::IsLessThanByRVA);
+ DCHECK(rel32_locations_.empty() ||
+ rel32_locations_.back()->rva() != kUnassignedRVA);
+
return true;
}
@@ -418,15 +416,18 @@
// Walk all the bytes in the file, whether or not in a section.
FileOffset file_offset = 0;
- std::vector<FileOffset> abs_offsets;
-
// File parsing follows file offset order, and we visit abs32 and rel32
// locations in lockstep. Therefore we need to extract and sort file offsets
- // of all abs32 and rel32 locations.
+ // of all abs32 and rel32 locations. For abs32, we copy the offsets to a new
+ // array.
+ std::vector<FileOffset> abs_offsets;
if (!RVAsToFileOffsets(abs32_locations_, &abs_offsets))
return false;
- std::sort(abs32_locations_.begin(), abs32_locations_.end());
+ std::sort(abs_offsets.begin(), abs_offsets.end());
+ // For rel32, TypedRVA (rather than raw offset) is stored, so sort-by-offset
+ // is performed in place to save memory. At the end of function we will
+ // sort-by-RVA.
if (!RVAsToFileOffsets(&rel32_locations_))
return false;
std::sort(rel32_locations_.begin(),
@@ -496,6 +497,10 @@
if (!ParseSimpleRegion(file_offset, length(), receptor))
return false;
+ // Restore original rel32 location order and sort by RVA order.
+ std::sort(rel32_locations_.begin(), rel32_locations_.end(),
+ TypedRVA::IsLessThanByRVA);
+
// Make certain we consume all of the relocations as expected
return (current_abs_offset == end_abs_offset);
}