Read Chrome build ID and store it in leak reports

components/metrics/leak_detector/gnu_build_id_reader.cc

+// Copyright 2016 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 "components/metrics/leak_detector/gnu_build_id_reader.h"
+
+#include <libelf.h>

[rickyz (no longer on Chrome), 2016/07/19 04:58:22]

Is just <elf.h> sufficient here?

[Simon Que, 2016/07/22 01:40:06]

Done.

+#include <string.h>

[Will Harris, 2016/07/22 01:58:02]

why is string needed?

[Simon Que, 2016/07/22 02:49:22]

For memcmp(). But I wonder if that could be replaced with a string comparison.

+
+#if defined(OS_CHROMEOS)
+#include <link.h>  // for dl_iterate_phdr
+#else
+#error "Getting binary mapping info is not supported on this platform."

[rickyz (no longer on Chrome), 2016/07/19 04:58:22]

Any reason to limit this to Chrome OS instead of Linux?

[Simon Que, 2016/07/22 01:40:06]

Using OS_POSIX or OS_LINUX cause the #else condition to be evaluated.

[rickyz (no longer on Chrome), 2016/07/22 02:47:31]

Yeah, what I meant was - if this works on OS_POSIX or OS_LINUX, is there any
reason that we we restrict this code to OS_CHROMEOS only?

Not a big deal either way, was just thinking it couldn't hurt to have the
function available on non-Chrome OS in case somebody might want to use it in the
future.

+#endif  // defined(OS_CHROMEOS)
+
+namespace metrics {
+namespace leak_detector {
+namespace gnu_build_id_reader {
+
+namespace {
+
+// Contains data passed to dl_iterate_phdr() for reading build ID.
+struct ReadBuildIDData {
+  // Points to a vector for storing the build ID.
+  std::vector<uint8_t>* build_id;
+  // Indicates whether build ID was read successfully.
+  bool success;
+};
+
+// Given a pointer and an offset, add the offset to the pointer and round it up
+// to the next uint32_t.
+const void* AlignPtrAndOffsetToUint32(const void* ptr, intptr_t offset) {
+  uintptr_t addr = reinterpret_cast<uintptr_t>(ptr) + offset;
+  uintptr_t aligned_addr =
+      (addr + sizeof(uint32_t) - 1) & ~(sizeof(uint32_t) - 1);
+  return reinterpret_cast<const void*>(aligned_addr);
+}
+
+// Searches for the ELF note containing the build ID within the data range
+// specified by [start, end). Returns the build ID in |*result|. If the build ID
+// is not found, |*result| will be unchanged. Returns true if the build ID was
+// successfully read or false otherwise.
+bool GetBuildIdFromNotes(const void* start,
+                         const void* end,
+                         std::vector<uint8_t>* result) {
+  using NoteHeaderPtr = const Elf32_Nhdr*;

[rickyz (no longer on Chrome), 2016/07/19 04:58:22]

This is the same for 64 vs. 32, but might as well use ElfW(Nhdr)*

[Simon Que, 2016/07/22 01:40:06]

Done.

+  NoteHeaderPtr note = reinterpret_cast<NoteHeaderPtr>(start);
+
+  while (note < end) {
+    NoteHeaderPtr name_ptr = &note[1];

[rickyz (no longer on Chrome), 2016/07/19 04:58:22]

Wouldn't use NoteHeaderPtr as the type here, since this isn't really pointing to
a header.

[Simon Que, 2016/07/22 01:40:06]

Done.

+    if (name_ptr > end) {
+      break;
+    }
+    // |desc_ptr| points the to the actual build ID data.
+    const uint8_t* desc_ptr = reinterpret_cast<const uint8_t*>(
+        AlignPtrAndOffsetToUint32(name_ptr, note->n_namesz));
+    if (note->n_type == NT_GNU_BUILD_ID &&
+        note->n_namesz == sizeof(ELF_NOTE_GNU) &&
+        memcmp((const char*)name_ptr, ELF_NOTE_GNU, sizeof(ELF_NOTE_GNU)) ==
+            0) {
+      result->assign(desc_ptr, desc_ptr + note->n_descsz);
+      return true;
+    }
+    NoteHeaderPtr next_ptr = reinterpret_cast<NoteHeaderPtr>(
+        AlignPtrAndOffsetToUint32(desc_ptr, note->n_descsz));
+    note = next_ptr;
+  }
+  return false;
+}
+
+// Callback for dl_iterate_phdr(). Finds the notes section and looks for the
+// build ID in there. |data| points to a ReadBuildIDData struct whose |build_id|
+// field should point to a valid std::vector<uint8_t>. Returns the build ID in
+// that field, and sets |ReadBuildIDData::success| based on whether the build ID
+// was successfully read.
+int FindNotesAndGetBuildID(struct dl_phdr_info* info,
+                           size_t /* size */,
+                           void* data) {
+  uintptr_t mapping_addr = reinterpret_cast<uintptr_t>(info->dlpi_addr);
+  const ElfW(Ehdr)* file_header =
+      reinterpret_cast<const ElfW(Ehdr)*>(mapping_addr);
+
+  // Make sure that a valid |mapping_addr| was read.
+  if (!file_header || file_header->e_phentsize != sizeof(ElfW(Phdr))) {
+    return 1;
+  }
+
+  for (int i = 0; i < info->dlpi_phnum; i++) {
+    // Find the ELF segment header for the NOTES section.
+    for (int i = 0; i < info->dlpi_phnum; ++i) {

[Will Harris, 2016/07/19 02:51:38]

i masks variable i, is this what you meant?

[Simon Que, 2016/07/22 01:40:06]

No, accidentally had a duplicate nested loop.

+      const ElfW(Phdr)& segment_header = info->dlpi_phdr[i];
+      if (segment_header.p_type == PT_NOTE) {
+        // Elf64_Nhdr is the same as Elf32_Nhdr so we can use either here.

[rickyz (no longer on Chrome), 2016/07/19 04:58:22]

Was this a leftover comment from a previous iteration?

[Simon Que, 2016/07/22 01:40:06]

Done.

+        const void* note = reinterpret_cast<const void*>(
+            mapping_addr + segment_header.p_offset);

[rickyz (no longer on Chrome), 2016/07/19 04:58:22]

segment_header.p_vaddr is probably more accurate here and below.

[Simon Que, 2016/07/22 01:40:06]

Done.

+        const void* note_end = reinterpret_cast<const void*>(
+            mapping_addr + segment_header.p_offset + segment_header.p_memsz);
+        ReadBuildIDData* read_data = reinterpret_cast<ReadBuildIDData*>(data);
+        read_data->success =
+            GetBuildIdFromNotes(note, note_end, read_data->build_id);
+      }
+    }
+  }
+  return 1;

[Will Harris, 2016/07/19 02:51:38]

"The dl_iterate_phdr() function walks through the list of an application's
shared objects and calls the function callback once for each object, until
either all shared objects have been processed or callback returns a nonzero
value."

this callback will only be called once, is this desired?

[Simon Que, 2016/07/22 01:40:06]

Yes. We are only interested in the currently running binary, Chrome, which is
going to be the first one. Made a comment about it.

+}
+
+}  // namespace
+
+bool ReadBuildID(std::vector<uint8_t>* build_id) {
+  ReadBuildIDData data;
+  data.build_id = build_id;
+  data.success = false;
+
+#if defined(OS_CHROMEOS)
+  dl_iterate_phdr(FindNotesAndGetBuildID, &data);
+#endif  // defined(OS_CHROMEOS)
+
+  return data.success;
+}
+
+}  // namespace gnu_build_id_reader
+}  // namespace leak_detector
+}  // namespace metrics