Teach the ELF parser to handle multiple PT_NOTE phdrs.

It is legal for an ELF to contain multiple PT_NOTEs, and that is in
fact what lld's output looks like.

Testing: "make check" and breakpad_unittests when patched into
chromium.

Bug: chromium:716484
Change-Id: I01d3f8679961e2cb7e789d4007de8914c6af357d
Reviewed-on: https://chromium-review.googlesource.com/513512
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Reviewed-by: Ted Mielczarek <ted@mielczarek.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>

src/common/linux/elfutils.cc

@@ -78,14 +78,12 @@ void FindElfClassSection(const char *elf_base,
 template<typename ElfClass>
 void FindElfClassSegment(const char *elf_base,
                          typename ElfClass::Word segment_type,
-                         const void **segment_start,
+                         wasteful_vector<ElfSegment> *segments) {
-                         size_t *segment_size) {
   typedef typename ElfClass::Ehdr Ehdr;
   typedef typename ElfClass::Phdr Phdr;
 
   assert(elf_base);
-  assert(segment_start);
+  assert(segments);
-  assert(segment_size);
 
   assert(my_strncmp(elf_base, ELFMAG, SELFMAG) == 0);
 
@@ -97,9 +95,10 @@ void FindElfClassSegment(const char *elf_base,
 
   for (int i = 0; i < elf_header->e_phnum; ++i) {
     if (phdrs[i].p_type == segment_type) {
-      *segment_start = elf_base + phdrs[i].p_offset;
+      ElfSegment seg = {};
-      *segment_size = phdrs[i].p_filesz;
+      seg.start = elf_base + phdrs[i].p_offset;
-      return;
+      seg.size = phdrs[i].p_filesz;
+      segments->push_back(seg);
     }
   }
 }
@@ -150,16 +149,11 @@ bool FindElfSection(const void *elf_mapped_base,
   return false;
 }
 
-bool FindElfSegment(const void *elf_mapped_base,
+bool FindElfSegments(const void* elf_mapped_base,
                      uint32_t segment_type,
-                    const void **segment_start,
+                     wasteful_vector<ElfSegment>* segments) {
-                    size_t *segment_size) {
   assert(elf_mapped_base);
-  assert(segment_start);
+  assert(segments);
-  assert(segment_size);
-
-  *segment_start = NULL;
-  *segment_size = 0;
 
   if (!IsValidElf(elf_mapped_base))
     return false;
@@ -169,13 +163,11 @@ bool FindElfSegment(const void *elf_mapped_base,
     static_cast<const char*>(elf_mapped_base);
 
   if (cls == ELFCLASS32) {
-    FindElfClassSegment<ElfClass32>(elf_base, segment_type,
+    FindElfClassSegment<ElfClass32>(elf_base, segment_type, segments);
-                                    segment_start, segment_size);
+    return true;
-    return *segment_start != NULL;
   } else if (cls == ELFCLASS64) {
-    FindElfClassSegment<ElfClass64>(elf_base, segment_type,
+    FindElfClassSegment<ElfClass64>(elf_base, segment_type, segments);
-                                    segment_start, segment_size);
+    return true;
-    return *segment_start != NULL;
   }
 
   return false;

src/common/linux/elfutils.h

@@ -37,6 +37,8 @@
 #include <link.h>
 #include <stdint.h>
 
+#include "common/memory.h"
+
 namespace google_breakpad {
 
 // Traits classes so consumers can write templatized code to deal
@@ -99,14 +101,17 @@ FindElfSectionByName(const char* name,
                      const char* names_end,
                      int nsection);
 
-// Attempt to find the first segment of type |segment_type| in the ELF
+struct ElfSegment {
-// binary data at |elf_mapped_base|. On success, returns true and sets
+  const void* start;
-// |*segment_start| to point to the start of the segment data, and
+  size_t size;
-// and |*segment_size| to the size of the segment's data.
+};
-bool FindElfSegment(const void *elf_mapped_base,
+
+// Attempt to find all segments of type |segment_type| in the ELF
+// binary data at |elf_mapped_base|. On success, returns true and fills
+// |*segments| with a list of segments of the given type.
+bool FindElfSegments(const void* elf_mapped_base,
                      uint32_t segment_type,
-                    const void **segment_start,
+                     wasteful_vector<ElfSegment>* segments);
-                    size_t *segment_size);
 
 // Convert an offset from an Elf header into a pointer to the mapped
 // address in the current process. Takes an extra template parameter

src/common/linux/file_id.cc

@@ -95,18 +95,25 @@ static bool ElfClassBuildIDNoteIdentifier(const void *section, size_t length,
 // and copy it into |identifier|.
 static bool FindElfBuildIDNote(const void* elf_mapped_base,
                                wasteful_vector<uint8_t>& identifier) {
-  void* note_section;
+  PageAllocator allocator;
-  size_t note_size;
+  // lld normally creates 2 PT_NOTEs, gold normally creates 1.
-  if ((!FindElfSegment(elf_mapped_base, PT_NOTE,
+  auto_wasteful_vector<ElfSegment, 2> segs(&allocator);
-                       (const void**)&note_section, &note_size) ||
+  if (FindElfSegments(elf_mapped_base, PT_NOTE, &segs)) {
-      note_size == 0)  &&
+    for (ElfSegment& seg : segs) {
-      (!FindElfSection(elf_mapped_base, ".note.gnu.build-id", SHT_NOTE,
+      if (ElfClassBuildIDNoteIdentifier(seg.start, seg.size, identifier)) {
-                       (const void**)&note_section, &note_size) ||
+        return true;
-      note_size == 0)) {
+      }
-    return false;
+    }
   }
 
+  void* note_section;
+  size_t note_size;
+  if (FindElfSection(elf_mapped_base, ".note.gnu.build-id", SHT_NOTE,
+                     (const void**)&note_section, &note_size)) {
     return ElfClassBuildIDNoteIdentifier(note_section, note_size, identifier);
+  }
+
+  return false;
 }
 
 // Attempt to locate the .text section of an ELF binary and generate

src/common/linux/file_id_unittest.cc

@@ -278,6 +278,40 @@ TYPED_TEST(FileIDTest, BuildIDPH) {
   EXPECT_EQ(expected_identifier_string, identifier_string);
 }
 
+TYPED_TEST(FileIDTest, BuildIDMultiplePH) {
+  const uint8_t kExpectedIdentifierBytes[] =
+    {0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
+     0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F,
+     0x10, 0x11, 0x12, 0x13};
+  const string expected_identifier_string =
+      this->get_file_id(kExpectedIdentifierBytes);
+
+  ELF elf(EM_386, TypeParam::kClass, kLittleEndian);
+  Section text(kLittleEndian);
+  text.Append(4096, 0);
+  elf.AddSection(".text", text, SHT_PROGBITS);
+  Notes notes1(kLittleEndian);
+  notes1.AddNote(0, "Linux",
+                reinterpret_cast<const uint8_t *>("\0x42\0x02\0\0"), 4);
+  Notes notes2(kLittleEndian);
+  notes2.AddNote(NT_GNU_BUILD_ID, "GNU", kExpectedIdentifierBytes,
+                 sizeof(kExpectedIdentifierBytes));
+  int note1_idx = elf.AddSection(".note1", notes1, SHT_NOTE);
+  int note2_idx = elf.AddSection(".note2", notes2, SHT_NOTE);
+  elf.AddSegment(note1_idx, note1_idx, PT_NOTE);
+  elf.AddSegment(note2_idx, note2_idx, PT_NOTE);
+  elf.Finish();
+  this->GetElfContents(elf);
+
+  id_vector identifier(this->make_vector());
+  EXPECT_TRUE(FileID::ElfFileIdentifierFromMappedFile(this->elfdata,
+                                                      identifier));
+  EXPECT_EQ(sizeof(kExpectedIdentifierBytes), identifier.size());
+
+  string identifier_string = FileID::ConvertIdentifierToUUIDString(identifier);
+  EXPECT_EQ(expected_identifier_string, identifier_string);
+}
+
 // Test to make sure two files with different text sections produce
 // different hashes when not using a build id.
 TYPED_TEST(FileIDTest, UniqueHashes) {