Fix thread unwind in CallStack.
The CallStack unwind of a different thread was broken since it
wasn't properly setting the tid value.
Fix this problem and add new unit tests to verify the behavior.
Bug: 246405269
Test: New unit tests pass.
Test: Ran unit tests for 1000 operations to verify not flaky.
Change-Id: I00342e6cdcdb4bcb68f29734dadee6c987c98040
diff --git a/libutils/Android.bp b/libutils/Android.bp
index 7939e82..c744b53 100644
--- a/libutils/Android.bp
+++ b/libutils/Android.bp
@@ -299,6 +299,7 @@
srcs: [
"BitSet_test.cpp",
+ "CallStack_test.cpp",
"Errors_test.cpp",
"FileMap_test.cpp",
"LruCache_test.cpp",
@@ -319,11 +320,14 @@
"SystemClock_test.cpp",
],
shared_libs: [
- "libz",
- "liblog",
- "libcutils",
- "libutils",
"libbase",
+ "libcutils",
+ "liblog",
+ "liblzma",
+ "libunwindstack",
+ "libutils",
+ "libutilscallstack",
+ "libz",
],
},
linux: {
@@ -334,9 +338,12 @@
},
host: {
static_libs: [
- "libutils",
- "liblog",
"libbase",
+ "liblog",
+ "liblzma",
+ "libunwindstack_no_dex",
+ "libutils",
+ "libutilscallstack",
],
},
},
diff --git a/libutils/CallStack.cpp b/libutils/CallStack.cpp
index f19ba6a..4dcb35b 100644
--- a/libutils/CallStack.cpp
+++ b/libutils/CallStack.cpp
@@ -49,7 +49,7 @@
unwindstack::AndroidUnwinderData data;
std::optional<pid_t> tid_val;
if (tid != -1) {
- *tid_val = tid;
+ tid_val = tid;
}
if (!unwinder.Unwind(tid_val, data)) {
ALOGW("%s: Failed to unwind callstack: %s", __FUNCTION__, data.GetErrorString().c_str());
diff --git a/libutils/CallStack_test.cpp b/libutils/CallStack_test.cpp
new file mode 100644
index 0000000..2ea1911
--- /dev/null
+++ b/libutils/CallStack_test.cpp
@@ -0,0 +1,62 @@
+/*
+ * 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 <unistd.h>
+
+#include <thread>
+
+#include <android-base/threads.h>
+#include <gtest/gtest.h>
+#include <utils/CallStack.h>
+
+[[clang::noinline]] extern "C" void CurrentCaller(android::String8& backtrace) {
+ android::CallStack cs;
+ cs.update();
+ backtrace = cs.toString();
+}
+
+TEST(CallStackTest, current_backtrace) {
+ android::String8 backtrace;
+ CurrentCaller(backtrace);
+
+ ASSERT_NE(-1, backtrace.find("(CurrentCaller")) << "Full backtrace:\n" << backtrace;
+}
+
+[[clang::noinline]] extern "C" void ThreadBusyWait(std::atomic<pid_t>* tid, volatile bool* done) {
+ *tid = android::base::GetThreadId();
+ while (!*done) {
+ }
+}
+
+TEST(CallStackTest, thread_backtrace) {
+ // Use a volatile to avoid any problems unwinding since sometimes
+ // accessing a std::atomic does not include unwind data at every
+ // instruction and leads to failed unwinds.
+ volatile bool done = false;
+ std::atomic<pid_t> tid = -1;
+ std::thread thread([&tid, &done]() { ThreadBusyWait(&tid, &done); });
+
+ while (tid == -1) {
+ }
+
+ android::CallStack cs;
+ cs.update(0, tid);
+
+ done = true;
+ thread.join();
+
+ ASSERT_NE(-1, cs.toString().find("(ThreadBusyWait")) << "Full backtrace:\n" << cs.toString();
+}