[Lldb-commits] [PATCH] D86417: [lldb] do not propagate eTrapHandlerFrame repeatedly

[Luboš Luňák via Phabricator via lldb-commits]

llunak created this revision.
llunak added reviewers: JosephTremoulet, jasonmolenda.
llunak added a project: LLDB.
Herald added a subscriber: JDevlieghere.
llunak requested review of this revision.

This fixes handle-abrt test on my x86_64 openSUSE 15.2. The regression was introduced by 31e6dbe1c6a6e418bd65847610d5052cc59978b2.
What happens is that the the point of the breapoint the stack is:

  frame #0: 0x00000000004006bb a.out`handler(sig=6) at main.c:7:5
  frame #1: 0x00007ffff7a555a0 libc.so.6`__restore_rt
  frame #2: 0x00007ffff7a55520 libc.so.6`raise + 272
  frame #3: 0x00007ffff7a56b01 libc.so.6`abort + 337
  frame #4: 0x00000000004006e9 a.out`abort_caller at main.c:12:5
  frame #5: 0x0000000000400743 a.out`main at main.c:23:5

The frame for '__restore_rt' is set to be eTrapHandlerFrame. And its parent 'raise' is set so as well because of 'GetNextFrame()->m_frame_type == eTrapHandlerFrame '. And then the same is done for 'abort', which seems incorrect, a frame two frames away from the trap frame should be an ordinary frame that does not need special handling.

I don't have detailed enough knowledge of this are to be certain about what happens, but this patch basically changes all 'GetNextFrame()->m_frame_type == eTrapHandlerFrame' to be false if the actual trap frame is only the next-next frame. This fixes the handle-abrt test for me, and all the other tests still pass. The actually affected code is the "if (behaves_like_zeroth_frame && process) {" block in RegisterContextUnwind::GetFullUnwindPlanForFrame(), merely adding "&& false" there avoids the problem as well. Why disabling that code matters here, I have no idea.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D86417

Files:
  lldb/include/lldb/Target/RegisterContextUnwind.h
  lldb/source/Target/RegisterContextUnwind.cpp


Index: lldb/source/Target/RegisterContextUnwind.cpp
===================================================================
--- lldb/source/Target/RegisterContextUnwind.cpp
+++ lldb/source/Target/RegisterContextUnwind.cpp
@@ -655,7 +655,8 @@
 
   // If we're in _sigtramp(), unwinding past this frame requires special
   // knowledge.
-  if (m_frame_type == eTrapHandlerFrame || m_frame_type == eDebuggerFrame)
+  if (m_frame_type == eTrapHandlerFrame ||
+      m_frame_type == eTrapHandlerParentFrame || m_frame_type == eDebuggerFrame)
     return unwind_plan_sp;
 
   unwind_plan_sp = func_unwinders_sp->GetUnwindPlanFastUnwind(
@@ -807,7 +808,9 @@
   // section, so prefer that if available. On other platforms we may need to
   // provide a platform-specific UnwindPlan which encodes the details of how to
   // unwind out of sigtramp.
-  if (m_frame_type == eTrapHandlerFrame && process) {
+  if ((m_frame_type == eTrapHandlerFrame ||
+       m_frame_type == eTrapHandlerParentFrame) &&
+      process) {
     m_fast_unwind_plan_sp.reset();
     unwind_plan_sp =
         func_unwinders_sp->GetEHFrameUnwindPlan(process->GetTarget());
@@ -1099,7 +1102,8 @@
 // so this method helps to disambiguate that.
 
 bool RegisterContextUnwind::IsTrapHandlerFrame() const {
-  return m_frame_type == eTrapHandlerFrame;
+  return m_frame_type == eTrapHandlerFrame ||
+         m_frame_type == eTrapHandlerParentFrame;
 }
 
 // A skip frame is a bogus frame on the stack -- but one where we're likely to
@@ -1216,9 +1220,10 @@
         // value instead of the Return Address register.
         // If $pc is not available, fall back to the RA reg.
         UnwindPlan::Row::RegisterLocation scratch;
-        if (m_frame_type == eTrapHandlerFrame &&
-            active_row->GetRegisterInfo 
-              (pc_regnum.GetAsKind (unwindplan_registerkind), scratch)) {
+        if ((m_frame_type == eTrapHandlerFrame ||
+             m_frame_type == eTrapHandlerParentFrame) &&
+            active_row->GetRegisterInfo(
+                pc_regnum.GetAsKind(unwindplan_registerkind), scratch)) {
           UnwindLogMsg("Providing pc register instead of rewriting to "
                        "RA reg because this is a trap handler and there is "
                        "a location for the saved pc register value.");
@@ -1769,7 +1774,7 @@
     return;
   }
 
-  m_frame_type = eTrapHandlerFrame;
+  m_frame_type = eTrapHandlerParentFrame;
 
   if (m_current_offset_backed_up_one != m_current_offset) {
     // We backed up the pc by 1 to compute the symbol context, but
Index: lldb/include/lldb/Target/RegisterContextUnwind.h
===================================================================
--- lldb/include/lldb/Target/RegisterContextUnwind.h
+++ lldb/include/lldb/Target/RegisterContextUnwind.h
@@ -71,6 +71,7 @@
   enum FrameType {
     eNormalFrame,
     eTrapHandlerFrame,
+    eTrapHandlerParentFrame, // a parent frame of eTrapHandlerFrame
     eDebuggerFrame, // a debugger inferior function call frame; we get caller's
                     // registers from debugger
     eSkipFrame,     // The unwind resulted in a bogus frame but may get back on


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D86417.287249.patch
Type: text/x-patch
Size: 3147 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20200823/e1dc0277/attachment.bin>