From a46de76024ad3799739b9a6715e5b03bb8cdeac0 Mon Sep 17 00:00:00 2001 From: Christopher Ferris Date: Wed, 3 Apr 2019 14:14:30 -0700 Subject: [PATCH] Fix off by one reading build id. Update unit tests and add new build id displaying in offline unwinds. Bug: 129873279 Test: All unit tests pass. Test: Verify that debuggerd displays build id properly. Change-Id: I97f4a204842447a20c812f535a458155b937d5e1 Merged-In: I97f4a204842447a20c812f535a458155b937d5e1 (cherry picked from commit 1760b45709df4d925c3a131db0b443b585837703) --- libunwindstack/ElfInterface.cpp | 2 +- libunwindstack/tests/ElfInterfaceTest.cpp | 47 +++++++++++++--------- libunwindstack/tests/UnwindOfflineTest.cpp | 17 ++++++++ 3 files changed, 47 insertions(+), 19 deletions(-) diff --git a/libunwindstack/ElfInterface.cpp b/libunwindstack/ElfInterface.cpp index e09a2aef4..32c637f85 100644 --- a/libunwindstack/ElfInterface.cpp +++ b/libunwindstack/ElfInterface.cpp @@ -285,7 +285,7 @@ std::string ElfInterface::ReadBuildID() { if (gnu_build_id_size_ - offset < hdr.n_descsz || hdr.n_descsz == 0) { return ""; } - std::string build_id(hdr.n_descsz - 1, '\0'); + std::string build_id(hdr.n_descsz, '\0'); if (memory_->ReadFully(gnu_build_id_offset_ + offset, &build_id[0], hdr.n_descsz)) { return build_id; } diff --git a/libunwindstack/tests/ElfInterfaceTest.cpp b/libunwindstack/tests/ElfInterfaceTest.cpp index d895863bb..cdc927aba 100644 --- a/libunwindstack/tests/ElfInterfaceTest.cpp +++ b/libunwindstack/tests/ElfInterfaceTest.cpp @@ -1192,14 +1192,16 @@ void ElfInterfaceTest::BuildID() { char note_section[128]; Nhdr note_header = {}; note_header.n_namesz = 4; // "GNU" - note_header.n_descsz = 8; // "BUILDID" + note_header.n_descsz = 7; // "BUILDID" note_header.n_type = NT_GNU_BUILD_ID; memcpy(¬e_section, ¬e_header, sizeof(note_header)); size_t note_offset = sizeof(note_header); + // The note information contains the GNU and trailing '\0'. memcpy(¬e_section[note_offset], "GNU", sizeof("GNU")); note_offset += sizeof("GNU"); - memcpy(¬e_section[note_offset], "BUILDID", sizeof("BUILDID")); - note_offset += sizeof("BUILDID"); + // This part of the note does not contain any trailing '\0'. + memcpy(¬e_section[note_offset], "BUILDID", 7); + note_offset += 8; Shdr shdr = {}; shdr.sh_type = SHT_NOTE; @@ -1244,24 +1246,27 @@ void ElfInterfaceTest::BuildIDTwoNotes() { char note_section[128]; Nhdr note_header = {}; note_header.n_namesz = 8; // "WRONG" aligned to 4 - note_header.n_descsz = 8; // "BUILDID" + note_header.n_descsz = 7; // "BUILDID" note_header.n_type = NT_GNU_BUILD_ID; memcpy(¬e_section, ¬e_header, sizeof(note_header)); size_t note_offset = sizeof(note_header); memcpy(¬e_section[note_offset], "WRONG", sizeof("WRONG")); note_offset += 8; - memcpy(¬e_section[note_offset], "BUILDID", sizeof("BUILDID")); - note_offset += sizeof("BUILDID"); + // This part of the note does not contain any trailing '\0'. + memcpy(¬e_section[note_offset], "BUILDID", 7); + note_offset += 8; note_header.n_namesz = 4; // "GNU" - note_header.n_descsz = 8; // "BUILDID" + note_header.n_descsz = 7; // "BUILDID" note_header.n_type = NT_GNU_BUILD_ID; memcpy(¬e_section[note_offset], ¬e_header, sizeof(note_header)); note_offset += sizeof(note_header); + // The note information contains the GNU and trailing '\0'. memcpy(¬e_section[note_offset], "GNU", sizeof("GNU")); note_offset += sizeof("GNU"); - memcpy(¬e_section[note_offset], "BUILDID", sizeof("BUILDID")); - note_offset += sizeof("BUILDID"); + // This part of the note does not contain any trailing '\0'. + memcpy(¬e_section[note_offset], "BUILDID", 7); + note_offset += 8; Shdr shdr = {}; shdr.sh_type = SHT_NOTE; @@ -1306,14 +1311,16 @@ void ElfInterfaceTest::BuildIDSectionTooSmallForName () { char note_section[128]; Nhdr note_header = {}; note_header.n_namesz = 4; // "GNU" - note_header.n_descsz = 8; // "BUILDID" + note_header.n_descsz = 7; // "BUILDID" note_header.n_type = NT_GNU_BUILD_ID; memcpy(¬e_section, ¬e_header, sizeof(note_header)); size_t note_offset = sizeof(note_header); + // The note information contains the GNU and trailing '\0'. memcpy(¬e_section[note_offset], "GNU", sizeof("GNU")); note_offset += sizeof("GNU"); - memcpy(¬e_section[note_offset], "BUILDID", sizeof("BUILDID")); - note_offset += sizeof("BUILDID"); + // This part of the note does not contain any trailing '\0'. + memcpy(¬e_section[note_offset], "BUILDID", 7); + note_offset += 8; Shdr shdr = {}; shdr.sh_type = SHT_NOTE; @@ -1358,14 +1365,16 @@ void ElfInterfaceTest::BuildIDSectionTooSmallForDesc () { char note_section[128]; Nhdr note_header = {}; note_header.n_namesz = 4; // "GNU" - note_header.n_descsz = 8; // "BUILDID" + note_header.n_descsz = 7; // "BUILDID" note_header.n_type = NT_GNU_BUILD_ID; memcpy(¬e_section, ¬e_header, sizeof(note_header)); size_t note_offset = sizeof(note_header); + // The note information contains the GNU and trailing '\0'. memcpy(¬e_section[note_offset], "GNU", sizeof("GNU")); note_offset += sizeof("GNU"); - memcpy(¬e_section[note_offset], "BUILDID", sizeof("BUILDID")); - note_offset += sizeof("BUILDID"); + // This part of the note does not contain any trailing '\0'. + memcpy(¬e_section[note_offset], "BUILDID", 7); + note_offset += 8; Shdr shdr = {}; shdr.sh_type = SHT_NOTE; @@ -1410,14 +1419,16 @@ void ElfInterfaceTest::BuildIDSectionTooSmallForHeader () { char note_section[128]; Nhdr note_header = {}; note_header.n_namesz = 4; // "GNU" - note_header.n_descsz = 8; // "BUILDID" + note_header.n_descsz = 7; // "BUILDID" note_header.n_type = NT_GNU_BUILD_ID; memcpy(¬e_section, ¬e_header, sizeof(note_header)); size_t note_offset = sizeof(note_header); + // The note information contains the GNU and trailing '\0'. memcpy(¬e_section[note_offset], "GNU", sizeof("GNU")); note_offset += sizeof("GNU"); - memcpy(¬e_section[note_offset], "BUILDID", sizeof("BUILDID")); - note_offset += sizeof("BUILDID"); + // This part of the note does not contain any trailing '\0'. + memcpy(¬e_section[note_offset], "BUILDID", 7); + note_offset += 8; Shdr shdr = {}; shdr.sh_type = SHT_NOTE; diff --git a/libunwindstack/tests/UnwindOfflineTest.cpp b/libunwindstack/tests/UnwindOfflineTest.cpp index 0867561d5..e3c646a74 100644 --- a/libunwindstack/tests/UnwindOfflineTest.cpp +++ b/libunwindstack/tests/UnwindOfflineTest.cpp @@ -204,6 +204,7 @@ static std::string DumpFrames(Unwinder& unwinder) { TEST_F(UnwindOfflineTest, pc_straddle_arm) { ASSERT_NO_FATAL_FAILURE(Init("straddle_arm/", ARCH_ARM)); + std::unique_ptr regs_copy(regs_->Clone()); Unwinder unwinder(128, maps_.get(), regs_.get(), process_memory_); unwinder.Unwind(); @@ -223,6 +224,22 @@ TEST_F(UnwindOfflineTest, pc_straddle_arm) { EXPECT_EQ(0xe9c86730U, unwinder.frames()[2].sp); EXPECT_EQ(0xf3367147U, unwinder.frames()[3].pc); EXPECT_EQ(0xe9c86778U, unwinder.frames()[3].sp); + + // Display build ids now. + unwinder.SetRegs(regs_copy.get()); + unwinder.SetDisplayBuildID(true); + unwinder.Unwind(); + + frame_info = DumpFrames(unwinder); + ASSERT_EQ(4U, unwinder.NumFrames()) << "Unwind:\n" << frame_info; + EXPECT_EQ( + " #00 pc 0001a9f8 libc.so (abort+64) (BuildId: 2dd0d4ba881322a0edabeed94808048c)\n" + " #01 pc 00006a1b libbase.so (android::base::DefaultAborter(char const*)+6) (BuildId: " + "ed43842c239cac1a618e600ea91c4cbd)\n" + " #02 pc 00007441 libbase.so (android::base::LogMessage::~LogMessage()+748) (BuildId: " + "ed43842c239cac1a618e600ea91c4cbd)\n" + " #03 pc 00015147 /does/not/exist/libhidlbase.so\n", + frame_info); } TEST_F(UnwindOfflineTest, pc_in_gnu_debugdata_arm) {