[Courgette] ELF: Fix abs32 / rel32 ordering in ParseFile() and restrict rel32 parsing to .text.

courgette/disassembler_elf_32.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "courgette/disassembler_elf_32.h"
 
 #include <algorithm>
 
 #include "base/logging.h"
 #include "courgette/assembly_program.h"
@@ skipping 151 matching lines @@
 
   if (!ParseAbs32Relocs())
     return false;
 
   if (!ParseRel32RelocsFromSections())
     return false;
 
   if (!ParseFile(target))
     return false;
 
+  std::sort(rel32_locations_.begin(),
+            rel32_locations_.end(),
+            TypedRVA::IsLessThanByRVA);
+  DCHECK(rel32_locations_.empty() ||
+         rel32_locations_.back()->rva() != kUnassignedRVA);
+
   target->DefaultAssignIndexes();
   return true;
 }
 
 bool DisassemblerElf32::UpdateLength() {
   Elf32_Off result = 0;
 
   // Find the end of the last section
   for (Elf32_Half section_id = 0; section_id < SectionHeaderCount();
        ++section_id) {
@@ skipping 94 matching lines @@
   }
   return true;
 }
 
 CheckBool DisassemblerElf32::ParseFile(AssemblyProgram* program) {
   // 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.
   if (!RVAsToFileOffsets(abs32_locations_, &abs_offsets))
     return false;
+  std::sort(abs32_locations_.begin(), abs32_locations_.end());
 
   if (!RVAsToFileOffsets(&rel32_locations_))
     return false;
+  std::sort(rel32_locations_.begin(),
+            rel32_locations_.end(),
+            TypedRVA::IsLessThanByFileOffset);
 
   std::vector<FileOffset>::iterator current_abs_offset = abs_offsets.begin();
+  std::vector<FileOffset>::iterator end_abs_offset = abs_offsets.end();
+
   ScopedVector<TypedRVA>::iterator current_rel = rel32_locations_.begin();
-
-  std::vector<FileOffset>::iterator end_abs_offset = abs_offsets.end();
   ScopedVector<TypedRVA>::iterator end_rel = rel32_locations_.end();
 
   // Visit section headers ordered by file offset.
   for (Elf32_Half section_id : section_header_file_offset_order_) {
     const Elf32_Shdr* section_header = SectionHeader(section_id);
 
     if (section_header->sh_type == SHT_NOBITS)
       continue;
 
     if (!ParseSimpleRegion(file_offset, section_header->sh_offset, program))
@@ skipping 199 matching lines @@
           return true;
       }
     }
   }
 
   return false;
 }
 
 CheckBool DisassemblerElf32::ParseRel32RelocsFromSections() {
   rel32_locations_.clear();
+  bool found_rel32 = false;
 
   // Loop through sections for relocation sections
   for (Elf32_Half section_id = 0; section_id < SectionHeaderCount();
        ++section_id) {
     const Elf32_Shdr* section_header = SectionHeader(section_id);
 
-    // TODO(huangs): Add better checks to skip non-code sections.
     // Some debug sections can have sh_type=SHT_PROGBITS but sh_addr=0.
     if (section_header->sh_type != SHT_PROGBITS ||
         section_header->sh_addr == 0)
       continue;
 
+    // Heuristic: Only consider ".text" section.
+    std::string section_name;
+    if (!SectionName(*section_header, &section_name))
+      return false;
+    if (section_name != ".text")
+      continue;
+
+    found_rel32 = true;
     if (!ParseRel32RelocsFromSection(section_header))
       return false;
   }
+  if (!found_rel32)
+    VLOG(1) << "Warning: Found no rel32 addresses. Missing .text section?";

[Will Harris, 2016/04/28 19:45:49]

what does this warning mean if it's seen? is there any action that a user of
courgette can take?

[huangs, 2016/04/28 19:53:45]

The warning says "your ELF file is weird because it's missing .text section". 
This should be a rare occurrence, but it's worth noting since otherwise
Courgette will silently produce suboptimal patches.  

On seeing this message, the user can reflect on why they going against
convention.  If there's a good reason, then we should revisit the heuristic.

[Will Harris, 2016/04/28 20:04:02]

Acknowledged.

 
-  std::sort(rel32_locations_.begin(),
-            rel32_locations_.end(),
-            TypedRVA::IsLessThan);
-  DCHECK(rel32_locations_.empty() ||
-         rel32_locations_.back()->rva() != kUnassignedRVA);
   return true;
 }
 
 }  // namespace courgette