[lldb][swift] Evaluate entry_value(async_reg) in terms of CFA

Prior to this commit, evaluating the dwarf expression `entry_value(async_reg)`
is done by finding the value of the asynchronous register in the parent frame.

To enable the above, the unwinder must pretend there is a real function call
between the parent frame and the current frame, and that the async register is
set by the parent frame prior to making the 'call'. None of this is actually
true, and it creates a lot of work for the unwinder (see the amount of code
deleted there).

Here is further evidence of how awkward this is. Suppose you have this call
stack:
```
  A <--- younger frame, top of the stack
  B
  C <--- older frame, bottom of the stack
```
When the unwinder is creating the frame of C from the register state of B, it
must know whether A was an indirect (Q) funclet or not, because that determined
how the frame of B was produced from the register state of A. This is very
unusual, in fact, the unwinder doesn't even have access to such information (we
had to use a "dummy" register for this).

This patch changes how `entry_value(async_reg)` (or
`entry_value(async_reg),deref` for Q_funclets) is evaluated: this expression is
equivalent to the CFA (the async context) of the current frame. Since we no
longer need to peek at the parent frame, the unwinder no longer needs to perform
the work described previously. The unwinder can instead provide the continuation
funclet with the register contents they will _actually_ have when the funclet
runs.

This patch also addresses a more subtle issue. In Q funclets, after a certain
instruction, `entry_value(async_reg)` produces a pointer to memory that has been
freed, as Q funclets free the async context of funclet that just finished
executing. If the debugger attempts to evaluate `entry_value(async_reg), deref`
as two separate operations, it will be accessing freed heap memory.  By
converting that operation sequence into `DW_OP_call_frame_cfa`, we bypass the
issue.

Author: felipepiovezan

lldb/source/Expression/DWARFExpression.cpp

@@ -532,6 +532,74 @@ bool DWARFExpression::LinkThreadLocalStorage(
   return true;
 }
 
+/// Returns true if \c opcodes contains the opcode for the register used for the
+/// Swift Async Context (x22 for aarch64, r14 for x86-64). It must also not
+/// contain any other opcodes.
+static bool IsAsyncRegCtxExpr(DataExtractor &opcodes,
+                              llvm::Triple::ArchType triple) {
+  offset_t offset = 0;
+  const uint8_t op_async_ctx_reg = opcodes.GetU8(&offset);
+  if (opcodes.BytesLeft(offset) != 0)
+    return false;
+
+  // FIXME: could this be exposed through the ABI/Language plugins?
+  return (triple == llvm::Triple::x86_64 && op_async_ctx_reg == DW_OP_reg14) ||
+         (triple == llvm::Triple::aarch64 && op_async_ctx_reg == DW_OP_reg22);
+}
+
+/// If \c opcodes contain the location of asynchronous contexts in Swift,
+/// evaluates DW_OP_call_frame_cfa, returns its result, and updates
+/// \c current_offset. Otherwise, does nothing. This is possible because, for
+/// async frames, the language unwinder treats the asynchronous context as the
+/// CFA of the frame.
+static llvm::Expected<Value> SwiftAsyncEvaluate_DW_OP_entry_value(
+    ExecutionContext &exe_ctx, StackFrame &current_frame,
+    const DWARFUnit *dwarf_cu, Function &func, const DataExtractor &opcodes,
+    offset_t &current_offset) {
+  auto func_name = func.GetMangled().GetMangledName();
+  if (!SwiftLanguageRuntime::IsAnySwiftAsyncFunctionSymbol(func_name))
+    return llvm::createStringError(
+        "SwiftAsyncEvaluate_DW_OP_entry_value: not an async function");
+
+  offset_t new_offset = current_offset;
+  const uint32_t subexpr_len = opcodes.GetULEB128(&new_offset);
+  const void *subexpr_data = opcodes.GetData(&new_offset, subexpr_len);
+  if (!subexpr_data)
+    return llvm::createStringError(
+        "SwiftAsyncEvaluate_DW_OP_entry_value: failed to extract subexpr");
+
+  DataExtractor subexpr_extractor(
+      subexpr_data, subexpr_len, opcodes.GetByteOrder(),
+      opcodes.GetAddressByteSize(), opcodes.getTargetByteSize());
+  if (!IsAsyncRegCtxExpr(subexpr_extractor,
+                         exe_ctx.GetTargetRef().GetArchitecture().GetMachine()))
+    return llvm::createStringError("SwiftAsyncEvaluate_DW_OP_entry_value: "
+                                   "missing async context register opcode");
+
+  // Q funclets require an extra level of indirection.
+  if (SwiftLanguageRuntime::IsSwiftAsyncAwaitResumePartialFunctionSymbol(
+          func_name)) {
+    const uint8_t maybe_op_deref = opcodes.GetU8(&new_offset);
+    if (maybe_op_deref != DW_OP_deref)
+      return llvm::createStringError("SwiftAsyncEvaluate_DW_OP_entry_value: "
+                                     "missing DW_OP_deref in Q funclet");
+  }
+
+  static const uint8_t cfa_opcode = DW_OP_call_frame_cfa;
+  DataExtractor cfa_expr_data(&cfa_opcode, 1, opcodes.GetByteOrder(),
+                              opcodes.GetAddressByteSize(),
+                              opcodes.getTargetByteSize());
+  DWARFExpressionList cfa_expr(func.CalculateSymbolContextModule(),
+                               cfa_expr_data, dwarf_cu);
+  llvm::Expected<Value> maybe_result = cfa_expr.Evaluate(
+      &exe_ctx, current_frame.GetRegisterContext().get(), LLDB_INVALID_ADDRESS,
+      /*initial_value_ptr=*/nullptr,
+      /*object_address_ptr=*/nullptr);
+  if (maybe_result)
+    current_offset = new_offset;
+  return maybe_result;
+}
+
 static llvm::Error
 Evaluate_DW_OP_entry_value(std::vector<Value> &stack, const DWARFUnit *dwarf_cu,
                            ExecutionContext *exe_ctx, RegisterContext *reg_ctx,
@@ -629,20 +697,19 @@ Evaluate_DW_OP_entry_value(std::vector<Value> &stack, const DWARFUnit *dwarf_cu,
   if (!current_func)
     return llvm::createStringError("no current function");
 
+  if (llvm::Expected<Value> result = SwiftAsyncEvaluate_DW_OP_entry_value(
+          *exe_ctx, *current_frame, dwarf_cu, *current_func, opcodes,
+          opcode_offset)) {
+    stack.push_back(*result);
+    return llvm::Error::success();
+  } else
+    LLDB_LOG_ERROR(log, result.takeError(), "{0}");
+
   CallEdge *call_edge = nullptr;
   ModuleList &modlist = target.GetImages();
   ExecutionContext parent_exe_ctx = *exe_ctx;
   parent_exe_ctx.SetFrameSP(parent_frame);
   Function *parent_func = nullptr;
-#ifdef LLDB_ENABLE_SWIFT
-  // Swift async function arguments are represented relative to a
-  // DW_OP_entry_value that fetches the async context register. This
-  // register is known to the unwinder and can always be restored
-  // therefore it is not necessary to match up a call site parameter
-  // with it.
-  auto fn_name = current_func->GetMangled().GetMangledName().GetStringRef();
-  if (!SwiftLanguageRuntime::IsAnySwiftAsyncFunctionSymbol(fn_name)) {
-#endif
 
   parent_func =
     parent_frame->GetSymbolContext(eSymbolContextFunction).function;
@@ -677,25 +744,16 @@ Evaluate_DW_OP_entry_value(std::vector<Value> &stack, const DWARFUnit *dwarf_cu,
   if (!call_edge)
     return llvm::createStringError("no unambiguous edge from parent "
                                    "to current function");
-#ifdef LLDB_ENABLE_SWIFT
-  }
-#endif
 
   // 3. Attempt to locate the DW_OP_entry_value expression in the set of
   //    available call site parameters. If found, evaluate the corresponding
   //    parameter in the context of the parent frame.
   const uint32_t subexpr_len = opcodes.GetULEB128(&opcode_offset);
-#ifdef LLDB_ENABLE_SWIFT
-  lldb::offset_t subexpr_offset = opcode_offset;
-#endif
   const void *subexpr_data = opcodes.GetData(&opcode_offset, subexpr_len);
   if (!subexpr_data)
     return llvm::createStringError("subexpr could not be read");
 
   const CallSiteParameter *matched_param = nullptr;
-#ifdef LLDB_ENABLE_SWIFT
-  if (call_edge) {
-#endif
   for (const CallSiteParameter &param : call_edge->GetCallSiteParameters()) {
     DataExtractor param_subexpr_extractor;
     if (!param.LocationInCallee.GetExpressionData(param_subexpr_extractor))
@@ -721,25 +779,10 @@ Evaluate_DW_OP_entry_value(std::vector<Value> &stack, const DWARFUnit *dwarf_cu,
   }
   if (!matched_param)
     return llvm::createStringError("no matching call site param found");
-#ifdef LLDB_ENABLE_SWIFT
-  }
-  std::optional<DWARFExpressionList> subexpr;
-  if (!matched_param) {
-    auto *ctx_func = parent_func ? parent_func : current_func;
-    subexpr.emplace(ctx_func->CalculateSymbolContextModule(),
-                    DataExtractor(opcodes, subexpr_offset, subexpr_len),
-                    dwarf_cu);
-  }
-#endif
 
   // TODO: Add support for DW_OP_push_object_address within a DW_OP_entry_value
   // subexpresion whenever llvm does.
-#ifdef LLDB_ENABLE_SWIFT
-  const DWARFExpressionList &param_expr =
-      matched_param ? matched_param->LocationInCaller : *subexpr;
-#else
   const DWARFExpressionList &param_expr = matched_param->LocationInCaller;
-#endif
 
   llvm::Expected<Value> maybe_result = param_expr.Evaluate(
       &parent_exe_ctx, parent_frame->GetRegisterContext().get(),

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

@@ -2539,11 +2539,6 @@ struct AsyncUnwindRegisterNumbers {
   uint32_t async_ctx_regnum;
   uint32_t fp_regnum;
   uint32_t pc_regnum;
-  /// A register to use as a marker to indicate how the async context is passed
-  /// to the function (indirectly, or not). This needs to be communicated to the
-  /// frames below us as they need to react differently. There is no good way to
-  /// expose this, so we set another dummy register to communicate this state.
-  uint32_t dummy_regnum;
 
   RegisterKind GetRegisterKind() const { return lldb::eRegisterKindDWARF; }
 };
@@ -2557,15 +2552,13 @@ GetAsyncUnwindRegisterNumbers(llvm::Triple::ArchType triple) {
     regnums.async_ctx_regnum = dwarf_r14_x86_64;
     regnums.fp_regnum = dwarf_rbp_x86_64;
     regnums.pc_regnum = dwarf_rip_x86_64;
-    regnums.dummy_regnum = dwarf_r15_x86_64;
     return regnums;
   }
   case llvm::Triple::aarch64: {
     AsyncUnwindRegisterNumbers regnums;
     regnums.async_ctx_regnum = arm64_dwarf::x22;
     regnums.fp_regnum = arm64_dwarf::fp;
     regnums.pc_regnum = arm64_dwarf::pc;
-    regnums.dummy_regnum = arm64_dwarf::x23;
     return regnums;
   }
   default:
@@ -2811,18 +2804,11 @@ SwiftLanguageRuntime::GetRuntimeUnwindPlan(ProcessSP process_sp,
   // The CFA of a funclet is its own async context.
   row->GetCFAValue().SetIsConstant(*async_ctx);
 
-  // The value of the async register in the parent frame is the entry value of
-  // the async register in the current frame. This mimics a function call, as
-  // if the parent funclet had called the current funclet.
-  row->SetRegisterLocationToIsConstant(regnums->async_ctx_regnum, *async_reg,
+  // The value of the async register in the parent frame (which is the
+  // continuation funclet) is the async context of this frame.
+  row->SetRegisterLocationToIsConstant(regnums->async_ctx_regnum, *async_ctx,
                                        /*can_replace=*/false);
 
-  // The parent frame needs to know how to interpret the value it is given for
-  // its own async register. A dummy register is used to communicate that.
-  if (!indirect_context)
-    row->SetRegisterLocationToIsConstant(regnums->dummy_regnum, 0,
-                                         /*can_replace=*/false);
-
   if (std::optional<addr_t> pc_after_prologue =
           TrySkipVirtualParentProlog(*async_ctx, *process_sp))
     row->SetRegisterLocationToIsConstant(regnums->pc_regnum, *pc_after_prologue,
@@ -2855,59 +2841,13 @@ UnwindPlanSP SwiftLanguageRuntime::GetFollowAsyncContextUnwindPlan(
   if (!regnums)
     return UnwindPlanSP();
 
-  const bool is_indirect =
-      regctx->ReadRegisterAsUnsigned(regnums->dummy_regnum, (uint64_t)-1ll) ==
-      (uint64_t)-1ll;
-  // In the general case, the async register setup by the frame above us
-  // should be dereferenced twice to get our context, except when the frame
-  // above us is an async frame on the OS stack that takes its context directly
-  // (see discussion in GetRuntimeUnwindPlan()). The availability of
-  // dummy_regnum is used as a marker for this situation.
-  if (!is_indirect) {
-    row->GetCFAValue().SetIsRegisterDereferenced(regnums->async_ctx_regnum);
-    row->SetRegisterLocationToSame(regnums->async_ctx_regnum, false);
-  } else {
-    static const uint8_t async_dwarf_expression_x86_64[] = {
-        llvm::dwarf::DW_OP_regx, dwarf_r14_x86_64, // DW_OP_regx, reg
-        llvm::dwarf::DW_OP_deref,                  // DW_OP_deref
-        llvm::dwarf::DW_OP_deref,                  // DW_OP_deref
-    };
-    static const uint8_t async_dwarf_expression_arm64[] = {
-        llvm::dwarf::DW_OP_regx, arm64_dwarf::x22, // DW_OP_regx, reg
-        llvm::dwarf::DW_OP_deref,                  // DW_OP_deref
-        llvm::dwarf::DW_OP_deref,                  // DW_OP_deref
-    };
-
-    const unsigned expr_size = sizeof(async_dwarf_expression_x86_64);
-    static_assert(sizeof(async_dwarf_expression_x86_64) ==
-                      sizeof(async_dwarf_expression_arm64),
-                  "Expressions of different sizes");
-
-    const uint8_t *expression = nullptr;
-    if (arch.GetMachine() == llvm::Triple::x86_64)
-      expression = async_dwarf_expression_x86_64;
-    else if (arch.GetMachine() == llvm::Triple::aarch64)
-      expression = async_dwarf_expression_arm64;
-    else
-      llvm_unreachable("Unsupported architecture");
-
-    // Note how the register location gets the same expression pointer with a
-    // different size. We just skip the trailing deref for it.
-    assert(expression[expr_size - 1] == llvm::dwarf::DW_OP_deref &&
-           "Should skip a deref");
-    row->GetCFAValue().SetIsDWARFExpression(expression, expr_size);
-    row->SetRegisterLocationToIsDWARFExpression(
-        regnums->async_ctx_regnum, expression, expr_size - 1, false);
-  }
-
-  // Suppose this is unwinding frame #2 of a call stack. The value given for
-  // the async register has two possible values, depending on what frame #1
-  // expects:
-  // 1. The CFA of frame #1, direct ABI, dereferencing it once produces CFA of
-  // Frame #2.
-  // 2. The CFA of frame #0, indirect ABI, dereferencing it twice produces CFA
-  // of Frame #2.
-  const unsigned num_indirections = 1 + is_indirect;
+  row->GetCFAValue().SetIsRegisterDereferenced(regnums->async_ctx_regnum);
+  // The value of the async register in the parent frame (which is the
+  // continuation funclet) is the async context of this frame.
+  row->SetRegisterLocationToIsCFAPlusOffset(regnums->async_ctx_regnum,
+                                            /*offset*/ 0, false);
+
+  const unsigned num_indirections = 1;
   if (std::optional<addr_t> pc_after_prologue = TrySkipVirtualParentProlog(
           GetAsyncContext(regctx), *process_sp, num_indirections))
     row->SetRegisterLocationToIsConstant(regnums->pc_regnum, *pc_after_prologue,

lldb/test/API/lang/swift/async/frame/variables_multiple_frames/TestSwiftAsyncFrameVarMultipleFrames.py

@@ -4,6 +4,7 @@
 import lldbsuite.test.lldbutil as lldbutil
 
 
+@skipIf(archs=no_match(["arm64", "arm64e", "x86_64"]))
 class TestCase(lldbtest.TestBase):
 
     mydir = lldbtest.TestBase.compute_mydir(__file__)
@@ -37,6 +38,26 @@ def check_pcs(self, async_frames, process, target):
             prologue_to_skip = parent_frame.GetFunction().GetPrologueByteSize()
             self.assertEqual(continuation_ptr + prologue_to_skip, parent_frame.GetPC())
 
+    def check_async_regs_one_frame(self, frame, process):
+        async_reg_name = "r14" if self.getArchitecture() == "x86_64" else "x22"
+
+        cfa = frame.GetCFA()
+        is_indirect = "await resume" in frame.GetFunctionName()
+        async_register = frame.FindRegister(async_reg_name).GetValueAsUnsigned()
+
+        if is_indirect:
+            deref_async_reg = self.read_ptr_from_memory(process, async_register)
+            self.assertEqual(deref_async_reg, cfa)
+        else:
+            self.assertEqual(async_register, cfa)
+
+    def check_async_regs(self, async_frames, process):
+        for frame in async_frames:
+            # The frames from the implicit main function don't have a demangled
+            # name, so we can't test whether they are a Q funclet or not.
+            if "Main" in frame.GetFunctionName():
+                break
+            self.check_async_regs_one_frame(frame, process)
 
     def check_variables(self, async_frames, expected_values):
         for frame, expected_value in zip(async_frames, expected_values):
@@ -58,6 +79,7 @@ def test(self):
         self.check_cfas(async_frames, process)
         self.check_pcs(async_frames, process, target)
         self.check_variables(async_frames, ["222", "333", "444", "555"])
+        self.check_async_regs(async_frames, process)
 
         target.DeleteAllBreakpoints()
         target.BreakpointCreateBySourceRegex("breakpoint2", source_file)
@@ -68,6 +90,7 @@ def test(self):
         self.check_cfas(async_frames, process)
         self.check_pcs(async_frames, process, target)
         self.check_variables(async_frames, ["111", "222", "333", "444", "555"])
+        self.check_async_regs(async_frames, process)
 
         # Now stop at the Q funclet right after the await to ASYNC___1
         target.DeleteAllBreakpoints()
@@ -77,6 +100,7 @@ def test(self):
         self.check_cfas(async_frames, process)
         self.check_pcs(async_frames, process, target)
         self.check_variables(async_frames, ["222", "333", "444", "555"])
+        self.check_async_regs(async_frames, process)
 
         target.DeleteAllBreakpoints()
         target.BreakpointCreateBySourceRegex("breakpoint3", source_file)
@@ -85,3 +109,4 @@ def test(self):
         self.check_cfas(async_frames, process)
         self.check_pcs(async_frames, process, target)
         self.check_variables(async_frames, ["222", "333", "444", "555"])
+        self.check_async_regs(async_frames, process)