From 6c8f80aa8b3ba8120c4158c069bb298c044dedf9 Mon Sep 17 00:00:00 2001 From: Ted Mielczarek Date: Tue, 5 Apr 2016 09:34:20 -0400 Subject: [PATCH] Switch the Linux minidump writer to use MDCVInfoELF for CV data. This preserves full build ids in minidumps, which are useful for tracking down the right version of system libraries from Linux distributions. The default build id produced by GNU binutils' ld is a 160-bit SHA-1 hash of some parts of the binary, which is exactly 20 bytes: https://sourceware.org/binutils/docs-2.26/ld/Options.html#index-g_t_002d_002dbuild_002did-292 The bulk of the changes here are to change the signatures of the FileID methods to use a wasteful_vector instead of raw pointers, since build ids can be of arbitrary length. The previous change that added support for this in the processor code preserved the return value of `Minidump::debug_identifier()` as the current `GUID+age` treatment for backwards-compatibility, and exposed the full build id from `Minidump::code_identifier()`, which was previously stubbed out for Linux dumps. This change keeps the debug ID in the `dump_syms` output the same to match. R=mark@chromium.org, thestig@chromium.org BUG= Review URL: https://codereview.chromium.org/1688743002 . --- Makefile.am | 4 +- Makefile.in | 5 +- .../handler/exception_handler_unittest.cc | 15 +- .../microdump_writer/microdump_writer.cc | 21 ++- .../linux/minidump_writer/linux_dumper.cc | 3 +- .../linux/minidump_writer/linux_dumper.h | 3 +- .../linux_ptrace_dumper_unittest.cc | 33 ++-- .../linux/minidump_writer/minidump_writer.cc | 48 ++--- .../minidump_writer_unittest.cc | 87 +++++---- src/common/linux/dump_symbols.cc | 36 ++-- src/common/linux/file_id.cc | 70 +++---- src/common/linux/file_id.h | 31 +-- src/common/linux/file_id_unittest.cc | 178 ++++++++++++------ src/common/memory.h | 52 ++++- src/common/memory_unittest.cc | 27 +++ src/processor/minidump_unittest.cc | 74 ++++++++ 16 files changed, 460 insertions(+), 227 deletions(-) diff --git a/Makefile.am b/Makefile.am index a8ec3f50..8af3830e 100644 --- a/Makefile.am +++ b/Makefile.am @@ -527,8 +527,10 @@ src_client_linux_linux_client_unittest_shlib_DEPENDENCIES = \ src/libbreakpad.a src_client_linux_linux_client_unittest_SOURCES = +# The extra-long build id is for a test in minidump_writer_unittest.cc. src_client_linux_linux_client_unittest_LDFLAGS = \ - -Wl,-rpath,'$$ORIGIN' + -Wl,-rpath,'$$ORIGIN' \ + -Wl,--build-id=0x000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f if ANDROID_HOST src_client_linux_linux_client_unittest_LDFLAGS += \ -llog diff --git a/Makefile.in b/Makefile.in index d145d08a..f5b66618 100644 --- a/Makefile.in +++ b/Makefile.in @@ -2264,8 +2264,11 @@ TESTS = $(check_PROGRAMS) $(check_SCRIPTS) @LINUX_HOST_TRUE@ src/libbreakpad.a @LINUX_HOST_TRUE@src_client_linux_linux_client_unittest_SOURCES = +# The extra-long build id is for a test in minidump_writer_unittest.cc. @LINUX_HOST_TRUE@src_client_linux_linux_client_unittest_LDFLAGS = \ -@LINUX_HOST_TRUE@ -Wl,-rpath,'$$ORIGIN' $(am__append_24) +@LINUX_HOST_TRUE@ -Wl,-rpath,'$$ORIGIN' \ +@LINUX_HOST_TRUE@ -Wl,--build-id=0x000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f \ +@LINUX_HOST_TRUE@ $(am__append_24) @LINUX_HOST_TRUE@src_client_linux_linux_client_unittest_LDADD = \ @LINUX_HOST_TRUE@ src/client/linux/linux_client_unittest_shlib \ @LINUX_HOST_TRUE@ $(TEST_LIBS) diff --git a/src/client/linux/handler/exception_handler_unittest.cc b/src/client/linux/handler/exception_handler_unittest.cc index 439b3c38..4eb7b73c 100644 --- a/src/client/linux/handler/exception_handler_unittest.cc +++ b/src/client/linux/handler/exception_handler_unittest.cc @@ -45,7 +45,6 @@ #include "client/linux/handler/exception_handler.h" #include "client/linux/minidump_writer/minidump_writer.h" #include "common/linux/eintr_wrapper.h" -#include "common/linux/file_id.h" #include "common/linux/ignore_ret.h" #include "common/linux/linux_libc_support.h" #include "common/tests/auto_tempdir.h" @@ -817,19 +816,7 @@ TEST(ExceptionHandlerTest, ModuleInfo) { 0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99, 0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF }; - char module_identifier_buffer[kGUIDStringSize]; - FileID::ConvertIdentifierToString(kModuleGUID, - module_identifier_buffer, - sizeof(module_identifier_buffer)); - string module_identifier(module_identifier_buffer); - // Strip out dashes - size_t pos; - while ((pos = module_identifier.find('-')) != string::npos) { - module_identifier.erase(pos, 1); - } - // And append a zero, because module IDs include an "age" field - // which is always zero on Linux. - module_identifier += "0"; + const string module_identifier = "33221100554477668899AABBCCDDEEFF0"; // Get some memory. char* memory = diff --git a/src/client/linux/microdump_writer/microdump_writer.cc b/src/client/linux/microdump_writer/microdump_writer.cc index 91697ed8..d459d9ec 100644 --- a/src/client/linux/microdump_writer/microdump_writer.cc +++ b/src/client/linux/microdump_writer/microdump_writer.cc @@ -34,17 +34,22 @@ #include +#include + #include "client/linux/dump_writer_common/thread_info.h" #include "client/linux/dump_writer_common/ucontext_reader.h" #include "client/linux/handler/exception_handler.h" #include "client/linux/handler/microdump_extra_info.h" #include "client/linux/log/log.h" #include "client/linux/minidump_writer/linux_ptrace_dumper.h" +#include "common/linux/file_id.h" #include "common/linux/linux_libc_support.h" namespace { +using google_breakpad::auto_wasteful_vector; using google_breakpad::ExceptionHandler; +using google_breakpad::kDefaultBuildIdSize; using google_breakpad::LinuxDumper; using google_breakpad::LinuxPtraceDumper; using google_breakpad::MappingInfo; @@ -336,18 +341,28 @@ class MicrodumpWriter { bool member, unsigned int mapping_id, const uint8_t* identifier) { - MDGUID module_identifier; + + auto_wasteful_vector identifier_bytes( + dumper_->allocator()); + if (identifier) { // GUID was provided by caller. - my_memcpy(&module_identifier, identifier, sizeof(MDGUID)); + identifier_bytes.insert(identifier_bytes.end(), + identifier, + identifier + sizeof(MDGUID)); } else { dumper_->ElfFileIdentifierForMapping( mapping, member, mapping_id, - reinterpret_cast(&module_identifier)); + identifier_bytes); } + // Copy as many bytes of |identifier| as will fit into a MDGUID + MDGUID module_identifier = {0}; + memcpy(&module_identifier, &identifier_bytes[0], + std::min(sizeof(MDGUID), identifier_bytes.size())); + char file_name[NAME_MAX]; char file_path[NAME_MAX]; dumper_->GetMappingEffectiveNameAndPath( diff --git a/src/client/linux/minidump_writer/linux_dumper.cc b/src/client/linux/minidump_writer/linux_dumper.cc index 060e6c7c..cfa28a87 100644 --- a/src/client/linux/minidump_writer/linux_dumper.cc +++ b/src/client/linux/minidump_writer/linux_dumper.cc @@ -121,9 +121,8 @@ bool LinuxDumper::ElfFileIdentifierForMapping(const MappingInfo& mapping, bool member, unsigned int mapping_id, - uint8_t identifier[sizeof(MDGUID)]) { + wasteful_vector& identifier) { assert(!member || mapping_id < mappings_.size()); - my_memset(identifier, 0, sizeof(MDGUID)); if (IsMappedFileOpenUnsafe(mapping)) return false; diff --git a/src/client/linux/minidump_writer/linux_dumper.h b/src/client/linux/minidump_writer/linux_dumper.h index f7fe1dd9..c3c79926 100644 --- a/src/client/linux/minidump_writer/linux_dumper.h +++ b/src/client/linux/minidump_writer/linux_dumper.h @@ -49,6 +49,7 @@ #include "client/linux/dump_writer_common/mapping_info.h" #include "client/linux/dump_writer_common/thread_info.h" +#include "common/linux/file_id.h" #include "common/memory.h" #include "google_breakpad/common/minidump_format.h" @@ -129,7 +130,7 @@ class LinuxDumper { bool ElfFileIdentifierForMapping(const MappingInfo& mapping, bool member, unsigned int mapping_id, - uint8_t identifier[sizeof(MDGUID)]); + wasteful_vector& identifier); uintptr_t crash_address() const { return crash_address_; } void set_crash_address(uintptr_t crash_address) { diff --git a/src/client/linux/minidump_writer/linux_ptrace_dumper_unittest.cc b/src/client/linux/minidump_writer/linux_ptrace_dumper_unittest.cc index 838ea5f6..be533e15 100644 --- a/src/client/linux/minidump_writer/linux_ptrace_dumper_unittest.cc +++ b/src/client/linux/minidump_writer/linux_ptrace_dumper_unittest.cc @@ -66,6 +66,7 @@ using namespace google_breakpad; namespace { +typedef wasteful_vector id_vector; typedef testing::Test LinuxPtraceDumperTest; /* Fixture for running tests in a child process. */ @@ -105,11 +106,17 @@ class LinuxPtraceDumperChildTest : public testing::Test { * This is achieved by defining a TestBody macro further below. */ virtual void RealTestBody() = 0; + + id_vector make_vector() { + return id_vector(&allocator, kDefaultBuildIdSize); + } + private: static const int kFatalFailure = 1; static const int kNonFatalFailure = 2; pid_t child_pid_; + PageAllocator allocator; }; } // namespace @@ -310,14 +317,15 @@ TEST_F(LinuxPtraceDumperChildTest, LinuxGateMappingID) { // Need to suspend the child so ptrace actually works. ASSERT_TRUE(dumper.ThreadsSuspend()); - uint8_t identifier[sizeof(MDGUID)]; + id_vector identifier(make_vector()); ASSERT_TRUE(dumper.ElfFileIdentifierForMapping(*mappings[index], true, index, identifier)); - uint8_t empty_identifier[sizeof(MDGUID)]; - memset(empty_identifier, 0, sizeof(empty_identifier)); - EXPECT_NE(0, memcmp(empty_identifier, identifier, sizeof(identifier))); + + id_vector empty_identifier(make_vector()); + empty_identifier.resize(kDefaultBuildIdSize, 0); + EXPECT_NE(empty_identifier, identifier); EXPECT_TRUE(dumper.ThreadsResume()); } #endif @@ -343,19 +351,18 @@ TEST_F(LinuxPtraceDumperChildTest, FileIDsMatch) { } ASSERT_TRUE(found_exe); - uint8_t identifier1[sizeof(MDGUID)]; - uint8_t identifier2[sizeof(MDGUID)]; + id_vector identifier1(make_vector()); + id_vector identifier2(make_vector()); EXPECT_TRUE(dumper.ElfFileIdentifierForMapping(*mappings[i], true, i, identifier1)); FileID fileid(exe_name); EXPECT_TRUE(fileid.ElfFileIdentifier(identifier2)); - char identifier_string1[37]; - char identifier_string2[37]; - FileID::ConvertIdentifierToString(identifier1, identifier_string1, - 37); - FileID::ConvertIdentifierToString(identifier2, identifier_string2, - 37); - EXPECT_STREQ(identifier_string1, identifier_string2); + + string identifier_string1 = + FileID::ConvertIdentifierToUUIDString(identifier1); + string identifier_string2 = + FileID::ConvertIdentifierToUUIDString(identifier2); + EXPECT_EQ(identifier_string1, identifier_string2); } /* Get back to normal behavior of TEST*() macros wrt TestBody. */ diff --git a/src/client/linux/minidump_writer/minidump_writer.cc b/src/client/linux/minidump_writer/minidump_writer.cc index 3103761f..f407caa7 100644 --- a/src/client/linux/minidump_writer/minidump_writer.cc +++ b/src/client/linux/minidump_writer/minidump_writer.cc @@ -73,6 +73,7 @@ #include "client/linux/minidump_writer/linux_ptrace_dumper.h" #include "client/linux/minidump_writer/proc_cpuinfo_reader.h" #include "client/minidump_file_writer.h" +#include "common/linux/file_id.h" #include "common/linux/linux_libc_support.h" #include "common/minidump_type_helper.h" #include "google_breakpad/common/minidump_format.h" @@ -81,8 +82,10 @@ namespace { using google_breakpad::AppMemoryList; +using google_breakpad::auto_wasteful_vector; using google_breakpad::ExceptionHandler; using google_breakpad::CpuSet; +using google_breakpad::kDefaultBuildIdSize; using google_breakpad::LineReader; using google_breakpad::LinuxDumper; using google_breakpad::LinuxPtraceDumper; @@ -546,41 +549,40 @@ class MinidumpWriter { mod->base_of_image = mapping.start_addr; mod->size_of_image = mapping.size; - uint8_t cv_buf[MDCVInfoPDB70_minsize + NAME_MAX]; - uint8_t* cv_ptr = cv_buf; + auto_wasteful_vector identifier_bytes( + dumper_->allocator()); - const uint32_t cv_signature = MD_CVINFOPDB70_SIGNATURE; - my_memcpy(cv_ptr, &cv_signature, sizeof(cv_signature)); - cv_ptr += sizeof(cv_signature); - uint8_t* signature = cv_ptr; - cv_ptr += sizeof(MDGUID); if (identifier) { // GUID was provided by caller. - my_memcpy(signature, identifier, sizeof(MDGUID)); + identifier_bytes.insert(identifier_bytes.end(), + identifier, + identifier + sizeof(MDGUID)); } else { // Note: ElfFileIdentifierForMapping() can manipulate the |mapping.name|. - dumper_->ElfFileIdentifierForMapping(mapping, member, - mapping_id, signature); + dumper_->ElfFileIdentifierForMapping(mapping, + member, + mapping_id, + identifier_bytes); + } + + if (!identifier_bytes.empty()) { + UntypedMDRVA cv(&minidump_writer_); + if (!cv.Allocate(MDCVInfoELF_minsize + identifier_bytes.size())) + return false; + + const uint32_t cv_signature = MD_CVINFOELF_SIGNATURE; + cv.Copy(&cv_signature, sizeof(cv_signature)); + cv.Copy(cv.position() + sizeof(cv_signature), &identifier_bytes[0], + identifier_bytes.size()); + + mod->cv_record = cv.location(); } - my_memset(cv_ptr, 0, sizeof(uint32_t)); // Set age to 0 on Linux. - cv_ptr += sizeof(uint32_t); char file_name[NAME_MAX]; char file_path[NAME_MAX]; dumper_->GetMappingEffectiveNameAndPath( mapping, file_path, sizeof(file_path), file_name, sizeof(file_name)); - const size_t file_name_len = my_strlen(file_name); - UntypedMDRVA cv(&minidump_writer_); - if (!cv.Allocate(MDCVInfoPDB70_minsize + file_name_len + 1)) - return false; - - // Write pdb_file_name - my_memcpy(cv_ptr, file_name, file_name_len + 1); - cv.Copy(cv_buf, MDCVInfoPDB70_minsize + file_name_len + 1); - - mod->cv_record = cv.location(); - MDLocationDescriptor ld; if (!minidump_writer_.WriteString(file_path, my_strlen(file_path), &ld)) return false; diff --git a/src/client/linux/minidump_writer/minidump_writer_unittest.cc b/src/client/linux/minidump_writer/minidump_writer_unittest.cc index e1046e12..db7d4f5d 100644 --- a/src/client/linux/minidump_writer/minidump_writer_unittest.cc +++ b/src/client/linux/minidump_writer/minidump_writer_unittest.cc @@ -54,10 +54,6 @@ using namespace google_breakpad; -// Length of a formatted GUID string = -// sizeof(MDGUID) * 2 + 4 (for dashes) + 1 (null terminator) -const int kGUIDStringSize = 37; - namespace { typedef testing::Test MinidumpWriterTest; @@ -137,19 +133,7 @@ TEST(MinidumpWriterTest, MappingInfo) { 0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99, 0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF }; - char module_identifier_buffer[kGUIDStringSize]; - FileID::ConvertIdentifierToString(kModuleGUID, - module_identifier_buffer, - sizeof(module_identifier_buffer)); - string module_identifier(module_identifier_buffer); - // Strip out dashes - size_t pos; - while ((pos = module_identifier.find('-')) != string::npos) { - module_identifier.erase(pos, 1); - } - // And append a zero, because module IDs include an "age" field - // which is always zero on Linux. - module_identifier += "0"; + const string module_identifier = "33221100554477668899AABBCCDDEEFF0"; // Get some memory. char* memory = @@ -230,6 +214,53 @@ TEST(MinidumpWriterTest, MappingInfo) { close(fds[1]); } +// Test that a binary with a longer-than-usual build id note +// makes its way all the way through to the minidump unscathed. +// The linux_client_unittest is linked with an explicit --build-id +// in Makefile.am. +TEST(MinidumpWriterTest, BuildIDLong) { + int fds[2]; + ASSERT_NE(-1, pipe(fds)); + + const pid_t child = fork(); + if (child == 0) { + close(fds[1]); + char b; + IGNORE_RET(HANDLE_EINTR(read(fds[0], &b, sizeof(b)))); + close(fds[0]); + syscall(__NR_exit); + } + close(fds[0]); + + ExceptionHandler::CrashContext context; + memset(&context, 0, sizeof(context)); + ASSERT_EQ(0, getcontext(&context.context)); + context.tid = child; + + AutoTempDir temp_dir; + const string dump_path = temp_dir.path() + kMDWriterUnitTestFileName; + + EXPECT_TRUE(WriteMinidump(dump_path.c_str(), + child, &context, sizeof(context))); + close(fds[1]); + + // Read the minidump. Load the module list, and ensure that + // the main module has the correct debug id and code id. + Minidump minidump(dump_path); + ASSERT_TRUE(minidump.Read()); + + MinidumpModuleList* module_list = minidump.GetModuleList(); + ASSERT_TRUE(module_list); + const MinidumpModule* module = module_list->GetMainModule(); + ASSERT_TRUE(module); + const string module_identifier = "030201000504070608090A0B0C0D0E0F0"; + // This is passed explicitly to the linker in Makefile.am + const string build_id = + "000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f"; + EXPECT_EQ(module_identifier, module->debug_identifier()); + EXPECT_EQ(build_id, module->code_identifier()); +} + // Test that mapping info can be specified, and that it overrides // existing mappings that are wholly contained within the specified // range. @@ -245,19 +276,7 @@ TEST(MinidumpWriterTest, MappingInfoContained) { 0x00, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88, 0x99, 0xAA, 0xBB, 0xCC, 0xDD, 0xEE, 0xFF }; - char module_identifier_buffer[kGUIDStringSize]; - FileID::ConvertIdentifierToString(kModuleGUID, - module_identifier_buffer, - sizeof(module_identifier_buffer)); - string module_identifier(module_identifier_buffer); - // Strip out dashes - size_t pos; - while ((pos = module_identifier.find('-')) != string::npos) { - module_identifier.erase(pos, 1); - } - // And append a zero, because module IDs include an "age" field - // which is always zero on Linux. - module_identifier += "0"; + const string module_identifier = "33221100554477668899AABBCCDDEEFF0"; // mmap a file AutoTempDir temp_dir; @@ -410,12 +429,10 @@ TEST(MinidumpWriterTest, DeletedBinary) { EXPECT_STREQ(binpath.c_str(), module->code_file().c_str()); // Check that the file ID is correct. FileID fileid(helper_path.c_str()); - uint8_t identifier[sizeof(MDGUID)]; + PageAllocator allocator; + wasteful_vector identifier(&allocator, kDefaultBuildIdSize); EXPECT_TRUE(fileid.ElfFileIdentifier(identifier)); - char identifier_string[kGUIDStringSize]; - FileID::ConvertIdentifierToString(identifier, - identifier_string, - kGUIDStringSize); + string identifier_string = FileID::ConvertIdentifierToUUIDString(identifier); string module_identifier(identifier_string); // Strip out dashes size_t pos; diff --git a/src/common/linux/dump_symbols.cc b/src/common/linux/dump_symbols.cc index 5d2aac7e..d31e2c2d 100644 --- a/src/common/linux/dump_symbols.cc +++ b/src/common/linux/dump_symbols.cc @@ -64,6 +64,7 @@ #include "common/linux/elfutils-inl.h" #include "common/linux/elf_symbols_to_module.h" #include "common/linux/file_id.h" +#include "common/memory.h" #include "common/module.h" #include "common/scoped_ptr.h" #ifndef NO_STABS_SUPPORT @@ -82,14 +83,18 @@ using google_breakpad::DwarfLineToModule; using google_breakpad::ElfClass; using google_breakpad::ElfClass32; using google_breakpad::ElfClass64; +using google_breakpad::FileID; using google_breakpad::FindElfSectionByName; using google_breakpad::GetOffset; using google_breakpad::IsValidElf; +using google_breakpad::kDefaultBuildIdSize; using google_breakpad::Module; +using google_breakpad::PageAllocator; #ifndef NO_STABS_SUPPORT using google_breakpad::StabsToModule; #endif using google_breakpad::scoped_ptr; +using google_breakpad::wasteful_vector; // Define AARCH64 ELF architecture if host machine does not include this define. #ifndef EM_AARCH64 @@ -854,25 +859,6 @@ const char* ElfArchitecture(const typename ElfClass::Ehdr* elf_header) { } } -// Format the Elf file identifier in IDENTIFIER as a UUID with the -// dashes removed. -string FormatIdentifier(unsigned char identifier[16]) { - char identifier_str[40]; - google_breakpad::FileID::ConvertIdentifierToString( - identifier, - identifier_str, - sizeof(identifier_str)); - string id_no_dash; - for (int i = 0; identifier_str[i] != '\0'; ++i) - if (identifier_str[i] != '-') - id_no_dash += identifier_str[i]; - // Add an extra "0" by the end. PDB files on Windows have an 'age' - // number appended to the end of the file identifier; this isn't - // really used or necessary on other platforms, but be consistent. - id_no_dash += '0'; - return id_no_dash; -} - // Return the non-directory portion of FILENAME: the portion after the // last slash, or the whole filename if there are no slashes. string BaseFileName(const string &filename) { @@ -924,9 +910,10 @@ bool ReadSymbolDataElfClass(const typename ElfClass::Ehdr* elf_header, *out_module = NULL; - unsigned char identifier[16]; - if (!google_breakpad::FileID::ElfFileIdentifierFromMappedFile(elf_header, - identifier)) { + PageAllocator allocator; + wasteful_vector identifier(&allocator, kDefaultBuildIdSize); + if (!FileID::ElfFileIdentifierFromMappedFile(elf_header, + identifier)) { fprintf(stderr, "%s: unable to generate file identifier\n", obj_filename.c_str()); return false; @@ -946,7 +933,10 @@ bool ReadSymbolDataElfClass(const typename ElfClass::Ehdr* elf_header, string name = BaseFileName(obj_filename); string os = "Linux"; - string id = FormatIdentifier(identifier); + // Add an extra "0" at the end. PDB files on Windows have an 'age' + // 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"; LoadSymbolsInfo info(debug_dirs); scoped_ptr module(new Module(name, os, architecture, id)); diff --git a/src/common/linux/file_id.cc b/src/common/linux/file_id.cc index 00b37313..e8dc528d 100644 --- a/src/common/linux/file_id.cc +++ b/src/common/linux/file_id.cc @@ -39,6 +39,7 @@ #include #include +#include #include "common/linux/elf_gnu_compat.h" #include "common/linux/elfutils.h" @@ -46,8 +47,13 @@ #include "common/linux/memory_mapped_file.h" #include "third_party/lss/linux_syscall_support.h" +using std::string; + namespace google_breakpad { +// Used in a few places for backwards-compatibility. +const size_t kMDGUIDSize = sizeof(MDGUID); + FileID::FileID(const char* path) : path_(path) {} // ELF note name and desc are 32-bits word padded. @@ -58,7 +64,7 @@ FileID::FileID(const char* path) : path_(path) {} template static bool ElfClassBuildIDNoteIdentifier(const void *section, size_t length, - uint8_t identifier[kMDGUIDSize]) { + wasteful_vector& identifier) { typedef typename ElfClass::Nhdr Nhdr; const void* section_end = reinterpret_cast(section) + length; @@ -76,21 +82,19 @@ static bool ElfClassBuildIDNoteIdentifier(const void *section, size_t length, return false; } - const char* build_id = reinterpret_cast(note_header) + + const uint8_t* build_id = reinterpret_cast(note_header) + sizeof(Nhdr) + NOTE_PADDING(note_header->n_namesz); - // Copy as many bits of the build ID as will fit - // into the GUID space. - my_memset(identifier, 0, kMDGUIDSize); - memcpy(identifier, build_id, - std::min(kMDGUIDSize, (size_t)note_header->n_descsz)); + identifier.insert(identifier.end(), + build_id, + build_id + note_header->n_descsz); return true; } // Attempt to locate a .note.gnu.build-id section in an ELF binary -// and copy as many bytes of it as will fit into |identifier|. -static bool FindElfBuildIDNote(const void *elf_mapped_base, - uint8_t identifier[kMDGUIDSize]) { +// and copy it into |identifier|. +static bool FindElfBuildIDNote(const void* elf_mapped_base, + wasteful_vector& identifier) { void* note_section; size_t note_size; int elfclass; @@ -116,8 +120,10 @@ static bool FindElfBuildIDNote(const void *elf_mapped_base, // Attempt to locate the .text section of an ELF binary and generate // a simple hash by XORing the first page worth of bytes into |identifier|. -static bool HashElfTextSection(const void *elf_mapped_base, - uint8_t identifier[kMDGUIDSize]) { +static bool HashElfTextSection(const void* elf_mapped_base, + wasteful_vector& identifier) { + identifier.resize(kMDGUIDSize); + void* text_section; size_t text_size; if (!FindElfSection(elf_mapped_base, ".text", SHT_PROGBITS, @@ -126,7 +132,9 @@ static bool HashElfTextSection(const void *elf_mapped_base, return false; } - my_memset(identifier, 0, kMDGUIDSize); + // Only provide |kMDGUIDSize| bytes to keep identifiers produced by this + // function backwards-compatible. + my_memset(&identifier[0], 0, kMDGUIDSize); const uint8_t* ptr = reinterpret_cast(text_section); const uint8_t* ptr_end = ptr + std::min(text_size, static_cast(4096)); while (ptr < ptr_end) { @@ -139,7 +147,7 @@ static bool HashElfTextSection(const void *elf_mapped_base, // static bool FileID::ElfFileIdentifierFromMappedFile(const void* base, - uint8_t identifier[kMDGUIDSize]) { + wasteful_vector& identifier) { // Look for a build id note first. if (FindElfBuildIDNote(base, identifier)) return true; @@ -148,7 +156,7 @@ bool FileID::ElfFileIdentifierFromMappedFile(const void* base, return HashElfTextSection(base, identifier); } -bool FileID::ElfFileIdentifier(uint8_t identifier[kMDGUIDSize]) { +bool FileID::ElfFileIdentifier(wasteful_vector& identifier) { MemoryMappedFile mapped_file(path_.c_str(), 0); if (!mapped_file.data()) // Should probably check if size >= ElfW(Ehdr)? return false; @@ -156,13 +164,16 @@ bool FileID::ElfFileIdentifier(uint8_t identifier[kMDGUIDSize]) { return ElfFileIdentifierFromMappedFile(mapped_file.data(), identifier); } +// This function is not ever called in an unsafe context, so it's OK +// to allocate memory and use libc. // static -void FileID::ConvertIdentifierToString(const uint8_t identifier[kMDGUIDSize], - char* buffer, int buffer_length) { - uint8_t identifier_swapped[kMDGUIDSize]; +string FileID::ConvertIdentifierToUUIDString( + const wasteful_vector& identifier) { + uint8_t identifier_swapped[kMDGUIDSize] = { 0 }; // Endian-ness swap to match dump processor expectation. - memcpy(identifier_swapped, identifier, kMDGUIDSize); + memcpy(identifier_swapped, &identifier[0], + std::min(kMDGUIDSize, identifier.size())); uint32_t* data1 = reinterpret_cast(identifier_swapped); *data1 = htonl(*data1); uint16_t* data2 = reinterpret_cast(identifier_swapped + 4); @@ -170,22 +181,13 @@ void FileID::ConvertIdentifierToString(const uint8_t identifier[kMDGUIDSize], uint16_t* data3 = reinterpret_cast(identifier_swapped + 6); *data3 = htons(*data3); - int buffer_idx = 0; - for (unsigned int idx = 0; - (buffer_idx < buffer_length) && (idx < kMDGUIDSize); - ++idx) { - int hi = (identifier_swapped[idx] >> 4) & 0x0F; - int lo = (identifier_swapped[idx]) & 0x0F; - - if (idx == 4 || idx == 6 || idx == 8 || idx == 10) - buffer[buffer_idx++] = '-'; - - buffer[buffer_idx++] = (hi >= 10) ? 'A' + hi - 10 : '0' + hi; - buffer[buffer_idx++] = (lo >= 10) ? 'A' + lo - 10 : '0' + lo; + string result; + for (unsigned int idx = 0; idx < kMDGUIDSize; ++idx) { + char buf[3]; + snprintf(buf, sizeof(buf), "%02X", identifier_swapped[idx]); + result.append(buf); } - - // NULL terminate - buffer[(buffer_idx < buffer_length) ? buffer_idx : buffer_idx - 1] = 0; + return result; } } // namespace google_breakpad diff --git a/src/common/linux/file_id.h b/src/common/linux/file_id.h index 2642722a..80e5a8e9 100644 --- a/src/common/linux/file_id.h +++ b/src/common/linux/file_id.h @@ -37,10 +37,15 @@ #include #include "common/linux/guid_creator.h" +#include "common/memory.h" namespace google_breakpad { -static const size_t kMDGUIDSize = sizeof(MDGUID); +// GNU binutils' ld defaults to 'sha1', which is 160 bits == 20 bytes, +// so this is enough to fit that, which most binaries will use. +// This is just a sensible default for auto_wasteful_vector so most +// callers can get away with stack allocation. +static const size_t kDefaultBuildIdSize = 20; class FileID { public: @@ -48,25 +53,25 @@ class FileID { ~FileID() {} // Load the identifier for the elf file path specified in the constructor into - // |identifier|. Return false if the identifier could not be created for the - // file. + // |identifier|. + // // The current implementation will look for a .note.gnu.build-id // section and use that as the file id, otherwise it falls back to // XORing the first 4096 bytes of the .text section to generate an identifier. - bool ElfFileIdentifier(uint8_t identifier[kMDGUIDSize]); + bool ElfFileIdentifier(wasteful_vector& identifier); // Load the identifier for the elf file mapped into memory at |base| into - // |identifier|. Return false if the identifier could not be created for the + // |identifier|. Return false if the identifier could not be created for this // file. - static bool ElfFileIdentifierFromMappedFile(const void* base, - uint8_t identifier[kMDGUIDSize]); + static bool ElfFileIdentifierFromMappedFile( + const void* base, + wasteful_vector& identifier); - // Convert the |identifier| data to a NULL terminated string. The string will - // be formatted as a UUID (e.g., 22F065BB-FC9C-49F7-80FE-26A7CEBD7BCE). - // The |buffer| should be at least 37 bytes long to receive all of the data - // and termination. Shorter buffers will contain truncated data. - static void ConvertIdentifierToString(const uint8_t identifier[kMDGUIDSize], - char* buffer, int buffer_length); + // Convert the |identifier| data to a string. The string will + // be formatted as a UUID in all uppercase without dashes. + // (e.g., 22F065BBFC9C49F780FE26A7CEBD7BCE). + static std::string ConvertIdentifierToUUIDString( + const wasteful_vector& identifier); private: // Storage for the path specified diff --git a/src/common/linux/file_id_unittest.cc b/src/common/linux/file_id_unittest.cc index 760eae82..ec11bb25 100644 --- a/src/common/linux/file_id_unittest.cc +++ b/src/common/linux/file_id_unittest.cc @@ -33,6 +33,7 @@ #include #include +#include #include "common/linux/elf_gnu_compat.h" #include "common/linux/elfutils.h" @@ -45,13 +46,11 @@ #include "breakpad_googletest_includes.h" using namespace google_breakpad; -using google_breakpad::ElfClass32; -using google_breakpad::ElfClass64; -using google_breakpad::SafeReadLink; using google_breakpad::synth_elf::ELF; using google_breakpad::synth_elf::Notes; using google_breakpad::test_assembler::kLittleEndian; using google_breakpad::test_assembler::Section; +using std::vector; using ::testing::Types; namespace { @@ -64,6 +63,8 @@ void PopulateSection(Section* section, int size, int prime_number) { section->Append(1, (i % prime_number) % 256); } +typedef wasteful_vector id_vector; + } // namespace #ifndef __ANDROID__ @@ -87,19 +88,20 @@ TEST(FileIDStripTest, StripSelf) { sprintf(cmdline, "strip \"%s\"", templ.c_str()); ASSERT_EQ(0, system(cmdline)) << "Failed to execute: " << cmdline; - uint8_t identifier1[sizeof(MDGUID)]; - uint8_t identifier2[sizeof(MDGUID)]; + PageAllocator allocator; + id_vector identifier1(&allocator, kDefaultBuildIdSize); + id_vector identifier2(&allocator, kDefaultBuildIdSize); + FileID fileid1(exe_name); EXPECT_TRUE(fileid1.ElfFileIdentifier(identifier1)); FileID fileid2(templ.c_str()); EXPECT_TRUE(fileid2.ElfFileIdentifier(identifier2)); - char identifier_string1[37]; - char identifier_string2[37]; - FileID::ConvertIdentifierToString(identifier1, identifier_string1, - 37); - FileID::ConvertIdentifierToString(identifier2, identifier_string2, - 37); - EXPECT_STREQ(identifier_string1, identifier_string2); + + string identifier_string1 = + FileID::ConvertIdentifierToUUIDString(identifier1); + string identifier_string2 = + FileID::ConvertIdentifierToUUIDString(identifier2); + EXPECT_EQ(identifier_string1, identifier_string2); } #endif // !__ANDROID__ @@ -116,8 +118,22 @@ public: elfdata = &elfdata_v[0]; } + id_vector make_vector() { + return id_vector(&allocator, kDefaultBuildIdSize); + } + + template + string get_file_id(const uint8_t (&data)[N]) { + id_vector expected_identifier(make_vector()); + expected_identifier.insert(expected_identifier.end(), + &data[0], + data + N); + return FileID::ConvertIdentifierToUUIDString(expected_identifier); + } + vector elfdata_v; uint8_t* elfdata; + PageAllocator allocator; }; typedef Types ElfClasses; @@ -125,10 +141,8 @@ typedef Types ElfClasses; TYPED_TEST_CASE(FileIDTest, ElfClasses); TYPED_TEST(FileIDTest, ElfClass) { - uint8_t identifier[sizeof(MDGUID)]; const char expected_identifier_string[] = - "80808080-8080-0000-0000-008080808080"; - char identifier_string[sizeof(expected_identifier_string)]; + "80808080808000000000008080808080"; const size_t kTextSectionSize = 128; ELF elf(EM_386, TypeParam::kClass, kLittleEndian); @@ -140,58 +154,106 @@ TYPED_TEST(FileIDTest, ElfClass) { elf.Finish(); this->GetElfContents(elf); + id_vector identifier(this->make_vector()); EXPECT_TRUE(FileID::ElfFileIdentifierFromMappedFile(this->elfdata, identifier)); - FileID::ConvertIdentifierToString(identifier, identifier_string, - sizeof(identifier_string)); - EXPECT_STREQ(expected_identifier_string, identifier_string); + string identifier_string = FileID::ConvertIdentifierToUUIDString(identifier); + EXPECT_EQ(expected_identifier_string, identifier_string); } TYPED_TEST(FileIDTest, BuildID) { - const uint8_t kExpectedIdentifier[sizeof(MDGUID)] = + const uint8_t kExpectedIdentifierBytes[] = {0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, - 0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F}; - char expected_identifier_string[] = - "00000000-0000-0000-0000-000000000000"; - FileID::ConvertIdentifierToString(kExpectedIdentifier, - expected_identifier_string, - sizeof(expected_identifier_string)); - - uint8_t identifier[sizeof(MDGUID)]; - char identifier_string[sizeof(expected_identifier_string)]; + 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 notes(kLittleEndian); - notes.AddNote(NT_GNU_BUILD_ID, "GNU", kExpectedIdentifier, - sizeof(kExpectedIdentifier)); + notes.AddNote(NT_GNU_BUILD_ID, "GNU", kExpectedIdentifierBytes, + sizeof(kExpectedIdentifierBytes)); elf.AddSection(".note.gnu.build-id", notes, SHT_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()); - FileID::ConvertIdentifierToString(identifier, identifier_string, - sizeof(identifier_string)); - EXPECT_STREQ(expected_identifier_string, identifier_string); + string identifier_string = FileID::ConvertIdentifierToUUIDString(identifier); + EXPECT_EQ(expected_identifier_string, identifier_string); +} + +// Test that a build id note with fewer bytes than usual is handled. +TYPED_TEST(FileIDTest, BuildIDShort) { + const uint8_t kExpectedIdentifierBytes[] = + {0x00, 0x01, 0x02, 0x03}; + 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 notes(kLittleEndian); + notes.AddNote(NT_GNU_BUILD_ID, "GNU", kExpectedIdentifierBytes, + sizeof(kExpectedIdentifierBytes)); + elf.AddSection(".note.gnu.build-id", notes, SHT_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 that a build id note with more bytes than usual is handled. +TYPED_TEST(FileIDTest, BuildIDLong) { + const uint8_t kExpectedIdentifierBytes[] = + {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 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 notes(kLittleEndian); + notes.AddNote(NT_GNU_BUILD_ID, "GNU", kExpectedIdentifierBytes, + sizeof(kExpectedIdentifierBytes)); + elf.AddSection(".note.gnu.build-id", notes, SHT_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); } TYPED_TEST(FileIDTest, BuildIDPH) { - const uint8_t kExpectedIdentifier[sizeof(MDGUID)] = + const uint8_t kExpectedIdentifierBytes[] = {0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, - 0x08, 0x09, 0x0A, 0x0B, 0x0C, 0x0D, 0x0E, 0x0F}; - char expected_identifier_string[] = - "00000000-0000-0000-0000-000000000000"; - FileID::ConvertIdentifierToString(kExpectedIdentifier, - expected_identifier_string, - sizeof(expected_identifier_string)); - - uint8_t identifier[sizeof(MDGUID)]; - char identifier_string[sizeof(expected_identifier_string)]; + 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); @@ -200,31 +262,25 @@ TYPED_TEST(FileIDTest, BuildIDPH) { Notes notes(kLittleEndian); notes.AddNote(0, "Linux", reinterpret_cast("\0x42\0x02\0\0"), 4); - notes.AddNote(NT_GNU_BUILD_ID, "GNU", kExpectedIdentifier, - sizeof(kExpectedIdentifier)); + notes.AddNote(NT_GNU_BUILD_ID, "GNU", kExpectedIdentifierBytes, + sizeof(kExpectedIdentifierBytes)); int note_idx = elf.AddSection(".note", notes, SHT_NOTE); elf.AddSegment(note_idx, note_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()); - FileID::ConvertIdentifierToString(identifier, identifier_string, - sizeof(identifier_string)); - EXPECT_STREQ(expected_identifier_string, identifier_string); + 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) { - char identifier_string_1[] = - "00000000-0000-0000-0000-000000000000"; - char identifier_string_2[] = - "00000000-0000-0000-0000-000000000000"; - uint8_t identifier_1[sizeof(MDGUID)]; - uint8_t identifier_2[sizeof(MDGUID)]; - { ELF elf1(EM_386, TypeParam::kClass, kLittleEndian); Section foo_1(kLittleEndian); @@ -237,10 +293,11 @@ TYPED_TEST(FileIDTest, UniqueHashes) { this->GetElfContents(elf1); } + id_vector identifier_1(this->make_vector()); EXPECT_TRUE(FileID::ElfFileIdentifierFromMappedFile(this->elfdata, identifier_1)); - FileID::ConvertIdentifierToString(identifier_1, identifier_string_1, - sizeof(identifier_string_1)); + string identifier_string_1 = + FileID::ConvertIdentifierToUUIDString(identifier_1); { ELF elf2(EM_386, TypeParam::kClass, kLittleEndian); @@ -254,10 +311,11 @@ TYPED_TEST(FileIDTest, UniqueHashes) { this->GetElfContents(elf2); } + id_vector identifier_2(this->make_vector()); EXPECT_TRUE(FileID::ElfFileIdentifierFromMappedFile(this->elfdata, identifier_2)); - FileID::ConvertIdentifierToString(identifier_2, identifier_string_2, - sizeof(identifier_string_2)); + string identifier_string_2 = + FileID::ConvertIdentifierToUUIDString(identifier_2); - EXPECT_STRNE(identifier_string_1, identifier_string_2); + EXPECT_NE(identifier_string_1, identifier_string_2); } diff --git a/src/common/memory.h b/src/common/memory.h index d6aa137d..16a612b8 100644 --- a/src/common/memory.h +++ b/src/common/memory.h @@ -64,7 +64,8 @@ class PageAllocator { : page_size_(getpagesize()), last_(NULL), current_page_(NULL), - page_offset_(0) { + page_offset_(0), + pages_allocated_(0) { } ~PageAllocator() { @@ -112,6 +113,8 @@ class PageAllocator { return false; } + unsigned long pages_allocated() { return pages_allocated_; } + private: uint8_t *GetNPages(size_t num_pages) { #if defined(__x86_64__) || defined(__aarch64__) || defined(__aarch64__) || \ @@ -136,6 +139,8 @@ class PageAllocator { header->num_pages = num_pages; last_ = header; + pages_allocated_ += num_pages; + return reinterpret_cast(a); } @@ -157,6 +162,7 @@ class PageAllocator { PageHeader *last_; uint8_t *current_page_; size_t page_offset_; + unsigned long pages_allocated_; }; // Wrapper to use with STL containers @@ -165,12 +171,30 @@ struct PageStdAllocator : public std::allocator { typedef typename std::allocator::pointer pointer; typedef typename std::allocator::size_type size_type; - explicit PageStdAllocator(PageAllocator& allocator): allocator_(allocator) {} + explicit PageStdAllocator(PageAllocator& allocator) : allocator_(allocator), + stackdata_(NULL), + stackdata_size_(0) + {} + template PageStdAllocator(const PageStdAllocator& other) - : allocator_(other.allocator_) {} + : allocator_(other.allocator_), + stackdata_(nullptr), + stackdata_size_(0) + {} + + explicit PageStdAllocator(PageAllocator& allocator, + pointer stackdata, + size_type stackdata_size) : allocator_(allocator), + stackdata_(stackdata), + stackdata_size_(stackdata_size) + {} inline pointer allocate(size_type n, const void* = 0) { - return static_cast(allocator_.Alloc(sizeof(T) * n)); + const size_type size = sizeof(T) * n; + if (size <= stackdata_size_) { + return stackdata_; + } + return static_cast(allocator_.Alloc(size)); } inline void deallocate(pointer, size_type) { @@ -188,6 +212,8 @@ struct PageStdAllocator : public std::allocator { template friend struct PageStdAllocator; PageAllocator& allocator_; + pointer stackdata_; + size_type stackdata_size_; }; // A wasteful vector is a std::vector, except that it allocates memory from a @@ -200,6 +226,24 @@ class wasteful_vector : public std::vector > { : std::vector >(PageStdAllocator(*allocator)) { std::vector >::reserve(size_hint); } + protected: + wasteful_vector(PageStdAllocator allocator) + : std::vector >(allocator) {} +}; + +// auto_wasteful_vector allocates space on the stack for N entries to avoid +// using the PageAllocator for small data, while still allowing for larger data. +template +class auto_wasteful_vector : public wasteful_vector { + T stackdata_[N]; + public: + auto_wasteful_vector(PageAllocator* allocator) + : wasteful_vector( + PageStdAllocator(*allocator, + &stackdata_[0], + sizeof(stackdata_))) { + std::vector >::reserve(N); + } }; } // namespace google_breakpad diff --git a/src/common/memory_unittest.cc b/src/common/memory_unittest.cc index 1e511ca5..b271bc3c 100644 --- a/src/common/memory_unittest.cc +++ b/src/common/memory_unittest.cc @@ -38,11 +38,13 @@ typedef testing::Test PageAllocatorTest; TEST(PageAllocatorTest, Setup) { PageAllocator allocator; + EXPECT_EQ(0, allocator.pages_allocated()); } TEST(PageAllocatorTest, SmallObjects) { PageAllocator allocator; + EXPECT_EQ(0, allocator.pages_allocated()); for (unsigned i = 1; i < 1024; ++i) { uint8_t *p = reinterpret_cast(allocator.Alloc(i)); ASSERT_FALSE(p == NULL); @@ -53,8 +55,10 @@ TEST(PageAllocatorTest, SmallObjects) { TEST(PageAllocatorTest, LargeObject) { PageAllocator allocator; + EXPECT_EQ(0, allocator.pages_allocated()); uint8_t *p = reinterpret_cast(allocator.Alloc(10000)); ASSERT_FALSE(p == NULL); + EXPECT_EQ(3, allocator.pages_allocated()); for (unsigned i = 1; i < 10; ++i) { uint8_t *p = reinterpret_cast(allocator.Alloc(i)); ASSERT_FALSE(p == NULL); @@ -75,6 +79,7 @@ TEST(WastefulVectorTest, Setup) { TEST(WastefulVectorTest, Simple) { PageAllocator allocator_; + EXPECT_EQ(0, allocator_.pages_allocated()); wasteful_vector v(&allocator_); for (unsigned i = 0; i < 256; ++i) { @@ -84,6 +89,7 @@ TEST(WastefulVectorTest, Simple) { } ASSERT_FALSE(v.empty()); ASSERT_EQ(v.size(), 256u); + EXPECT_EQ(1, allocator_.pages_allocated()); for (unsigned i = 0; i < 256; ++i) ASSERT_EQ(v[i], i); } @@ -91,7 +97,28 @@ TEST(WastefulVectorTest, Simple) { TEST(WastefulVectorTest, UsesPageAllocator) { PageAllocator allocator_; wasteful_vector v(&allocator_); + EXPECT_EQ(1, allocator_.pages_allocated()); v.push_back(1); ASSERT_TRUE(allocator_.OwnsPointer(&v[0])); } + +TEST(WastefulVectorTest, AutoWastefulVector) { + PageAllocator allocator_; + EXPECT_EQ(0, allocator_.pages_allocated()); + + auto_wasteful_vector v(&allocator_); + EXPECT_EQ(0, allocator_.pages_allocated()); + + v.push_back(1); + EXPECT_EQ(0, allocator_.pages_allocated()); + EXPECT_FALSE(allocator_.OwnsPointer(&v[0])); + + v.resize(4); + EXPECT_EQ(0, allocator_.pages_allocated()); + EXPECT_FALSE(allocator_.OwnsPointer(&v[0])); + + v.resize(10); + EXPECT_EQ(1, allocator_.pages_allocated()); + EXPECT_TRUE(allocator_.OwnsPointer(&v[0])); +} diff --git a/src/processor/minidump_unittest.cc b/src/processor/minidump_unittest.cc index 4c5a7b2f..d29e9f4e 100644 --- a/src/processor/minidump_unittest.cc +++ b/src/processor/minidump_unittest.cc @@ -625,6 +625,80 @@ TEST(Dump, CVELFShort) { ASSERT_EQ("B4CDA95F0000000000000000000000000", md_module->debug_identifier()); } +// Test that a build_id that's very long is handled properly. +TEST(Dump, CVELFLong) { + Dump dump(0, kLittleEndian); + String module_name(dump, "elf module"); + Section cv_info(dump); + cv_info + .D32(MD_CVINFOELF_SIGNATURE) // signature + // build_id, lots of bytes + .Append("\x5f\xa9\xcd\xb4\x10\x53\xdf\x1b\x86\xfa\xb7\x33\xb4\xdf" + "\x37\x38\xce\xa3\x4a\x87\x01\x02\x03\x04\x05\x06\x07\x08" + "\x09\x0a\x0b\x0c\x0d\x0e\x0f"); + + + const MDRawSystemInfo linux_x86 = { + MD_CPU_ARCHITECTURE_X86, // processor_architecture + 6, // processor_level + 0xd08, // processor_revision + 1, // number_of_processors + 0, // product_type + 0, // major_version + 0, // minor_version + 0, // build_number + MD_OS_LINUX, // platform_id + 0xdeadbeef, // csd_version_rva + 0x100, // suite_mask + 0, // reserved2 + { // cpu + { // x86_cpu_info + { 0x756e6547, 0x49656e69, 0x6c65746e }, // vendor_id + 0x6d8, // version_information + 0xafe9fbff, // feature_information + 0xffffffff // amd_extended_cpu_features + } + } + }; + String csd_version(dump, "Literally Linux"); + SystemInfo system_info(dump, linux_x86, csd_version); + + Module module(dump, 0xa90206ca83eb2852ULL, 0xada542bd, + module_name, + 0xb1054d2a, + 0x34571371, + fixed_file_info, // from synth_minidump_unittest_data.h + &cv_info, nullptr); + + dump.Add(&module); + dump.Add(&module_name); + dump.Add(&cv_info); + dump.Add(&system_info); + dump.Add(&csd_version); + dump.Finish(); + + string contents; + ASSERT_TRUE(dump.GetContents(&contents)); + istringstream minidump_stream(contents); + Minidump minidump(minidump_stream); + ASSERT_TRUE(minidump.Read()); + ASSERT_EQ(2U, minidump.GetDirectoryEntryCount()); + + MinidumpModuleList *md_module_list = minidump.GetModuleList(); + ASSERT_TRUE(md_module_list != NULL); + ASSERT_EQ(1U, md_module_list->module_count()); + + const MinidumpModule *md_module = md_module_list->GetModuleAtIndex(0); + ASSERT_TRUE(md_module != NULL); + // just the build_id, directly + ASSERT_EQ( + "5fa9cdb41053df1b86fab733b4df3738cea34a870102030405060708090a0b0c0d0e0f", + md_module->code_identifier()); + // build_id truncated to GUID length and treated as such, with zero + // age appended. + ASSERT_EQ("B4CDA95F53101BDF86FAB733B4DF37380", md_module->debug_identifier()); +} + TEST(Dump, OneSystemInfo) { Dump dump(0, kLittleEndian); String csd_version(dump, "Petulant Pierogi");