DavidSpickett updated this revision to Diff 382575.
DavidSpickett added a comment.

Turns out comparing thread.get_thread_frames() doesn't work because
you end up getting references to the same frames. Instead save the info
we want to compare.

One obvious reason this tactic should have failed is that the libc raise
frame will have a different ID between the two backtraces. This is why I've
ignored ID in the comparion.

Doing this showed me that sp and fp should be set, so I've got fp from
the sigcontext also.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112069/new/

https://reviews.llvm.org/D112069

Files:
  lldb/include/lldb/Target/Platform.h
  lldb/source/API/SBFrame.cpp
  lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
  lldb/source/Plugins/Platform/Linux/PlatformLinux.h
  lldb/source/Target/RegisterContextUnwind.cpp
  lldb/test/API/functionalities/signal/handle-abrt/TestHandleAbort.py

Index: lldb/test/API/functionalities/signal/handle-abrt/TestHandleAbort.py
===================================================================
--- lldb/test/API/functionalities/signal/handle-abrt/TestHandleAbort.py
+++ lldb/test/API/functionalities/signal/handle-abrt/TestHandleAbort.py
@@ -7,7 +7,21 @@
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
-
+from collections import namedtuple
+
+# Since frames internally point to ExecutionContextRef which itself points
+# to something else, we can't make a deep copy of them from Python.
+# Instead save what we care about for comparison purposes.
+# Ignoring the frame ID because what is frame 0 in the first catch will
+# be frame 2 in the handler backtrace.
+class FrameInfo(namedtuple('FrameInfo', ['sp', 'fp', 'function_name'])):
+    # So assert failures are more readable
+    def __repr__(self):
+        return "SP: 0x{:x} FP: 0x{:x} Fn: {}".format(
+            self.sp, self.fp, self.function_name)
+
+def make_frame_info(frame):
+    return FrameInfo(frame.GetSP(), frame.GetFP(), frame.GetDisplayFunctionName())
 
 class HandleAbortTestCase(TestBase):
 
@@ -16,8 +30,6 @@
     NO_DEBUG_INFO_TESTCASE = True
 
     @skipIfWindows  # signals do not exist on Windows
-    # Fails on Ubuntu Focal
-    @skipIf(archs=["aarch64"], oslist=["linux"])
     @expectedFailureNetBSD
     def test_inferior_handle_sigabrt(self):
         """Inferior calls abort() and handles the resultant SIGABRT.
@@ -47,6 +59,9 @@
         self.assertEqual(thread.GetStopReasonDataAtIndex(0),
                          signo, "The stop signal should be SIGABRT")
 
+        # Save the backtrace frames to compare to the handler backtrace later.
+        signal_frames = [make_frame_info(f) for f in thread.get_thread_frames()]
+
         # Continue to breakpoint in abort handler
         bkpt = target.FindBreakpointByID(
             lldbutil.run_break_set_by_source_regexp(self, "Set a breakpoint here"))
@@ -59,12 +74,20 @@
         self.assertEqual(frame.GetDisplayFunctionName(), "handler", "Unexpected break?")
 
         # Expect that unwinding should find 'abort_caller'
-        foundFoo = False
-        for frame in thread:
+        found_caller = False
+        for frame in thread.get_thread_frames():
             if frame.GetDisplayFunctionName() == "abort_caller":
-                foundFoo = True
+                found_caller = True
+                break
+
+        self.assertTrue(found_caller, "Unwinding did not find func that called abort")
+
+        # The signal handler backtrace has extra frames at the start, remove those
+        handler_frames = thread.get_thread_frames()[-len(signal_frames):]
+        handler_frames = [make_frame_info(f) for f in handler_frames]
 
-        self.assertTrue(foundFoo, "Unwinding did not find func that called abort")
+        # Check that frames present in both backtraces have the same addresses.
+        self.assertEqual(signal_frames, handler_frames, "Common backtrace frames do not match")
 
         # Continue until we exit.
         process.Continue()
Index: lldb/source/Target/RegisterContextUnwind.cpp
===================================================================
--- lldb/source/Target/RegisterContextUnwind.cpp
+++ lldb/source/Target/RegisterContextUnwind.cpp
@@ -900,6 +900,17 @@
   // unwind out of sigtramp.
   if (m_frame_type == eTrapHandlerFrame && process) {
     m_fast_unwind_plan_sp.reset();
+
+    if (m_sym_ctx_valid) {
+      lldb::PlatformSP platform = process->GetTarget().GetPlatform();
+      unwind_plan_sp = platform->GetTrapHandlerUnwindPlan(
+          process->GetTarget().GetArchitecture().GetMachine(),
+          GetSymbolOrFunctionName(m_sym_ctx));
+
+      if (unwind_plan_sp)
+        return unwind_plan_sp;
+    }
+
     unwind_plan_sp =
         func_unwinders_sp->GetEHFrameUnwindPlan(process->GetTarget());
     if (!unwind_plan_sp)
Index: lldb/source/Plugins/Platform/Linux/PlatformLinux.h
===================================================================
--- lldb/source/Plugins/Platform/Linux/PlatformLinux.h
+++ lldb/source/Plugins/Platform/Linux/PlatformLinux.h
@@ -50,6 +50,9 @@
 
   void CalculateTrapHandlerSymbolNames() override;
 
+  lldb::UnwindPlanSP GetTrapHandlerUnwindPlan(llvm::Triple::ArchType machine,
+                                              ConstString name) override;
+
   MmapArgList GetMmapArgumentList(const ArchSpec &arch, lldb::addr_t addr,
                                   lldb::addr_t length, unsigned prot,
                                   unsigned flags, lldb::addr_t fd,
Index: lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
===================================================================
--- lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
+++ lldb/source/Plugins/Platform/Linux/PlatformLinux.cpp
@@ -14,9 +14,11 @@
 #include <sys/utsname.h>
 #endif
 
+#include "Utility/ARM64_DWARF_Registers.h"
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/PluginManager.h"
 #include "lldb/Host/HostInfo.h"
+#include "lldb/Symbol/UnwindPlan.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/Target.h"
 #include "lldb/Utility/FileSpec.h"
@@ -251,6 +253,63 @@
   m_trap_handlers.push_back(ConstString("__restore_rt"));
 }
 
+static lldb::UnwindPlanSP GetAArch64TrapHanlderUnwindPlan(ConstString name) {
+  UnwindPlanSP unwind_plan_sp;
+  if (name != "__kernel_rt_sigreturn")
+    return unwind_plan_sp;
+
+  UnwindPlan::RowSP row = std::make_shared<UnwindPlan::Row>();
+  row->SetOffset(0);
+
+  // In the signal trampoline frame, sp points to an rt_sigframe[1], which is:
+  //  - 128-byte siginfo struct
+  //  - ucontext struct:
+  //     - 8-byte long (uc_flags)
+  //     - 8-byte pointer (uc_link)
+  //     - 24-byte stack_t
+  //     - 128-byte signal set
+  //     - 8 bytes of padding because sigcontext has 16-byte alignment
+  //     - sigcontext/mcontext_t
+  // [1]
+  // https://github.com/torvalds/linux/blob/master/arch/arm64/kernel/signal.c
+  int32_t offset = 128 + 8 + 8 + 24 + 128 + 8;
+  // Then sigcontext[2] is:
+  // - 8 byte fault address
+  // - 31 8 byte registers
+  // - 8 byte sp
+  // - 8 byte pc
+  // [2]
+  // https://github.com/torvalds/linux/blob/master/arch/arm64/include/uapi/asm/sigcontext.h
+  offset += 8 + (31 * 8);
+  row->GetCFAValue().SetIsRegisterPlusOffset(arm64_dwarf::sp, offset);
+  // Aka x29
+  row->SetRegisterLocationToAtCFAPlusOffset(arm64_dwarf::fp, -16, false);
+  row->SetRegisterLocationToAtCFAPlusOffset(arm64_dwarf::sp, 0, false);
+  row->SetRegisterLocationToAtCFAPlusOffset(arm64_dwarf::pc, 8, false);
+
+  unwind_plan_sp = std::make_shared<UnwindPlan>(eRegisterKindGeneric);
+  unwind_plan_sp->AppendRow(row);
+  unwind_plan_sp->SetSourceName("AArch64 Linux sigcontext");
+  unwind_plan_sp->SetSourcedFromCompiler(eLazyBoolYes);
+  // Because sp is the same throughout the function
+  unwind_plan_sp->SetUnwindPlanValidAtAllInstructions(eLazyBoolYes);
+  unwind_plan_sp->SetUnwindPlanForSignalTrap(eLazyBoolYes);
+  unwind_plan_sp->SetRegisterKind(eRegisterKindDWARF);
+
+  return unwind_plan_sp;
+}
+
+lldb::UnwindPlanSP
+PlatformLinux::GetTrapHandlerUnwindPlan(llvm::Triple::ArchType machine,
+                                        ConstString name) {
+  if ((machine == llvm::Triple::ArchType::aarch64 ||
+       machine == llvm::Triple::ArchType::aarch64_be ||
+       machine == llvm::Triple::ArchType::aarch64_32))
+    return GetAArch64TrapHanlderUnwindPlan(name);
+
+  return {};
+}
+
 MmapArgList PlatformLinux::GetMmapArgumentList(const ArchSpec &arch,
                                                addr_t addr, addr_t length,
                                                unsigned prot, unsigned flags,
Index: lldb/source/API/SBFrame.cpp
===================================================================
--- lldb/source/API/SBFrame.cpp
+++ lldb/source/API/SBFrame.cpp
@@ -66,7 +66,6 @@
 
 SBFrame::SBFrame(const SBFrame &rhs) : m_opaque_sp() {
   LLDB_RECORD_CONSTRUCTOR(SBFrame, (const lldb::SBFrame &), rhs);
-
   m_opaque_sp = clone(rhs.m_opaque_sp);
 }
 
Index: lldb/include/lldb/Target/Platform.h
===================================================================
--- lldb/include/lldb/Target/Platform.h
+++ lldb/include/lldb/Target/Platform.h
@@ -719,6 +719,24 @@
   ///     A list of symbol names.  The list may be empty.
   virtual const std::vector<ConstString> &GetTrapHandlerSymbolNames();
 
+  /// Try to get a specific unwind plan for a named trap handler.
+  /// The default is not to have specific unwind plans for trap handlers.
+  ///
+  /// \param[in] machine
+  ///     The architecture of the current target.
+  ///
+  /// \param[in] name
+  ///     Name of the trap handler function.
+  ///
+  /// \return
+  ///     A specific unwind plan for that trap handler, or an empty
+  ///     shared pointer. The latter means there is no specific plan,
+  ///     unwind as normal.
+  virtual lldb::UnwindPlanSP
+  GetTrapHandlerUnwindPlan(llvm::Triple::ArchType machine, ConstString name) {
+    return {};
+  }
+
   /// Find a support executable that may not live within in the standard
   /// locations related to LLDB.
   ///
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to