Allow multiple threads sharing a map to unwind.
Add a mutex in MapInfo, and a mutex in Elf. Lock the creation of an Elf
file using the MapInfo mutex, and lock when calling Step, GetFunctionName,
or GetSoname since they can modify information in the object. It might
be beneficial to use a fine grained lock in the future.
Change the Maps object to contain a vector of MapInfo pointers rather
than the total objects. This avoids copying this data around.
Add a test to libbacktrace to verify that sharing a map while doing
unwinds in different threads works.
Add concurrency tests in libunwindstack to verify the locking works.
Add always inline to the RegsGetLocal arm and aarch64 functions. I had
a case where clang did not inline the code, so make sure this is specified.
Bug: 68813077
Test: New unit tests to cover the case. Passes all unit tests.
Test: Ran a monkey test while dumping bugreports and verified that
Test: no crashes in libunwind.
Test: Remove the locking and verified that all of the concurrenty tests fail.
Change-Id: I769e728c676f6bdae9e64ce4cdc03b6749beae03
diff --git a/libunwindstack/tests/MapInfoGetElfTest.cpp b/libunwindstack/tests/MapInfoGetElfTest.cpp
index 0b70d13..9973794 100644
--- a/libunwindstack/tests/MapInfoGetElfTest.cpp
+++ b/libunwindstack/tests/MapInfoGetElfTest.cpp
@@ -23,7 +23,9 @@
#include <sys/types.h>
#include <unistd.h>
+#include <atomic>
#include <memory>
+#include <thread>
#include <vector>
#include <android-base/file.h>
@@ -67,52 +69,52 @@
};
TEST_F(MapInfoGetElfTest, invalid) {
- MapInfo info{.start = 0x1000, .end = 0x2000, .offset = 0, .flags = PROT_READ, .name = ""};
+ MapInfo info(0x1000, 0x2000, 0, PROT_READ, "");
// The map is empty, but this should still create an invalid elf object.
- std::unique_ptr<Elf> elf(info.GetElf(process_memory_, false));
- ASSERT_TRUE(elf.get() != nullptr);
+ Elf* elf = info.GetElf(process_memory_, false);
+ ASSERT_TRUE(elf != nullptr);
ASSERT_FALSE(elf->valid());
}
TEST_F(MapInfoGetElfTest, valid32) {
- MapInfo info{.start = 0x3000, .end = 0x4000, .offset = 0, .flags = PROT_READ, .name = ""};
+ MapInfo info(0x3000, 0x4000, 0, PROT_READ, "");
Elf32_Ehdr ehdr;
TestInitEhdr<Elf32_Ehdr>(&ehdr, ELFCLASS32, EM_ARM);
memory_->SetMemory(0x3000, &ehdr, sizeof(ehdr));
- std::unique_ptr<Elf> elf(info.GetElf(process_memory_, false));
- ASSERT_TRUE(elf.get() != nullptr);
+ Elf* elf = info.GetElf(process_memory_, false);
+ ASSERT_TRUE(elf != nullptr);
ASSERT_TRUE(elf->valid());
EXPECT_EQ(static_cast<uint32_t>(EM_ARM), elf->machine_type());
EXPECT_EQ(ELFCLASS32, elf->class_type());
}
TEST_F(MapInfoGetElfTest, valid64) {
- MapInfo info{.start = 0x8000, .end = 0x9000, .offset = 0, .flags = PROT_READ, .name = ""};
+ MapInfo info(0x8000, 0x9000, 0, PROT_READ, "");
Elf64_Ehdr ehdr;
TestInitEhdr<Elf64_Ehdr>(&ehdr, ELFCLASS64, EM_AARCH64);
memory_->SetMemory(0x8000, &ehdr, sizeof(ehdr));
- std::unique_ptr<Elf> elf(info.GetElf(process_memory_, false));
- ASSERT_TRUE(elf.get() != nullptr);
+ Elf* elf = info.GetElf(process_memory_, false);
+ ASSERT_TRUE(elf != nullptr);
ASSERT_TRUE(elf->valid());
EXPECT_EQ(static_cast<uint32_t>(EM_AARCH64), elf->machine_type());
EXPECT_EQ(ELFCLASS64, elf->class_type());
}
TEST_F(MapInfoGetElfTest, gnu_debugdata_do_not_init32) {
- MapInfo info{.start = 0x4000, .end = 0x8000, .offset = 0, .flags = PROT_READ, .name = ""};
+ MapInfo info(0x4000, 0x8000, 0, PROT_READ, "");
TestInitGnuDebugdata<Elf32_Ehdr, Elf32_Shdr>(ELFCLASS32, EM_ARM, false,
[&](uint64_t offset, const void* ptr, size_t size) {
memory_->SetMemory(0x4000 + offset, ptr, size);
});
- std::unique_ptr<Elf> elf(info.GetElf(process_memory_, false));
- ASSERT_TRUE(elf.get() != nullptr);
+ Elf* elf = info.GetElf(process_memory_, false);
+ ASSERT_TRUE(elf != nullptr);
ASSERT_TRUE(elf->valid());
EXPECT_EQ(static_cast<uint32_t>(EM_ARM), elf->machine_type());
EXPECT_EQ(ELFCLASS32, elf->class_type());
@@ -120,15 +122,15 @@
}
TEST_F(MapInfoGetElfTest, gnu_debugdata_do_not_init64) {
- MapInfo info{.start = 0x6000, .end = 0x8000, .offset = 0, .flags = PROT_READ, .name = ""};
+ MapInfo info(0x6000, 0x8000, 0, PROT_READ, "");
TestInitGnuDebugdata<Elf64_Ehdr, Elf64_Shdr>(ELFCLASS64, EM_AARCH64, false,
[&](uint64_t offset, const void* ptr, size_t size) {
memory_->SetMemory(0x6000 + offset, ptr, size);
});
- std::unique_ptr<Elf> elf(info.GetElf(process_memory_, false));
- ASSERT_TRUE(elf.get() != nullptr);
+ Elf* elf = info.GetElf(process_memory_, false);
+ ASSERT_TRUE(elf != nullptr);
ASSERT_TRUE(elf->valid());
EXPECT_EQ(static_cast<uint32_t>(EM_AARCH64), elf->machine_type());
EXPECT_EQ(ELFCLASS64, elf->class_type());
@@ -136,15 +138,15 @@
}
TEST_F(MapInfoGetElfTest, gnu_debugdata_init32) {
- MapInfo info{.start = 0x2000, .end = 0x3000, .offset = 0, .flags = PROT_READ, .name = ""};
+ MapInfo info(0x2000, 0x3000, 0, PROT_READ, "");
TestInitGnuDebugdata<Elf32_Ehdr, Elf32_Shdr>(ELFCLASS32, EM_ARM, true,
[&](uint64_t offset, const void* ptr, size_t size) {
memory_->SetMemory(0x2000 + offset, ptr, size);
});
- std::unique_ptr<Elf> elf(info.GetElf(process_memory_, true));
- ASSERT_TRUE(elf.get() != nullptr);
+ Elf* elf = info.GetElf(process_memory_, true);
+ ASSERT_TRUE(elf != nullptr);
ASSERT_TRUE(elf->valid());
EXPECT_EQ(static_cast<uint32_t>(EM_ARM), elf->machine_type());
EXPECT_EQ(ELFCLASS32, elf->class_type());
@@ -152,15 +154,15 @@
}
TEST_F(MapInfoGetElfTest, gnu_debugdata_init64) {
- MapInfo info{.start = 0x5000, .end = 0x8000, .offset = 0, .flags = PROT_READ, .name = ""};
+ MapInfo info(0x5000, 0x8000, 0, PROT_READ, "");
TestInitGnuDebugdata<Elf64_Ehdr, Elf64_Shdr>(ELFCLASS64, EM_AARCH64, true,
[&](uint64_t offset, const void* ptr, size_t size) {
memory_->SetMemory(0x5000 + offset, ptr, size);
});
- std::unique_ptr<Elf> elf(info.GetElf(process_memory_, true));
- ASSERT_TRUE(elf.get() != nullptr);
+ Elf* elf = info.GetElf(process_memory_, true);
+ ASSERT_TRUE(elf != nullptr);
ASSERT_TRUE(elf->valid());
EXPECT_EQ(static_cast<uint32_t>(EM_AARCH64), elf->machine_type());
EXPECT_EQ(ELFCLASS64, elf->class_type());
@@ -168,32 +170,36 @@
}
TEST_F(MapInfoGetElfTest, end_le_start) {
- MapInfo info{.start = 0x1000, .end = 0x1000, .offset = 0, .flags = PROT_READ, .name = elf_.path};
+ MapInfo info(0x1000, 0x1000, 0, PROT_READ, elf_.path);
Elf32_Ehdr ehdr;
TestInitEhdr<Elf32_Ehdr>(&ehdr, ELFCLASS32, EM_ARM);
ASSERT_TRUE(android::base::WriteFully(elf_.fd, &ehdr, sizeof(ehdr)));
- std::unique_ptr<Elf> elf(info.GetElf(process_memory_, false));
+ Elf* elf = info.GetElf(process_memory_, false);
+ ASSERT_TRUE(elf != nullptr);
ASSERT_FALSE(elf->valid());
+ delete info.elf;
info.elf = nullptr;
info.end = 0xfff;
- elf.reset(info.GetElf(process_memory_, false));
+ elf = info.GetElf(process_memory_, false);
+ ASSERT_TRUE(elf != nullptr);
ASSERT_FALSE(elf->valid());
// Make sure this test is valid.
+ delete info.elf;
info.elf = nullptr;
info.end = 0x2000;
- elf.reset(info.GetElf(process_memory_, false));
+ elf = info.GetElf(process_memory_, false);
+ ASSERT_TRUE(elf != nullptr);
ASSERT_TRUE(elf->valid());
}
// Verify that if the offset is non-zero but there is no elf at the offset,
// that the full file is used.
TEST_F(MapInfoGetElfTest, file_backed_non_zero_offset_full_file) {
- MapInfo info{
- .start = 0x1000, .end = 0x2000, .offset = 0x100, .flags = PROT_READ, .name = elf_.path};
+ MapInfo info(0x1000, 0x2000, 0x100, PROT_READ, elf_.path);
std::vector<uint8_t> buffer(0x1000);
memset(buffer.data(), 0, buffer.size());
@@ -202,7 +208,8 @@
memcpy(buffer.data(), &ehdr, sizeof(ehdr));
ASSERT_TRUE(android::base::WriteFully(elf_.fd, buffer.data(), buffer.size()));
- std::unique_ptr<Elf> elf(info.GetElf(process_memory_, false));
+ Elf* elf = info.GetElf(process_memory_, false);
+ ASSERT_TRUE(elf != nullptr);
ASSERT_TRUE(elf->valid());
ASSERT_TRUE(elf->memory() != nullptr);
ASSERT_EQ(0x100U, info.elf_offset);
@@ -221,8 +228,7 @@
// Verify that if the offset is non-zero and there is an elf at that
// offset, that only part of the file is used.
TEST_F(MapInfoGetElfTest, file_backed_non_zero_offset_partial_file) {
- MapInfo info{
- .start = 0x1000, .end = 0x2000, .offset = 0x2000, .flags = PROT_READ, .name = elf_.path};
+ MapInfo info(0x1000, 0x2000, 0x2000, PROT_READ, elf_.path);
std::vector<uint8_t> buffer(0x4000);
memset(buffer.data(), 0, buffer.size());
@@ -231,7 +237,8 @@
memcpy(&buffer[info.offset], &ehdr, sizeof(ehdr));
ASSERT_TRUE(android::base::WriteFully(elf_.fd, buffer.data(), buffer.size()));
- std::unique_ptr<Elf> elf(info.GetElf(process_memory_, false));
+ Elf* elf = info.GetElf(process_memory_, false);
+ ASSERT_TRUE(elf != nullptr);
ASSERT_TRUE(elf->valid());
ASSERT_TRUE(elf->memory() != nullptr);
ASSERT_EQ(0U, info.elf_offset);
@@ -251,8 +258,7 @@
// embedded elf is bigger than the initial map, the new object is larger
// than the original map size. Do this for a 32 bit elf and a 64 bit elf.
TEST_F(MapInfoGetElfTest, file_backed_non_zero_offset_partial_file_whole_elf32) {
- MapInfo info{
- .start = 0x5000, .end = 0x6000, .offset = 0x1000, .flags = PROT_READ, .name = elf_.path};
+ MapInfo info(0x5000, 0x6000, 0x1000, PROT_READ, elf_.path);
std::vector<uint8_t> buffer(0x4000);
memset(buffer.data(), 0, buffer.size());
@@ -264,7 +270,8 @@
memcpy(&buffer[info.offset], &ehdr, sizeof(ehdr));
ASSERT_TRUE(android::base::WriteFully(elf_.fd, buffer.data(), buffer.size()));
- std::unique_ptr<Elf> elf(info.GetElf(process_memory_, false));
+ Elf* elf = info.GetElf(process_memory_, false);
+ ASSERT_TRUE(elf != nullptr);
ASSERT_TRUE(elf->valid());
ASSERT_TRUE(elf->memory() != nullptr);
ASSERT_EQ(0U, info.elf_offset);
@@ -279,8 +286,7 @@
}
TEST_F(MapInfoGetElfTest, file_backed_non_zero_offset_partial_file_whole_elf64) {
- MapInfo info{
- .start = 0x7000, .end = 0x8000, .offset = 0x1000, .flags = PROT_READ, .name = elf_.path};
+ MapInfo info(0x7000, 0x8000, 0x1000, PROT_READ, elf_.path);
std::vector<uint8_t> buffer(0x4000);
memset(buffer.data(), 0, buffer.size());
@@ -292,7 +298,8 @@
memcpy(&buffer[info.offset], &ehdr, sizeof(ehdr));
ASSERT_TRUE(android::base::WriteFully(elf_.fd, buffer.data(), buffer.size()));
- std::unique_ptr<Elf> elf(info.GetElf(process_memory_, false));
+ Elf* elf = info.GetElf(process_memory_, false);
+ ASSERT_TRUE(elf != nullptr);
ASSERT_TRUE(elf->valid());
ASSERT_TRUE(elf->memory() != nullptr);
ASSERT_EQ(0U, info.elf_offset);
@@ -307,7 +314,7 @@
}
TEST_F(MapInfoGetElfTest, process_memory_not_read_only) {
- MapInfo info{.start = 0x9000, .end = 0xa000, .offset = 0x1000, .flags = 0, .name = ""};
+ MapInfo info(0x9000, 0xa000, 0x1000, 0, "");
// Create valid elf data in process memory only.
Elf64_Ehdr ehdr;
@@ -317,21 +324,19 @@
ehdr.e_shnum = 4;
memory_->SetMemory(0x9000, &ehdr, sizeof(ehdr));
- std::unique_ptr<Elf> elf(info.GetElf(process_memory_, false));
+ Elf* elf = info.GetElf(process_memory_, false);
+ ASSERT_TRUE(elf != nullptr);
ASSERT_FALSE(elf->valid());
+ delete info.elf;
info.elf = nullptr;
info.flags = PROT_READ;
- elf.reset(info.GetElf(process_memory_, false));
+ elf = info.GetElf(process_memory_, false);
ASSERT_TRUE(elf->valid());
}
TEST_F(MapInfoGetElfTest, check_device_maps) {
- MapInfo info{.start = 0x7000,
- .end = 0x8000,
- .offset = 0x1000,
- .flags = PROT_READ | MAPS_FLAGS_DEVICE_MAP,
- .name = "/dev/something"};
+ MapInfo info(0x7000, 0x8000, 0x1000, PROT_READ | MAPS_FLAGS_DEVICE_MAP, "/dev/something");
// Create valid elf data in process memory for this to verify that only
// the name is causing invalid elf data.
@@ -342,20 +347,68 @@
ehdr.e_shnum = 4;
memory_->SetMemory(0x7000, &ehdr, sizeof(ehdr));
- std::unique_ptr<Elf> elf(info.GetElf(process_memory_, false));
+ Elf* elf = info.GetElf(process_memory_, false);
+ ASSERT_TRUE(elf != nullptr);
ASSERT_FALSE(elf->valid());
// Set the name to nothing to verify that it still fails.
+ delete info.elf;
info.elf = nullptr;
info.name = "";
- elf.reset(info.GetElf(process_memory_, false));
+ elf = info.GetElf(process_memory_, false);
ASSERT_FALSE(elf->valid());
// Change the flags and verify the elf is valid now.
+ delete info.elf;
info.elf = nullptr;
info.flags = PROT_READ;
- elf.reset(info.GetElf(process_memory_, false));
+ elf = info.GetElf(process_memory_, false);
ASSERT_TRUE(elf->valid());
}
+TEST_F(MapInfoGetElfTest, multiple_thread_get_elf) {
+ static constexpr size_t kNumConcurrentThreads = 100;
+
+ Elf64_Ehdr ehdr;
+ TestInitEhdr<Elf64_Ehdr>(&ehdr, ELFCLASS64, EM_X86_64);
+ ehdr.e_shoff = 0x2000;
+ ehdr.e_shentsize = sizeof(Elf64_Shdr) + 100;
+ ehdr.e_shnum = 4;
+ memory_->SetMemory(0x7000, &ehdr, sizeof(ehdr));
+
+ Elf* elf_in_threads[kNumConcurrentThreads];
+ std::vector<std::thread*> threads;
+
+ std::atomic_bool wait;
+ wait = true;
+ // Create all of the threads and have them do the GetElf at the same time
+ // to make it likely that a race will occur.
+ MapInfo info(0x7000, 0x8000, 0x1000, PROT_READ, "");
+ for (size_t i = 0; i < kNumConcurrentThreads; i++) {
+ std::thread* thread = new std::thread([i, this, &wait, &info, &elf_in_threads]() {
+ while (wait)
+ ;
+ Elf* elf = info.GetElf(process_memory_, false);
+ elf_in_threads[i] = elf;
+ });
+ threads.push_back(thread);
+ }
+ ASSERT_TRUE(info.elf == nullptr);
+
+ // Set them all going and wait for the threads to finish.
+ wait = false;
+ for (auto thread : threads) {
+ thread->join();
+ delete thread;
+ }
+
+ // Now verify that all of the elf files are exactly the same and valid.
+ Elf* elf = info.elf;
+ ASSERT_TRUE(elf != nullptr);
+ EXPECT_TRUE(elf->valid());
+ for (size_t i = 0; i < kNumConcurrentThreads; i++) {
+ EXPECT_EQ(elf, elf_in_threads[i]) << "Thread " << i << " mismatched.";
+ }
+}
+
} // namespace unwindstack