Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 15fee822 authored by Christopher Ferris's avatar Christopher Ferris
Browse files

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
parent 61c82abe
Loading
Loading
Loading
Loading
+12 −5
Original line number Diff line number Diff line
@@ -299,6 +299,7 @@ cc_test {

    srcs: [
        "BitSet_test.cpp",
        "CallStack_test.cpp",
        "Errors_test.cpp",
        "FileMap_test.cpp",
        "LruCache_test.cpp",
@@ -319,11 +320,14 @@ cc_test {
                "SystemClock_test.cpp",
            ],
            shared_libs: [
                "libz",
                "liblog",
                "libbase",
                "libcutils",
                "liblog",
                "liblzma",
                "libunwindstack",
                "libutils",
                "libbase",
                "libutilscallstack",
                "libz",
            ],
        },
        linux: {
@@ -334,9 +338,12 @@ cc_test {
        },
        host: {
            static_libs: [
                "libutils",
                "liblog",
                "libbase",
                "liblog",
                "liblzma",
                "libunwindstack_no_dex",
                "libutils",
                "libutilscallstack",
            ],
        },
    },
+1 −1
Original line number Diff line number Diff line
@@ -49,7 +49,7 @@ void CallStack::update(int32_t ignoreDepth, pid_t tid) {
    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());
+62 −0
Original line number Diff line number Diff line
/*
 * 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();
}