diff options
Diffstat (limited to 'src/client/linux/minidump_writer')
5 files changed, 100 insertions, 74 deletions
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<uint8_t>& 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<uint8_t>& 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<uint8_t> 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<uint8_t, kDefaultBuildIdSize> 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<uint8_t> 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; |