From c44217f6468152bf5693df7ec78a48d97e2b0e83 Mon Sep 17 00:00:00 2001 From: Ted Mielczarek Date: Fri, 10 Jun 2016 13:23:29 -0400 Subject: [PATCH] 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 | 4 +- src/common/linux/dump_symbols_unittest.cc | 76 ++++++++++++++++------- src/common/linux/elfutils.h | 8 +++ src/common/linux/file_id.cc | 26 +++++--- src/common/linux/file_id.h | 4 ++ src/common/linux/file_id_unittest.cc | 17 +++++ src/common/module.cc | 8 ++- src/common/module.h | 5 +- src/common/module_unittest.cc | 11 ++++ 9 files changed, 125 insertions(+), 34 deletions(-) diff --git a/src/common/linux/dump_symbols.cc b/src/common/linux/dump_symbols.cc index 6b27120a..40b9e478 100644 --- a/src/common/linux/dump_symbols.cc +++ b/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; } diff --git a/src/common/linux/dump_symbols_unittest.cc b/src/common/linux/dump_symbols_unittest.cc index 3f86dbe6..bb7b2007 100644 --- a/src/common/linux/dump_symbols_unittest.cc +++ b/src/common/linux/dump_symbols_unittest.cc @@ -40,6 +40,8 @@ #include #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 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 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) { - ELF elf(EM_386, ELFCLASS32, kLittleEndian); +TYPED_TEST(DumpSymbols, SimplePublic) { + 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); - syms.AddSymbol("superfunc", (uint32_t)0x1000, (uint32_t)0x10, + SymbolTable syms(kLittleEndian, TypeParam::kAddrSize, table); + 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(), 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" - "PUBLIC 1000 0 superfunc\n", - s.str()); + const string expected = + string("MODULE Linux ") + TypeParam::kMachineName + + " 000000000000000000000000000000000 foo\n" + "INFO CODE_ID 00000000000000000000000000000000\n" + "PUBLIC 1000 0 superfunc\n"; + EXPECT_EQ(expected, s.str()); delete module; } -TEST_F(DumpSymbols, SimplePublic64) { - ELF elf(EM_X86_64, ELFCLASS64, kLittleEndian); +TYPED_TEST(DumpSymbols, SimpleBuildID) { + 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); - syms.AddSymbol("superfunc", (uint64_t)0x1000, (uint64_t)0x10, - ELF64_ST_INFO(STB_GLOBAL, STT_FUNC), + SymbolTable syms(kLittleEndian, TypeParam::kAddrSize, table); + 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); 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(), 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" - "PUBLIC 1000 0 superfunc\n", - s.str()); + const string expected = + string("MODULE Linux ") + TypeParam::kMachineName + + " 030201000504070608090A0B0C0D0E0F0 foo\n" + "INFO CODE_ID 000102030405060708090A0B0C0D0E0F10111213\n" + "PUBLIC 1000 0 superfunc\n"; + EXPECT_EQ(expected, s.str()); + delete module; } } // namespace google_breakpad diff --git a/src/common/linux/elfutils.h b/src/common/linux/elfutils.h index dccdc235..f34ba831 100644 --- a/src/common/linux/elfutils.h +++ b/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); diff --git a/src/common/linux/file_id.cc b/src/common/linux/file_id.cc index e8dc528d..02207758 100644 --- a/src/common/linux/file_id.cc +++ b/src/common/linux/file_id.cc @@ -164,8 +164,18 @@ bool FileID::ElfFileIdentifier(wasteful_vector& 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& identifier) { @@ -181,13 +191,13 @@ string FileID::ConvertIdentifierToUUIDString( uint16_t* data3 = reinterpret_cast(identifier_swapped + 6); *data3 = htons(*data3); - string result; - for (unsigned int idx = 0; idx < kMDGUIDSize; ++idx) { - char buf[3]; - snprintf(buf, sizeof(buf), "%02X", identifier_swapped[idx]); - result.append(buf); - } - return result; + return bytes_to_hex_string(identifier_swapped, kMDGUIDSize); +} + +// static +string FileID::ConvertIdentifierToString( + const wasteful_vector& identifier) { + return bytes_to_hex_string(&identifier[0], identifier.size()); } } // namespace google_breakpad diff --git a/src/common/linux/file_id.h b/src/common/linux/file_id.h index 80e5a8e9..a1d2fd6e 100644 --- a/src/common/linux/file_id.h +++ b/src/common/linux/file_id.h @@ -73,6 +73,10 @@ class FileID { static std::string ConvertIdentifierToUUIDString( const wasteful_vector& identifier); + // Convert the entire |identifier| data to a hex string. + static std::string ConvertIdentifierToString( + const wasteful_vector& identifier); + private: // Storage for the path specified std::string path_; diff --git a/src/common/linux/file_id_unittest.cc b/src/common/linux/file_id_unittest.cc index ec11bb25..3a819303 100644 --- a/src/common/linux/file_id_unittest.cc +++ b/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)); +} diff --git a/src/common/module.cc b/src/common/module.cc index fa798f48..b5fcb623 100644 --- a/src/common/module.cc +++ b/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(); diff --git a/src/common/module.h b/src/common/module.h index 65b5595d..6c2bb278 100644 --- a/src/common/module.h +++ b/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 diff --git a/src/common/module_unittest.cc b/src/common/module_unittest.cc index 0b643271..78406e37 100644 --- a/src/common/module_unittest.cc +++ b/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);