Merge pull request #7123 from thuanquang/fix/guest-frame-chain-zero-record

fix(libkernel): end guest thread, fiber and context frame chains at a zero record

core/libs/prx/libSceFiber/Export.cpp

@@ -167,6 +167,9 @@ Aps5FiberSwitchStack_nid_no_patch:
 Aps5FiberTrampoline_nid_no_patch:
     mov %r12, %rcx
     and $-16, %rsp
+    push $0
+    push $0
+    mov %rsp, %rbp
     sub $32, %rsp
     call Aps5FiberMain_nid_no_patch
     ud2
@@ -236,6 +239,9 @@ asm(".text\n"
     APS5_ASM_FUNCTION("Aps5FiberTrampoline_nid_no_patch") R"(
     mov %r12, %rdi
     and $-16, %rsp
+    push $0
+    push $0
+    mov %rsp, %rbp
     call )" APS5_ASM_SYMBOL("Aps5FiberMain_nid_no_patch") R"(
     ud2
 )" APS5_ASM_FUNCTION_END("Aps5FiberTrampoline_nid_no_patch"));

core/libs/prx/libSceLibcInternal/CMakeLists.txt

@@ -1,5 +1,6 @@
 add_library(libSceLibcInternal SHARED EXCLUDE_FROM_ALL Export.cpp FileTime.cpp Getopt.cpp SignalList.cpp Spawn.cpp UserContext.cpp)
 target_include_directories(libSceLibcInternal PRIVATE ${LIBS_INCLUDE_DIR})
+set_source_files_properties(UserContext.cpp PROPERTIES COMPILE_OPTIONS -fno-omit-frame-pointer)
 target_link_libraries(libSceLibcInternal PRIVATE ${CMAKE_DL_LIBS})
 target_sources(libSceLibcInternal PRIVATE Directory.cpp)
 target_sources(libSceLibcInternal PRIVATE Err.cpp)

core/libs/prx/libkernel/CMakeLists.txt

@@ -478,6 +478,17 @@ if(BUILD_TESTING)
         set_tests_properties(guest_pthread_attr_stack PROPERTIES ENVIRONMENT_MODIFICATION
             "PATH=path_list_prepend:$<TARGET_FILE_DIR:libkernel>")
     endif()
+    add_test_executable(guest_pthread_frame_chain_tests ${LIBS_INCLUDE_DIR}/tests/GuestPthreadFrameChain.cpp)
+    target_include_directories(guest_pthread_frame_chain_tests PRIVATE ${LIBS_INCLUDE_DIR})
+    target_compile_options(guest_pthread_frame_chain_tests PRIVATE -fno-omit-frame-pointer)
+    target_link_libraries(guest_pthread_frame_chain_tests PRIVATE libkernel libc)
+    configure_windows_unwind(guest_pthread_frame_chain_tests)
+    add_test(NAME guest_pthread_frame_chain COMMAND guest_pthread_frame_chain_tests)
+    set_tests_properties(guest_pthread_frame_chain PROPERTIES TIMEOUT 20)
+    if(WIN32)
+        set_tests_properties(guest_pthread_frame_chain PROPERTIES ENVIRONMENT_MODIFICATION
+            "PATH=path_list_prepend:$<TARGET_FILE_DIR:libkernel>")
+    endif()
     add_test_executable(guest_pthread_mutexattr_tests ${LIBS_INCLUDE_DIR}/tests/GuestPthreadMutexattr.cpp)
     target_include_directories(guest_pthread_mutexattr_tests PRIVATE ${LIBS_INCLUDE_DIR})
     target_link_libraries(guest_pthread_mutexattr_tests PRIVATE libkernel libc)
@@ -736,6 +747,7 @@ if(BUILD_TESTING)
     endif()
     add_test_executable(guest_user_context_tests ${LIBS_INCLUDE_DIR}/tests/GuestUserContext.cpp)
     target_include_directories(guest_user_context_tests PRIVATE ${LIBS_INCLUDE_DIR})
+    target_compile_options(guest_user_context_tests PRIVATE -fno-omit-frame-pointer)
     target_link_libraries(guest_user_context_tests PRIVATE libSceLibcInternal libkernel libc)
     configure_windows_unwind(guest_user_context_tests)
     add_test(NAME guest_user_context COMMAND guest_user_context_tests)

core/libs/prx/libkernel/Pthread/src/Thread.cpp

@@ -3,6 +3,7 @@
 #include "../include/WindowsThreadLocal.hpp"
 #include "../include/Cancel.hpp"
 #include "prx/libc/include/General.hpp"
+#include "prx/libc/include/general/AsmMacros.hpp"
 #include "prx/libkernel/KernelErrors.hpp"
 #include "prx/libc/include/CpuTopology.hpp"
 #include "prx/libkernel/DirectMemory/DirectMemory.hpp"
@@ -545,6 +546,31 @@ void ThreadCancel::EndWait() {
     BeginWait(nullptr);
 }
 
+extern "C" void* APS5_VABI Aps5CallGuestEntry_nid_no_patch(PthreadEntry entry, void* arg);
+
+asm(".text\n"
+    APS5_ASM_FUNCTION("Aps5CallGuestEntry_nid_no_patch") R"(
+    .cfi_startproc
+    push %rbp
+    .cfi_adjust_cfa_offset 8
+    .cfi_rel_offset %rbp, 0
+    push $0
+    .cfi_adjust_cfa_offset 8
+    push $0
+    .cfi_adjust_cfa_offset 8
+    mov %rsp, %rbp
+    mov %rdi, %rax
+    mov %rsi, %rdi
+    call *%rax
+    add $16, %rsp
+    .cfi_adjust_cfa_offset -16
+    pop %rbp
+    .cfi_adjust_cfa_offset -8
+    .cfi_restore %rbp
+    ret
+    .cfi_endproc
+)" APS5_ASM_FUNCTION_END("Aps5CallGuestEntry_nid_no_patch"));
+
 static void RunThread(std::unique_ptr<ThreadArgs> args) {
     APS5_LOG_OUT("RunThread entry=0x%llx arg=%p self=%p", static_cast<unsigned long long>(reinterpret_cast<std::uintptr_t>(args->entry)), args->arg, static_cast<void*>(args->self));
     const auto entry = args->entry;
@@ -561,7 +587,7 @@ static void RunThread(std::unique_ptr<ThreadArgs> args) {
 #ifndef _WIN32
     pthread_sigmask(SIG_SETMASK, &signalMask, nullptr);
 #endif
-    finishThread(self, entry(arg));
+    finishThread(self, Aps5CallGuestEntry_nid_no_patch(entry, arg));
     CurrentGuestState().current = nullptr;
 }
 

core/libs/tests/Fiber.cpp

@@ -180,13 +180,17 @@ void APS5_VABI FrameChainEntry(std::uint64_t, std::uint64_t) {
     auto* frame = static_cast<std::uintptr_t*>(__builtin_frame_address(0));
     const auto low = reinterpret_cast<std::uintptr_t>(frame);
     const auto high = low + (1u << 20);
-    for (int depth = 0; depth < 100 && frame != nullptr; ++depth) {
+    for (int depth = 0; depth < 100; ++depth) {
         const auto address = reinterpret_cast<std::uintptr_t>(frame);
         Require(address >= low && address + 2 * sizeof(std::uintptr_t) <= high, "frame chain stays on the fiber's stack");
-        frame = reinterpret_cast<std::uintptr_t*>(frame[0]);
-        Require(frame == nullptr || reinterpret_cast<std::uintptr_t>(frame) > address, "frame chain climbs the stack");
+        if (frame[1] == 0) {
+            g_chainEnded = frame[0] == 0;
+            break;
+        }
+        auto* next = reinterpret_cast<std::uintptr_t*>(frame[0]);
+        Require(reinterpret_cast<std::uintptr_t>(next) > address, "frame chain climbs the stack to a frame whose return address is 0");
+        frame = next;
     }
-    g_chainEnded = frame == nullptr;
     std::uint64_t received = 0;
     for (;;) sceFiberReturnToThread(0, &received);
 }
@@ -207,7 +211,7 @@ void RunFrameChain() {
     auto* fiber = reinterpret_cast<FiberObject*>(&g_chainStorage);
     Require(_sceFiberInitializeImpl_nid_postfix(fiber, "chain", FrameChainEntry, 0, g_chainContext.data(), g_chainContext.size(), nullptr, 0) == 0, "initialize chain");
     Require(sceFiberRun_nid_postfix(fiber, 0, nullptr) == 0, "run chain");
-    Require(g_chainEnded, "frame chain of a fiber ends with a null frame");
+    Require(g_chainEnded, "frame chain of a fiber ends at a frame whose saved frame pointer and return address are 0");
     Require(sceFiberFinalize(fiber) == 0, "finalize chain");
 }
 

core/libs/tests/GuestPthreadFrameChain.cpp

@@ -0,0 +1,70 @@
+#include "SceTypes.hpp"
+#include <cstdint>
+#include <cstdio>
+#include <cstdlib>
+
+extern "C" {
+int APS5_VABI scePthreadCreate(Pthread* thread, const PthreadAttr* attr, PthreadEntry entry, void* arg, const char* name);
+int APS5_VABI scePthreadJoin(Pthread thread, void** retval);
+}
+
+static constexpr int SCE_OK = 0;
+static constexpr std::uintptr_t ChainReach = std::uintptr_t{64} << 10;
+
+static void Require(bool value, const char* what) {
+    if (value) return;
+    std::fprintf(stderr, "guest pthread frame chain: %s\n", what);
+    std::abort();
+}
+
+struct Walk {
+    int frames = 0;
+    bool leftStack = false;
+    bool ended = false;
+    bool endedOnZeroRecord = false;
+};
+
+static Walk g_walk;
+
+[[gnu::noinline]] static void WalkFrameChain() {
+    auto* frame = static_cast<std::uintptr_t*>(__builtin_frame_address(0));
+    const auto low = reinterpret_cast<std::uintptr_t>(frame);
+    for (int depth = 0; depth < 100; ++depth) {
+        const auto address = reinterpret_cast<std::uintptr_t>(frame);
+        if (address < low || address - low > ChainReach) {
+            g_walk.leftStack = true;
+            return;
+        }
+        ++g_walk.frames;
+        if (frame[1] == 0) {
+            g_walk.ended = true;
+            g_walk.endedOnZeroRecord = frame[0] == 0;
+            return;
+        }
+        auto* next = reinterpret_cast<std::uintptr_t*>(frame[0]);
+        if (reinterpret_cast<std::uintptr_t>(next) <= address) {
+            g_walk.leftStack = true;
+            return;
+        }
+        frame = next;
+    }
+}
+
+static void* APS5_VABI WalkingEntry(void* arg) {
+    WalkFrameChain();
+    return arg;
+}
+
+int main() {
+    Pthread thread = nullptr;
+    Require(scePthreadCreate(&thread, nullptr, WalkingEntry, &g_walk, "frame chain") == SCE_OK, "create a guest thread");
+    void* result = nullptr;
+    Require(scePthreadJoin(thread, &result) == SCE_OK, "join the guest thread");
+    Require(result == &g_walk, "the guest entry gets its argument and its return value reaches scePthreadJoin");
+    Require(!g_walk.leftStack, "the frame chain of a guest thread stays on its stack and climbs it");
+    Require(g_walk.ended, "the frame chain of a guest thread ends at a frame whose return address is 0");
+    Require(g_walk.endedOnZeroRecord, "the last frame of a guest thread's chain also has a saved frame pointer of 0");
+    Require(g_walk.frames == 3, "the chain is the walker's frame, the guest entry's frame and the zero record");
+    std::puts("guest pthread frame chain tests passed");
+    return 0;
+}

core/libs/tests/GuestUserContext.cpp

@@ -149,6 +149,7 @@ Stack g_jobStack;
 Stack g_exitStack;
 std::uint64_t g_arguments[6] = {};
 int g_entries = 0;
+bool g_contextChainEnded = false;
 Fibre g_dispatcher = {};
 Fibre g_job = {};
 int g_jobRuns = 0;
@@ -193,6 +194,23 @@ void ThrowAndCatch() {
     Require(caught, "a host exception is caught on a context stack");
 }
 
+[[gnu::noinline]] void WalkContextChain() {
+    auto* frame = static_cast<std::uintptr_t*>(__builtin_frame_address(0));
+    const auto low = reinterpret_cast<std::uintptr_t>(g_contextStack.bytes);
+    const auto high = low + StackBytes;
+    for (int depth = 0; depth < 100; ++depth) {
+        const auto address = reinterpret_cast<std::uintptr_t>(frame);
+        if (address < low || address + 2 * sizeof(std::uintptr_t) > high) return;
+        if (frame[1] == 0) {
+            g_contextChainEnded = frame[0] == 0;
+            return;
+        }
+        auto* next = reinterpret_cast<std::uintptr_t*>(frame[0]);
+        if (reinterpret_cast<std::uintptr_t>(next) <= address) return;
+        frame = next;
+    }
+}
+
 [[gnu::noinline]] void GrowStack(int depth) {
     volatile unsigned char page[16 * 1024];
     page[0] = static_cast<unsigned char>(depth);
@@ -341,6 +359,7 @@ extern "C" void APS5_VABI ContextBody(std::uint64_t first, std::uint64_t second,
     g_arguments[4] = fifth;
     g_arguments[5] = sixth;
     CheckStackBounds(&g_contextStack, "the stack bounds follow setcontext onto the context stack");
+    WalkContextChain();
     ThrowAndCatch();
     ++g_entries;
 }
@@ -401,6 +420,7 @@ void CheckMakecontext() {
         Require(false, "setcontext into the made context returned");
     }
     Require(g_entries == 1, "the made context runs once and resumes uc_link");
+    Require(g_contextChainEnded, "the frame chain of a made context ends on its stack at a frame whose saved frame pointer and return address are 0");
     for (unsigned index = 0; index < 6; ++index) Require(g_arguments[index] == 0x8000000000000001ull + index, "makecontext passes six 64-bit arguments");
     const auto stackLow = reinterpret_cast<std::uint64_t>(g_contextStack.bytes);
     Require(EntryStackPointer > stackLow && EntryStackPointer < stackLow + StackBytes, "the made context runs on its own stack");