Dump INFO CODE_ID containing Build ID in Linux dump_syms

I'd like to have the Build ID available for our symbol server
uploading, and this will make it easy.

Most of this change is me rewriting dump_symbols_unittest to be
typed tests so I could add a new test there.

R=mark@chromium.org
BUG=

Review URL: https://codereview.chromium.org/2052263002 .

src/common/linux/dump_symbols.cc

@@ -926,8 +926,10 @@ bool InitModuleForElfClass(const typename ElfClass::Ehdr* elf_header,
   // number appended to the end of the file identifier; this isn't
   // really used or necessary on other platforms, but be consistent.
   string id = FileID::ConvertIdentifierToUUIDString(identifier) + "0";
+  // This is just the raw Build ID in hex.
+  string code_id = FileID::ConvertIdentifierToString(identifier);
 
-  module.reset(new Module(name, os, architecture, id));
+  module.reset(new Module(name, os, architecture, id, code_id));
 
   return true;
 }

src/common/linux/dump_symbols_unittest.cc

@@ -40,6 +40,8 @@
 #include <vector>
 
 #include "breakpad_googletest_includes.h"
+#include "common/linux/elf_gnu_compat.h"
+#include "common/linux/elfutils.h"
 #include "common/linux/dump_symbols.h"
 #include "common/linux/synth_elf.h"
 #include "common/module.h"
@@ -54,6 +56,7 @@ bool ReadSymbolDataInternal(const uint8_t* obj_file,
                             Module** module);
 
 using google_breakpad::synth_elf::ELF;
+using google_breakpad::synth_elf::Notes;
 using google_breakpad::synth_elf::StringTable;
 using google_breakpad::synth_elf::SymbolTable;
 using google_breakpad::test_assembler::kLittleEndian;
@@ -61,7 +64,9 @@ using google_breakpad::test_assembler::Section;
 using std::stringstream;
 using std::vector;
 using ::testing::Test;
+using ::testing::Types;
 
+template<typename ElfClass>
 class DumpSymbols : public Test {
  public:
   void GetElfContents(ELF& elf) {
@@ -78,7 +83,11 @@ class DumpSymbols : public Test {
   uint8_t* elfdata;
 };
 
-TEST_F(DumpSymbols, Invalid) {
+typedef Types<ElfClass32, ElfClass64> ElfClasses;
+
+TYPED_TEST_CASE(DumpSymbols, ElfClasses);
+
+TYPED_TEST(DumpSymbols, Invalid) {
   Elf32_Ehdr header;
   memset(&header, 0, sizeof(header));
   Module* module;
@@ -90,8 +99,8 @@ TEST_F(DumpSymbols, Invalid) {
                                       &module));
 }
 
-TEST_F(DumpSymbols, SimplePublic32) {
+TYPED_TEST(DumpSymbols, SimplePublic) {
-  ELF elf(EM_386, ELFCLASS32, kLittleEndian);
+  ELF elf(TypeParam::kMachine, TypeParam::kClass, kLittleEndian);
   // Zero out text section for simplicity.
   Section text(kLittleEndian);
   text.Append(4096, 0);
@@ -99,8 +108,11 @@ TEST_F(DumpSymbols, SimplePublic32) {
 
   // Add a public symbol.
   StringTable table(kLittleEndian);
-  SymbolTable syms(kLittleEndian, 4, table);
+  SymbolTable syms(kLittleEndian, TypeParam::kAddrSize, table);
-  syms.AddSymbol("superfunc", (uint32_t)0x1000, (uint32_t)0x10,
+  syms.AddSymbol("superfunc",
+                   (typename TypeParam::Addr)0x1000,
+                   (typename TypeParam::Addr)0x10,
+                 // ELF32_ST_INFO works for 32-or 64-bit.
                  ELF32_ST_INFO(STB_GLOBAL, STT_FUNC),
                  SHN_UNDEF + 1);
   int index = elf.AddSection(".dynstr", table, SHT_STRTAB);
@@ -109,14 +121,14 @@ TEST_F(DumpSymbols, SimplePublic32) {
                  SHF_ALLOC,           // flags
                  0,                   // addr
                  index,               // link
-                 sizeof(Elf32_Sym));  // entsize
+                 sizeof(typename TypeParam::Sym));  // entsize
 
   elf.Finish();
-  GetElfContents(elf);
+  this->GetElfContents(elf);
 
   Module* module;
   DumpOptions options(ALL_SYMBOL_DATA, true);
-  EXPECT_TRUE(ReadSymbolDataInternal(elfdata,
+  EXPECT_TRUE(ReadSymbolDataInternal(this->elfdata,
                                      "foo",
                                      vector<string>(),
                                      options,
@@ -124,24 +136,40 @@ TEST_F(DumpSymbols, SimplePublic32) {
 
   stringstream s;
   module->Write(s, ALL_SYMBOL_DATA);
-  EXPECT_EQ("MODULE Linux x86 000000000000000000000000000000000 foo\n"
+  const string expected =
-            "PUBLIC 1000 0 superfunc\n",
+    string("MODULE Linux ") + TypeParam::kMachineName
-            s.str());
+    + " 000000000000000000000000000000000 foo\n"
+    "INFO CODE_ID 00000000000000000000000000000000\n"
+    "PUBLIC 1000 0 superfunc\n";
+  EXPECT_EQ(expected, s.str());
   delete module;
 }
 
-TEST_F(DumpSymbols, SimplePublic64) {
+TYPED_TEST(DumpSymbols, SimpleBuildID) {
-  ELF elf(EM_X86_64, ELFCLASS64, kLittleEndian);
+  ELF elf(TypeParam::kMachine, TypeParam::kClass, kLittleEndian);
   // Zero out text section for simplicity.
   Section text(kLittleEndian);
   text.Append(4096, 0);
   elf.AddSection(".text", text, SHT_PROGBITS);
 
+  // Add a Build ID
+  const uint8_t kExpectedIdentifierBytes[] =
+    {0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
+     0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F,
+     0x10, 0x11, 0x12, 0x13};
+  Notes notes(kLittleEndian);
+  notes.AddNote(NT_GNU_BUILD_ID, "GNU", kExpectedIdentifierBytes,
+                sizeof(kExpectedIdentifierBytes));
+  elf.AddSection(".note.gnu.build-id", notes, SHT_NOTE);
+
   // Add a public symbol.
   StringTable table(kLittleEndian);
-  SymbolTable syms(kLittleEndian, 8, table);
+  SymbolTable syms(kLittleEndian, TypeParam::kAddrSize, table);
-  syms.AddSymbol("superfunc", (uint64_t)0x1000, (uint64_t)0x10,
+  syms.AddSymbol("superfunc",
-                 ELF64_ST_INFO(STB_GLOBAL, STT_FUNC),
+                   (typename TypeParam::Addr)0x1000,
+                   (typename TypeParam::Addr)0x10,
+                 // ELF32_ST_INFO works for 32-or 64-bit.
+                 ELF32_ST_INFO(STB_GLOBAL, STT_FUNC),
                  SHN_UNDEF + 1);
   int index = elf.AddSection(".dynstr", table, SHT_STRTAB);
   elf.AddSection(".dynsym", syms,
@@ -149,14 +177,14 @@ TEST_F(DumpSymbols, SimplePublic64) {
                  SHF_ALLOC,           // flags
                  0,                   // addr
                  index,               // link
-                 sizeof(Elf64_Sym));  // entsize
+                 sizeof(typename TypeParam::Sym));  // entsize
 
   elf.Finish();
-  GetElfContents(elf);
+  this->GetElfContents(elf);
 
   Module* module;
   DumpOptions options(ALL_SYMBOL_DATA, true);
-  EXPECT_TRUE(ReadSymbolDataInternal(elfdata,
+  EXPECT_TRUE(ReadSymbolDataInternal(this->elfdata,
                                      "foo",
                                      vector<string>(),
                                      options,
@@ -164,9 +192,13 @@ TEST_F(DumpSymbols, SimplePublic64) {
 
   stringstream s;
   module->Write(s, ALL_SYMBOL_DATA);
-  EXPECT_EQ("MODULE Linux x86_64 000000000000000000000000000000000 foo\n"
+  const string expected =
-            "PUBLIC 1000 0 superfunc\n",
+    string("MODULE Linux ") + TypeParam::kMachineName
-            s.str());
+    + " 030201000504070608090A0B0C0D0E0F0 foo\n"
+    "INFO CODE_ID 000102030405060708090A0B0C0D0E0F10111213\n"
+    "PUBLIC 1000 0 superfunc\n";
+  EXPECT_EQ(expected, s.str());
+  delete module;
 }
 
 }  // namespace google_breakpad

src/common/linux/elfutils.h

@@ -49,9 +49,13 @@ struct ElfClass32 {
   typedef Elf32_Shdr Shdr;
   typedef Elf32_Half Half;
   typedef Elf32_Off Off;
+  typedef Elf32_Sym Sym;
   typedef Elf32_Word Word;
+
   static const int kClass = ELFCLASS32;
+  static const uint16_t kMachine = EM_386;
   static const size_t kAddrSize = sizeof(Elf32_Addr);
+  static constexpr const char* kMachineName = "x86";
 };
 
 struct ElfClass64 {
@@ -62,9 +66,13 @@ struct ElfClass64 {
   typedef Elf64_Shdr Shdr;
   typedef Elf64_Half Half;
   typedef Elf64_Off Off;
+  typedef Elf64_Sym Sym;
   typedef Elf64_Word Word;
+
   static const int kClass = ELFCLASS64;
+  static const uint16_t kMachine = EM_X86_64;
   static const size_t kAddrSize = sizeof(Elf64_Addr);
+  static constexpr const char* kMachineName = "x86_64";
 };
 
 bool IsValidElf(const void* elf_header);

src/common/linux/file_id.cc

@@ -164,8 +164,18 @@ bool FileID::ElfFileIdentifier(wasteful_vector<uint8_t>& identifier) {
   return ElfFileIdentifierFromMappedFile(mapped_file.data(), identifier);
 }
 
-// This function is not ever called in an unsafe context, so it's OK
+// These three functions are not ever called in an unsafe context, so it's OK
 // to allocate memory and use libc.
+static string bytes_to_hex_string(const uint8_t* bytes, size_t count) {
+  string result;
+  for (unsigned int idx = 0; idx < count; ++idx) {
+    char buf[3];
+    snprintf(buf, sizeof(buf), "%02X", bytes[idx]);
+    result.append(buf);
+  }
+  return result;
+}
+
 // static
 string FileID::ConvertIdentifierToUUIDString(
     const wasteful_vector<uint8_t>& identifier) {
@@ -181,13 +191,13 @@ string FileID::ConvertIdentifierToUUIDString(
   uint16_t* data3 = reinterpret_cast<uint16_t*>(identifier_swapped + 6);
   *data3 = htons(*data3);
 
-  string result;
+  return bytes_to_hex_string(identifier_swapped, kMDGUIDSize);
-  for (unsigned int idx = 0; idx < kMDGUIDSize; ++idx) {
+}
-    char buf[3];
+
-    snprintf(buf, sizeof(buf), "%02X", identifier_swapped[idx]);
+// static
-    result.append(buf);
+string FileID::ConvertIdentifierToString(
-  }
+    const wasteful_vector<uint8_t>& identifier) {
-  return result;
+  return bytes_to_hex_string(&identifier[0], identifier.size());
 }
 
 }  // namespace google_breakpad

src/common/linux/file_id.h

@@ -73,6 +73,10 @@ class FileID {
   static std::string ConvertIdentifierToUUIDString(
       const wasteful_vector<uint8_t>& identifier);
 
+  // Convert the entire |identifier| data to a hex string.
+  static std::string ConvertIdentifierToString(
+      const wasteful_vector<uint8_t>& identifier);
+
  private:
   // Storage for the path specified
   std::string path_;

src/common/linux/file_id_unittest.cc

@@ -319,3 +319,20 @@ TYPED_TEST(FileIDTest, UniqueHashes) {
 
   EXPECT_NE(identifier_string_1, identifier_string_2);
 }
+
+TYPED_TEST(FileIDTest, ConvertIdentifierToString) {
+  const uint8_t kIdentifierBytes[] =
+    {0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07,
+     0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F,
+     0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17,
+     0x18, 0x19, 0x1A, 0x1B, 0x1C, 0x1D, 0x1E, 0x1F};
+  const char* kExpected =
+    "000102030405060708090A0B0C0D0E0F101112131415161718191A1B1C1D1E1F";
+
+  id_vector identifier(this->make_vector());
+  identifier.insert(identifier.end(),
+                    kIdentifierBytes,
+                    kIdentifierBytes + sizeof(kIdentifierBytes));
+  ASSERT_EQ(kExpected,
+            FileID::ConvertIdentifierToString(identifier));
+}

src/common/module.cc

@@ -49,11 +49,13 @@ using std::hex;
 
 
 Module::Module(const string &name, const string &os,
-               const string &architecture, const string &id) :
+               const string &architecture, const string &id,
+               const string &code_id /* = "" */) :
     name_(name),
     os_(os),
     architecture_(architecture),
     id_(id),
+    code_id_(code_id),
     load_address_(0) { }
 
 Module::~Module() {
@@ -235,6 +237,10 @@ bool Module::Write(std::ostream &stream, SymbolData symbol_data) {
   if (!stream.good())
     return ReportError();
 
+  if (!code_id_.empty()) {
+    stream << "INFO CODE_ID " << code_id_ << endl;
+  }
+
   if (symbol_data != ONLY_CFI) {
     AssignSourceIds();
 

src/common/module.h

@@ -179,7 +179,7 @@ class Module {
   // Create a new module with the given name, operating system,
   // architecture, and ID string.
   Module(const string &name, const string &os, const string &architecture,
-         const string &id);
+         const string &id, const string &code_id = "");
   ~Module();
 
   // Set the module's load address to LOAD_ADDRESS; addresses given
@@ -281,6 +281,7 @@ class Module {
   string os() const { return os_; }
   string architecture() const { return architecture_; }
   string identifier() const { return id_; }
+  string code_identifier() const { return code_id_; }
 
  private:
   // Report an error that has occurred writing the symbol file, using
@@ -293,7 +294,7 @@ class Module {
   static bool WriteRuleMap(const RuleMap &rule_map, std::ostream &stream);
 
   // Module header entries.
-  string name_, os_, architecture_, id_;
+  string name_, os_, architecture_, id_, code_id_;
 
   // The module's nominal load address.  Addresses for functions and
   // lines are absolute, assuming the module is loaded at this

src/common/module_unittest.cc

@@ -64,6 +64,7 @@ static Module::Function *generate_duplicate_function(const string &name) {
 #define MODULE_OS "os-name"
 #define MODULE_ARCH "architecture"
 #define MODULE_ID "id-string"
+#define MODULE_CODE_ID "code-id-string"
 
 TEST(Write, Header) {
   stringstream s;
@@ -74,6 +75,16 @@ TEST(Write, Header) {
                contents.c_str());
 }
 
+TEST(Write, HeaderCodeId) {
+  stringstream s;
+  Module m(MODULE_NAME, MODULE_OS, MODULE_ARCH, MODULE_ID, MODULE_CODE_ID);
+  m.Write(s, ALL_SYMBOL_DATA);
+  string contents = s.str();
+  EXPECT_STREQ("MODULE os-name architecture id-string name with spaces\n"
+               "INFO CODE_ID code-id-string\n",
+               contents.c_str());
+}
+
 TEST(Write, OneLineFunc) {
   stringstream s;
   Module m(MODULE_NAME, MODULE_OS, MODULE_ARCH, MODULE_ID);