[Lldb-commits] [PATCH] D114861: Don't consider frames with different PCs as a loop

[Tatyana Krasnukha via Phabricator via lldb-commits]

tatyana-krasnukha created this revision.
tatyana-krasnukha added reviewers: labath, jasonmolenda.
tatyana-krasnukha added a project: LLDB.
Herald added a subscriber: JDevlieghere.
tatyana-krasnukha requested review of this revision.
Herald added a subscriber: lldb-commits.

A compiler can produce FDE records with no CFA offset if it doesn't save anything on stack for a frame. This is what the MetaWare compiler does for some functions - it doesn't update CFA since it saves return address to a register.
In this case LLDB fails to unwind the stack complaining about a loop. This patch checks also the return addresses of the frames, there is no loop if they differ.

This is also consistent with how UnwindLLDB::GetOneMoreFrame identifies infinite loops.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D114861

Files:
  lldb/source/Target/RegisterContextUnwind.cpp


Index: lldb/source/Target/RegisterContextUnwind.cpp
===================================================================
--- lldb/source/Target/RegisterContextUnwind.cpp
+++ lldb/source/Target/RegisterContextUnwind.cpp
@@ -690,28 +690,29 @@
   // -- or even more devious, we can actually oscillate between two CFA values.
   // Detect that here and break out to avoid a possible infinite loop in lldb
   // trying to unwind the stack. To detect when we have the same CFA value
-  // multiple times, we compare the
-  // CFA of the current
-  // frame with the 2nd next frame because in some specail case (e.g. signal
-  // hanlders, hand written assembly without ABI compliance) we can have 2
-  // frames with the same
-  // CFA (in theory we
-  // can have arbitrary number of frames with the same CFA, but more then 2 is
-  // very very unlikely)
+  // multiple times, we compare the CFA of the current frame with the 2nd next
+  // frame because in some specail case (e.g. signal hanlders, hand written
+  // assembly without ABI compiance) we can have 2 frames with the same CFA (in
+  // theory we can have arbitrary number of frames with the same CFA, but more
+  // then 2 is very very unlikely).
 
   RegisterContextUnwind::SharedPtr next_frame = GetNextFrame();
-  if (next_frame) {
-    RegisterContextUnwind::SharedPtr next_next_frame =
-        next_frame->GetNextFrame();
-    addr_t next_next_frame_cfa = LLDB_INVALID_ADDRESS;
-    if (next_next_frame && next_next_frame->GetCFA(next_next_frame_cfa)) {
-      if (next_next_frame_cfa == m_cfa) {
-        // We have a loop in the stack unwind
-        return true;
-      }
-    }
-  }
-  return false;
+  if (!next_frame)
+    return false;
+
+  RegisterContextUnwind::SharedPtr next_next_frame = next_frame->GetNextFrame();
+  if (!next_next_frame)
+    return false;
+
+  // Since the return address value can be restored not only from the stack but
+  // also from a register, don't consider frames with the same CFAs and different
+  // PCs as a loop.
+  if (m_current_pc.IsValid() && (m_current_pc != next_next_frame->GetPC()))
+    return false;
+
+  addr_t next_next_frame_cfa = LLDB_INVALID_ADDRESS;
+  return next_next_frame->GetCFA(next_next_frame_cfa) &&
+         next_next_frame_cfa == m_cfa;
 }
 
 bool RegisterContextUnwind::IsFrameZero() const { return m_frame_number == 0; }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D114861.390594.patch
Type: text/x-patch
Size: 2358 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20211201/623ff52b/attachment-0001.bin>