Fix swift async frames unwinding unwinding #9025

felipepiovezan · 2024-07-29T21:57:28Z

This series of patches aims to solve the problem where LLDB is unable to print variables in backtraces involving swift async functions. The only frame where this works today is the first async frame, which is a real frame and is physically on the stack. We fail to do so for all virtual frames.

The underlying problem is simple: when unwinding the PC, we use the first instruction of the continuation funclets. This instruction is part of the prologue, where no variables are in scope.

The solution is similarly simple: we just offset the PC by the size of the prologue of the continuation funclet.

These commits are meant to be reviewed independently, please the commit messages too.

The first commit is a cherry-pick from: llvm#100624

felipepiovezan · 2024-07-29T21:58:11Z

~~Requires: swiftlang/swift#75553~~
edit: no longer true, reworked to remove the Q funclet fixes

jimingham · 2024-07-29T23:57:15Z

lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp

+// Creates an expression accessing (fp - 8), with an optional dereference
+// operation. This is only valid for x86_64 or aarch64.
+llvm::ArrayRef<uint8_t>
+getAsyncRegFromFramePointerDWARFExpr(llvm::Triple::ArchType triple,


lldb functions start with an Upper case letter.

Fixed in the new version

adrian-prantl · 2024-07-30T16:01:44Z

lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp

@@ -2586,6 +2582,39 @@ lldb::addr_t SwiftLanguageRuntime::GetAsyncContext(RegisterContext *regctx) {
  return LLDB_INVALID_ADDRESS;
 }

+// Creates an expression accessing (fp - 8), with an optional dereference


adrian-prantl · 2024-07-30T16:05:47Z

lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp

+  // works.
+  static const uint8_t g_cfa_dwarf_expression_x86_64[] = {
+      llvm::dwarf::DW_OP_breg6, // DW_OP_breg6, register 6 == rbp
+      0x78,                     //    sleb128 -8 (ptrsize)


I don't understand this comment. Yes, it's 128-8, but sleb128 is a variable-length encoding, so I'm confused.

I also understand this is not new in your patch.

Can you elaborate? -8 is 0x78 in sleb128 encoding, and it takes a single byte to represent it.
The comment is explaining where the magic 0x78 comes from.

lldb/include/lldb/Symbol/UnwindPlan.h

adrian-prantl · 2024-07-30T17:42:02Z

lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp

@@ -2765,9 +2761,9 @@ SwiftLanguageRuntime::GetRuntimeUnwindPlan(ProcessSP process_sp,

 // Creates an UnwindPlan for following the AsyncContext chain
 // up the stack, from a current AsyncContext frame.


This should be a doxygen comment on the declaration.

adrian-prantl · 2024-07-30T17:43:45Z

lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp


+  auto pc_after_prologue = [&]() -> std::optional<addr_t> {
+    // If we're in the prologue, we can just use our x22, it is unmodified.


is there a generic name for x22 (since this is arch specific?)

context_reg or some thing like that?

calling is "async_reg" to match other comments in this file which refer to it as "asynchronous register"

adrian-prantl · 2024-07-30T17:45:02Z

lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp

+    // Otherwise, check the ABI guaranteed slot for the saved x22 reg: *(fp - 8).
+    Status error;
+    addr_t x22_entry_value;
+    process_sp->ReadMemory(fp - ptr_size, &x22_entry_value, ptr_size, error);


We should comment that this is true on all ABIs, otherwise this screams for being an ABI callback.

adrian-prantl · 2024-07-30T17:52:55Z

lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp

-  row->SetRegisterLocationToAtCFAPlusOffset(regnums->pc_regnum, ptr_size,
-                                            false);
+  // Since we pretend x22 is the register "passed" to our child frame, there
+  // is always at least one indirection.


Can you add the missing detail to this comment that explains why the indirection follows from it being a virtual frame?

I've reworded this entire paragraph to remove this part of the explanation that requires a lot of background information.

adrian-prantl · 2024-07-30T17:57:02Z

lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp

+  if (async_reg_val == LLDB_INVALID_ADDRESS || async_reg_val == 0)
+    return {};
+
+  const auto ptr_size = 8;


get this from process?

adrian-prantl · 2024-07-30T17:58:16Z

lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp

+    return {};
+
+  Address pc;
+  Target &target(process.GetTarget());


Target &target = process.GetTarget(); ?

felipepiovezan

Address review comments

lldb/include/lldb/Symbol/UnwindPlan.h

felipepiovezan · 2024-07-30T18:08:32Z

lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp

+    return {};
+
+  Address pc;
+  Target &target(process.GetTarget());


felipepiovezan · 2024-07-30T18:08:49Z

lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp

+  if (async_reg_val == LLDB_INVALID_ADDRESS || async_reg_val == 0)
+    return {};
+
+  const auto ptr_size = 8;


felipepiovezan · 2024-07-30T18:13:23Z

lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp

+  // works.
+  static const uint8_t g_cfa_dwarf_expression_x86_64[] = {
+      llvm::dwarf::DW_OP_breg6, // DW_OP_breg6, register 6 == rbp
+      0x78,                     //    sleb128 -8 (ptrsize)


Can you elaborate? -8 is 0x78 in sleb128 encoding, and it takes a single byte to represent it.
The comment is explaining where the magic 0x78 comes from.

felipepiovezan · 2024-07-30T18:24:50Z

lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp


+  auto pc_after_prologue = [&]() -> std::optional<addr_t> {
+    // If we're in the prologue, we can just use our x22, it is unmodified.


calling is "async_reg" to match other comments in this file which refer to it as "asynchronous register"

felipepiovezan · 2024-07-30T18:25:44Z

lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp

+    // Otherwise, check the ABI guaranteed slot for the saved x22 reg: *(fp - 8).
+    Status error;
+    addr_t x22_entry_value;
+    process_sp->ReadMemory(fp - ptr_size, &x22_entry_value, ptr_size, error);


felipepiovezan · 2024-07-30T19:47:19Z

lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp

-  row->SetRegisterLocationToAtCFAPlusOffset(regnums->pc_regnum, ptr_size,
-                                            false);
+  // Since we pretend x22 is the register "passed" to our child frame, there
+  // is always at least one indirection.


I've reworded this entire paragraph to remove this part of the explanation that requires a lot of background information.

felipepiovezan · 2024-07-30T20:14:40Z

lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp

@@ -2765,9 +2761,9 @@ SwiftLanguageRuntime::GetRuntimeUnwindPlan(ProcessSP process_sp,

 // Creates an UnwindPlan for following the AsyncContext chain
 // up the stack, from a current AsyncContext frame.


felipepiovezan · 2024-07-30T20:18:41Z

lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp

@@ -2586,6 +2582,39 @@ lldb::addr_t SwiftLanguageRuntime::GetAsyncContext(RegisterContext *regctx) {
  return LLDB_INVALID_ADDRESS;
 }

+// Creates an expression accessing (fp - 8), with an optional dereference


This will enable subsequent commits to introduce calls to other member functions from that method.

felipepiovezan · 2024-07-31T17:53:24Z

Removed the last two commits, which dealt with Q funclets as frame 0 and required compiler changes.
This PR no longer needs any compiler changes, going to propose it for the 6.0 branch

felipepiovezan · 2024-07-31T18:16:50Z

@swift-ci test

lldb/include/lldb/Symbol/UnwindPlan.h

lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp

adrian-prantl · 2024-07-31T19:51:16Z

lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp

+
+  // Compute the CFA of this frame.
+  addr_t cfa = async_reg_val;
+  while (num_indirections--) {


The fact that this underflows on the last iteration bothers me a bit.
for (; num_indirections != 0; --num_indirections) ?

Done, good catch!

lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp

lldb/source/Symbol/UnwindPlan.cpp

adrian-prantl · 2024-07-31T19:55:13Z

lldb/test/API/lang/swift/async/frame/variables_multiple_frames/main.swift

+
+func ASYNC___2___() async -> Int {
+  var myvar = 222;
+  let result = await ASYNC___1___() //breakpoint1


// breakpoint1

Variables are never available in the prologue of a function. By unwinding the pc of the virtual frames as the first instruction of the continuation function, LLDB is unable to display any variable information. This patch addresses the issue by computing the continuation pointer from the context of the function being unwound, and then querying the continuation's SymbolContext to determine its prologue size.

felipepiovezan

Addressed review comments

felipepiovezan · 2024-07-31T20:42:42Z

lldb/test/API/lang/swift/async/frame/variables_multiple_frames/main.swift

+
+func ASYNC___2___() async -> Int {
+  var myvar = 222;
+  let result = await ASYNC___1___() //breakpoint1


lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp

felipepiovezan · 2024-07-31T20:48:17Z

lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp

+
+  // Compute the CFA of this frame.
+  addr_t cfa = async_reg_val;
+  while (num_indirections--) {


Done, good catch!

lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp

felipepiovezan · 2024-07-31T21:13:42Z

@swift-ci test

felipepiovezan · 2024-07-31T21:16:02Z

@swift-ci test platform windows

felipepiovezan · 2024-07-31T21:16:32Z

@swift-ci test windows platform

felipepiovezan · 2024-07-31T21:53:58Z

@swift-ci test windows platform

felipepiovezan mentioned this pull request Jul 29, 2024

[DebugInfo] Dont generate info for __swift_async_resume_project_conte… swiftlang/swift#75553

Closed

jimingham reviewed Jul 29, 2024

View reviewed changes

felipepiovezan force-pushed the felipe/unwinding_pc_patches_to_release branch from 9964bc1 to 317e4a6 Compare July 30, 2024 15:21

adrian-prantl reviewed Jul 30, 2024

View reviewed changes

felipepiovezan force-pushed the felipe/unwinding_pc_patches_to_release branch from 317e4a6 to 1aa6a85 Compare July 30, 2024 20:18

felipepiovezan commented Jul 30, 2024

View reviewed changes

felipepiovezan added 2 commits July 31, 2024 10:49

[LLDB] Add constant value mode for RegisterLocation

2672795

[LLDB][swift][nfc] Make unwind method a member

bffca2b

This will enable subsequent commits to introduce calls to other member functions from that method.

felipepiovezan force-pushed the felipe/unwinding_pc_patches_to_release branch from 1aa6a85 to 8e0cfd2 Compare July 31, 2024 17:51

felipepiovezan changed the base branch from stable/20230725 to swift/release/6.0 July 31, 2024 17:52

felipepiovezan marked this pull request as ready for review July 31, 2024 18:16

adrian-prantl reviewed Jul 31, 2024

View reviewed changes

lldb/include/lldb/Symbol/UnwindPlan.h Show resolved Hide resolved

adrian-prantl reviewed Jul 31, 2024

View reviewed changes

lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp Show resolved Hide resolved

adrian-prantl reviewed Jul 31, 2024

View reviewed changes

lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp Show resolved Hide resolved

adrian-prantl reviewed Jul 31, 2024

View reviewed changes

lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp Show resolved Hide resolved

adrian-prantl reviewed Jul 31, 2024

View reviewed changes

lldb/source/Symbol/UnwindPlan.cpp Show resolved Hide resolved

adrian-prantl reviewed Jul 31, 2024

View reviewed changes

felipepiovezan added 2 commits July 31, 2024 13:42

[LLDB][swift] Add test demonstrating failed async variable inspection

4e5e68f

felipepiovezan force-pushed the felipe/unwinding_pc_patches_to_release branch from 8e0cfd2 to 85069c8 Compare July 31, 2024 20:55

felipepiovezan commented Jul 31, 2024

View reviewed changes

adrian-prantl approved these changes Jul 31, 2024

View reviewed changes

adrian-prantl merged commit d85bebe into swiftlang:swift/release/6.0 Aug 1, 2024
3 checks passed

felipepiovezan mentioned this pull request Aug 16, 2024

[lldb][Swift] Small steps towards fixing StepOut in async functions #9138

Merged

		@@ -2765,9 +2761,9 @@ SwiftLanguageRuntime::GetRuntimeUnwindPlan(ProcessSP process_sp,

		// Creates an UnwindPlan for following the AsyncContext chain
		// up the stack, from a current AsyncContext frame.


		auto pc_after_prologue = [&]() -> std::optional<addr_t> {
		// If we're in the prologue, we can just use our x22, it is unmodified.

Fix swift async frames unwinding unwinding #9025

Fix swift async frames unwinding unwinding #9025

Uh oh!

Conversation

felipepiovezan commented Jul 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

felipepiovezan commented Jul 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

felipepiovezan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

felipepiovezan commented Jul 31, 2024

Uh oh!

felipepiovezan commented Jul 31, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

felipepiovezan commented Jul 29, 2024 •

edited

Loading

felipepiovezan commented Jul 29, 2024 •

edited

Loading