diff options
author | Christopher Ferris <cferris@google.com> | 2023-11-17 00:15:48 +0000 |
---|---|---|
committer | Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com> | 2023-11-17 00:15:48 +0000 |
commit | 18891d9faf230f5edbf99e9ac69d944782b8f65f (patch) | |
tree | 4d8f7afe0de7c22c8381ebccae20fc54ab90cb21 | |
parent | 7c553de67b663b05b1f3df64c4583404e37a1289 (diff) | |
parent | 35964ca66ac8683a1f63a1633f97ba4da4a65d94 (diff) | |
download | unwinding-18891d9faf230f5edbf99e9ac69d944782b8f65f.tar.gz |
Change MemoryBuffer to use a vector. am: 35964ca66a
Original change: https://android-review.googlesource.com/c/platform/system/unwinding/+/2833274
Change-Id: I2607dac124fd1f8d9464e4aa3ae59927ce4cce06
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
-rw-r--r-- | libunwindstack/DexFile.cpp | 8 | ||||
-rw-r--r-- | libunwindstack/JitDebug.cpp | 6 | ||||
-rw-r--r-- | libunwindstack/Memory.cpp | 23 | ||||
-rw-r--r-- | libunwindstack/MemoryBuffer.h | 34 | ||||
-rw-r--r-- | libunwindstack/tests/MemoryBufferTest.cpp | 61 | ||||
-rw-r--r-- | libunwindstack/tests/MemoryXzTest.cpp | 4 |
6 files changed, 80 insertions, 56 deletions
diff --git a/libunwindstack/DexFile.cpp b/libunwindstack/DexFile.cpp index cb2a28a..0bb069f 100644 --- a/libunwindstack/DexFile.cpp +++ b/libunwindstack/DexFile.cpp @@ -108,11 +108,9 @@ std::shared_ptr<DexFile> DexFile::Create(uint64_t base_addr, uint64_t file_size, } // Fallback: make copy in local buffer. - std::unique_ptr<MemoryBuffer> copy(new MemoryBuffer); - if (!copy->Resize(file_size)) { - return nullptr; - } - if (!memory->ReadFully(base_addr, copy->GetPtr(0), file_size)) { + std::unique_ptr<MemoryBuffer> copy(new MemoryBuffer(file_size)); + uint8_t* dst_ptr = copy->GetPtr(0); + if (dst_ptr == nullptr || !memory->ReadFully(base_addr, dst_ptr, file_size)) { return nullptr; } std::unique_ptr<art_api::dex::DexFile> dex; diff --git a/libunwindstack/JitDebug.cpp b/libunwindstack/JitDebug.cpp index 20e981e..422071d 100644 --- a/libunwindstack/JitDebug.cpp +++ b/libunwindstack/JitDebug.cpp @@ -26,9 +26,9 @@ namespace unwindstack { template <> bool GlobalDebugInterface<Elf>::Load(Maps*, std::shared_ptr<Memory>& memory, uint64_t addr, uint64_t size, /*out*/ std::shared_ptr<Elf>& elf) { - MemoryBuffer* buffer = new MemoryBuffer; - std::shared_ptr<Memory> copy(buffer); - if (!buffer->Resize(size) || !memory->ReadFully(addr, buffer->GetPtr(0), size)) { + std::shared_ptr<Memory> copy(new MemoryBuffer(size)); + uint8_t* dst_ptr = copy->GetPtr(0); + if (dst_ptr == nullptr || !memory->ReadFully(addr, dst_ptr, size)) { return false; } elf.reset(new Elf(copy)); diff --git a/libunwindstack/Memory.cpp b/libunwindstack/Memory.cpp index 29fc5cd..258c54a 100644 --- a/libunwindstack/Memory.cpp +++ b/libunwindstack/Memory.cpp @@ -228,21 +228,28 @@ std::shared_ptr<Memory> Memory::CreateOfflineMemory(const uint8_t* data, uint64_ } size_t MemoryBuffer::Read(uint64_t addr, void* dst, size_t size) { - if (addr >= size_) { + if (addr < offset_) { + return 0; + } + addr -= offset_; + size_t raw_size = raw_.size(); + if (addr >= raw_size) { return 0; } - size_t bytes_left = size_ - static_cast<size_t>(addr); - const unsigned char* actual_base = static_cast<const unsigned char*>(raw_) + addr; + size_t bytes_left = raw_size - static_cast<size_t>(addr); size_t actual_len = std::min(bytes_left, size); - - memcpy(dst, actual_base, actual_len); + memcpy(dst, &raw_[addr], actual_len); return actual_len; } -uint8_t* MemoryBuffer::GetPtr(size_t offset) { - if (offset < size_) { - return &raw_[offset]; +uint8_t* MemoryBuffer::GetPtr(size_t addr) { + if (addr < offset_) { + return nullptr; + } + addr -= offset_; + if (addr < raw_.size()) { + return &raw_[addr]; } return nullptr; } diff --git a/libunwindstack/MemoryBuffer.h b/libunwindstack/MemoryBuffer.h index a5b5743..a19b331 100644 --- a/libunwindstack/MemoryBuffer.h +++ b/libunwindstack/MemoryBuffer.h @@ -18,7 +18,6 @@ #include <stdint.h> -#include <string> #include <vector> #include <unwindstack/Memory.h> @@ -27,31 +26,28 @@ namespace unwindstack { class MemoryBuffer : public Memory { public: - MemoryBuffer() = default; - virtual ~MemoryBuffer() { free(raw_); } + // If a size is too large, assume it's likely corrupted data, and set to zero. + MemoryBuffer(size_t size) : raw_(size > kMaxBufferSize ? 0 : size), offset_(0) {} + MemoryBuffer(size_t size, uint64_t offset) + : raw_(size > kMaxBufferSize ? 0 : size), offset_(offset) {} + virtual ~MemoryBuffer() = default; size_t Read(uint64_t addr, void* dst, size_t size) override; uint8_t* GetPtr(size_t offset) override; - bool Resize(size_t size) { - void* new_raw = realloc(raw_, size); - if (new_raw == nullptr) { - free(raw_); - raw_ = nullptr; - size_ = 0; - return false; - } - raw_ = reinterpret_cast<uint8_t*>(new_raw); - size_ = size; - return true; - } - - uint64_t Size() { return size_; } + uint64_t Size() { return raw_.size(); } private: - uint8_t* raw_ = nullptr; - size_t size_ = 0; + std::vector<uint8_t> raw_; + uint64_t offset_; + + // This class is only used for global data and a compressed .debug_frame in + // the library code. The limit of 10MB is way over what any valid existing + // globals data section is expected to be. A 50MB shared library only contains + // a .debug_frame that is < 100KB in size. Therefore, 10MB should be able to + // handle any valid large shared library with a valid large .debug_frame. + static constexpr size_t kMaxBufferSize = 10 * 1024 * 1024; }; } // namespace unwindstack diff --git a/libunwindstack/tests/MemoryBufferTest.cpp b/libunwindstack/tests/MemoryBufferTest.cpp index 91b20dd..d375f0f 100644 --- a/libunwindstack/tests/MemoryBufferTest.cpp +++ b/libunwindstack/tests/MemoryBufferTest.cpp @@ -27,14 +27,12 @@ namespace unwindstack { class MemoryBufferTest : public ::testing::Test { protected: - void SetUp() override { - ResetLogs(); - memory_.reset(new MemoryBuffer); - } + void SetUp() override { ResetLogs(); } std::unique_ptr<MemoryBuffer> memory_; }; TEST_F(MemoryBufferTest, empty) { + memory_.reset(new MemoryBuffer(0)); ASSERT_EQ(0U, memory_->Size()); std::vector<uint8_t> buffer(1024); ASSERT_FALSE(memory_->ReadFully(0, buffer.data(), 1)); @@ -43,7 +41,7 @@ TEST_F(MemoryBufferTest, empty) { } TEST_F(MemoryBufferTest, write_read) { - ASSERT_TRUE(memory_->Resize(256)); + memory_.reset(new MemoryBuffer(256)); ASSERT_EQ(256U, memory_->Size()); ASSERT_TRUE(memory_->GetPtr(0) != nullptr); ASSERT_TRUE(memory_->GetPtr(1) != nullptr); @@ -63,7 +61,8 @@ TEST_F(MemoryBufferTest, write_read) { } TEST_F(MemoryBufferTest, read_failures) { - ASSERT_TRUE(memory_->Resize(100)); + memory_.reset(new MemoryBuffer(100)); + ASSERT_EQ(100U, memory_->Size()); std::vector<uint8_t> buffer(200); ASSERT_FALSE(memory_->ReadFully(0, buffer.data(), 101)); ASSERT_FALSE(memory_->ReadFully(100, buffer.data(), 1)); @@ -73,14 +72,14 @@ TEST_F(MemoryBufferTest, read_failures) { } TEST_F(MemoryBufferTest, read_failure_overflow) { - ASSERT_TRUE(memory_->Resize(100)); + memory_.reset(new MemoryBuffer(100)); + ASSERT_EQ(100U, memory_->Size()); std::vector<uint8_t> buffer(200); - ASSERT_FALSE(memory_->ReadFully(UINT64_MAX - 100, buffer.data(), 200)); } -TEST_F(MemoryBufferTest, Read) { - ASSERT_TRUE(memory_->Resize(256)); +TEST_F(MemoryBufferTest, read_checks) { + memory_.reset(new MemoryBuffer(256)); ASSERT_EQ(256U, memory_->Size()); ASSERT_TRUE(memory_->GetPtr(0) != nullptr); ASSERT_TRUE(memory_->GetPtr(1) != nullptr); @@ -99,19 +98,43 @@ TEST_F(MemoryBufferTest, Read) { } } -TEST_F(MemoryBufferTest, Resize) { - ASSERT_TRUE(memory_->Resize(256)); - - ASSERT_TRUE(memory_->Resize(1024)); +TEST_F(MemoryBufferTest, resize_too_large) { + memory_.reset(new MemoryBuffer(SIZE_MAX)); + ASSERT_EQ(0U, memory_->Size()); } -extern "C" void __hwasan_init() __attribute__((weak)); +TEST_F(MemoryBufferTest, read_checks_non_zero_offset) { + memory_.reset(new MemoryBuffer(256, 1000)); + ASSERT_EQ(256U, memory_->Size()); + ASSERT_TRUE(memory_->GetPtr(0) == nullptr); + ASSERT_TRUE(memory_->GetPtr(1) == nullptr); + ASSERT_TRUE(memory_->GetPtr(999) == nullptr); + ASSERT_TRUE(memory_->GetPtr(1000) != nullptr); + ASSERT_TRUE(memory_->GetPtr(1255) != nullptr); + ASSERT_TRUE(memory_->GetPtr(1256) == nullptr); + + uint8_t* data = memory_->GetPtr(1000); + for (size_t i = 0; i < memory_->Size(); i++) { + data[i] = i; + } -TEST_F(MemoryBufferTest, Resize_too_large) { - if (&__hwasan_init != 0) { - GTEST_SKIP() << "Tests fails hwasan allocation size too large check."; + std::vector<uint8_t> buffer(memory_->Size()); + ASSERT_EQ(128U, memory_->Read(1128, buffer.data(), buffer.size())); + for (size_t i = 0; i < 128; i++) { + ASSERT_EQ(128 + i, buffer[i]) << "Failed at byte " << i; } - ASSERT_FALSE(memory_->Resize(SIZE_MAX)); +} + +TEST_F(MemoryBufferTest, reads_non_zero_offset) { + memory_.reset(new MemoryBuffer(256, 1000)); + ASSERT_EQ(256U, memory_->Size()); + std::vector<uint8_t> buffer(256); + ASSERT_EQ(0U, memory_->Read(900, buffer.data(), buffer.size())); + ASSERT_EQ(0U, memory_->Read(999, buffer.data(), buffer.size())); + ASSERT_EQ(256U, memory_->Read(1000, buffer.data(), buffer.size())); + ASSERT_EQ(246U, memory_->Read(1010, buffer.data(), buffer.size())); + ASSERT_EQ(1U, memory_->Read(1255, buffer.data(), buffer.size())); + ASSERT_EQ(0U, memory_->Read(1256, buffer.data(), buffer.size())); } } // namespace unwindstack diff --git a/libunwindstack/tests/MemoryXzTest.cpp b/libunwindstack/tests/MemoryXzTest.cpp index 17562dc..6597240 100644 --- a/libunwindstack/tests/MemoryXzTest.cpp +++ b/libunwindstack/tests/MemoryXzTest.cpp @@ -32,8 +32,8 @@ class MemoryXzTest : public ::testing::Test { std::string data; // NB: It is actually binary data. EXPECT_TRUE(android::base::ReadFileToString(dir + filename, &data)) << filename; EXPECT_GT(data.size(), 0u); - auto memory = std::make_unique<MemoryBuffer>(); - EXPECT_TRUE(memory->Resize(data.size())); + auto memory = std::make_unique<MemoryBuffer>(data.size()); + EXPECT_EQ(data.size(), memory->Size()); memcpy(memory->GetPtr(0), data.data(), data.size()); return memory; } |