Merge pull request #9025 from felipepiovezan/felipe/unwinding_pc_patches_to_release

adrian-prantl · web-flow · commit d85bebe1b468 · 2024-08-01T13:59:03.000-07:00
Fix swift async frames unwinding unwinding
diff --git a/lldb/include/lldb/Symbol/UnwindPlan.h b/lldb/include/lldb/Symbol/UnwindPlan.h
@@ -68,7 +68,8 @@ class UnwindPlan {
         isAFAPlusOffset,   // reg = AFA + offset
         inOtherRegister,   // reg = other reg
         atDWARFExpression, // reg = deref(eval(dwarf_expr))
-        isDWARFExpression  // reg = eval(dwarf_expr)
+        isDWARFExpression, // reg = eval(dwarf_expr)
+        isConstant         // reg = constant
       };
 
       RegisterLocation() : m_location() {}
@@ -105,6 +106,15 @@ class UnwindPlan {
 
       bool IsDWARFExpression() const { return m_type == isDWARFExpression; }
 
+      bool IsConstant() const { return m_type == isConstant; }
+
+      void SetIsConstant(uint64_t value) {
+        m_type = isConstant;
+        m_location.constant_value = value;
+      }
+
+      uint64_t GetConstant() const { return m_location.constant_value; }
+
       void SetAtCFAPlusOffset(int32_t offset) {
         m_type = atCFAPlusOffset;
         m_location.offset = offset;
@@ -192,6 +202,8 @@ class UnwindPlan {
           const uint8_t *opcodes;
           uint16_t length;
         } expr;
+        // For m_type == isConstant
+        uint64_t constant_value;
       } m_location;
     };
 
@@ -361,6 +373,10 @@ class UnwindPlan {
     bool SetRegisterLocationToIsDWARFExpression(uint32_t reg_num,
                                                 const uint8_t *opcodes,
                                                 uint32_t len, bool can_replace);
+
+    bool SetRegisterLocationToIsConstant(uint32_t reg_num, uint64_t constant,
+                                         bool can_replace);
+
     // When this UnspecifiedRegistersAreUndefined mode is
     // set, any register that is not specified by this Row will
     // be described as Undefined.
diff --git a/lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp b/lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.cpp
@@ -99,10 +99,6 @@ static bool IsModuleSwiftRuntime(lldb_private::Process &process,
   return module.GetFileSpec().GetFilename() == GetStandardLibraryName(process);
 }
 
-static UnwindPlanSP
-GetFollowAsyncContextUnwindPlan(RegisterContext *regctx, ArchSpec &arch,
-                                bool &behaves_like_zeroth_frame);
-
 AppleObjCRuntimeV2 *
 SwiftLanguageRuntime::GetObjCRuntime(lldb_private::Process &process) {
   if (auto objc_runtime = ObjCLanguageRuntime::Get(process)) {
@@ -2608,7 +2604,7 @@ SwiftLanguageRuntime::GetRuntimeUnwindPlan(ProcessSP process_sp,
   addr_t fp = regctx->GetFP(LLDB_INVALID_ADDRESS);
   if (fp == LLDB_INVALID_ADDRESS) {
     if (GetAsyncContext(regctx) != LLDB_INVALID_ADDRESS)
-      return GetFollowAsyncContextUnwindPlan(regctx, arch,
+      return GetFollowAsyncContextUnwindPlan(process_sp, regctx, arch,
                                              behaves_like_zeroth_frame);
     return UnwindPlanSP();
   }
@@ -2749,9 +2745,31 @@ SwiftLanguageRuntime::GetRuntimeUnwindPlan(ProcessSP process_sp,
         regnums->dummy_regnum, g_dummy_dwarf_expression,
         sizeof(g_dummy_dwarf_expression), false);
   }
-  row->SetRegisterLocationToAtCFAPlusOffset(regnums->pc_regnum, ptr_size,
-                                            false);
 
+  std::optional<addr_t> pc_after_prologue = [&]() -> std::optional<addr_t> {
+    // In the prologue, use the async_reg as is, it has not been clobbered.
+    if (in_prologue)
+      return TrySkipVirtualParentProlog(GetAsyncContext(regctx), *process_sp,
+                                        indirect_context);
+
+    // Both ABIs (x86_64 and aarch64) guarantee the async reg is saved at:
+    // *(fp - 8).
+    Status error;
+    addr_t async_reg_entry_value = LLDB_INVALID_ADDRESS;
+    process_sp->ReadMemory(fp - ptr_size, &async_reg_entry_value, ptr_size,
+                           error);
+    if (error.Fail())
+      return {};
+    return TrySkipVirtualParentProlog(async_reg_entry_value, *process_sp,
+                                      indirect_context);
+  }();
+
+  if (pc_after_prologue)
+    row->SetRegisterLocationToIsConstant(regnums->pc_regnum, *pc_after_prologue,
+                                         false);
+  else
+    row->SetRegisterLocationToAtCFAPlusOffset(regnums->pc_regnum, ptr_size,
+                                              false);
   row->SetUnspecifiedRegistersAreUndefined(true);
 
   UnwindPlanSP plan = std::make_shared<UnwindPlan>(lldb::eRegisterKindDWARF);
@@ -2763,11 +2781,9 @@ SwiftLanguageRuntime::GetRuntimeUnwindPlan(ProcessSP process_sp,
   return plan;
 }
 
-// Creates an UnwindPlan for following the AsyncContext chain
-// up the stack, from a current AsyncContext frame.
-static UnwindPlanSP
-GetFollowAsyncContextUnwindPlan(RegisterContext *regctx, ArchSpec &arch,
-                                bool &behaves_like_zeroth_frame) {
+UnwindPlanSP SwiftLanguageRuntime::GetFollowAsyncContextUnwindPlan(
+    ProcessSP process_sp, RegisterContext *regctx, ArchSpec &arch,
+    bool &behaves_like_zeroth_frame) {
   LLDB_SCOPED_TIMER();
 
   UnwindPlan::RowSP row(new UnwindPlan::Row);
@@ -2779,13 +2795,15 @@ GetFollowAsyncContextUnwindPlan(RegisterContext *regctx, ArchSpec &arch,
   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 (regctx->ReadRegisterAsUnsigned(regnums->dummy_regnum, (uint64_t)-1ll) !=
-      (uint64_t)-1ll) {
+  if (!is_indirect) {
     row->GetCFAValue().SetIsRegisterDereferenced(regnums->async_ctx_regnum);
     row->SetRegisterLocationToSame(regnums->async_ctx_regnum, false);
   } else {
@@ -2822,8 +2840,21 @@ GetFollowAsyncContextUnwindPlan(RegisterContext *regctx, ArchSpec &arch,
         regnums->async_ctx_regnum, expression, expr_size - 1, false);
   }
 
-  row->SetRegisterLocationToAtCFAPlusOffset(regnums->pc_regnum, ptr_size,
-                                            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;
+  if (std::optional<addr_t> pc_after_prologue = TrySkipVirtualParentProlog(
+          GetAsyncContext(regctx), *process_sp, num_indirections))
+    row->SetRegisterLocationToIsConstant(regnums->pc_regnum, *pc_after_prologue,
+                                         false);
+  else
+    row->SetRegisterLocationToAtCFAPlusOffset(regnums->pc_regnum, ptr_size,
+                                              false);
 
   row->SetUnspecifiedRegistersAreUndefined(true);
 
@@ -2837,4 +2868,51 @@ GetFollowAsyncContextUnwindPlan(RegisterContext *regctx, ArchSpec &arch,
   return plan;
 }
 
+std::optional<lldb::addr_t> SwiftLanguageRuntime::TrySkipVirtualParentProlog(
+    lldb::addr_t async_reg_val, Process &process, unsigned num_indirections) {
+  assert(num_indirections <= 2 &&
+         "more than two dereferences should not be needed");
+  if (async_reg_val == LLDB_INVALID_ADDRESS || async_reg_val == 0)
+    return {};
+
+  const auto ptr_size = process.GetAddressByteSize();
+  Status error;
+
+  // Compute the CFA of this frame.
+  addr_t cfa = async_reg_val;
+  for (; num_indirections != 0; --num_indirections) {
+    process.ReadMemory(cfa, &cfa, ptr_size, error);
+    if (error.Fail())
+      return {};
+  }
+
+  // The last funclet will have a zero CFA, we don't want to read that.
+  if (cfa == 0)
+    return {};
+
+  // Get the PC of the parent frame, i.e. the continuation pointer, which is
+  // the second field of the CFA.
+  addr_t pc_location = cfa + ptr_size;
+  addr_t pc_value = LLDB_INVALID_ADDRESS;
+  process.ReadMemory(pc_location, &pc_value, ptr_size, error);
+  if (error.Fail())
+    return {};
+
+  Address pc;
+  Target &target = process.GetTarget();
+  pc.SetLoadAddress(pc_value, &target);
+  if (!pc.IsValid())
+    return {};
+
+  SymbolContext sc;
+  if (!pc.CalculateSymbolContext(&sc,
+                                 eSymbolContextFunction | eSymbolContextSymbol))
+    return {};
+  if (!sc.symbol && !sc.function)
+    return {};
+
+  auto prologue_size = sc.symbol ? sc.symbol->GetPrologueByteSize()
+                                 : sc.function->GetPrologueByteSize();
+  return pc_value + prologue_size;
+}
 } // namespace lldb_private
diff --git a/lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.h b/lldb/source/Plugins/LanguageRuntime/Swift/SwiftLanguageRuntime.h
@@ -482,6 +482,21 @@ class SwiftLanguageRuntime : public LanguageRuntime {
   bool GetTargetOfPartialApply(SymbolContext &curr_sc, ConstString &apply_name,
                                SymbolContext &sc);
   AppleObjCRuntimeV2 *GetObjCRuntime();
+
+private:
+  /// Creates an UnwindPlan for following the AsyncContext chain up the stack,
+  /// from a current AsyncContext frame.
+  lldb::UnwindPlanSP
+  GetFollowAsyncContextUnwindPlan(lldb::ProcessSP process_sp,
+                                  RegisterContext *regctx, ArchSpec &arch,
+                                  bool &behaves_like_zeroth_frame);
+
+  /// Given the async register of a funclet, extract its continuation pointer,
+  /// compute the prologue size of the continuation function, and return the
+  /// address of the first non-prologue instruction.
+  std::optional<lldb::addr_t>
+  TrySkipVirtualParentProlog(lldb::addr_t async_reg_val, Process &process,
+                             unsigned num_indirections);
 };
 
 } // namespace lldb_private
diff --git a/lldb/source/Symbol/UnwindPlan.cpp b/lldb/source/Symbol/UnwindPlan.cpp
@@ -46,6 +46,8 @@ operator==(const UnwindPlan::Row::RegisterLocation &rhs) const {
         return !memcmp(m_location.expr.opcodes, rhs.m_location.expr.opcodes,
                        m_location.expr.length);
       break;
+    case isConstant:
+      return m_location.constant_value == rhs.m_location.constant_value;
     }
   }
   return false;
@@ -153,6 +155,9 @@ void UnwindPlan::Row::RegisterLocation::Dump(Stream &s,
     if (m_type == atDWARFExpression)
       s.PutChar(']');
   } break;
+  case isConstant:
+    s.Printf("=0x%" PRIx64, m_location.constant_value);
+    break;
   }
 }
 
@@ -362,6 +367,17 @@ bool UnwindPlan::Row::SetRegisterLocationToIsDWARFExpression(
   return true;
 }
 
+bool UnwindPlan::Row::SetRegisterLocationToIsConstant(uint32_t reg_num,
+                                                      uint64_t constant,
+                                                      bool can_replace) {
+  if (!can_replace &&
+      m_register_locations.find(reg_num) != m_register_locations.end())
+    return false;
+  RegisterLocation reg_loc;
+  reg_loc.SetIsConstant(constant);
+  m_register_locations[reg_num] = reg_loc;
+  return true;
+}
 
 bool UnwindPlan::Row::operator==(const UnwindPlan::Row &rhs) const {
   return m_offset == rhs.m_offset && m_cfa_value == rhs.m_cfa_value &&
diff --git a/lldb/source/Target/RegisterContextUnwind.cpp b/lldb/source/Target/RegisterContextUnwind.cpp
@@ -1692,6 +1692,15 @@ RegisterContextUnwind::SavedLocationForRegister(
     return UnwindLLDB::RegisterSearchResult::eRegisterNotFound;
   }
 
+  if (unwindplan_regloc.IsConstant()) {
+    regloc.type = UnwindLLDB::RegisterLocation::eRegisterValueInferred;
+    regloc.location.inferred_value = unwindplan_regloc.GetConstant();
+    m_registers[regnum.GetAsKind(eRegisterKindLLDB)] = regloc;
+    UnwindLogMsg("supplying caller's register %s (%d) via constant value",
+                 regnum.GetName(), regnum.GetAsKind(eRegisterKindLLDB));
+    return UnwindLLDB::RegisterSearchResult::eRegisterFound;
+  }
+
   UnwindLogMsg("no save location for %s (%d) in this stack frame",
                regnum.GetName(), regnum.GetAsKind(eRegisterKindLLDB));
 
diff --git a/lldb/test/API/lang/swift/async/frame/variables_multiple_frames/Makefile b/lldb/test/API/lang/swift/async/frame/variables_multiple_frames/Makefile
@@ -0,0 +1,3 @@
+SWIFT_SOURCES := main.swift
+SWIFTFLAGS_EXTRAS := -parse-as-library
+include Makefile.rules
diff --git a/lldb/test/API/lang/swift/async/frame/variables_multiple_frames/TestSwiftAsyncFrameVarMultipleFrames.py b/lldb/test/API/lang/swift/async/frame/variables_multiple_frames/TestSwiftAsyncFrameVarMultipleFrames.py
@@ -0,0 +1,78 @@
+import lldb
+from lldbsuite.test.decorators import *
+import lldbsuite.test.lldbtest as lldbtest
+import lldbsuite.test.lldbutil as lldbutil
+
+
+class TestCase(lldbtest.TestBase):
+
+    mydir = lldbtest.TestBase.compute_mydir(__file__)
+
+    # Check that the CFA chain is correctly built
+    def check_cfas(self, async_frames, process):
+        async_cfas = list(map(lambda frame: frame.GetCFA(), async_frames))
+        expected_cfas = [async_cfas[0]]
+        # The CFA chain ends in nullptr.
+        while expected_cfas[-1] != 0:
+            error = lldb.SBError()
+            expected_cfas.append(
+                process.ReadPointerFromMemory(expected_cfas[-1], error)
+            )
+            self.assertSuccess(error, "Managed to read cfa memory")
+
+        self.assertEqual(async_cfas, expected_cfas[:-1])
+
+    def check_pcs(self, async_frames, process, target):
+        for idx, frame in enumerate(async_frames[:-1]):
+            # Read the continuation pointer from the second field of the CFA.
+            error = lldb.SBError()
+            continuation_ptr = process.ReadPointerFromMemory(
+                frame.GetCFA() + target.addr_size, error
+            )
+            self.assertSuccess(error, "Managed to read context memory")
+
+            # The PC of the previous frame should be the continuation pointer
+            # with the funclet's prologue skipped.
+            parent_frame = async_frames[idx + 1]
+            prologue_to_skip = parent_frame.GetFunction().GetPrologueByteSize()
+            self.assertEqual(continuation_ptr + prologue_to_skip, parent_frame.GetPC())
+
+
+    def check_variables(self, async_frames, expected_values):
+        for (frame, expected_value) in zip(async_frames, expected_values):
+            myvar = frame.FindVariable("myvar")
+            lldbutil.check_variable(self, myvar, False, value=expected_value)
+
+    @swiftTest
+    @skipIf(oslist=["windows", "linux"])
+    def test(self):
+        """Test `frame variable` in async functions"""
+        self.build()
+
+        source_file = lldb.SBFileSpec("main.swift")
+        target, process, _, _ = lldbutil.run_to_source_breakpoint(
+            self, "breakpoint1", source_file
+        )
+
+        async_frames = process.GetSelectedThread().frames
+        self.check_cfas(async_frames, process)
+        self.check_pcs(async_frames, process, target)
+        self.check_variables(async_frames, ["222", "333", "444", "555"])
+
+        target.DeleteAllBreakpoints()
+        target.BreakpointCreateBySourceRegex("breakpoint2", source_file)
+        process.Continue()
+        # First frame is from a synchronous function
+        frames = process.GetSelectedThread().frames
+        async_frames = frames[1:]
+        self.check_cfas(async_frames, process)
+        self.check_pcs(async_frames, process, target)
+        self.check_variables(async_frames, ["111", "222", "333", "444", "555"])
+
+        target.DeleteAllBreakpoints()
+        target.BreakpointCreateBySourceRegex("breakpoint3", source_file)
+        process.Continue()
+        async_frames = process.GetSelectedThread().frames
+        self.check_cfas(async_frames, process)
+        self.check_pcs(async_frames, process, target)
+        self.check_variables(async_frames, ["222", "333", "444", "555"])
diff --git a/lldb/test/API/lang/swift/async/frame/variables_multiple_frames/main.swift b/lldb/test/API/lang/swift/async/frame/variables_multiple_frames/main.swift
diff --git a/lldb/test/API/lang/swift/async/unwind/backtrace_locals/TestSwiftAsyncBacktraceLocals.py b/lldb/test/API/lang/swift/async/unwind/backtrace_locals/TestSwiftAsyncBacktraceLocals.py

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+SWIFT_SOURCES := main.swift`
	`2`	`+SWIFTFLAGS_EXTRAS := -parse-as-library`
	`3`	`+include Makefile.rules`