Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 9e484bdb authored by Christopher Ferris's avatar Christopher Ferris
Browse files

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
parent 5d89e6c7

libunwindstack/DwarfEhFrame.cpp

+2 −2
Original line number Diff line number Diff line
@@ -100,7 +100,7 @@ const typename DwarfEhFrame<AddressType>::FdeInfo* DwarfEhFrame<AddressType>::Ge 
    fde_info_.erase(index);

    return nullptr;

  }

  info->pc = value;

  info->pc = value + 4;

  return info;

}
@@ -175,7 +175,7 @@ bool DwarfEhFrame<AddressType>::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) {

libunwindstack/DwarfMemory.cpp

+1 −1
Original line number Diff line number Diff line
@@ -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.

libunwindstack/tests/DwarfEhFrameTest.cpp

+8 −8
Original line number Diff line number Diff line
@@ -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);

}

libunwindstack/tests/DwarfMemoryTest.cpp

+22 −0
Original line number Diff line number Diff line
@@ -52,6 +52,8 @@ class DwarfMemoryTest : public ::testing::Test { 
  void ReadEncodedValue_non_zero_adjust();

  template <typename AddressType>

  void ReadEncodedValue_overflow();

  template <typename AddressType>

  void ReadEncodedValue_high_bit_set();



  MemoryFake memory_;

  std::unique_ptr<DwarfMemory> dwarf_mem_;
@@ -435,6 +437,26 @@ TEST_F(DwarfMemoryTest, ReadEncodedValue_overflow_uint64_t) { 
  ReadEncodedValue_overflow<uint64_t>();

}



template <typename AddressType>

void DwarfMemoryTest::ReadEncodedValue_high_bit_set() {

  uint64_t value;

  memory_.SetData32(0, 0x15234);

  ASSERT_FALSE(dwarf_mem_->ReadEncodedValue<AddressType>(0xc3, &value));



  dwarf_mem_->set_func_offset(0x60000);

  dwarf_mem_->set_cur_offset(0);

  ASSERT_TRUE(dwarf_mem_->ReadEncodedValue<AddressType>(0xc3, &value));

  ASSERT_EQ(0x75234U, value);

}



TEST_F(DwarfMemoryTest, ReadEncodedValue_high_bit_set_uint32_t) {

  ReadEncodedValue_high_bit_set<uint32_t>();

}



TEST_F(DwarfMemoryTest, ReadEncodedValue_high_bit_set_uint64_t) {

  ReadEncodedValue_high_bit_set<uint64_t>();

}



TEST_F(DwarfMemoryTest, AdjustEncodedValue_absptr) {

  uint64_t value = 0x1234;

  ASSERT_TRUE(dwarf_mem_->AdjustEncodedValue(0x00, &value));