diff --git a/debuggerd/Android.bp b/debuggerd/Android.bp index 2b5f4f646..17a9f3a7c 100644 --- a/debuggerd/Android.bp +++ b/debuggerd/Android.bp @@ -193,7 +193,6 @@ cc_test { "libdebuggerd/test/elf_fake.cpp", "libdebuggerd/test/log_fake.cpp", "libdebuggerd/test/open_files_list_test.cpp", - "libdebuggerd/test/property_fake.cpp", "libdebuggerd/test/ptrace_fake.cpp", "libdebuggerd/test/tombstone_test.cpp", ], diff --git a/debuggerd/crash_dump.cpp b/debuggerd/crash_dump.cpp index 6ef3ed650..827420e1b 100644 --- a/debuggerd/crash_dump.cpp +++ b/debuggerd/crash_dump.cpp @@ -348,11 +348,6 @@ int main(int argc, char** argv) { LOG(FATAL) << "failed to create backtrace map"; } } - std::unique_ptr backtrace_map_new; - backtrace_map_new.reset(BacktraceMap::CreateNew(main_tid)); - if (!backtrace_map_new) { - LOG(FATAL) << "failed to create backtrace map new"; - } // Collect the list of open files. OpenFilesList open_files; @@ -432,9 +427,8 @@ int main(int argc, char** argv) { dump_backtrace(output_fd.get(), backtrace_map.get(), target, main_tid, process_name, threads, 0); } else { ATRACE_NAME("engrave_tombstone"); - engrave_tombstone(output_fd.get(), backtrace_map.get(), backtrace_map_new.get(), &open_files, - target, main_tid, process_name, threads, abort_address, - fatal_signal ? &amfd_data : nullptr); + engrave_tombstone(output_fd.get(), backtrace_map.get(), &open_files, target, main_tid, + process_name, threads, abort_address, fatal_signal ? &amfd_data : nullptr); } // We don't actually need to PTRACE_DETACH, as long as our tracees aren't in diff --git a/debuggerd/libdebuggerd/include/libdebuggerd/tombstone.h b/debuggerd/libdebuggerd/include/libdebuggerd/tombstone.h index 45740df7d..79743b61c 100644 --- a/debuggerd/libdebuggerd/include/libdebuggerd/tombstone.h +++ b/debuggerd/libdebuggerd/include/libdebuggerd/tombstone.h @@ -35,10 +35,10 @@ class BacktraceMap; int open_tombstone(std::string* path); /* Creates a tombstone file and writes the crash dump to it. */ -void engrave_tombstone(int tombstone_fd, BacktraceMap* map, BacktraceMap* map_new, - const OpenFilesList* open_files, pid_t pid, pid_t tid, - const std::string& process_name, const std::map& threads, - uintptr_t abort_msg_address, std::string* amfd_data); +void engrave_tombstone(int tombstone_fd, BacktraceMap* map, const OpenFilesList* open_files, + pid_t pid, pid_t tid, const std::string& process_name, + const std::map& threads, uintptr_t abort_msg_address, + std::string* amfd_data); void engrave_tombstone_ucontext(int tombstone_fd, uintptr_t abort_msg_address, siginfo_t* siginfo, ucontext_t* ucontext); diff --git a/debuggerd/libdebuggerd/test/property_fake.cpp b/debuggerd/libdebuggerd/test/property_fake.cpp deleted file mode 100644 index 02069f1af..000000000 --- a/debuggerd/libdebuggerd/test/property_fake.cpp +++ /dev/null @@ -1,45 +0,0 @@ -/* - * Copyright (C) 2015 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 - -#include -#include - -#include - -std::unordered_map g_properties; - -extern "C" int property_set(const char* name, const char* value) { - if (g_properties.count(name) != 0) { - g_properties.erase(name); - } - g_properties[name] = value; - return 0; -} - -extern "C" int property_get(const char* key, char* value, const char* default_value) { - if (g_properties.count(key) == 0) { - if (default_value == nullptr) { - return 0; - } - strncpy(value, default_value, PROP_VALUE_MAX-1); - } else { - strncpy(value, g_properties[key].c_str(), PROP_VALUE_MAX-1); - } - value[PROP_VALUE_MAX-1] = '\0'; - return strlen(value); -} diff --git a/debuggerd/libdebuggerd/test/tombstone_test.cpp b/debuggerd/libdebuggerd/test/tombstone_test.cpp index 934ceba49..59a43b73b 100644 --- a/debuggerd/libdebuggerd/test/tombstone_test.cpp +++ b/debuggerd/libdebuggerd/test/tombstone_test.cpp @@ -19,8 +19,9 @@ #include #include -#include #include +#include +#include #include "libdebuggerd/utility.h" @@ -639,7 +640,10 @@ TEST_F(TombstoneTest, dump_log_file_error) { TEST_F(TombstoneTest, dump_header_info) { dump_header_info(&log_); - std::string expected = "Build fingerprint: 'unknown'\nRevision: 'unknown'\n"; + std::string expected = android::base::StringPrintf( + "Build fingerprint: '%s'\nRevision: '%s'\n", + android::base::GetProperty("ro.build.fingerprint", "unknown").c_str(), + android::base::GetProperty("ro.revision", "unknown").c_str()); expected += android::base::StringPrintf("ABI: '%s'\n", ABI_STRING); ASSERT_STREQ(expected.c_str(), amfd_data_.c_str()); } diff --git a/debuggerd/libdebuggerd/tombstone.cpp b/debuggerd/libdebuggerd/tombstone.cpp index 6fb29a938..725c42cad 100644 --- a/debuggerd/libdebuggerd/tombstone.cpp +++ b/debuggerd/libdebuggerd/tombstone.cpp @@ -496,55 +496,8 @@ __attribute__((weak)) void dump_registers(log_t* log, const ucontext_t*) { _LOG(log, logtype::REGISTERS, " register dumping unimplemented on this architecture"); } -static bool verify_backtraces_equal(Backtrace* back1, Backtrace* back2) { - if (back1->NumFrames() != back2->NumFrames()) { - return false; - } - std::string back1_str; - std::string back2_str; - for (size_t i = 0; i < back1->NumFrames(); i++) { - back1_str += back1->FormatFrameData(i); - back2_str += back2->FormatFrameData(i); - } - return back1_str == back2_str; -} - -static void log_mismatch_data(log_t* log, Backtrace* backtrace) { - _LOG(log, logtype::THREAD, "MISMATCH: This unwind is different.\n"); - if (backtrace->NumFrames() == 0) { - _LOG(log, logtype::THREAD, "MISMATCH: No frames in new backtrace.\n"); - return; - } - _LOG(log, logtype::THREAD, "MISMATCH: Backtrace from new unwinder.\n"); - for (size_t i = 0; i < backtrace->NumFrames(); i++) { - _LOG(log, logtype::THREAD, "MISMATCH: %s\n", backtrace->FormatFrameData(i).c_str()); - } - - // Get the stack trace up to 8192 bytes. - std::vector buffer(8192 / sizeof(uint64_t)); - size_t bytes = - backtrace->Read(backtrace->GetFrame(0)->sp, reinterpret_cast(buffer.data()), - buffer.size() * sizeof(uint64_t)); - std::string log_data; - for (size_t i = 0; i < bytes / sizeof(uint64_t); i++) { - if ((i % 4) == 0) { - if (!log_data.empty()) { - _LOG(log, logtype::THREAD, "MISMATCH: stack_data%s\n", log_data.c_str()); - log_data = ""; - } - } - log_data += android::base::StringPrintf(" 0x%016" PRIx64, buffer[i]); - } - - if (!log_data.empty()) { - _LOG(log, logtype::THREAD, "MISMATCH: data%s\n", log_data.c_str()); - } - - // If there is any leftover (bytes % sizeof(uint64_t) != 0, ignore it for now. -} - -static bool dump_thread(log_t* log, pid_t pid, pid_t tid, const std::string& process_name, - const std::string& thread_name, BacktraceMap* map, BacktraceMap* map_new, +static void dump_thread(log_t* log, pid_t pid, pid_t tid, const std::string& process_name, + const std::string& thread_name, BacktraceMap* map, uintptr_t abort_msg_address, bool primary_thread) { log->current_tid = tid; if (!primary_thread) { @@ -558,18 +511,7 @@ static bool dump_thread(log_t* log, pid_t pid, pid_t tid, const std::string& pro dump_abort_message(backtrace.get(), log, abort_msg_address); } dump_registers(log, tid); - bool matches = true; if (backtrace->Unwind(0)) { - // Use the new method and verify it is the same as old. - std::unique_ptr backtrace_new(Backtrace::CreateNew(pid, tid, map_new)); - if (!backtrace_new->Unwind(0)) { - _LOG(log, logtype::THREAD, "Failed to unwind with new unwinder: %s\n", - backtrace_new->GetErrorString(backtrace_new->GetError()).c_str()); - matches = false; - } else if (!verify_backtraces_equal(backtrace.get(), backtrace_new.get())) { - log_mismatch_data(log, backtrace_new.get()); - matches = false; - } dump_backtrace_and_stack(backtrace.get(), log); } else { ALOGE("Unwind failed: pid = %d, tid = %d", pid, tid); @@ -583,8 +525,6 @@ static bool dump_thread(log_t* log, pid_t pid, pid_t tid, const std::string& pro } log->current_tid = log->crashed_tid; - - return matches; } // Reads the contents of the specified log device, filters out the entries @@ -718,18 +658,16 @@ static void dump_logs(log_t* log, pid_t pid, unsigned int tail) { } // Dumps all information about the specified pid to the tombstone. -static void dump_crash(log_t* log, BacktraceMap* map, BacktraceMap* map_new, - const OpenFilesList* open_files, pid_t pid, pid_t tid, - const std::string& process_name, const std::map& threads, - uintptr_t abort_msg_address) { +static void dump_crash(log_t* log, BacktraceMap* map, const OpenFilesList* open_files, pid_t pid, + pid_t tid, const std::string& process_name, + const std::map& threads, uintptr_t abort_msg_address) { // don't copy log messages to tombstone unless this is a dev device bool want_logs = GetBoolProperty("ro.debuggable", false); _LOG(log, logtype::HEADER, "*** *** *** *** *** *** *** *** *** *** *** *** *** *** *** ***\n"); dump_header_info(log); - bool new_unwind_matches = dump_thread(log, pid, tid, process_name, threads.find(tid)->second, map, - map_new, abort_msg_address, true); + dump_thread(log, pid, tid, process_name, threads.find(tid)->second, map, abort_msg_address, true); if (want_logs) { dump_logs(log, pid, 5); } @@ -739,9 +677,7 @@ static void dump_crash(log_t* log, BacktraceMap* map, BacktraceMap* map_new, const std::string& thread_name = it.second; if (thread_tid != tid) { - bool match = - dump_thread(log, pid, thread_tid, process_name, thread_name, map, map_new, 0, false); - new_unwind_matches = new_unwind_matches && match; + dump_thread(log, pid, thread_tid, process_name, thread_name, map, 0, false); } } @@ -753,26 +689,18 @@ static void dump_crash(log_t* log, BacktraceMap* map, BacktraceMap* map_new, if (want_logs) { dump_logs(log, pid, 0); } - if (!new_unwind_matches) { - _LOG(log, logtype::THREAD, "MISMATCH: New and old unwinder do not agree.\n"); - _LOG(log, logtype::THREAD, "MISMATCH: If you see this please file a bug in:\n"); - _LOG(log, logtype::THREAD, - "MISMATCH: Android > Android OS & Apps > Runtime > native > tools " - "(debuggerd/gdb/init/simpleperf/strace/valgrind)\n"); - _LOG(log, logtype::THREAD, "MISMATCH: and attach this tombstone.\n"); - } } -void engrave_tombstone(int tombstone_fd, BacktraceMap* map, BacktraceMap* map_new, - const OpenFilesList* open_files, pid_t pid, pid_t tid, - const std::string& process_name, const std::map& threads, - uintptr_t abort_msg_address, std::string* amfd_data) { +void engrave_tombstone(int tombstone_fd, BacktraceMap* map, const OpenFilesList* open_files, + pid_t pid, pid_t tid, const std::string& process_name, + const std::map& threads, uintptr_t abort_msg_address, + std::string* amfd_data) { log_t log; log.current_tid = tid; log.crashed_tid = tid; log.tfd = tombstone_fd; log.amfd_data = amfd_data; - dump_crash(&log, map, map_new, open_files, pid, tid, process_name, threads, abort_msg_address); + dump_crash(&log, map, open_files, pid, tid, process_name, threads, abort_msg_address); } void engrave_tombstone_ucontext(int tombstone_fd, uintptr_t abort_msg_address, siginfo_t* siginfo,