[libunwind] [libunwind] Detect cycles of length 1 (PR #103476)

Michael Kolupaev via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 13 14:38:49 PDT 2024


https://github.com/al13n321 created https://github.com/llvm/llvm-project/pull/103476

If an unwind step leaves both the instruction pointer and the stack pointer unchanged, stop unwinding. Otherwise we'd be stuck in a loop producing the same stack frame over and over.

This happens with musl libc in __clone(), which has incorrect DWARF unwind information.

GDB also handles it using cycle detection, but it checks for cycles of any length rather than just length 1. That seems unnecessary here, at least until anyone encounters such cycles in practice.

Tested manually in https://github.com/ClickHouse/libunwind/pull/33

>From cdac646034cf9f7bf6326bcd28a3626db8650ed7 Mon Sep 17 00:00:00 2001
From: Michael Kolupaev <michael.kolupaev at clickhouse.com>
Date: Tue, 13 Aug 2024 21:31:30 +0000
Subject: [PATCH] [libunwind] Detect cycles of length 1

---
 libunwind/src/UnwindCursor.hpp | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/libunwind/src/UnwindCursor.hpp b/libunwind/src/UnwindCursor.hpp
index ce6dced535e781..b6c51446cb7d47 100644
--- a/libunwind/src/UnwindCursor.hpp
+++ b/libunwind/src/UnwindCursor.hpp
@@ -2923,6 +2923,9 @@ template <typename A, typename R> int UnwindCursor<A, R>::step(bool stage2) {
   if (_unwindInfoMissing)
     return UNW_STEP_END;
 
+  unw_word_t previousIP = getReg(UNW_REG_IP);
+  unw_word_t previousSP = getReg(UNW_REG_SP);
+
   // Use unwinding info to modify register set as if function returned.
   int result;
 #if defined(_LIBUNWIND_CHECK_LINUX_SIGRETURN)
@@ -2951,6 +2954,14 @@ template <typename A, typename R> int UnwindCursor<A, R>::step(bool stage2) {
 
   // update info based on new PC
   if (result == UNW_STEP_SUCCESS) {
+    // Detect cycles of length 1. In particular this happens in musl's
+    // __clone(), which has incorrect DWARF unwind information.
+    // We don't check all registers, so it's not strictly guaranteed that
+    // unwinding would be stuck in a cycle, but seems like a reasonable
+    // heuristic.
+    if (getReg(UNW_REG_SP) == previousSP && getReg(UNW_REG_IP) == previousIP)
+      return UNW_EBADFRAME;
+
     this->setInfoBasedOnIPRegister(true);
     if (_unwindInfoMissing)
       return UNW_STEP_END;



More information about the cfe-commits mailing list