[lldb] Use consistent CFA before/after prologue of async functions (#4806)

Previously, `SwiftLanguageRuntime::GetRuntimeUnwindPlan` would not generate an _async_ unwind plan when stopped inside the prologue. The reason was that the logic couldn't distinguish between an async function and a sync function called by an async function. This happens because – in a prologue, the register values may make it look like an async function (specifically the extended frame marker bit set on the frame pointer).

To determine whether lldb is stopped in an async function, in addition to checking the extended frame marker, it can look for marker nodes in the symbol's demangle tree.

This all seemed fine initially, but then we discovered some logic bugs within thread plans. The logic bugs were caused by CFA values varying at different parts of the function.

By not returning an async unwind plan during the prologue, the effect is that the function call gets a standard (thread based) CFA (Canonical Frame Address). The standard CFA is the stack pointer ($sp) value at the call site. However once execution proceeds past the prologue, for the same function, lldb returns an async unwind plan. For an async unwind, the CFA is taken to be the async context passed into the function (`x22` on arm64, `r14` on x86-64). The problem is that now the CFA varies across the function. From the DWARF standard:

> The algorithm to compute CFA changes as you progress through the prologue and epilogue code. (By definition, the CFA value does not change.)

Between the logic bugs and DWARF, it's best to keep the CFA consistent throughout a function. This change does that by returning an async unwind plan even in the prologue. This makes the unwind plan logic more branch-y and complex than it was. A follow up change is to refactor this code as well as document it better. For diff readability, those changes will come separately.

rdar://88142757

Author: kastiglione

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

@@ -2462,33 +2462,49 @@ SwiftLanguageRuntime::GetRuntimeUnwindPlan(ProcessSP process_sp,
                                             eSymbolContextSymbol))
       return UnwindPlanSP();
 
+  Address func_start_addr;
+  uint32_t prologue_size;
+  ConstString mangled_name;
   if (sc.function) {
-    Address func_start_addr = sc.function->GetAddressRange().GetBaseAddress();
-    AddressRange prologue_range(func_start_addr,
-                                sc.function->GetPrologueByteSize());
-    if (prologue_range.ContainsLoadAddress(pc, &target) ||
-        func_start_addr == pc) {
-      return UnwindPlanSP();
-    }
+    func_start_addr = sc.function->GetAddressRange().GetBaseAddress();
+    prologue_size = sc.function->GetPrologueByteSize();
+    mangled_name = sc.function->GetMangled().GetMangledName();
   } else if (sc.symbol) {
-    Address func_start_addr = sc.symbol->GetAddress();
-    AddressRange prologue_range(func_start_addr,
-                                sc.symbol->GetPrologueByteSize());
-    if (prologue_range.ContainsLoadAddress(pc, &target) ||
-        func_start_addr == pc) {
-      return UnwindPlanSP();
-    }
+    func_start_addr = sc.symbol->GetAddress();
+    prologue_size = sc.symbol->GetPrologueByteSize();
+    mangled_name = sc.symbol->GetMangled().GetMangledName();
+  } else {
+    return UnwindPlanSP();
   }
 
-  addr_t saved_fp = LLDB_INVALID_ADDRESS;
-  Status error;
-  if (!process_sp->ReadMemory(fp, &saved_fp, 8, error))
-    return UnwindPlanSP();
+  AddressRange prologue_range(func_start_addr, prologue_size);
+  bool in_prologue = (func_start_addr == pc ||
+                      prologue_range.ContainsLoadAddress(pc, &target));
 
-  // Get the high nibble of the dreferenced fp; if the 60th bit is set,
-  // this is the transition to a swift async AsyncContext chain.
-  if ((saved_fp & (0xfULL << 60)) >> 60 != 1)
-    return UnwindPlanSP();
+  if (in_prologue) {
+    if (!IsAnySwiftAsyncFunctionSymbol(mangled_name.GetStringRef()))
+      return UnwindPlanSP();
+  } else {
+    addr_t saved_fp = LLDB_INVALID_ADDRESS;
+    Status error;
+    if (!process_sp->ReadMemory(fp, &saved_fp, 8, error))
+      return UnwindPlanSP();
+
+    // Get the high nibble of the dreferenced fp; if the 60th bit is set,
+    // this is the transition to a swift async AsyncContext chain.
+    if ((saved_fp & (0xfULL << 60)) >> 60 != 1)
+      return UnwindPlanSP();
+  }
+
+  // The coroutine funclets split from an async function have 2 different ABIs:
+  //  - Async suspend partial functions and the first funclet get their async
+  //    context directly in the async register.
+  //  - Async await resume partial functions take their context indirectly, it
+  //    needs to be dereferenced to get the actual function's context.
+  // The debug info for locals reflects this difference, so our unwinding of the
+  // context register needs to reflect it too.
+  bool indirect_context =
+      IsSwiftAsyncAwaitResumePartialFunctionSymbol(mangled_name.GetStringRef());
 
   UnwindPlan::RowSP row(new UnwindPlan::Row);
   const int32_t ptr_size = 8;
@@ -2529,30 +2545,30 @@ SwiftLanguageRuntime::GetRuntimeUnwindPlan(ProcessSP process_sp,
   else
     llvm_unreachable("Unsupported architecture");
 
-  row->GetCFAValue().SetIsDWARFExpression(expr, expr_size);
-  // The coroutine funclets split from an async function have 2 different ABIs:
-  //  - Async suspend partial functions and the first funclet get their async
-  //    context directly in the async register.
-  //  - Async await resume partial functions take their context indirectly, it
-  //    needs to be dereferenced to get the actual function's context.
-  // The debug info for locals reflects this difference, so our unwinding of the
-  // context register needs to reflect it too.
-  bool indirect_context =
-      sc.symbol ? IsSwiftAsyncAwaitResumePartialFunctionSymbol(
-                      sc.symbol->GetMangled().GetMangledName().GetStringRef())
-                : false;
+  if (in_prologue) {
+    if (indirect_context)
+      row->GetCFAValue().SetIsRegisterDereferenced(regnums->async_ctx_regnum);
+    else
+      row->GetCFAValue().SetIsRegisterPlusOffset(regnums->async_ctx_regnum, 0);
+  } else {
+    row->GetCFAValue().SetIsDWARFExpression(expr, expr_size);
+  }
 
   if (indirect_context) {
-    // In a "resume" coroutine, the passed context argument needs to be
-    // dereferenced once to get the context. This is reflected in the debug
-    // info so we need to account for it and report am async register value
-    // that needs to be dereferenced to get to the context.
-    // Note that the size passed for the DWARF expression is the size of the
-    // array minus one. This skips the last deref for this use.
-    assert(expr[expr_size - 1] == llvm::dwarf::DW_OP_deref &&
-           "Should skip a deref");
-    row->SetRegisterLocationToIsDWARFExpression(regnums->async_ctx_regnum, expr,
-                                                expr_size - 1, false);
+    if (in_prologue) {
+      row->SetRegisterLocationToSame(regnums->async_ctx_regnum, false);
+    } else {
+      // In a "resume" coroutine, the passed context argument needs to be
+      // dereferenced once to get the context. This is reflected in the debug
+      // info so we need to account for it and report am async register value
+      // that needs to be dereferenced to get to the context.
+      // Note that the size passed for the DWARF expression is the size of the
+      // array minus one. This skips the last deref for this use.
+      assert(expr[expr_size - 1] == llvm::dwarf::DW_OP_deref &&
+             "Should skip a deref");
+      row->SetRegisterLocationToIsDWARFExpression(regnums->async_ctx_regnum,
+                                                  expr, expr_size - 1, false);
+    }
   } else {
     // In the first part of a split async function, the context is passed
     // directly, so we can use the CFA value directly.

lldb/test/API/lang/swift/async/frame/variable/TestSwiftAsyncFrameVar.py

@@ -9,55 +9,38 @@ class TestCase(lldbtest.TestBase):
 
     @swiftTest
     @skipIf(oslist=['windows', 'linux'])
-    @expectedFailureAll(bugnumber="rdar://88142757")
     def test(self):
         """Test `frame variable` in async functions"""
         self.build()
 
-        # Setting a breakpoint on "inner" results in a breakpoint at the start
-        # of each coroutine "funclet" function.
-        _, process, _, _ = lldbutil.run_to_name_breakpoint(self, 'inner')
-
-        # The step-over actions in the below commands may not be needed in the
-        # future, but for now they are. This comment describes why. Take the
-        # following line of code:
-        #     let x = await asyncFunc()
-        # Some breakpoints, including the ones in this test, resolve to
-        # locations that are at the start of resume functions. At the start of
-        # the resume function, the assignment may not be complete. In order to
-        # ensure assignment takes place, step-over is used to take execution to
-        # the next line.
-
-        stop_num = 0
-        while process.state == lldb.eStateStopped:
-            thread = process.GetSelectedThread()
-            frame = thread.frames[0]
-            if stop_num == 0:
-                # Start of the function.
-                pass
-            elif stop_num == 1:
-                # After first await, read `a`.
-                a = frame.FindVariable("a")
-                self.assertTrue(a.IsValid())
-                self.assertEqual(a.unsigned, 0)
-                # Step to complete `a`'s assignment (stored in the stack).
-                thread.StepOver()
-                self.assertGreater(a.unsigned, 0)
-            elif stop_num == 2:
-                # After second, read `a` and `b`.
-                # At this point, `a` can be read from the async context.
-                a = frame.FindVariable("a")
-                self.assertTrue(a.IsValid())
-                self.assertGreater(a.unsigned, 0)
-                b = frame.FindVariable("b")
-                self.assertTrue(b.IsValid())
-                self.assertEqual(b.unsigned, 0)
-                # Step to complete `b`'s assignment (stored in the stack).
-                thread.StepOver()
-                self.assertGreater(b.unsigned, 0)
-            else:
-                # Unexpected stop.
-                self.assertTrue(False)
-
-            stop_num += 1
-            process.Continue()
+        source_file = lldb.SBFileSpec("main.swift")
+        target, process, _, _ = lldbutil.run_to_source_breakpoint(
+            self, "// break one", source_file)
+
+        # At "break one", only the `a` variable should have a value.
+        frame = process.GetSelectedThread().frames[0]
+        a = frame.FindVariable("a")
+        self.assertTrue(a.IsValid())
+        self.assertGreater(a.unsigned, 0)
+        b = frame.FindVariable("b")
+        self.assertTrue(b.IsValid())
+        self.assertEqual(b.unsigned, 0)
+
+        # The first breakpoint resolves to multiple locations, but only the
+        # first location is needed. Now that we've stopped, delete it to
+        # prevent the other locations from interrupting the test.
+        target.DeleteAllBreakpoints()
+
+        # Setup, and run to, the next breakpoint.
+        target.BreakpointCreateBySourceRegex("// break two", source_file)
+        self.setAsync(False)
+        process.Continue()
+
+        # At "break two", both `a` and `b` should have values.
+        frame = process.GetSelectedThread().frames[0]
+        a = frame.FindVariable("a")
+        self.assertTrue(a.IsValid())
+        self.assertGreater(a.unsigned, 0)
+        b = frame.FindVariable("b")
+        self.assertTrue(b.IsValid())
+        self.assertGreater(b.unsigned, 0)

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

@@ -4,8 +4,8 @@ func randInt(_ i: Int) async -> Int {
 
 func inner() async {
   let a = await randInt(30)
-  let b = await randInt(a + 11)
-  use(a, b)
+  let b = await randInt(a + 11) // break one
+  use(a, b) // break two
 }
 
 func use<T>(_ t: T...) {}