Skip to content

Conversation

@Nerixyz
Copy link
Contributor

@Nerixyz Nerixyz commented Nov 26, 2025

When a PDB is loaded through target symbols add <pdb-path>, its m_objectfile_sp is an ObjectFilePDB instead of ObjectFilePECOFF (the debugged module). In both the native and DIA plugin, some paths assumed that m_objectfile_sp is the debugged module. With this PR, they go through m_objfile_sp->GetModule()->GetObjectFile().

For the DIA plugin, this lead to an assertion failure (#169628 (comment)) and for both plugins, it meant that the symbol table wasn't loaded.

@llvmbot
Copy link
Member

llvmbot commented Nov 26, 2025

@llvm/pr-subscribers-lldb

Author: nerix (Nerixyz)

Changes

When a PDB is loaded through target symbols add &lt;pdb-path&gt;, its m_objectfile_sp is an ObjectFilePDB instead of ObjectFilePECOFF (the debugged module). In both the native and DIA plugin, some paths assumed that m_objectfile_sp is the debugged module. With this PR, they go through m_objfile_sp-&gt;GetModule()-&gt;GetObjectFile().

For the DIA plugin, this lead to an assertion failure (#169628 (comment)) and for both plugins, it meant that the symbol table wasn't loaded.


Full diff: https://github.com/llvm/llvm-project/pull/169728.diff

3 Files Affected:

  • (modified) lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp (+2-1)
  • (modified) lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp (+6-3)
  • (added) lldb/test/Shell/SymbolFile/PDB/add-symbols.cpp (+39)
diff --git a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
index aaec1600dacff..40e783f9bad38 100644
--- a/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
+++ b/lldb/source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp
@@ -1126,7 +1126,8 @@ lldb::LanguageType SymbolFileNativePDB::ParseLanguage(CompileUnit &comp_unit) {
 }
 
 void SymbolFileNativePDB::AddSymbols(Symtab &symtab) {
-  auto *section_list = m_objfile_sp->GetSectionList();
+  auto *section_list =
+      m_objfile_sp->GetModule()->GetObjectFile()->GetSectionList();
   if (!section_list)
     return;
 
diff --git a/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp b/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
index 0ccb1804bb13a..97c995fc9b22a 100644
--- a/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
+++ b/lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
@@ -287,8 +287,10 @@ uint32_t SymbolFilePDB::CalculateAbilities() {
 }
 
 void SymbolFilePDB::InitializeObject() {
-  lldb::addr_t obj_load_address =
-      m_objfile_sp->GetBaseAddress().GetFileAddress();
+  lldb::addr_t obj_load_address = m_objfile_sp->GetModule()
+                                      ->GetObjectFile()
+                                      ->GetBaseAddress()
+                                      .GetFileAddress();
   lldbassert(obj_load_address && obj_load_address != LLDB_INVALID_ADDRESS);
   m_session_up->setLoadAddress(obj_load_address);
   if (!m_global_scope_up)
@@ -1479,7 +1481,8 @@ void SymbolFilePDB::AddSymbols(lldb_private::Symtab &symtab) {
   if (!results)
     return;
 
-  auto section_list = m_objfile_sp->GetSectionList();
+  auto section_list =
+      m_objfile_sp->GetModule()->GetObjectFile()->GetSectionList();
   if (!section_list)
     return;
 
diff --git a/lldb/test/Shell/SymbolFile/PDB/add-symbols.cpp b/lldb/test/Shell/SymbolFile/PDB/add-symbols.cpp
new file mode 100644
index 0000000000000..ef7690b1720a6
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/PDB/add-symbols.cpp
@@ -0,0 +1,39 @@
+// REQUIRES: lld, target-windows
+
+// Test that `target symbols add <pdb>` works.
+// RUN: %build --compiler=clang-cl --nodefaultlib --output=%t.exe %s
+// RUN: mv %t.pdb %t-renamed.pdb
+
+// RUN: env LLDB_USE_NATIVE_PDB_READER=0 %lldb \
+// RUN:   -o "b main" \
+// RUN:   -o "target symbols add %t-renamed.pdb" \
+// RUN:   -o r \
+// RUN:   -o "target variable a" \
+// RUN:   -o "target modules dump symtab" \
+// RUN:   -b %t.exe | FileCheck %s
+
+// RUN: env LLDB_USE_NATIVE_PDB_READER=1 %lldb \
+// RUN:   -o "b main" \
+// RUN:   -o "target symbols add %t-renamed.pdb" \
+// RUN:   -o r \
+// RUN:   -o "target variable a" \
+// RUN:   -o "target modules dump symtab" \
+// RUN:   -b %t.exe | FileCheck %s
+
+// CHECK: target create
+// CHECK: (lldb) b main
+// CHECK-NEXT: Breakpoint 1: no locations (pending).
+// CHECK: (lldb) target symbols add
+// CHECK: 1 location added to breakpoint 1
+// CHECK: (lldb) r
+// CHECK: * thread #1, stop reason = breakpoint 1.1
+// CHECK: (lldb) target variable a
+// CHECK-NEXT: (A) a = (x = 47)
+// CHECK: (lldb) target modules dump symtab
+// CHECK: [{{.*}} main
+
+struct A {
+  int x = 47;
+};
+A a;
+int main() {}

Copy link
Collaborator

@DavidSpickett DavidSpickett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Some questions about the tools but there are probably good reasons for your choices.

@DavidSpickett
Copy link
Collaborator

I did not realise this test could run anywhere, knowing that resolves all my comments, thanks.

@Nerixyz Nerixyz merged commit cc72171 into llvm:main Nov 28, 2025
12 checks passed
@slydiman
Copy link
Contributor

lldb-x86_64-win is broken after this patch.

FAIL: lldb-shell::add-symbols.cpp
[view all 86 lines](https://lab.llvm.org/buildbot/#/builders/211/builds/4162/steps/10/logs/FAIL__lldb-shell__add-symbols_cpp) 
# | Input file: <stdin>
# | Check file: C:\buildbot\as-builder-10\lldb-x86-64\llvm-project\lldb\test\Shell\SymbolFile\PDB\add-symbols.cpp
# | 
# | -dump-input=help explains the following input dump.
# | 
# | Input was:
# | <<<<<<
# |             .
# |             .
# |             .
# |             7: Breakpoint 1: no locations (pending). 
# |             8: WARNING: Unable to resolve breakpoint to any actual locations. 
# |             9: (lldb) target symbols add C:\buildbot\as-builder-10\lldb-x86-64\build\tools\lldb\test\Shell\SymbolFile\PDB\Output\add-symbols.cpp.tmp-renamed.pdb 
# |            10: symbol file 'C:\buildbot\as-builder-10\lldb-x86-64\build\tools\lldb\test\Shell\SymbolFile\PDB\Output\add-symbols.cpp.tmp-renamed.pdb' has been added to 'C:\buildbot\as-builder-10\lldb-x86-64\build\tools\lldb\test\Shell\SymbolFile\PDB\Output\add-symbols.cpp.tmp.exe' 
# |            11: (lldb) r 
# |            12: 1 location added to breakpoint 1 
# | check:28'0                                     X error: no match found
# |            13: (lldb) Process 15812 launched: 'C:\buildbot\as-builder-10\lldb-x86-64\build\tools\lldb\test\Shell\SymbolFile\PDB\Output\add-symbols.cpp.tmp.exe' (x86_64) 
# | check:28'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
# | check:28'1     ?                                                                                                                                                          possible intended match
# |            14: Process 15812 stopped 
# | check:28'0     ~~~~~~~~~~~~~~~~~~~~~~
# |            15: * thread #1, stop reason = breakpoint 1.1 
# | check:28'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
# |            16:  frame #0: 0x00007ff7fa381000 add-symbols.cpp.tmp.exe`main at add-symbols.cpp:39 
# | check:28'0     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
# |            17:  36 int x = 47; 
# | check:28'0     ~~~~~~~~~~~~~~~~
# |            18:  37 }; 
# | check:28'0     ~~~~~~~
# |             .
# |             .
# |             .
# | >>>>>>
# `-----------------------------
# error: command failed with exit status: 1

Nerixyz added a commit that referenced this pull request Nov 28, 2025
The test was flaky, because it assumed that the breakpoint was always
resolved before `r` was executed
(#169728 (comment)).
This PR removes the check for this order. It still checks that the
breakpoint is resolved before it is hit.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants