Merge changes I6791dd72,I887d5073
* changes:
Reduce priority of merge threads
Tune snapshot-merge performance
diff --git a/debuggerd/Android.bp b/debuggerd/Android.bp
index 5c7d847..ad0231d 100644
--- a/debuggerd/Android.bp
+++ b/debuggerd/Android.bp
@@ -293,13 +293,6 @@
"libdebuggerd/test/utility_test.cpp",
],
- product_variables: {
- malloc_not_svelte: {
- srcs: ["libdebuggerd/test/scudo_test.cpp"],
- header_libs: ["scudo_headers"],
- },
- },
-
target: {
android: {
srcs: [
diff --git a/debuggerd/libdebuggerd/include/libdebuggerd/scudo.h b/debuggerd/libdebuggerd/include/libdebuggerd/scudo.h
index 68bfd5b..a506859 100644
--- a/debuggerd/libdebuggerd/include/libdebuggerd/scudo.h
+++ b/debuggerd/libdebuggerd/include/libdebuggerd/scudo.h
@@ -34,10 +34,9 @@
class ScudoCrashData {
public:
- ScudoCrashData() = default;
+ ScudoCrashData() = delete;
~ScudoCrashData() = default;
-
- bool SetErrorInfo(unwindstack::Memory* process_memory, const ProcessInfo& process_info);
+ ScudoCrashData(unwindstack::Memory* process_memory, const ProcessInfo& process_info);
bool CrashIsMine() const;
diff --git a/debuggerd/libdebuggerd/scudo.cpp b/debuggerd/libdebuggerd/scudo.cpp
index 9483e59..5d861f8 100644
--- a/debuggerd/libdebuggerd/scudo.cpp
+++ b/debuggerd/libdebuggerd/scudo.cpp
@@ -14,11 +14,6 @@
* limitations under the License.
*/
-#include <stdint.h>
-#include <unistd.h>
-
-#include <vector>
-
#include "libdebuggerd/scudo.h"
#include "libdebuggerd/tombstone.h"
@@ -30,92 +25,57 @@
#include "tombstone.pb.h"
-bool ScudoCrashData::SetErrorInfo(unwindstack::Memory* process_memory,
- const ProcessInfo& process_info) {
+std::unique_ptr<char[]> AllocAndReadFully(unwindstack::Memory* process_memory, uint64_t addr,
+ size_t size) {
+ auto buf = std::make_unique<char[]>(size);
+ if (!process_memory->ReadFully(addr, buf.get(), size)) {
+ return std::unique_ptr<char[]>();
+ }
+ return buf;
+}
+
+ScudoCrashData::ScudoCrashData(unwindstack::Memory* process_memory,
+ const ProcessInfo& process_info) {
if (!process_info.has_fault_address) {
- return false;
+ return;
}
- std::vector<char> stack_depot(__scudo_get_stack_depot_size());
- if (!process_memory->ReadFully(process_info.scudo_stack_depot, stack_depot.data(),
- stack_depot.size())) {
- return false;
+ auto stack_depot = AllocAndReadFully(process_memory, process_info.scudo_stack_depot,
+ __scudo_get_stack_depot_size());
+ auto region_info = AllocAndReadFully(process_memory, process_info.scudo_region_info,
+ __scudo_get_region_info_size());
+ auto ring_buffer = AllocAndReadFully(process_memory, process_info.scudo_ring_buffer,
+ __scudo_get_ring_buffer_size());
+ if (!stack_depot || !region_info || !ring_buffer) {
+ return;
}
- std::vector<char> region_info(__scudo_get_region_info_size());
- if (!process_memory->ReadFully(process_info.scudo_region_info, region_info.data(),
- region_info.size())) {
- return false;
- }
- std::vector<char> ring_buffer(__scudo_get_ring_buffer_size());
- if (!process_memory->ReadFully(process_info.scudo_ring_buffer, ring_buffer.data(),
- ring_buffer.size())) {
- return false;
- }
-
- uintptr_t page_size = getpagesize();
untagged_fault_addr_ = process_info.untagged_fault_address;
- uintptr_t fault_page = untagged_fault_addr_ & ~(page_size - 1);
+ uintptr_t fault_page = untagged_fault_addr_ & ~(PAGE_SIZE - 1);
- // Attempt to get 16 pages before the fault page and 16 pages after.
- constexpr size_t kExtraPages = 16;
- std::vector<char> memory(page_size * (kExtraPages * 2 + 1));
-
- // Read faulting page first.
- size_t memory_index = kExtraPages;
- if (!process_memory->ReadFully(fault_page, &memory[memory_index * page_size], page_size)) {
- return false;
+ uintptr_t memory_begin = fault_page - PAGE_SIZE * 16;
+ if (memory_begin > fault_page) {
+ return;
}
- // Attempt to read the pages after the fault page, stop as soon as we
- // fail to read.
- uintptr_t read_addr = fault_page;
- if (!__builtin_add_overflow(fault_page, page_size, &read_addr)) {
- memory_index++;
- for (size_t i = 0; i < kExtraPages; i++, memory_index++) {
- if (!process_memory->ReadFully(read_addr, &memory[memory_index * page_size], page_size)) {
- break;
- }
- if (__builtin_add_overflow(read_addr, page_size, &read_addr)) {
- break;
- }
- }
- }
- uintptr_t memory_end = read_addr;
-
- // Attempt to read the pages before the fault page, stop as soon as we
- // fail to read.
- memory_index = kExtraPages;
- if (fault_page > 0) {
- read_addr = fault_page - page_size;
- for (size_t i = 0; i < kExtraPages; i++, memory_index--) {
- if (!process_memory->ReadFully(read_addr, &memory[(memory_index - 1) * page_size],
- page_size)) {
- break;
- }
- if (read_addr == 0) {
- memory_index--;
- break;
- }
- read_addr -= page_size;
- }
- }
- size_t start_memory_index = memory_index;
- uintptr_t memory_begin = fault_page - (kExtraPages - memory_index) * page_size;
-
- std::vector<long> memory_tags((memory_end - memory_begin) / kTagGranuleSize);
- read_addr = memory_begin;
- for (size_t i = 0; i < memory_tags.size(); i++) {
- memory_tags[i] = process_memory->ReadTag(read_addr);
- read_addr += kTagGranuleSize;
+ uintptr_t memory_end = fault_page + PAGE_SIZE * 16;
+ if (memory_end < fault_page) {
+ return;
}
- __scudo_get_error_info(
- &error_info_, process_info.maybe_tagged_fault_address, stack_depot.data(), region_info.data(),
- ring_buffer.data(), &memory[start_memory_index * page_size],
- reinterpret_cast<const char*>(memory_tags.data()), memory_begin, memory_end - memory_begin);
+ auto memory = std::make_unique<char[]>(memory_end - memory_begin);
+ for (auto i = memory_begin; i != memory_end; i += PAGE_SIZE) {
+ process_memory->ReadFully(i, memory.get() + i - memory_begin, PAGE_SIZE);
+ }
- return true;
+ auto memory_tags = std::make_unique<char[]>((memory_end - memory_begin) / kTagGranuleSize);
+ for (auto i = memory_begin; i != memory_end; i += kTagGranuleSize) {
+ memory_tags[(i - memory_begin) / kTagGranuleSize] = process_memory->ReadTag(i);
+ }
+
+ __scudo_get_error_info(&error_info_, process_info.maybe_tagged_fault_address, stack_depot.get(),
+ region_info.get(), ring_buffer.get(), memory.get(), memory_tags.get(),
+ memory_begin, memory_end - memory_begin);
}
bool ScudoCrashData::CrashIsMine() const {
diff --git a/debuggerd/libdebuggerd/test/scudo_test.cpp b/debuggerd/libdebuggerd/test/scudo_test.cpp
deleted file mode 100644
index d8fc6a7..0000000
--- a/debuggerd/libdebuggerd/test/scudo_test.cpp
+++ /dev/null
@@ -1,233 +0,0 @@
-/*
- * Copyright (C) 2022 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#include <stdlib.h>
-#include <unistd.h>
-
-#include <gmock/gmock.h>
-#include <gtest/gtest.h>
-
-#include "libdebuggerd/scudo.h"
-#include "libdebuggerd/types.h"
-#include "unwindstack/Memory.h"
-
-#include "log_fake.h"
-
-#include <inttypes.h>
-
-// This needs to match the kExtraPages from ScudoCrashData::SetErrorInfo.
-constexpr uint64_t kMaxPages = 16;
-
-class MemoryAlwaysZero : public unwindstack::Memory {
- public:
- MemoryAlwaysZero() = default;
- virtual ~MemoryAlwaysZero() = default;
-
- size_t Read(uint64_t addr, void* buffer, size_t size) override {
- if (test_unreadable_addrs_.count(addr) != 0) {
- return 0;
- }
- test_read_addrs_.insert(addr);
- memset(buffer, 0, size);
- return size;
- }
-
- void TestAddUnreadableAddress(uint64_t addr) { test_unreadable_addrs_.insert(addr); }
-
- void TestClearAddresses() {
- test_read_addrs_.clear();
- test_unreadable_addrs_.clear();
- }
-
- std::set<uint64_t>& test_read_addrs() { return test_read_addrs_; }
-
- private:
- std::set<uint64_t> test_unreadable_addrs_;
-
- std::set<uint64_t> test_read_addrs_;
-};
-
-TEST(ScudoTest, no_fault_address) {
- MemoryAlwaysZero process_memory;
- ProcessInfo info;
- info.has_fault_address = false;
- info.untagged_fault_address = 0x5000;
- info.scudo_stack_depot = 0x1000;
- info.scudo_region_info = 0x2000;
- info.scudo_ring_buffer = 0x3000;
-
- ScudoCrashData crash;
- ASSERT_FALSE(crash.SetErrorInfo(&process_memory, info));
-
- info.has_fault_address = true;
- ASSERT_TRUE(crash.SetErrorInfo(&process_memory, info));
-}
-
-TEST(ScudoTest, scudo_data_read_check) {
- MemoryAlwaysZero process_memory;
- ProcessInfo info;
- info.has_fault_address = true;
- info.untagged_fault_address = 0x5000;
- info.scudo_stack_depot = 0x1000;
- info.scudo_region_info = 0x2000;
- info.scudo_ring_buffer = 0x3000;
-
- ScudoCrashData crash;
- ASSERT_TRUE(crash.SetErrorInfo(&process_memory, info));
-
- // Stack Depot unreadable
- process_memory.TestClearAddresses();
- process_memory.TestAddUnreadableAddress(0x1000);
- ASSERT_FALSE(crash.SetErrorInfo(&process_memory, info));
-
- // The Region Info doesn't exist for 32 bit.
-#if defined(__LP64__)
- // Region Info unreadable
- process_memory.TestClearAddresses();
- process_memory.TestAddUnreadableAddress(0x2000);
- ASSERT_FALSE(crash.SetErrorInfo(&process_memory, info));
-#endif
-
- // Ring Buffer unreadable
- process_memory.TestClearAddresses();
- process_memory.TestAddUnreadableAddress(0x3000);
- ASSERT_FALSE(crash.SetErrorInfo(&process_memory, info));
-
- // Verify that with all scudo data readable, the error info works.
- process_memory.TestClearAddresses();
- ASSERT_TRUE(crash.SetErrorInfo(&process_memory, info));
-}
-
-TEST(ScudoTest, fault_page_unreadable) {
- MemoryAlwaysZero process_memory;
- ProcessInfo info;
- info.has_fault_address = true;
- info.untagged_fault_address = 0x5124;
- info.scudo_stack_depot = 0x1000;
- info.scudo_region_info = 0x2000;
- info.scudo_ring_buffer = 0x3000;
-
- ScudoCrashData crash;
- ASSERT_TRUE(crash.SetErrorInfo(&process_memory, info));
-
- uint64_t fault_page = info.untagged_fault_address & ~(getpagesize() - 1);
- process_memory.TestAddUnreadableAddress(fault_page);
- ASSERT_FALSE(crash.SetErrorInfo(&process_memory, info));
-}
-
-TEST(ScudoTest, pages_before_fault_unreadable) {
- MemoryAlwaysZero process_memory;
- ProcessInfo info;
- info.has_fault_address = true;
- info.untagged_fault_address = 0x15124;
- info.scudo_stack_depot = 0x1000;
- info.scudo_region_info = 0x2000;
- info.scudo_ring_buffer = 0x3000;
-
- ScudoCrashData crash;
- ASSERT_TRUE(crash.SetErrorInfo(&process_memory, info));
-
- uint64_t page_size = getpagesize();
- uint64_t fault_page = info.untagged_fault_address & ~(page_size - 1);
-
- std::vector<uint64_t> expected_reads = {0x1000, 0x2000, 0x3000};
- for (size_t i = 0; i <= kMaxPages; i++) {
- expected_reads.emplace_back(fault_page + i * page_size);
- }
-
- // Loop through and make pages before the fault page unreadable.
- for (size_t i = 1; i <= kMaxPages + 1; i++) {
- process_memory.TestClearAddresses();
- uint64_t unreadable_addr = fault_page - i * page_size;
- SCOPED_TRACE(testing::Message()
- << "Failed at unreadable address 0x" << std::hex << unreadable_addr);
- process_memory.TestAddUnreadableAddress(unreadable_addr);
- ASSERT_TRUE(crash.SetErrorInfo(&process_memory, info));
- ASSERT_THAT(process_memory.test_read_addrs(),
- testing::UnorderedElementsAreArray(expected_reads));
- // Need to add the previous unreadable_addr to the list of expected addresses.
- expected_reads.emplace_back(unreadable_addr);
- }
-}
-
-TEST(ScudoTest, pages_after_fault_unreadable) {
- MemoryAlwaysZero process_memory;
- ProcessInfo info;
- info.has_fault_address = true;
- info.untagged_fault_address = 0x15124;
- info.scudo_stack_depot = 0x1000;
- info.scudo_region_info = 0x2000;
- info.scudo_ring_buffer = 0x3000;
-
- ScudoCrashData crash;
- ASSERT_TRUE(crash.SetErrorInfo(&process_memory, info));
-
- uint64_t page_size = getpagesize();
- uint64_t fault_page = info.untagged_fault_address & ~(page_size - 1);
-
- std::vector<uint64_t> expected_reads = {0x1000, 0x2000, 0x3000};
- for (size_t i = 0; i <= kMaxPages; i++) {
- expected_reads.emplace_back(fault_page - i * page_size);
- }
-
- // Loop through and make pages after the fault page unreadable.
- for (size_t i = 1; i <= kMaxPages + 1; i++) {
- process_memory.TestClearAddresses();
- uint64_t unreadable_addr = fault_page + i * page_size;
- SCOPED_TRACE(testing::Message()
- << "Failed at unreadable address 0x" << std::hex << unreadable_addr);
- process_memory.TestAddUnreadableAddress(unreadable_addr);
- ASSERT_TRUE(crash.SetErrorInfo(&process_memory, info));
- ASSERT_THAT(process_memory.test_read_addrs(),
- testing::UnorderedElementsAreArray(expected_reads));
- // Need to add the previous unreadable_addr to the list of expected addresses.
- expected_reads.emplace_back(unreadable_addr);
- }
-}
-
-// Make sure that if the fault address is low, you won't underflow.
-TEST(ScudoTest, fault_address_low) {
- MemoryAlwaysZero process_memory;
- ProcessInfo info;
- info.has_fault_address = true;
- info.scudo_stack_depot = 0x21000;
- info.scudo_region_info = 0x22000;
- info.scudo_ring_buffer = 0x23000;
-
- ScudoCrashData crash;
- ASSERT_TRUE(crash.SetErrorInfo(&process_memory, info));
-
- uint64_t page_size = getpagesize();
- for (size_t i = 0; i < kMaxPages + 1; i++) {
- process_memory.TestClearAddresses();
- info.untagged_fault_address = 0x124 + i * getpagesize();
- SCOPED_TRACE(testing::Message()
- << "Failed with fault address 0x" << std::hex << info.untagged_fault_address);
- ASSERT_TRUE(crash.SetErrorInfo(&process_memory, info));
- std::vector<uint64_t> expected_reads = {0x21000, 0x22000, 0x23000};
- uint64_t fault_page = info.untagged_fault_address & ~(page_size - 1);
- expected_reads.emplace_back(fault_page);
- for (size_t j = 1; j <= kMaxPages; j++) {
- expected_reads.emplace_back(fault_page + j * page_size);
- }
- while (fault_page != 0) {
- fault_page -= page_size;
- expected_reads.emplace_back(fault_page);
- }
- ASSERT_THAT(process_memory.test_read_addrs(),
- testing::UnorderedElementsAreArray(expected_reads));
- }
-}
diff --git a/debuggerd/libdebuggerd/tombstone_proto.cpp b/debuggerd/libdebuggerd/tombstone_proto.cpp
index 6e1ce8f..159ebc8 100644
--- a/debuggerd/libdebuggerd/tombstone_proto.cpp
+++ b/debuggerd/libdebuggerd/tombstone_proto.cpp
@@ -193,9 +193,8 @@
static void dump_probable_cause(Tombstone* tombstone, unwindstack::AndroidUnwinder* unwinder,
const ProcessInfo& process_info, const ThreadInfo& main_thread) {
#if defined(USE_SCUDO)
- ScudoCrashData scudo_crash_data;
- if (scudo_crash_data.SetErrorInfo(unwinder->GetProcessMemory().get(), process_info) &&
- scudo_crash_data.CrashIsMine()) {
+ ScudoCrashData scudo_crash_data(unwinder->GetProcessMemory().get(), process_info);
+ if (scudo_crash_data.CrashIsMine()) {
scudo_crash_data.AddCauseProtos(tombstone, unwinder);
return;
}