diff options
author | Christopher Ferris | 2018-02-16 15:48:19 -0600 |
---|---|---|
committer | Christopher Ferris | 2018-02-16 16:52:38 -0600 |
commit | d9575b668bd122f1ad80767bc36024e4571ffeb0 (patch) | |
tree | 6d52a08b98a3f1c429fbba400b71193d4e3bb510 /libunwindstack | |
parent | 76eda07ff6bacb5972bec7f59f0d36ac076028ec (diff) | |
download | platform-system-core-d9575b668bd122f1ad80767bc36024e4571ffeb0.tar.gz platform-system-core-d9575b668bd122f1ad80767bc36024e4571ffeb0.tar.xz platform-system-core-d9575b668bd122f1ad80767bc36024e4571ffeb0.zip |
Modify elf cache to handle elf_offsets properly.
Bug: 73498823
Test: All unit tests pass.
Test: Simpleperf run that previously failed, passes now.
Change-Id: Iff3a1f2f641a46ab9a0326579af3649f0c76fc65
Diffstat (limited to 'libunwindstack')
-rw-r--r-- | libunwindstack/Elf.cpp | 55 | ||||
-rw-r--r-- | libunwindstack/MapInfo.cpp | 14 | ||||
-rw-r--r-- | libunwindstack/include/unwindstack/Elf.h | 6 | ||||
-rw-r--r-- | libunwindstack/tests/ElfCacheTest.cpp | 63 |
4 files changed, 116 insertions, 22 deletions
diff --git a/libunwindstack/Elf.cpp b/libunwindstack/Elf.cpp index dbf772e5d..02f8a9ae0 100644 --- a/libunwindstack/Elf.cpp +++ b/libunwindstack/Elf.cpp | |||
@@ -20,6 +20,7 @@ | |||
20 | #include <memory> | 20 | #include <memory> |
21 | #include <mutex> | 21 | #include <mutex> |
22 | #include <string> | 22 | #include <string> |
23 | #include <utility> | ||
23 | 24 | ||
24 | #define LOG_TAG "unwind" | 25 | #define LOG_TAG "unwind" |
25 | #include <log/log.h> | 26 | #include <log/log.h> |
@@ -36,7 +37,7 @@ | |||
36 | namespace unwindstack { | 37 | namespace unwindstack { |
37 | 38 | ||
38 | bool Elf::cache_enabled_; | 39 | bool Elf::cache_enabled_; |
39 | std::unordered_map<std::string, std::shared_ptr<Elf>>* Elf::cache_; | 40 | std::unordered_map<std::string, std::pair<std::shared_ptr<Elf>, bool>>* Elf::cache_; |
40 | std::mutex* Elf::cache_lock_; | 41 | std::mutex* Elf::cache_lock_; |
41 | 42 | ||
42 | bool Elf::Init(bool init_gnu_debugdata) { | 43 | bool Elf::Init(bool init_gnu_debugdata) { |
@@ -308,7 +309,7 @@ uint64_t Elf::GetLoadBias(Memory* memory) { | |||
308 | void Elf::SetCachingEnabled(bool enable) { | 309 | void Elf::SetCachingEnabled(bool enable) { |
309 | if (!cache_enabled_ && enable) { | 310 | if (!cache_enabled_ && enable) { |
310 | cache_enabled_ = true; | 311 | cache_enabled_ = true; |
311 | cache_ = new std::unordered_map<std::string, std::shared_ptr<Elf>>; | 312 | cache_ = new std::unordered_map<std::string, std::pair<std::shared_ptr<Elf>, bool>>; |
312 | cache_lock_ = new std::mutex; | 313 | cache_lock_ = new std::mutex; |
313 | } else if (cache_enabled_ && !enable) { | 314 | } else if (cache_enabled_ && !enable) { |
314 | cache_enabled_ = false; | 315 | cache_enabled_ = false; |
@@ -326,18 +327,54 @@ void Elf::CacheUnlock() { | |||
326 | } | 327 | } |
327 | 328 | ||
328 | void Elf::CacheAdd(MapInfo* info) { | 329 | void Elf::CacheAdd(MapInfo* info) { |
329 | if (info->offset == 0) { | 330 | // If elf_offset != 0, then cache both name:offset and name. |
330 | (*cache_)[info->name] = info->elf; | 331 | // The cached name is used to do lookups if multiple maps for the same |
331 | } else { | 332 | // named elf file exist. |
332 | std::string name(info->name + ':' + std::to_string(info->offset)); | 333 | // For example, if there are two maps boot.odex:1000 and boot.odex:2000 |
333 | (*cache_)[name] = info->elf; | 334 | // where each reference the entire boot.odex, the cache will properly |
335 | // use the same cached elf object. | ||
336 | |||
337 | if (info->offset == 0 || info->elf_offset != 0) { | ||
338 | (*cache_)[info->name] = std::make_pair(info->elf, true); | ||
339 | } | ||
340 | |||
341 | if (info->offset != 0) { | ||
342 | // The second element in the pair indicates whether elf_offset should | ||
343 | // be set to offset when getting out of the cache. | ||
344 | (*cache_)[info->name + ':' + std::to_string(info->offset)] = | ||
345 | std::make_pair(info->elf, info->elf_offset != 0); | ||
334 | } | 346 | } |
335 | } | 347 | } |
336 | 348 | ||
337 | bool Elf::CacheGet(const std::string& name, std::shared_ptr<Elf>* elf) { | 349 | bool Elf::CacheAfterCreateMemory(MapInfo* info) { |
350 | if (info->name.empty() || info->offset == 0 || info->elf_offset == 0) { | ||
351 | return false; | ||
352 | } | ||
353 | |||
354 | auto entry = cache_->find(info->name); | ||
355 | if (entry == cache_->end()) { | ||
356 | return false; | ||
357 | } | ||
358 | |||
359 | // In this case, the whole file is the elf, and the name has already | ||
360 | // been cached. Add an entry at name:offset to get this directly out | ||
361 | // of the cache next time. | ||
362 | info->elf = entry->second.first; | ||
363 | (*cache_)[info->name + ':' + std::to_string(info->offset)] = std::make_pair(info->elf, true); | ||
364 | return true; | ||
365 | } | ||
366 | |||
367 | bool Elf::CacheGet(MapInfo* info) { | ||
368 | std::string name(info->name); | ||
369 | if (info->offset != 0) { | ||
370 | name += ':' + std::to_string(info->offset); | ||
371 | } | ||
338 | auto entry = cache_->find(name); | 372 | auto entry = cache_->find(name); |
339 | if (entry != cache_->end()) { | 373 | if (entry != cache_->end()) { |
340 | *elf = entry->second; | 374 | info->elf = entry->second.first; |
375 | if (entry->second.second) { | ||
376 | info->elf_offset = info->offset; | ||
377 | } | ||
341 | return true; | 378 | return true; |
342 | } | 379 | } |
343 | return false; | 380 | return false; |
diff --git a/libunwindstack/MapInfo.cpp b/libunwindstack/MapInfo.cpp index 0c153351d..39378a3af 100644 --- a/libunwindstack/MapInfo.cpp +++ b/libunwindstack/MapInfo.cpp | |||
@@ -117,23 +117,15 @@ Elf* MapInfo::GetElf(const std::shared_ptr<Memory>& process_memory, bool init_gn | |||
117 | if (Elf::CachingEnabled() && !name.empty()) { | 117 | if (Elf::CachingEnabled() && !name.empty()) { |
118 | Elf::CacheLock(); | 118 | Elf::CacheLock(); |
119 | locked = true; | 119 | locked = true; |
120 | if (offset != 0) { | 120 | if (Elf::CacheGet(this)) { |
121 | std::string hash(name + ':' + std::to_string(offset)); | ||
122 | if (Elf::CacheGet(hash, &elf)) { | ||
123 | Elf::CacheUnlock(); | ||
124 | return elf.get(); | ||
125 | } | ||
126 | } else if (Elf::CacheGet(name, &elf)) { | ||
127 | Elf::CacheUnlock(); | 121 | Elf::CacheUnlock(); |
128 | return elf.get(); | 122 | return elf.get(); |
129 | } | 123 | } |
130 | } | 124 | } |
131 | 125 | ||
132 | Memory* memory = CreateMemory(process_memory); | 126 | Memory* memory = CreateMemory(process_memory); |
133 | if (locked && offset != 0 && elf_offset != 0) { | 127 | if (locked) { |
134 | // In this case, the whole file is the elf, need to see if the elf | 128 | if (Elf::CacheAfterCreateMemory(this)) { |
135 | // data was cached. | ||
136 | if (Elf::CacheGet(name, &elf)) { | ||
137 | delete memory; | 129 | delete memory; |
138 | Elf::CacheUnlock(); | 130 | Elf::CacheUnlock(); |
139 | return elf.get(); | 131 | return elf.get(); |
diff --git a/libunwindstack/include/unwindstack/Elf.h b/libunwindstack/include/unwindstack/Elf.h index a874709d2..385973e9f 100644 --- a/libunwindstack/include/unwindstack/Elf.h +++ b/libunwindstack/include/unwindstack/Elf.h | |||
@@ -23,6 +23,7 @@ | |||
23 | #include <mutex> | 23 | #include <mutex> |
24 | #include <string> | 24 | #include <string> |
25 | #include <unordered_map> | 25 | #include <unordered_map> |
26 | #include <utility> | ||
26 | 27 | ||
27 | #include <unwindstack/ElfInterface.h> | 28 | #include <unwindstack/ElfInterface.h> |
28 | #include <unwindstack/Memory.h> | 29 | #include <unwindstack/Memory.h> |
@@ -103,7 +104,8 @@ class Elf { | |||
103 | static void CacheLock(); | 104 | static void CacheLock(); |
104 | static void CacheUnlock(); | 105 | static void CacheUnlock(); |
105 | static void CacheAdd(MapInfo* info); | 106 | static void CacheAdd(MapInfo* info); |
106 | static bool CacheGet(const std::string& name, std::shared_ptr<Elf>* elf); | 107 | static bool CacheGet(MapInfo* info); |
108 | static bool CacheAfterCreateMemory(MapInfo* info); | ||
107 | 109 | ||
108 | protected: | 110 | protected: |
109 | bool valid_ = false; | 111 | bool valid_ = false; |
@@ -120,7 +122,7 @@ class Elf { | |||
120 | std::unique_ptr<ElfInterface> gnu_debugdata_interface_; | 122 | std::unique_ptr<ElfInterface> gnu_debugdata_interface_; |
121 | 123 | ||
122 | static bool cache_enabled_; | 124 | static bool cache_enabled_; |
123 | static std::unordered_map<std::string, std::shared_ptr<Elf>>* cache_; | 125 | static std::unordered_map<std::string, std::pair<std::shared_ptr<Elf>, bool>>* cache_; |
124 | static std::mutex* cache_lock_; | 126 | static std::mutex* cache_lock_; |
125 | }; | 127 | }; |
126 | 128 | ||
diff --git a/libunwindstack/tests/ElfCacheTest.cpp b/libunwindstack/tests/ElfCacheTest.cpp index 0086c9edb..89331ea26 100644 --- a/libunwindstack/tests/ElfCacheTest.cpp +++ b/libunwindstack/tests/ElfCacheTest.cpp | |||
@@ -60,6 +60,7 @@ class ElfCacheTest : public ::testing::Test { | |||
60 | 60 | ||
61 | void VerifyWithinSameMap(bool cache_enabled); | 61 | void VerifyWithinSameMap(bool cache_enabled); |
62 | void VerifySameMap(bool cache_enabled); | 62 | void VerifySameMap(bool cache_enabled); |
63 | void VerifyWithinSameMapNeverReadAtZero(bool cache_enabled); | ||
63 | 64 | ||
64 | static std::shared_ptr<Memory> memory_; | 65 | static std::shared_ptr<Memory> memory_; |
65 | }; | 66 | }; |
@@ -198,4 +199,66 @@ TEST_F(ElfCacheTest, caching_valid_elf_offset_non_zero) { | |||
198 | VerifyWithinSameMap(true); | 199 | VerifyWithinSameMap(true); |
199 | } | 200 | } |
200 | 201 | ||
202 | // Verify that when reading from multiple non-zero offsets in the same map | ||
203 | // that when cached, all of the elf objects are the same. | ||
204 | void ElfCacheTest::VerifyWithinSameMapNeverReadAtZero(bool cache_enabled) { | ||
205 | if (!cache_enabled) { | ||
206 | Elf::SetCachingEnabled(false); | ||
207 | } | ||
208 | |||
209 | TemporaryFile tf; | ||
210 | ASSERT_TRUE(tf.fd != -1); | ||
211 | WriteElfFile(0, &tf, EM_ARM); | ||
212 | lseek(tf.fd, 0x500, SEEK_SET); | ||
213 | uint8_t value = 0; | ||
214 | write(tf.fd, &value, 1); | ||
215 | close(tf.fd); | ||
216 | |||
217 | uint64_t start = 0x1000; | ||
218 | uint64_t end = 0x20000; | ||
219 | // Multiple info sections at different offsets will have non-zero elf offsets. | ||
220 | MapInfo info300_1(start, end, 0x300, 0x5, tf.path); | ||
221 | MapInfo info300_2(start, end, 0x300, 0x5, tf.path); | ||
222 | MapInfo info400_1(start, end, 0x400, 0x5, tf.path); | ||
223 | MapInfo info400_2(start, end, 0x400, 0x5, tf.path); | ||
224 | |||
225 | Elf* elf300_1 = info300_1.GetElf(memory_, true); | ||
226 | ASSERT_TRUE(elf300_1->valid()); | ||
227 | EXPECT_EQ(ARCH_ARM, elf300_1->arch()); | ||
228 | Elf* elf300_2 = info300_2.GetElf(memory_, true); | ||
229 | ASSERT_TRUE(elf300_2->valid()); | ||
230 | EXPECT_EQ(ARCH_ARM, elf300_2->arch()); | ||
231 | EXPECT_EQ(0x300U, info300_1.elf_offset); | ||
232 | EXPECT_EQ(0x300U, info300_2.elf_offset); | ||
233 | if (cache_enabled) { | ||
234 | EXPECT_EQ(elf300_1, elf300_2); | ||
235 | } else { | ||
236 | EXPECT_NE(elf300_1, elf300_2); | ||
237 | } | ||
238 | |||
239 | Elf* elf400_1 = info400_1.GetElf(memory_, true); | ||
240 | ASSERT_TRUE(elf400_1->valid()); | ||
241 | EXPECT_EQ(ARCH_ARM, elf400_1->arch()); | ||
242 | Elf* elf400_2 = info400_2.GetElf(memory_, true); | ||
243 | ASSERT_TRUE(elf400_2->valid()); | ||
244 | EXPECT_EQ(ARCH_ARM, elf400_2->arch()); | ||
245 | EXPECT_EQ(0x400U, info400_1.elf_offset); | ||
246 | EXPECT_EQ(0x400U, info400_2.elf_offset); | ||
247 | if (cache_enabled) { | ||
248 | EXPECT_EQ(elf400_1, elf400_2); | ||
249 | EXPECT_EQ(elf300_1, elf400_1); | ||
250 | } else { | ||
251 | EXPECT_NE(elf400_1, elf400_2); | ||
252 | EXPECT_NE(elf300_1, elf400_1); | ||
253 | } | ||
254 | } | ||
255 | |||
256 | TEST_F(ElfCacheTest, no_caching_valid_elf_offset_non_zero_never_read_at_zero) { | ||
257 | VerifyWithinSameMapNeverReadAtZero(false); | ||
258 | } | ||
259 | |||
260 | TEST_F(ElfCacheTest, caching_valid_elf_offset_non_zero_never_read_at_zero) { | ||
261 | VerifyWithinSameMapNeverReadAtZero(true); | ||
262 | } | ||
263 | |||
201 | } // namespace unwindstack | 264 | } // namespace unwindstack |