diff options
author | Christopher Ferris | 2018-06-29 18:30:55 -0500 |
---|---|---|
committer | Christopher Ferris | 2018-07-12 14:45:31 -0500 |
commit | 5afddb06370dc8b3d04a31f6f3ae04a5fa379cde (patch) | |
tree | 274864d812331b87c63c44c344db70967d9b4033 /libunwindstack | |
parent | 32960e54b44042eb74c845fc9ae50aeb8b74841c (diff) | |
download | platform-system-core-5afddb06370dc8b3d04a31f6f3ae04a5fa379cde.tar.gz platform-system-core-5afddb06370dc8b3d04a31f6f3ae04a5fa379cde.tar.xz platform-system-core-5afddb06370dc8b3d04a31f6f3ae04a5fa379cde.zip |
Remove Memory::ReadField.
In almost all cases, it is faster to read the entire structure rather
than do multiple reads using ReadField. The only case where it would be
slower is if doing a remote unwind and ptrace is the only way to read. In
all other cases, it's a single system call. In the ptrace call, it will be
multiple calls. Given that it is unusual to be forced to use ptrace,
it's better to avoid it.
It also reduces the code complexity to do a single read, and avoids
issues where the code forgets to read the field it needs.
Test: Unit tests pass on host and target.
Change-Id: I7b3875b2c85d0d88115b1776e1be28521dc0b932
Diffstat (limited to 'libunwindstack')
-rw-r--r-- | libunwindstack/ElfInterface.cpp | 71 | ||||
-rw-r--r-- | libunwindstack/ElfInterfaceArm.cpp | 14 | ||||
-rw-r--r-- | libunwindstack/ElfInterfaceArm.h | 2 | ||||
-rw-r--r-- | libunwindstack/include/unwindstack/ElfInterface.h | 2 | ||||
-rw-r--r-- | libunwindstack/include/unwindstack/Memory.h | 12 | ||||
-rw-r--r-- | libunwindstack/tests/ElfInterfaceArmTest.cpp | 33 | ||||
-rw-r--r-- | libunwindstack/tests/MemoryTest.cpp | 34 |
7 files changed, 18 insertions, 150 deletions
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) | |||
204 | uint64_t offset = ehdr.e_phoff; | 204 | uint64_t offset = ehdr.e_phoff; |
205 | for (size_t i = 0; i < ehdr.e_phnum; i++, offset += ehdr.e_phentsize) { | 205 | for (size_t i = 0; i < ehdr.e_phnum; i++, offset += ehdr.e_phentsize) { |
206 | PhdrType phdr; | 206 | PhdrType phdr; |
207 | if (!memory_->ReadField(offset, &phdr, &phdr.p_type, sizeof(phdr.p_type))) { | 207 | if (!memory_->ReadFully(offset, &phdr, sizeof(phdr))) { |
208 | last_error_.code = ERROR_MEMORY_INVALID; | 208 | last_error_.code = ERROR_MEMORY_INVALID; |
209 | last_error_.address = | 209 | last_error_.address = offset; |
210 | offset + reinterpret_cast<uintptr_t>(&phdr.p_type) - reinterpret_cast<uintptr_t>(&phdr); | ||
211 | return false; | 210 | return false; |
212 | } | 211 | } |
213 | 212 | ||
214 | if (HandleType(offset, phdr.p_type)) { | ||
215 | continue; | ||
216 | } | ||
217 | |||
218 | switch (phdr.p_type) { | 213 | switch (phdr.p_type) { |
219 | case PT_LOAD: | 214 | case PT_LOAD: |
220 | { | 215 | { |
221 | // Get the flags first, if this isn't an executable header, ignore it. | ||
222 | if (!memory_->ReadField(offset, &phdr, &phdr.p_flags, sizeof(phdr.p_flags))) { | ||
223 | last_error_.code = ERROR_MEMORY_INVALID; | ||
224 | last_error_.address = offset + reinterpret_cast<uintptr_t>(&phdr.p_flags) - | ||
225 | reinterpret_cast<uintptr_t>(&phdr); | ||
226 | return false; | ||
227 | } | ||
228 | if ((phdr.p_flags & PF_X) == 0) { | 216 | if ((phdr.p_flags & PF_X) == 0) { |
229 | continue; | 217 | continue; |
230 | } | 218 | } |
231 | 219 | ||
232 | if (!memory_->ReadField(offset, &phdr, &phdr.p_vaddr, sizeof(phdr.p_vaddr))) { | ||
233 | last_error_.code = ERROR_MEMORY_INVALID; | ||
234 | last_error_.address = offset + reinterpret_cast<uintptr_t>(&phdr.p_vaddr) - | ||
235 | reinterpret_cast<uintptr_t>(&phdr); | ||
236 | return false; | ||
237 | } | ||
238 | if (!memory_->ReadField(offset, &phdr, &phdr.p_offset, sizeof(phdr.p_offset))) { | ||
239 | last_error_.code = ERROR_MEMORY_INVALID; | ||
240 | last_error_.address = offset + reinterpret_cast<uintptr_t>(&phdr.p_offset) - | ||
241 | reinterpret_cast<uintptr_t>(&phdr); | ||
242 | return false; | ||
243 | } | ||
244 | if (!memory_->ReadField(offset, &phdr, &phdr.p_memsz, sizeof(phdr.p_memsz))) { | ||
245 | last_error_.code = ERROR_MEMORY_INVALID; | ||
246 | last_error_.address = offset + reinterpret_cast<uintptr_t>(&phdr.p_memsz) - | ||
247 | reinterpret_cast<uintptr_t>(&phdr); | ||
248 | return false; | ||
249 | } | ||
250 | pt_loads_[phdr.p_offset] = LoadInfo{phdr.p_offset, phdr.p_vaddr, | 220 | pt_loads_[phdr.p_offset] = LoadInfo{phdr.p_offset, phdr.p_vaddr, |
251 | static_cast<size_t>(phdr.p_memsz)}; | 221 | static_cast<size_t>(phdr.p_memsz)}; |
252 | if (phdr.p_offset == 0) { | 222 | if (phdr.p_offset == 0) { |
@@ -256,46 +226,20 @@ bool ElfInterface::ReadProgramHeaders(const EhdrType& ehdr, uint64_t* load_bias) | |||
256 | } | 226 | } |
257 | 227 | ||
258 | case PT_GNU_EH_FRAME: | 228 | case PT_GNU_EH_FRAME: |
259 | if (!memory_->ReadField(offset, &phdr, &phdr.p_offset, sizeof(phdr.p_offset))) { | ||
260 | last_error_.code = ERROR_MEMORY_INVALID; | ||
261 | last_error_.address = offset + reinterpret_cast<uintptr_t>(&phdr.p_offset) - | ||
262 | reinterpret_cast<uintptr_t>(&phdr); | ||
263 | return false; | ||
264 | } | ||
265 | // This is really the pointer to the .eh_frame_hdr section. | 229 | // This is really the pointer to the .eh_frame_hdr section. |
266 | eh_frame_hdr_offset_ = phdr.p_offset; | 230 | eh_frame_hdr_offset_ = phdr.p_offset; |
267 | if (!memory_->ReadField(offset, &phdr, &phdr.p_memsz, sizeof(phdr.p_memsz))) { | ||
268 | last_error_.code = ERROR_MEMORY_INVALID; | ||
269 | last_error_.address = offset + reinterpret_cast<uintptr_t>(&phdr.p_memsz) - | ||
270 | reinterpret_cast<uintptr_t>(&phdr); | ||
271 | return false; | ||
272 | } | ||
273 | eh_frame_hdr_size_ = phdr.p_memsz; | 231 | eh_frame_hdr_size_ = phdr.p_memsz; |
274 | break; | 232 | break; |
275 | 233 | ||
276 | case PT_DYNAMIC: | 234 | case PT_DYNAMIC: |
277 | if (!memory_->ReadField(offset, &phdr, &phdr.p_offset, sizeof(phdr.p_offset))) { | ||
278 | last_error_.code = ERROR_MEMORY_INVALID; | ||
279 | last_error_.address = offset + reinterpret_cast<uintptr_t>(&phdr.p_offset) - | ||
280 | reinterpret_cast<uintptr_t>(&phdr); | ||
281 | return false; | ||
282 | } | ||
283 | dynamic_offset_ = phdr.p_offset; | 235 | dynamic_offset_ = phdr.p_offset; |
284 | if (!memory_->ReadField(offset, &phdr, &phdr.p_vaddr, sizeof(phdr.p_vaddr))) { | ||
285 | last_error_.code = ERROR_MEMORY_INVALID; | ||
286 | last_error_.address = offset + reinterpret_cast<uintptr_t>(&phdr.p_vaddr) - | ||
287 | reinterpret_cast<uintptr_t>(&phdr); | ||
288 | return false; | ||
289 | } | ||
290 | dynamic_vaddr_ = phdr.p_vaddr; | 236 | dynamic_vaddr_ = phdr.p_vaddr; |
291 | if (!memory_->ReadField(offset, &phdr, &phdr.p_memsz, sizeof(phdr.p_memsz))) { | ||
292 | last_error_.code = ERROR_MEMORY_INVALID; | ||
293 | last_error_.address = offset + reinterpret_cast<uintptr_t>(&phdr.p_memsz) - | ||
294 | reinterpret_cast<uintptr_t>(&phdr); | ||
295 | return false; | ||
296 | } | ||
297 | dynamic_size_ = phdr.p_memsz; | 237 | dynamic_size_ = phdr.p_memsz; |
298 | break; | 238 | break; |
239 | |||
240 | default: | ||
241 | HandleUnknownType(phdr.p_type, phdr.p_offset, phdr.p_filesz); | ||
242 | break; | ||
299 | } | 243 | } |
300 | } | 244 | } |
301 | return true; | 245 | return true; |
@@ -313,8 +257,7 @@ bool ElfInterface::ReadSectionHeaders(const EhdrType& ehdr) { | |||
313 | ShdrType shdr; | 257 | ShdrType shdr; |
314 | if (ehdr.e_shstrndx < ehdr.e_shnum) { | 258 | if (ehdr.e_shstrndx < ehdr.e_shnum) { |
315 | uint64_t sh_offset = offset + ehdr.e_shstrndx * ehdr.e_shentsize; | 259 | uint64_t sh_offset = offset + ehdr.e_shstrndx * ehdr.e_shentsize; |
316 | if (memory_->ReadField(sh_offset, &shdr, &shdr.sh_offset, sizeof(shdr.sh_offset)) && | 260 | if (memory_->ReadFully(sh_offset, &shdr, sizeof(shdr))) { |
317 | memory_->ReadField(sh_offset, &shdr, &shdr.sh_size, sizeof(shdr.sh_size))) { | ||
318 | sec_offset = shdr.sh_offset; | 261 | sec_offset = shdr.sh_offset; |
319 | sec_size = shdr.sh_size; | 262 | sec_size = shdr.sh_size; |
320 | } | 263 | } |
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) { | |||
87 | #define PT_ARM_EXIDX 0x70000001 | 87 | #define PT_ARM_EXIDX 0x70000001 |
88 | #endif | 88 | #endif |
89 | 89 | ||
90 | bool ElfInterfaceArm::HandleType(uint64_t offset, uint32_t type) { | 90 | void ElfInterfaceArm::HandleUnknownType(uint32_t type, uint64_t ph_offset, uint64_t ph_filesz) { |
91 | if (type != PT_ARM_EXIDX) { | 91 | if (type != PT_ARM_EXIDX) { |
92 | return false; | 92 | return; |
93 | } | ||
94 | |||
95 | Elf32_Phdr phdr; | ||
96 | if (!memory_->ReadFully(offset, &phdr, sizeof(phdr))) { | ||
97 | return true; | ||
98 | } | 93 | } |
99 | 94 | ||
100 | // The offset already takes into account the load bias. | 95 | // The offset already takes into account the load bias. |
101 | start_offset_ = phdr.p_offset; | 96 | start_offset_ = ph_offset; |
102 | 97 | ||
103 | // Always use filesz instead of memsz. In most cases they are the same, | 98 | // Always use filesz instead of memsz. In most cases they are the same, |
104 | // but some shared libraries wind up setting one correctly and not the other. | 99 | // but some shared libraries wind up setting one correctly and not the other. |
105 | total_entries_ = phdr.p_filesz / 8; | 100 | total_entries_ = ph_filesz / 8; |
106 | return true; | ||
107 | } | 101 | } |
108 | 102 | ||
109 | bool ElfInterfaceArm::Step(uint64_t pc, Regs* regs, Memory* process_memory, bool* finished) { | 103 | 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 { | |||
70 | 70 | ||
71 | bool FindEntry(uint32_t pc, uint64_t* entry_offset); | 71 | bool FindEntry(uint32_t pc, uint64_t* entry_offset); |
72 | 72 | ||
73 | bool HandleType(uint64_t offset, uint32_t type) override; | 73 | void HandleUnknownType(uint32_t type, uint64_t ph_offset, uint64_t ph_filesz) override; |
74 | 74 | ||
75 | bool Step(uint64_t pc, Regs* regs, Memory* process_memory, bool* finished) override; | 75 | bool Step(uint64_t pc, Regs* regs, Memory* process_memory, bool* finished) override; |
76 | 76 | ||
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 { | |||
118 | template <typename SymType> | 118 | template <typename SymType> |
119 | bool GetGlobalVariableWithTemplate(const std::string& name, uint64_t* memory_address); | 119 | bool GetGlobalVariableWithTemplate(const std::string& name, uint64_t* memory_address); |
120 | 120 | ||
121 | virtual bool HandleType(uint64_t, uint32_t) { return false; } | 121 | virtual void HandleUnknownType(uint32_t, uint64_t, uint64_t) {} |
122 | 122 | ||
123 | template <typename EhdrType> | 123 | template <typename EhdrType> |
124 | static void GetMaxSizeWithTemplate(Memory* memory, uint64_t* size); | 124 | 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 { | |||
41 | 41 | ||
42 | bool ReadFully(uint64_t addr, void* dst, size_t size); | 42 | bool ReadFully(uint64_t addr, void* dst, size_t size); |
43 | 43 | ||
44 | inline bool ReadField(uint64_t addr, void* start, void* field, size_t size) { | ||
45 | if (reinterpret_cast<uintptr_t>(field) < reinterpret_cast<uintptr_t>(start)) { | ||
46 | return false; | ||
47 | } | ||
48 | uint64_t offset = reinterpret_cast<uintptr_t>(field) - reinterpret_cast<uintptr_t>(start); | ||
49 | if (__builtin_add_overflow(addr, offset, &offset)) { | ||
50 | return false; | ||
51 | } | ||
52 | // The read will check if offset + size overflows. | ||
53 | return ReadFully(offset, field, size); | ||
54 | } | ||
55 | |||
56 | inline bool Read32(uint64_t addr, uint32_t* dst) { | 44 | inline bool Read32(uint64_t addr, uint32_t* dst) { |
57 | return ReadFully(addr, dst, sizeof(uint32_t)); | 45 | return ReadFully(addr, dst, sizeof(uint32_t)); |
58 | } | 46 | } |
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) { | |||
242 | ASSERT_EQ(0xa020U, entries[4]); | 242 | ASSERT_EQ(0xa020U, entries[4]); |
243 | } | 243 | } |
244 | 244 | ||
245 | TEST_F(ElfInterfaceArmTest, HandleType_not_arm_exidx) { | 245 | TEST_F(ElfInterfaceArmTest, HandleUnknownType_arm_exidx) { |
246 | ElfInterfaceArmFake interface(&memory_); | 246 | ElfInterfaceArmFake interface(&memory_); |
247 | 247 | ||
248 | ASSERT_FALSE(interface.HandleType(0x1000, PT_NULL)); | ||
249 | ASSERT_FALSE(interface.HandleType(0x1000, PT_LOAD)); | ||
250 | ASSERT_FALSE(interface.HandleType(0x1000, PT_DYNAMIC)); | ||
251 | ASSERT_FALSE(interface.HandleType(0x1000, PT_INTERP)); | ||
252 | ASSERT_FALSE(interface.HandleType(0x1000, PT_NOTE)); | ||
253 | ASSERT_FALSE(interface.HandleType(0x1000, PT_SHLIB)); | ||
254 | ASSERT_FALSE(interface.HandleType(0x1000, PT_PHDR)); | ||
255 | ASSERT_FALSE(interface.HandleType(0x1000, PT_TLS)); | ||
256 | ASSERT_FALSE(interface.HandleType(0x1000, PT_LOOS)); | ||
257 | ASSERT_FALSE(interface.HandleType(0x1000, PT_HIOS)); | ||
258 | ASSERT_FALSE(interface.HandleType(0x1000, PT_LOPROC)); | ||
259 | ASSERT_FALSE(interface.HandleType(0x1000, PT_HIPROC)); | ||
260 | ASSERT_FALSE(interface.HandleType(0x1000, PT_GNU_EH_FRAME)); | ||
261 | ASSERT_FALSE(interface.HandleType(0x1000, PT_GNU_STACK)); | ||
262 | } | ||
263 | |||
264 | TEST_F(ElfInterfaceArmTest, HandleType_arm_exidx) { | ||
265 | ElfInterfaceArmFake interface(&memory_); | ||
266 | |||
267 | Elf32_Phdr phdr = {}; | ||
268 | interface.FakeSetStartOffset(0x1000); | 248 | interface.FakeSetStartOffset(0x1000); |
269 | interface.FakeSetTotalEntries(100); | 249 | interface.FakeSetTotalEntries(100); |
270 | phdr.p_offset = 0x2000; | ||
271 | phdr.p_filesz = 0xa00; | ||
272 | 250 | ||
273 | // Verify that if reads fail, we don't set the values but still get true. | 251 | // Verify that if the type is not the one we want, we don't set the values. |
274 | ASSERT_TRUE(interface.HandleType(0x1000, 0x70000001)); | 252 | interface.HandleUnknownType(0x70000000, 0x2000, 320); |
275 | ASSERT_EQ(0x1000U, interface.start_offset()); | 253 | ASSERT_EQ(0x1000U, interface.start_offset()); |
276 | ASSERT_EQ(100U, interface.total_entries()); | 254 | ASSERT_EQ(100U, interface.total_entries()); |
277 | 255 | ||
278 | // Everything is correct and present. | 256 | // Everything is correct and present. |
279 | memory_.SetMemory(0x1000, &phdr, sizeof(phdr)); | 257 | interface.HandleUnknownType(0x70000001, 0x2000, 320); |
280 | ASSERT_TRUE(interface.HandleType(0x1000, 0x70000001)); | ||
281 | ASSERT_EQ(0x2000U, interface.start_offset()); | 258 | ASSERT_EQ(0x2000U, interface.start_offset()); |
282 | ASSERT_EQ(320U, interface.total_entries()); | 259 | ASSERT_EQ(40U, interface.total_entries()); |
283 | } | 260 | } |
284 | 261 | ||
285 | TEST_F(ElfInterfaceArmTest, StepExidx) { | 262 | 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 { | |||
51 | uint64_t four; | 51 | uint64_t four; |
52 | }; | 52 | }; |
53 | 53 | ||
54 | TEST(MemoryTest, read_field) { | ||
55 | MemoryFakeAlwaysReadZero memory; | ||
56 | |||
57 | FakeStruct data; | ||
58 | memset(&data, 0xff, sizeof(data)); | ||
59 | ASSERT_TRUE(memory.ReadField(0, &data, &data.one, sizeof(data.one))); | ||
60 | ASSERT_EQ(0, data.one); | ||
61 | |||
62 | memset(&data, 0xff, sizeof(data)); | ||
63 | ASSERT_TRUE(memory.ReadField(0, &data, &data.two, sizeof(data.two))); | ||
64 | ASSERT_FALSE(data.two); | ||
65 | |||
66 | memset(&data, 0xff, sizeof(data)); | ||
67 | ASSERT_TRUE(memory.ReadField(0, &data, &data.three, sizeof(data.three))); | ||
68 | ASSERT_EQ(0U, data.three); | ||
69 | |||
70 | memset(&data, 0xff, sizeof(data)); | ||
71 | ASSERT_TRUE(memory.ReadField(0, &data, &data.four, sizeof(data.four))); | ||
72 | ASSERT_EQ(0U, data.four); | ||
73 | } | ||
74 | |||
75 | TEST(MemoryTest, read_field_fails) { | ||
76 | MemoryFakeAlwaysReadZero memory; | ||
77 | |||
78 | FakeStruct data; | ||
79 | memset(&data, 0xff, sizeof(data)); | ||
80 | |||
81 | ASSERT_FALSE(memory.ReadField(UINT64_MAX, &data, &data.three, sizeof(data.three))); | ||
82 | |||
83 | // Field and start reversed, should fail. | ||
84 | ASSERT_FALSE(memory.ReadField(100, &data.two, &data, sizeof(data.two))); | ||
85 | ASSERT_FALSE(memory.ReadField(0, &data.two, &data, sizeof(data.two))); | ||
86 | } | ||
87 | |||
88 | TEST(MemoryTest, read_string) { | 54 | TEST(MemoryTest, read_string) { |
89 | std::string name("string_in_memory"); | 55 | std::string name("string_in_memory"); |
90 | 56 | ||