diff --git a/libunwindstack/ElfInterface.cpp b/libunwindstack/ElfInterface.cpp index 915cddb08..2c0045691 100644 --- a/libunwindstack/ElfInterface.cpp +++ b/libunwindstack/ElfInterface.cpp @@ -204,49 +204,19 @@ bool ElfInterface::ReadProgramHeaders(const EhdrType& ehdr, uint64_t* load_bias) uint64_t offset = ehdr.e_phoff; for (size_t i = 0; i < ehdr.e_phnum; i++, offset += ehdr.e_phentsize) { PhdrType phdr; - if (!memory_->ReadField(offset, &phdr, &phdr.p_type, sizeof(phdr.p_type))) { + if (!memory_->ReadFully(offset, &phdr, sizeof(phdr))) { last_error_.code = ERROR_MEMORY_INVALID; - last_error_.address = - offset + reinterpret_cast(&phdr.p_type) - reinterpret_cast(&phdr); + last_error_.address = offset; return false; } - if (HandleType(offset, phdr.p_type)) { - continue; - } - switch (phdr.p_type) { case PT_LOAD: { - // Get the flags first, if this isn't an executable header, ignore it. - if (!memory_->ReadField(offset, &phdr, &phdr.p_flags, sizeof(phdr.p_flags))) { - last_error_.code = ERROR_MEMORY_INVALID; - last_error_.address = offset + reinterpret_cast(&phdr.p_flags) - - reinterpret_cast(&phdr); - return false; - } if ((phdr.p_flags & PF_X) == 0) { continue; } - if (!memory_->ReadField(offset, &phdr, &phdr.p_vaddr, sizeof(phdr.p_vaddr))) { - last_error_.code = ERROR_MEMORY_INVALID; - last_error_.address = offset + reinterpret_cast(&phdr.p_vaddr) - - reinterpret_cast(&phdr); - return false; - } - if (!memory_->ReadField(offset, &phdr, &phdr.p_offset, sizeof(phdr.p_offset))) { - last_error_.code = ERROR_MEMORY_INVALID; - last_error_.address = offset + reinterpret_cast(&phdr.p_offset) - - reinterpret_cast(&phdr); - return false; - } - if (!memory_->ReadField(offset, &phdr, &phdr.p_memsz, sizeof(phdr.p_memsz))) { - last_error_.code = ERROR_MEMORY_INVALID; - last_error_.address = offset + reinterpret_cast(&phdr.p_memsz) - - reinterpret_cast(&phdr); - return false; - } pt_loads_[phdr.p_offset] = LoadInfo{phdr.p_offset, phdr.p_vaddr, static_cast(phdr.p_memsz)}; if (phdr.p_offset == 0) { @@ -256,46 +226,20 @@ bool ElfInterface::ReadProgramHeaders(const EhdrType& ehdr, uint64_t* load_bias) } case PT_GNU_EH_FRAME: - if (!memory_->ReadField(offset, &phdr, &phdr.p_offset, sizeof(phdr.p_offset))) { - last_error_.code = ERROR_MEMORY_INVALID; - last_error_.address = offset + reinterpret_cast(&phdr.p_offset) - - reinterpret_cast(&phdr); - return false; - } // This is really the pointer to the .eh_frame_hdr section. eh_frame_hdr_offset_ = phdr.p_offset; - if (!memory_->ReadField(offset, &phdr, &phdr.p_memsz, sizeof(phdr.p_memsz))) { - last_error_.code = ERROR_MEMORY_INVALID; - last_error_.address = offset + reinterpret_cast(&phdr.p_memsz) - - reinterpret_cast(&phdr); - return false; - } eh_frame_hdr_size_ = phdr.p_memsz; break; case PT_DYNAMIC: - if (!memory_->ReadField(offset, &phdr, &phdr.p_offset, sizeof(phdr.p_offset))) { - last_error_.code = ERROR_MEMORY_INVALID; - last_error_.address = offset + reinterpret_cast(&phdr.p_offset) - - reinterpret_cast(&phdr); - return false; - } dynamic_offset_ = phdr.p_offset; - if (!memory_->ReadField(offset, &phdr, &phdr.p_vaddr, sizeof(phdr.p_vaddr))) { - last_error_.code = ERROR_MEMORY_INVALID; - last_error_.address = offset + reinterpret_cast(&phdr.p_vaddr) - - reinterpret_cast(&phdr); - return false; - } dynamic_vaddr_ = phdr.p_vaddr; - if (!memory_->ReadField(offset, &phdr, &phdr.p_memsz, sizeof(phdr.p_memsz))) { - last_error_.code = ERROR_MEMORY_INVALID; - last_error_.address = offset + reinterpret_cast(&phdr.p_memsz) - - reinterpret_cast(&phdr); - return false; - } dynamic_size_ = phdr.p_memsz; break; + + default: + HandleUnknownType(phdr.p_type, phdr.p_offset, phdr.p_filesz); + break; } } return true; @@ -313,8 +257,7 @@ bool ElfInterface::ReadSectionHeaders(const EhdrType& ehdr) { ShdrType shdr; if (ehdr.e_shstrndx < ehdr.e_shnum) { uint64_t sh_offset = offset + ehdr.e_shstrndx * ehdr.e_shentsize; - if (memory_->ReadField(sh_offset, &shdr, &shdr.sh_offset, sizeof(shdr.sh_offset)) && - memory_->ReadField(sh_offset, &shdr, &shdr.sh_size, sizeof(shdr.sh_size))) { + if (memory_->ReadFully(sh_offset, &shdr, sizeof(shdr))) { sec_offset = shdr.sh_offset; sec_size = shdr.sh_size; } diff --git a/libunwindstack/ElfInterfaceArm.cpp b/libunwindstack/ElfInterfaceArm.cpp index a3244e824..3dd5d54fe 100644 --- a/libunwindstack/ElfInterfaceArm.cpp +++ b/libunwindstack/ElfInterfaceArm.cpp @@ -87,23 +87,17 @@ bool ElfInterfaceArm::GetPrel31Addr(uint32_t offset, uint32_t* addr) { #define PT_ARM_EXIDX 0x70000001 #endif -bool ElfInterfaceArm::HandleType(uint64_t offset, uint32_t type) { +void ElfInterfaceArm::HandleUnknownType(uint32_t type, uint64_t ph_offset, uint64_t ph_filesz) { if (type != PT_ARM_EXIDX) { - return false; - } - - Elf32_Phdr phdr; - if (!memory_->ReadFully(offset, &phdr, sizeof(phdr))) { - return true; + return; } // The offset already takes into account the load bias. - start_offset_ = phdr.p_offset; + start_offset_ = ph_offset; // Always use filesz instead of memsz. In most cases they are the same, // but some shared libraries wind up setting one correctly and not the other. - total_entries_ = phdr.p_filesz / 8; - return true; + total_entries_ = ph_filesz / 8; } bool ElfInterfaceArm::Step(uint64_t pc, Regs* regs, Memory* process_memory, bool* finished) { diff --git a/libunwindstack/ElfInterfaceArm.h b/libunwindstack/ElfInterfaceArm.h index 3bee9cff1..4c3a0c347 100644 --- a/libunwindstack/ElfInterfaceArm.h +++ b/libunwindstack/ElfInterfaceArm.h @@ -70,7 +70,7 @@ class ElfInterfaceArm : public ElfInterface32 { bool FindEntry(uint32_t pc, uint64_t* entry_offset); - bool HandleType(uint64_t offset, uint32_t type) override; + void HandleUnknownType(uint32_t type, uint64_t ph_offset, uint64_t ph_filesz) override; bool Step(uint64_t pc, Regs* regs, Memory* process_memory, bool* finished) override; diff --git a/libunwindstack/include/unwindstack/ElfInterface.h b/libunwindstack/include/unwindstack/ElfInterface.h index 0c588da7d..5c1210d68 100644 --- a/libunwindstack/include/unwindstack/ElfInterface.h +++ b/libunwindstack/include/unwindstack/ElfInterface.h @@ -118,7 +118,7 @@ class ElfInterface { template bool GetGlobalVariableWithTemplate(const std::string& name, uint64_t* memory_address); - virtual bool HandleType(uint64_t, uint32_t) { return false; } + virtual void HandleUnknownType(uint32_t, uint64_t, uint64_t) {} template static void GetMaxSizeWithTemplate(Memory* memory, uint64_t* size); diff --git a/libunwindstack/include/unwindstack/Memory.h b/libunwindstack/include/unwindstack/Memory.h index c0c07f4f9..dee5e983c 100644 --- a/libunwindstack/include/unwindstack/Memory.h +++ b/libunwindstack/include/unwindstack/Memory.h @@ -41,18 +41,6 @@ class Memory { bool ReadFully(uint64_t addr, void* dst, size_t size); - inline bool ReadField(uint64_t addr, void* start, void* field, size_t size) { - if (reinterpret_cast(field) < reinterpret_cast(start)) { - return false; - } - uint64_t offset = reinterpret_cast(field) - reinterpret_cast(start); - if (__builtin_add_overflow(addr, offset, &offset)) { - return false; - } - // The read will check if offset + size overflows. - return ReadFully(offset, field, size); - } - inline bool Read32(uint64_t addr, uint32_t* dst) { return ReadFully(addr, dst, sizeof(uint32_t)); } diff --git a/libunwindstack/tests/ElfInterfaceArmTest.cpp b/libunwindstack/tests/ElfInterfaceArmTest.cpp index a8bb4aaf4..43c6a9734 100644 --- a/libunwindstack/tests/ElfInterfaceArmTest.cpp +++ b/libunwindstack/tests/ElfInterfaceArmTest.cpp @@ -242,44 +242,21 @@ TEST_F(ElfInterfaceArmTest, iterate) { ASSERT_EQ(0xa020U, entries[4]); } -TEST_F(ElfInterfaceArmTest, HandleType_not_arm_exidx) { +TEST_F(ElfInterfaceArmTest, HandleUnknownType_arm_exidx) { ElfInterfaceArmFake interface(&memory_); - ASSERT_FALSE(interface.HandleType(0x1000, PT_NULL)); - ASSERT_FALSE(interface.HandleType(0x1000, PT_LOAD)); - ASSERT_FALSE(interface.HandleType(0x1000, PT_DYNAMIC)); - ASSERT_FALSE(interface.HandleType(0x1000, PT_INTERP)); - ASSERT_FALSE(interface.HandleType(0x1000, PT_NOTE)); - ASSERT_FALSE(interface.HandleType(0x1000, PT_SHLIB)); - ASSERT_FALSE(interface.HandleType(0x1000, PT_PHDR)); - ASSERT_FALSE(interface.HandleType(0x1000, PT_TLS)); - ASSERT_FALSE(interface.HandleType(0x1000, PT_LOOS)); - ASSERT_FALSE(interface.HandleType(0x1000, PT_HIOS)); - ASSERT_FALSE(interface.HandleType(0x1000, PT_LOPROC)); - ASSERT_FALSE(interface.HandleType(0x1000, PT_HIPROC)); - ASSERT_FALSE(interface.HandleType(0x1000, PT_GNU_EH_FRAME)); - ASSERT_FALSE(interface.HandleType(0x1000, PT_GNU_STACK)); -} - -TEST_F(ElfInterfaceArmTest, HandleType_arm_exidx) { - ElfInterfaceArmFake interface(&memory_); - - Elf32_Phdr phdr = {}; interface.FakeSetStartOffset(0x1000); interface.FakeSetTotalEntries(100); - phdr.p_offset = 0x2000; - phdr.p_filesz = 0xa00; - // Verify that if reads fail, we don't set the values but still get true. - ASSERT_TRUE(interface.HandleType(0x1000, 0x70000001)); + // Verify that if the type is not the one we want, we don't set the values. + interface.HandleUnknownType(0x70000000, 0x2000, 320); ASSERT_EQ(0x1000U, interface.start_offset()); ASSERT_EQ(100U, interface.total_entries()); // Everything is correct and present. - memory_.SetMemory(0x1000, &phdr, sizeof(phdr)); - ASSERT_TRUE(interface.HandleType(0x1000, 0x70000001)); + interface.HandleUnknownType(0x70000001, 0x2000, 320); ASSERT_EQ(0x2000U, interface.start_offset()); - ASSERT_EQ(320U, interface.total_entries()); + ASSERT_EQ(40U, interface.total_entries()); } TEST_F(ElfInterfaceArmTest, StepExidx) { diff --git a/libunwindstack/tests/MemoryTest.cpp b/libunwindstack/tests/MemoryTest.cpp index 4a9ed9f58..365598499 100644 --- a/libunwindstack/tests/MemoryTest.cpp +++ b/libunwindstack/tests/MemoryTest.cpp @@ -51,40 +51,6 @@ struct FakeStruct { uint64_t four; }; -TEST(MemoryTest, read_field) { - MemoryFakeAlwaysReadZero memory; - - FakeStruct data; - memset(&data, 0xff, sizeof(data)); - ASSERT_TRUE(memory.ReadField(0, &data, &data.one, sizeof(data.one))); - ASSERT_EQ(0, data.one); - - memset(&data, 0xff, sizeof(data)); - ASSERT_TRUE(memory.ReadField(0, &data, &data.two, sizeof(data.two))); - ASSERT_FALSE(data.two); - - memset(&data, 0xff, sizeof(data)); - ASSERT_TRUE(memory.ReadField(0, &data, &data.three, sizeof(data.three))); - ASSERT_EQ(0U, data.three); - - memset(&data, 0xff, sizeof(data)); - ASSERT_TRUE(memory.ReadField(0, &data, &data.four, sizeof(data.four))); - ASSERT_EQ(0U, data.four); -} - -TEST(MemoryTest, read_field_fails) { - MemoryFakeAlwaysReadZero memory; - - FakeStruct data; - memset(&data, 0xff, sizeof(data)); - - ASSERT_FALSE(memory.ReadField(UINT64_MAX, &data, &data.three, sizeof(data.three))); - - // Field and start reversed, should fail. - ASSERT_FALSE(memory.ReadField(100, &data.two, &data, sizeof(data.two))); - ASSERT_FALSE(memory.ReadField(0, &data.two, &data, sizeof(data.two))); -} - TEST(MemoryTest, read_string) { std::string name("string_in_memory");