From 9e484bdb4ac24841ce8956dd9f98c33a4be4c037 Mon Sep 17 00:00:00 2001 From: Christopher Ferris Date: Thu, 10 Aug 2017 17:37:32 -0700 Subject: [PATCH] Fix another set of bugs. - The pc read from the eh frame binary table of pc/fde offset is off by 4. I verified that on arm/arm64/x86/x86_64 the pc in this table matches the fde pc_start value. I did this by adding an error if this occurred and ran unwind_info over everything in system/lib, system/lib64, system/bin. - Fixed unit tests for the above change. - Fix a small bug in the processing encoded values. The high bit of the encoding should be masked off, but I wasn't doing that. That meant during processing of the fde, I was incorrectly returning an error because the encoded value was unknown. - Added a new test for this encoding change. Bug: 23762183 Test: Build and all unit tests pass. Also, see above comments. Change-Id: If074a410a1726392274cd72c64470ca0be48e0db --- libunwindstack/DwarfEhFrame.cpp | 4 ++-- libunwindstack/DwarfMemory.cpp | 2 +- libunwindstack/tests/DwarfEhFrameTest.cpp | 16 ++++++++-------- libunwindstack/tests/DwarfMemoryTest.cpp | 22 ++++++++++++++++++++++ 4 files changed, 33 insertions(+), 11 deletions(-) diff --git a/libunwindstack/DwarfEhFrame.cpp b/libunwindstack/DwarfEhFrame.cpp index d0b35c389..db8f558ff 100644 --- a/libunwindstack/DwarfEhFrame.cpp +++ b/libunwindstack/DwarfEhFrame.cpp @@ -100,7 +100,7 @@ const typename DwarfEhFrame::FdeInfo* DwarfEhFrame::Ge fde_info_.erase(index); return nullptr; } - info->pc = value; + info->pc = value + 4; return info; } @@ -175,7 +175,7 @@ bool DwarfEhFrame::GetFdeOffsetSequential(uint64_t pc, uint64_t* fd last_error_ = DWARF_ERROR_MEMORY_INVALID; return false; } - info->pc = value; + info->pc = value + 4; if (pc < info->pc) { if (prev_info == nullptr) { diff --git a/libunwindstack/DwarfMemory.cpp b/libunwindstack/DwarfMemory.cpp index b6e0412e5..901f49285 100644 --- a/libunwindstack/DwarfMemory.cpp +++ b/libunwindstack/DwarfMemory.cpp @@ -235,7 +235,7 @@ bool DwarfMemory::ReadEncodedValue(uint8_t encoding, uint64_t* value) { return false; } - return AdjustEncodedValue(encoding & 0xf0, value); + return AdjustEncodedValue(encoding & 0x70, value); } // Instantiate all of the needed template functions. diff --git a/libunwindstack/tests/DwarfEhFrameTest.cpp b/libunwindstack/tests/DwarfEhFrameTest.cpp index e9501e31c..07159b0e0 100644 --- a/libunwindstack/tests/DwarfEhFrameTest.cpp +++ b/libunwindstack/tests/DwarfEhFrameTest.cpp @@ -124,7 +124,7 @@ TYPED_TEST_P(DwarfEhFrameTest, GetFdeInfoFromIndex_read_pcrel) { auto info = this->eh_frame_->GetFdeInfoFromIndex(2); ASSERT_TRUE(info != nullptr); - EXPECT_EQ(0x1380U, info->pc); + EXPECT_EQ(0x1384U, info->pc); EXPECT_EQ(0x1540U, info->offset); } @@ -139,7 +139,7 @@ TYPED_TEST_P(DwarfEhFrameTest, GetFdeInfoFromIndex_read_datarel) { auto info = this->eh_frame_->GetFdeInfoFromIndex(2); ASSERT_TRUE(info != nullptr); - EXPECT_EQ(0x3340U, info->pc); + EXPECT_EQ(0x3344U, info->pc); EXPECT_EQ(0x3500U, info->offset); } @@ -153,7 +153,7 @@ TYPED_TEST_P(DwarfEhFrameTest, GetFdeInfoFromIndex_cached) { auto info = this->eh_frame_->GetFdeInfoFromIndex(2); ASSERT_TRUE(info != nullptr); - EXPECT_EQ(0x340U, info->pc); + EXPECT_EQ(0x344U, info->pc); EXPECT_EQ(0x500U, info->offset); // Clear the memory so that this will fail if it doesn't read cached data. @@ -161,7 +161,7 @@ TYPED_TEST_P(DwarfEhFrameTest, GetFdeInfoFromIndex_cached) { info = this->eh_frame_->GetFdeInfoFromIndex(2); ASSERT_TRUE(info != nullptr); - EXPECT_EQ(0x340U, info->pc); + EXPECT_EQ(0x344U, info->pc); EXPECT_EQ(0x500U, info->offset); } @@ -220,18 +220,18 @@ TYPED_TEST_P(DwarfEhFrameTest, GetFdeOffsetSequential) { // Verify that if entries is zero, that it fails. uint64_t fde_offset; - ASSERT_FALSE(this->eh_frame_->GetFdeOffsetSequential(0x340, &fde_offset)); + ASSERT_FALSE(this->eh_frame_->GetFdeOffsetSequential(0x344, &fde_offset)); this->eh_frame_->TestSetCurEntriesOffset(0x1040); - ASSERT_TRUE(this->eh_frame_->GetFdeOffsetSequential(0x340, &fde_offset)); + ASSERT_TRUE(this->eh_frame_->GetFdeOffsetSequential(0x344, &fde_offset)); EXPECT_EQ(0x500U, fde_offset); - ASSERT_TRUE(this->eh_frame_->GetFdeOffsetSequential(0x440, &fde_offset)); + ASSERT_TRUE(this->eh_frame_->GetFdeOffsetSequential(0x444, &fde_offset)); EXPECT_EQ(0x600U, fde_offset); // Expect that the data is cached so no more memory reads will occur. this->memory_.Clear(); - ASSERT_TRUE(this->eh_frame_->GetFdeOffsetSequential(0x440, &fde_offset)); + ASSERT_TRUE(this->eh_frame_->GetFdeOffsetSequential(0x444, &fde_offset)); EXPECT_EQ(0x600U, fde_offset); } diff --git a/libunwindstack/tests/DwarfMemoryTest.cpp b/libunwindstack/tests/DwarfMemoryTest.cpp index 08fe7cf39..f12d2fe3e 100644 --- a/libunwindstack/tests/DwarfMemoryTest.cpp +++ b/libunwindstack/tests/DwarfMemoryTest.cpp @@ -52,6 +52,8 @@ class DwarfMemoryTest : public ::testing::Test { void ReadEncodedValue_non_zero_adjust(); template void ReadEncodedValue_overflow(); + template + void ReadEncodedValue_high_bit_set(); MemoryFake memory_; std::unique_ptr dwarf_mem_; @@ -435,6 +437,26 @@ TEST_F(DwarfMemoryTest, ReadEncodedValue_overflow_uint64_t) { ReadEncodedValue_overflow(); } +template +void DwarfMemoryTest::ReadEncodedValue_high_bit_set() { + uint64_t value; + memory_.SetData32(0, 0x15234); + ASSERT_FALSE(dwarf_mem_->ReadEncodedValue(0xc3, &value)); + + dwarf_mem_->set_func_offset(0x60000); + dwarf_mem_->set_cur_offset(0); + ASSERT_TRUE(dwarf_mem_->ReadEncodedValue(0xc3, &value)); + ASSERT_EQ(0x75234U, value); +} + +TEST_F(DwarfMemoryTest, ReadEncodedValue_high_bit_set_uint32_t) { + ReadEncodedValue_high_bit_set(); +} + +TEST_F(DwarfMemoryTest, ReadEncodedValue_high_bit_set_uint64_t) { + ReadEncodedValue_high_bit_set(); +} + TEST_F(DwarfMemoryTest, AdjustEncodedValue_absptr) { uint64_t value = 0x1234; ASSERT_TRUE(dwarf_mem_->AdjustEncodedValue(0x00, &value));