From c964d56e673d70a286aa4bc5d0adaf18f14e6f69 Mon Sep 17 00:00:00 2001 From: Felipe de Azevedo Piovezan Date: Mon, 12 May 2025 08:34:35 -0700 Subject: [PATCH 1/4] [lldb][swift] Change unwind heuristic for Q funclets in arm64e With arm64e, many more branches are generated, which cause the debugger to stop more often while stepping. Among those stops are code regions where the debugger gets confused about what is stored in the async register stack slot. This commit revives a heuristic that was previously used by default for all architectures and that works in all PC addresses, but that sacrifices unwinding o recursive functions. It is only used if the target is arm64e. --- .../Swift/SwiftLanguageRuntime.cpp | 34 +++++++++++++++++-- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp b/lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp index 3800eebcf0dc1..26365c293dfce 100644 --- a/lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp +++ b/lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp @@ -2663,20 +2663,48 @@ static llvm::Expected ReadAsyncContextRegisterFromUnwind( return async_reg; } +static llvm::Expected +DoesContinuationPointToSameFunction(addr_t async_reg, SymbolContext &sc, + Process &process) { + llvm::Expected continuation_ptr = ReadPtrFromAddr( + process, async_reg, /*offset*/ process.GetAddressByteSize()); + if (!continuation_ptr) + return continuation_ptr.takeError(); + + Address continuation_addr; + continuation_addr.SetLoadAddress(process.FixCodeAddress(*continuation_ptr), + &process.GetTarget()); + if (sc.function) + return sc.function->GetAddressRange().ContainsLoadAddress( + continuation_addr, &process.GetTarget()); + assert(sc.symbol); + return sc.symbol->ContainsFileAddress(continuation_addr.GetFileAddress()); +} + /// Returns true if the async register should be dereferenced once to obtain the /// CFA of the currently executing function. This is the case at the start of /// "Q" funclets, before the low level code changes the meaning of the async /// register to not require the indirection. -/// The end of the prologue approximates the transition point. +/// The end of the prologue approximates the transition point well in non-arm64e +/// targets. /// FIXME: In the few instructions between the end of the prologue and the /// transition point, this approximation fails. rdar://139676623 static llvm::Expected IsIndirectContext(Process &process, StringRef mangled_name, - Address pc, SymbolContext &sc) { + Address pc, SymbolContext &sc, + addr_t async_reg) { if (!SwiftLanguageRuntime::IsSwiftAsyncAwaitResumePartialFunctionSymbol( mangled_name)) return false; + // For arm64e, pointer authentication generates branches that cause stepping + // algorithms to stop & unwind in more places. The "end of the prologue" + // approximation fails in those; instead, check whether the continuation + // pointer still points to the currently executing function. This works for + // all instructions, but fails when direct recursion is involved. + if (process.GetTarget().GetArchitecture().GetTriple().isArm64e()) + return DoesContinuationPointToSameFunction(async_reg, sc, process); + // This is checked prior to calling this function. assert(sc.function || sc.symbol); uint32_t prologue_size = sc.function ? sc.function->GetPrologueByteSize() @@ -2759,7 +2787,7 @@ SwiftLanguageRuntime::GetRuntimeUnwindPlan(ProcessSP process_sp, return log_expected(async_reg.takeError()); llvm::Expected maybe_indirect_context = - IsIndirectContext(*process_sp, mangled_name, pc, sc); + IsIndirectContext(*process_sp, mangled_name, pc, sc, *async_reg); if (!maybe_indirect_context) return log_expected(maybe_indirect_context.takeError()); From 2e847d0d3fe856f7bc5e89e79628da614c033992 Mon Sep 17 00:00:00 2001 From: Felipe de Azevedo Piovezan Date: Tue, 13 May 2025 09:39:15 -0700 Subject: [PATCH 2/4] [lldb][NFC] Delete dead code in SwiftLanguageRuntimeNames --- .../Swift/SwiftLanguageRuntimeNames.cpp | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntimeNames.cpp b/lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntimeNames.cpp index 98a9c1dfd2c9a..4356db594b44f 100644 --- a/lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntimeNames.cpp +++ b/lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntimeNames.cpp @@ -378,17 +378,6 @@ CreateRunThroughTaskSwitchThreadPlan(Thread &thread, if (!resume_fn_ptr) return {}; - auto arch = reg_ctx->CalculateTarget()->GetArchitecture(); - std::optional async_regs = - GetAsyncUnwindRegisterNumbers(arch.GetMachine()); - if (!async_regs) - return {}; - unsigned async_reg_number = reg_ctx->ConvertRegisterKindToRegisterNumber( - async_regs->GetRegisterKind(), async_regs->async_ctx_regnum); - uint64_t async_ctx = reg_ctx->ReadRegisterAsUnsigned(async_reg_number, 0); - if (!async_ctx) - return {}; - return std::make_shared(thread, resume_fn_ptr, /*stop_others*/ false); } From 2c318f1c78524baf6b99c254796fae41a4a14aff Mon Sep 17 00:00:00 2001 From: Felipe de Azevedo Piovezan Date: Tue, 13 May 2025 13:02:52 -0700 Subject: [PATCH 3/4] [lldb] Call FixCodeAddress prior to setting breakpoint in SwiftLanguageRuntime This is required so that the breakpoint may be resolved correctly in case the address has been signed. --- .../Plugins/LanguageRuntime/Swift/SwiftLanguageRuntimeNames.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntimeNames.cpp b/lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntimeNames.cpp index 4356db594b44f..e54dcef3d60f0 100644 --- a/lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntimeNames.cpp +++ b/lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntimeNames.cpp @@ -378,6 +378,7 @@ CreateRunThroughTaskSwitchThreadPlan(Thread &thread, if (!resume_fn_ptr) return {}; + resume_fn_ptr = thread.GetProcess()->FixCodeAddress(resume_fn_ptr); return std::make_shared(thread, resume_fn_ptr, /*stop_others*/ false); } From 6e361c7fd92a4413c8b5048a9ee67b91b10cffa6 Mon Sep 17 00:00:00 2001 From: Felipe de Azevedo Piovezan Date: Tue, 13 May 2025 13:04:40 -0700 Subject: [PATCH 4/4] [lldb] Call FixDataAddress when following Swift async chain The code comparing async context addresses need to strip any pointer authentication bits, otherwise the integer comparison will always fail. --- lldb/source/Target/StackID.cpp | 3 +- lldb/unittests/StackID/StackIDTest.cpp | 46 ++++++++++++++------------ 2 files changed, 26 insertions(+), 23 deletions(-) diff --git a/lldb/source/Target/StackID.cpp b/lldb/source/Target/StackID.cpp index 7f59fb4b175d7..19989d731d42c 100644 --- a/lldb/source/Target/StackID.cpp +++ b/lldb/source/Target/StackID.cpp @@ -83,6 +83,7 @@ bool lldb_private::operator!=(const StackID &lhs, const StackID &rhs) { static llvm::Expected IsReachableParent(lldb::addr_t source, lldb::addr_t maybe_parent, Process &process) { + maybe_parent = process.FixDataAddress(maybe_parent); auto max_num_frames = 512; for (lldb::addr_t parent_ctx = source; parent_ctx && max_num_frames; max_num_frames--) { @@ -94,7 +95,7 @@ static llvm::Expected IsReachableParent(lldb::addr_t source, return llvm::createStringError(llvm::formatv( "Failed to read parent async context of: {0:x}. Error: {1}", old_parent_ctx, error.AsCString())); - if (parent_ctx == maybe_parent) + if (process.FixDataAddress(parent_ctx) == maybe_parent) return true; } if (max_num_frames == 0) diff --git a/lldb/unittests/StackID/StackIDTest.cpp b/lldb/unittests/StackID/StackIDTest.cpp index 27f5e94b89063..de62f3cd9b4d2 100644 --- a/lldb/unittests/StackID/StackIDTest.cpp +++ b/lldb/unittests/StackID/StackIDTest.cpp @@ -76,25 +76,27 @@ struct MockStackID : StackID { }; TEST_F(StackIDTest, StackStackCFAComparison) { - auto process = MockProcess(m_target_sp, Listener::MakeListener("dummy")); + auto process = std::make_shared(m_target_sp, + Listener::MakeListener("dummy")); MockStackID small_cfa_on_stack(/*cfa*/ 10, OnStack::Yes); MockStackID big_cfa_on_stack(/*cfa*/ 100, OnStack::Yes); EXPECT_TRUE( - StackID::IsYounger(small_cfa_on_stack, big_cfa_on_stack, process)); + StackID::IsYounger(small_cfa_on_stack, big_cfa_on_stack, *process)); EXPECT_FALSE( - StackID::IsYounger(big_cfa_on_stack, small_cfa_on_stack, process)); + StackID::IsYounger(big_cfa_on_stack, small_cfa_on_stack, *process)); } TEST_F(StackIDTest, StackHeapCFAComparison) { - auto process = MockProcess(m_target_sp, Listener::MakeListener("dummy")); + auto process = std::make_shared(m_target_sp, + Listener::MakeListener("dummy")); MockStackID cfa_on_stack(/*cfa*/ 100, OnStack::Yes); MockStackID cfa_on_heap(/*cfa*/ 10, OnStack::No); - EXPECT_TRUE(StackID::IsYounger(cfa_on_stack, cfa_on_heap, process)); - EXPECT_FALSE(StackID::IsYounger(cfa_on_heap, cfa_on_stack, process)); + EXPECT_TRUE(StackID::IsYounger(cfa_on_stack, cfa_on_heap, *process)); + EXPECT_FALSE(StackID::IsYounger(cfa_on_heap, cfa_on_stack, *process)); } TEST_F(StackIDTest, HeapHeapCFAComparison) { @@ -107,21 +109,21 @@ TEST_F(StackIDTest, HeapHeapCFAComparison) { memory_map[100] = 108; memory_map[108] = 116; memory_map[116] = 0; - auto process = MockProcess(m_target_sp, Listener::MakeListener("dummy"), - std::move(memory_map)); + auto process = std::make_shared( + m_target_sp, Listener::MakeListener("dummy"), std::move(memory_map)); MockStackID oldest_cfa(/*cfa*/ 116, OnStack::No); MockStackID middle_cfa(/*cfa*/ 108, OnStack::No); MockStackID youngest_cfa(/*cfa*/ 100, OnStack::No); - EXPECT_TRUE(StackID::IsYounger(youngest_cfa, oldest_cfa, process)); - EXPECT_FALSE(StackID::IsYounger(oldest_cfa, youngest_cfa, process)); + EXPECT_TRUE(StackID::IsYounger(youngest_cfa, oldest_cfa, *process)); + EXPECT_FALSE(StackID::IsYounger(oldest_cfa, youngest_cfa, *process)); - EXPECT_TRUE(StackID::IsYounger(youngest_cfa, middle_cfa, process)); - EXPECT_FALSE(StackID::IsYounger(middle_cfa, youngest_cfa, process)); + EXPECT_TRUE(StackID::IsYounger(youngest_cfa, middle_cfa, *process)); + EXPECT_FALSE(StackID::IsYounger(middle_cfa, youngest_cfa, *process)); - EXPECT_TRUE(StackID::IsYounger(middle_cfa, oldest_cfa, process)); - EXPECT_FALSE(StackID::IsYounger(oldest_cfa, middle_cfa, process)); + EXPECT_TRUE(StackID::IsYounger(middle_cfa, oldest_cfa, *process)); + EXPECT_FALSE(StackID::IsYounger(oldest_cfa, middle_cfa, *process)); } TEST_F(StackIDTest, HeapHeapCFAComparisonDecreasing) { @@ -134,19 +136,19 @@ TEST_F(StackIDTest, HeapHeapCFAComparisonDecreasing) { memory_map[100] = 90; memory_map[90] = 80; memory_map[80] = 0; - auto process = MockProcess(m_target_sp, Listener::MakeListener("dummy"), - std::move(memory_map)); + auto process = std::make_shared( + m_target_sp, Listener::MakeListener("dummy"), std::move(memory_map)); MockStackID oldest_cfa(/*cfa*/ 80, OnStack::No); MockStackID middle_cfa(/*cfa*/ 90, OnStack::No); MockStackID youngest_cfa(/*cfa*/ 100, OnStack::No); - EXPECT_TRUE(StackID::IsYounger(youngest_cfa, oldest_cfa, process)); - EXPECT_FALSE(StackID::IsYounger(oldest_cfa, youngest_cfa, process)); + EXPECT_TRUE(StackID::IsYounger(youngest_cfa, oldest_cfa, *process)); + EXPECT_FALSE(StackID::IsYounger(oldest_cfa, youngest_cfa, *process)); - EXPECT_TRUE(StackID::IsYounger(youngest_cfa, middle_cfa, process)); - EXPECT_FALSE(StackID::IsYounger(middle_cfa, youngest_cfa, process)); + EXPECT_TRUE(StackID::IsYounger(youngest_cfa, middle_cfa, *process)); + EXPECT_FALSE(StackID::IsYounger(middle_cfa, youngest_cfa, *process)); - EXPECT_TRUE(StackID::IsYounger(middle_cfa, oldest_cfa, process)); - EXPECT_FALSE(StackID::IsYounger(oldest_cfa, middle_cfa, process)); + EXPECT_TRUE(StackID::IsYounger(middle_cfa, oldest_cfa, *process)); + EXPECT_FALSE(StackID::IsYounger(oldest_cfa, middle_cfa, *process)); }