https://github.com/labath created 
https://github.com/llvm/llvm-project/pull/125839

PR #86603 broke unwinding in for unwind info added via "target symbols add". 
#86770 attempted to fix this, but the fix was only partial -- it accepted new 
sources of unwind information, but didn't take into account that the symbol 
file can alter what lldb percieves as function boundaries.

A stripped file will not contain information about private (non-exported) 
symbols, which will make the public symbols appear very large. If lldb tries to 
unwind from such a function before symbols are added, then the cached unwind 
plan will prevent new (correct) unwind plans from being created.

target-symbols-add-unwind.test might have caught this, were it not for the fact 
that the "image show-unwind" command does *not* use cached unwind information 
(it recomputes it from scratch).

The changes in this patch come in three pieces:
- Clear cached unwind plans when adding symbols. Since the symbol boundaries 
can change, we cannot trust anything we've computed previously.
- Add a flag to "image show-unwind" to display the cached unwind information 
(mainly for the use in the test, but I think it's also generally useful).
- Rewrite the test to better and more reliably simulate the real-world 
scenario: I've swapped the running process for a core (minidump) file so it can 
run anywhere; used the caching version of the show-unwind command; and swapped 
C for assembly to better control the placement of symbols

>From 82e0ee5bf53dd6b886327021f5b145c942b43ff6 Mon Sep 17 00:00:00 2001
From: Pavel Labath <pa...@labath.sk>
Date: Wed, 5 Feb 2025 11:28:48 +0100
Subject: [PATCH] [lldb] Clear cached unwind plans when adding symbols

PR #86603 broke unwinding in for unwind info added via "target symbols
add". #86770 attempted to fix this, but the fix was only partial -- it
accepted new sources of unwind information, but didn't take into account
that the symbol file can alter what lldb percieves as function
boundaries.

A stripped file will not contain information about private
(non-exported) symbols, which will make the public symbols appear very
large. If lldb tries to unwind from such a function before symbols are
added, then the cached unwind plan will prevent new (correct) unwind
plans from being created.

target-symbols-add-unwind.test might have caught this, were it not for
the fact that the "image show-unwind" command does *not* use cached
unwind information (it recomputes it from scratch).

The changes in this patch come in three pieces:
- Clear cached unwind plans when adding symbols. Since the symbol
  boundaries can change, we cannot trust anything we've computed
  previously.
- Add a flag to "image show-unwind" to display the cached unwind
  information (mainly for the use in the test, but I think it's also
  generally useful).
- Rewrite the test to better and more reliably simulate the real-world
  scenario: I've swapped the running process for a core (minidump) file
  so it can run anywhere; used the caching version of the show-unwind
  command; and swapped C for assembly to better control the placement of
  symbols
---
 lldb/include/lldb/Symbol/UnwindTable.h        |   5 +-
 lldb/source/Commands/CommandObjectTarget.cpp  |  22 +++-
 lldb/source/Commands/Options.td               |   2 +
 lldb/source/Symbol/UnwindTable.cpp            |   3 +-
 .../Inputs/target-symbols-add-unwind.c        |   1 -
 .../SymbolFile/target-symbols-add-unwind.test | 118 +++++++++++++++---
 6 files changed, 125 insertions(+), 26 deletions(-)
 delete mode 100644 
lldb/test/Shell/SymbolFile/Inputs/target-symbols-add-unwind.c

diff --git a/lldb/include/lldb/Symbol/UnwindTable.h 
b/lldb/include/lldb/Symbol/UnwindTable.h
index 29b7fa61b4849e0..3166fdec6ebaa77 100644
--- a/lldb/include/lldb/Symbol/UnwindTable.h
+++ b/lldb/include/lldb/Symbol/UnwindTable.h
@@ -38,8 +38,9 @@ class UnwindTable {
   ArmUnwindInfo *GetArmUnwindInfo();
   SymbolFile *GetSymbolFile();
 
-  lldb::FuncUnwindersSP GetFuncUnwindersContainingAddress(const Address &addr,
-                                                          SymbolContext &sc);
+  lldb::FuncUnwindersSP
+  GetFuncUnwindersContainingAddress(const Address &addr,
+                                    const SymbolContext &sc);
 
   bool GetAllowAssemblyEmulationUnwindPlans();
 
diff --git a/lldb/source/Commands/CommandObjectTarget.cpp 
b/lldb/source/Commands/CommandObjectTarget.cpp
index d8265e41a7384eb..db142e8f0eaee77 100644
--- a/lldb/source/Commands/CommandObjectTarget.cpp
+++ b/lldb/source/Commands/CommandObjectTarget.cpp
@@ -3471,6 +3471,17 @@ class CommandObjectTargetModulesShowUnwind : public 
CommandObjectParsed {
         m_type = eLookupTypeFunctionOrSymbol;
         break;
 
+      case 'c':
+        bool value, success;
+        value = OptionArgParser::ToBoolean(option_arg, false, &success);
+        if (success) {
+          m_cached = value;
+        } else {
+          return Status::FromErrorStringWithFormatv(
+              "invalid boolean value '%s' passed for -G option", option_arg);
+        }
+        break;
+
       default:
         llvm_unreachable("Unimplemented option");
       }
@@ -3482,6 +3493,7 @@ class CommandObjectTargetModulesShowUnwind : public 
CommandObjectParsed {
       m_type = eLookupTypeInvalid;
       m_str.clear();
       m_addr = LLDB_INVALID_ADDRESS;
+      m_cached = false;
     }
 
     llvm::ArrayRef<OptionDefinition> GetDefinitions() override {
@@ -3494,6 +3506,7 @@ class CommandObjectTargetModulesShowUnwind : public 
CommandObjectParsed {
                                      // parsing options
     std::string m_str; // Holds name lookup
     lldb::addr_t m_addr = LLDB_INVALID_ADDRESS; // Holds the address to lookup
+    bool m_cached = false;
   };
 
   CommandObjectTargetModulesShowUnwind(CommandInterpreter &interpreter)
@@ -3583,9 +3596,12 @@ class CommandObjectTargetModulesShowUnwind : public 
CommandObjectParsed {
       if (abi)
         start_addr = abi->FixCodeAddress(start_addr);
 
-      FuncUnwindersSP func_unwinders_sp(
-          sc.module_sp->GetUnwindTable()
-              .GetUncachedFuncUnwindersContainingAddress(start_addr, sc));
+      UnwindTable &uw_table = sc.module_sp->GetUnwindTable();
+      FuncUnwindersSP func_unwinders_sp =
+          m_options.m_cached
+              ? uw_table.GetFuncUnwindersContainingAddress(start_addr, sc)
+              : uw_table.GetUncachedFuncUnwindersContainingAddress(start_addr,
+                                                                   sc);
       if (!func_unwinders_sp)
         continue;
 
diff --git a/lldb/source/Commands/Options.td b/lldb/source/Commands/Options.td
index 777f8c36c4916ca..8831fed38435b54 100644
--- a/lldb/source/Commands/Options.td
+++ b/lldb/source/Commands/Options.td
@@ -965,6 +965,8 @@ let Command = "target modules show unwind" in {
   def target_modules_show_unwind_address : Option<"address", "a">, Group<2>,
     Arg<"AddressOrExpression">, Desc<"Show unwind instructions for a function "
     "or symbol containing an address">;
+  def target_modules_show_unwind_cached : Option<"cached", "c">,
+    Arg<"Boolean">, Desc<"Show cached unwind information">;
 }
 
 let Command = "target modules lookup" in {
diff --git a/lldb/source/Symbol/UnwindTable.cpp 
b/lldb/source/Symbol/UnwindTable.cpp
index da88b0c9c4ea191..61d51192bf3d1e4 100644
--- a/lldb/source/Symbol/UnwindTable.cpp
+++ b/lldb/source/Symbol/UnwindTable.cpp
@@ -86,6 +86,7 @@ void UnwindTable::Initialize() {
 void UnwindTable::ModuleWasUpdated() {
   std::lock_guard<std::mutex> guard(m_mutex);
   m_scanned_all_unwind_sources = false;
+  m_unwinds.clear();
 }
 
 UnwindTable::~UnwindTable() = default;
@@ -118,7 +119,7 @@ UnwindTable::GetAddressRange(const Address &addr, const 
SymbolContext &sc) {
 
 FuncUnwindersSP
 UnwindTable::GetFuncUnwindersContainingAddress(const Address &addr,
-                                               SymbolContext &sc) {
+                                               const SymbolContext &sc) {
   Initialize();
 
   std::lock_guard<std::mutex> guard(m_mutex);
diff --git a/lldb/test/Shell/SymbolFile/Inputs/target-symbols-add-unwind.c 
b/lldb/test/Shell/SymbolFile/Inputs/target-symbols-add-unwind.c
deleted file mode 100644
index 237c8ce181774d9..000000000000000
--- a/lldb/test/Shell/SymbolFile/Inputs/target-symbols-add-unwind.c
+++ /dev/null
@@ -1 +0,0 @@
-int main() {}
diff --git a/lldb/test/Shell/SymbolFile/target-symbols-add-unwind.test 
b/lldb/test/Shell/SymbolFile/target-symbols-add-unwind.test
index 5420213d405e868..249d2ef79a891c4 100644
--- a/lldb/test/Shell/SymbolFile/target-symbols-add-unwind.test
+++ b/lldb/test/Shell/SymbolFile/target-symbols-add-unwind.test
@@ -1,27 +1,107 @@
-# TODO: When it's possible to run "image show-unwind" without a running
-# process, we can remove the unsupported line below, and hard-code an ELF
-# triple in the test.
-# UNSUPPORTED: system-windows, system-darwin
-
-# RUN: cd %T
-# RUN: %clang_host %S/Inputs/target-symbols-add-unwind.c -g \
-# RUN:   -fno-unwind-tables -fno-asynchronous-unwind-tables \
-# RUN:   -o target-symbols-add-unwind.debug
-# RUN: llvm-objcopy --strip-debug target-symbols-add-unwind.debug \
-# RUN:   target-symbols-add-unwind.stripped
-# RUN: %lldb target-symbols-add-unwind.stripped -s %s -o quit | FileCheck %s
-
-process launch --stop-at-entry
-image show-unwind -n main
-# CHECK-LABEL: image show-unwind -n main
+# NB: The minidump core file exists only because "image show-unwind" currently
+# requires a process to exist. If that changes, it can be removed.
+
+# REQUIRES: x86, lld
+
+# RUN: split-file %s %t
+# RUN: yaml2obj %t/a.core.yaml -o %t/a.core
+# RUN: %clang -c --target=x86_64-pc-linux %t/a.s -o %t/a.o
+# RUN: ld.lld --shared %t/a.o -o %t/a.debug --build-id=0xdeadbeef \
+# RUN:   --image-base=0x10000
+# RUN: llvm-objcopy --strip-all %t/a.debug %t/a.stripped
+# RUN: cd %t
+# RUN: %lldb -c %t/a.core \
+# RUN:   -o "settings set interpreter.stop-command-source-on-error false" \
+# RUN:   -s %t/commands -o quit | FileCheck %s
+
+#--- commands
+
+image add a.stripped
+image load --file a.stripped --slide 0
+image list
+# CHECK-LABEL: image list
+# CHECK: [  0] DEADBEEF 0x0000000000010000 a.stripped
+
+## Due to missing symbol information this (incorrectly) prints the unwind
+## information for public_fn1
+image show-unwind -n public_fn1 --cached true
+# CHECK-LABEL: image show-unwind -n public_fn1
+# CHECK-NEXT: UNWIND PLANS for a.stripped`public_fn1 (start addr 0x12000)
 # CHECK-NOT: debug_frame UnwindPlan:
 
-target symbols add -s target-symbols-add-unwind.stripped 
target-symbols-add-unwind.debug
+target symbols add -s a.stripped a.debug
 # CHECK-LABEL: target symbols add
 # CHECK: symbol file {{.*}} has been added to {{.*}}
 
-image show-unwind -n main
-# CHECK-LABEL: image show-unwind -n main
+image show-unwind -n private_fn --cached true
+# CHECK-LABEL: image show-unwind -n private_fn
+# CHECK-NEXT: UNWIND PLANS for a.stripped`private_fn (start addr 0x12010)
 # CHECK: debug_frame UnwindPlan:
 # CHECK-NEXT: This UnwindPlan originally sourced from DWARF CFI
 # CHECK-NEXT: This UnwindPlan is sourced from the compiler: yes.
+# CHECK-NEXT: This UnwindPlan is valid at all instruction locations: no.
+# CHECK-NEXT: This UnwindPlan is for a trap handler function: no.
+# CHECK-NEXT: Address range of this UnwindPlan: [a.stripped.PT_LOAD[1]..text + 
16-0x0000000000000013)
+
+
+#--- a.s
+
+        .text
+        .cfi_sections .debug_frame
+        .globl  public_fn1, public_fn2
+
+        .p2align 12
+public_fn1:
+        .cfi_startproc
+        pushq   %rbp
+        .cfi_def_cfa_offset 16
+        .cfi_offset %rbp, -16
+        popq    %rbp
+        .cfi_def_cfa %rsp, 8
+        retq
+        .cfi_endproc
+
+        .p2align 4
+private_fn:
+        .cfi_startproc
+        pushq   %rbp
+        .cfi_def_cfa_offset 16
+        .cfi_offset %rbp, -16
+        popq    %rbp
+        .cfi_def_cfa %rsp, 8
+        retq
+        .cfi_endproc
+
+        .p2align 4
+public_fn2:
+        .cfi_startproc
+        pushq   %rbp
+        .cfi_def_cfa_offset 16
+        .cfi_offset %rbp, -16
+        popq    %rbp
+        .cfi_def_cfa %rsp, 8
+        retq
+        .cfi_endproc
+
+#--- a.core.yaml
+--- !minidump
+Streams:
+  - Type:            SystemInfo
+    Processor Arch:  AMD64
+    Platform ID:     Linux
+    CPU:
+      Vendor ID:       GenuineIntel
+      Version Info:    0x00000000
+      Feature Info:    0x00000000
+  - Type:            ThreadList
+    Threads:
+      - Thread Id:       0x000074F3
+        Context:         
0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000B001000000000006CAE000000006B7FC05A0000C81D415A0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000A2BF9E5A6B7F0000000000000000000000000000000000008850C14BFD7F00009850C14BFD7F00000100000000000000B04AC14BFD7F0000000000000000000060812D01000000000800000000000000B065E05A6B7F00008004400000000000E050C14BFD7F00000000000000000000000000000000000004400000000000007F03FFFF0000FFFFFFFFFFFF000000000000000000000000801F00006B7F00000400000000000000B84CC14BFD7F0000304D405A6B7F0000C84DC14BFD7F0000C0AA405A6B7F00004F033D0000000000B84DC14BFD7F0000E84DC14BFD7F0000000000000000000000000000000000000070E05A6B7F000078629E5A6B7F0000C81D415A6B7F0000804F9E5A6B7F00000000000001000000E603000001000000E093115A6B7F0000804EC14BFD7F0000584EC14BFD7F000099ADC05A6B7F00000100000000000000AAAAD77D0000000002000000000000000800000000000000B065E05A6B7F0000E6B7C05A6B7F0000010000006B7F0000884DC14BFD7F0000106F7C5A6B7F0000984EC14BFD7F0000488B7C5A6B7F0000C4A71CB90000000001000000000000000800000000000000B065E05A6B7F000048B6C05A6B7F0000702AE25A6B7F0000D84DC14BFD7F000030489E5A6B7F0000E84EC14BFD7F0000E05E9E5A6B7F00000991F0460000000001000000000000000800000000000000B065E05A6B7F000048B6C05A6B7F00000100000000000000284EC14BFD7F00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000
+        Stack:
+          Start of Memory Range: 0x00007FFD4BC15080
+          Content:         30044000000000000000000000000000
+  - Type:            MemoryList
+    Memory Ranges:
+      - Start of Memory Range: 0x00007FFD4BC15080
+        Content:         30044000000000000000000000000000
+...

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to