Only call one unwinder.
Nobody is looking at the mismatches, and it can cause problems
with tombstone parsers.
Also, fix the dump_header_info test and remove unused properties_fake.cpp.
Test: Ran unit tests, verified tombstones still work.
Change-Id: I4261646016b4e84b26a5aee72f3227f1ce48ec9a
diff --git a/debuggerd/Android.bp b/debuggerd/Android.bp
index 2b5f4f6..17a9f3a 100644
--- a/debuggerd/Android.bp
+++ b/debuggerd/Android.bp
@@ -193,7 +193,6 @@
"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 6ef3ed6..827420e 100644
--- a/debuggerd/crash_dump.cpp
+++ b/debuggerd/crash_dump.cpp
@@ -348,11 +348,6 @@
LOG(FATAL) << "failed to create backtrace map";
}
}
- std::unique_ptr<BacktraceMap> 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 @@
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 45740df..79743b6 100644
--- a/debuggerd/libdebuggerd/include/libdebuggerd/tombstone.h
+++ b/debuggerd/libdebuggerd/include/libdebuggerd/tombstone.h
@@ -35,10 +35,10 @@
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<pid_t, std::string>& 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<pid_t, std::string>& 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 02069f1..0000000
--- 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 <string.h>
-
-#include <string>
-#include <unordered_map>
-
-#include <sys/system_properties.h>
-
-std::unordered_map<std::string, std::string> 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 934ceba..59a43b7 100644
--- a/debuggerd/libdebuggerd/test/tombstone_test.cpp
+++ b/debuggerd/libdebuggerd/test/tombstone_test.cpp
@@ -19,8 +19,9 @@
#include <memory>
#include <string>
-#include <gtest/gtest.h>
#include <android-base/file.h>
+#include <android-base/properties.h>
+#include <gtest/gtest.h>
#include "libdebuggerd/utility.h"
@@ -639,7 +640,10 @@
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 6fb29a9..725c42c 100644
--- a/debuggerd/libdebuggerd/tombstone.cpp
+++ b/debuggerd/libdebuggerd/tombstone.cpp
@@ -496,55 +496,8 @@
_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<uint64_t> buffer(8192 / sizeof(uint64_t));
- size_t bytes =
- backtrace->Read(backtrace->GetFrame(0)->sp, reinterpret_cast<uint8_t*>(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 @@
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> 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 @@
}
log->current_tid = log->crashed_tid;
-
- return matches;
}
// Reads the contents of the specified log device, filters out the entries
@@ -718,18 +658,16 @@
}
// 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<pid_t, std::string>& 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<pid_t, std::string>& 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 @@
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 @@
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<pid_t, std::string>& 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<pid_t, std::string>& 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,