Skip to content

Conversation

kuilpd
Copy link
Contributor

@kuilpd kuilpd commented Jul 10, 2025

Check symbol_context.comp_unit before accessing it to avoid nullptr dereferencing.

@kuilpd kuilpd requested a review from labath July 10, 2025 13:01
@kuilpd kuilpd added the lldb label Jul 10, 2025
@kuilpd kuilpd requested a review from JDevlieghere as a code owner July 10, 2025 13:02
@kuilpd
Copy link
Contributor Author

kuilpd commented Jul 10, 2025

lldb-shell-subprocess tests were failing because they add a setting settings set symbols.enable-external-lookup false, which disables resolving variables for compile units, but still marks them as resolved, so stack_frame->GetSymbolContext(lldb::eSymbolContextCompUnit) just doesn't do anything. I'm not sure if it's by design or not, but checking that comp_unit before accessing it avoids this issue, I guess I should have done that to begin with.

The other thing I noticed is that watchpoint command doesn't expect frame->GetValueForVariableExpressionPath to search for global variables (CommandObjectWatchpoint.cpp:842-847), so it has a second step after that using target->GetImages().FindGlobalVariables(). But after this fix, DIL finds the global anyway using the same method, so that seconds step is unused now. Should we address this somehow?

@llvmbot
Copy link
Member

llvmbot commented Jul 10, 2025

@llvm/pr-subscribers-lldb

Author: Ilia Kuklin (kuilpd)

Changes

Check symbol_context.comp_unit before accessing it to avoid nullptr dereferencing.


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

1 Files Affected:

  • (modified) lldb/source/ValueObject/DILEval.cpp (+3-2)
diff --git a/lldb/source/ValueObject/DILEval.cpp b/lldb/source/ValueObject/DILEval.cpp
index 8ca9b4215985d..fd3f9f8724608 100644
--- a/lldb/source/ValueObject/DILEval.cpp
+++ b/lldb/source/ValueObject/DILEval.cpp
@@ -50,8 +50,9 @@ lldb::ValueObjectSP LookupGlobalIdentifier(
   // Get a global variables list without the locals from the current frame
   SymbolContext symbol_context =
       stack_frame->GetSymbolContext(lldb::eSymbolContextCompUnit);
-  lldb::VariableListSP variable_list =
-      symbol_context.comp_unit->GetVariableList(true);
+  lldb::VariableListSP variable_list;
+  if (symbol_context.comp_unit)
+    variable_list = symbol_context.comp_unit->GetVariableList(true);
 
   name_ref.consume_front("::");
   lldb::ValueObjectSP value_sp;

@labath
Copy link
Collaborator

labath commented Jul 10, 2025

It's pretty much an accident we hit this in the test suite. Could you create a dedicated test for this. I guess the scenario is "running 'frame var' in a frame without debug info" (?)

@kuilpd
Copy link
Contributor Author

kuilpd commented Jul 10, 2025

It's pretty much an accident we hit this in the test suite. Could you create a dedicated test for this. I guess the scenario is "running 'frame var' in a frame without debug info" (?)

No debug information at all? Or test the same settings set symbols.enable-external-lookup false?

@labath
Copy link
Collaborator

labath commented Jul 10, 2025

No debug info in the currently selected frame. Or at least, I think so, based on looking at the code you're fixing. You can verify that by checking whether the current code crashes in that scenario. I'm right, it should be sufficient to build one file with -g0.

symbols.enable-external-lookup is sort of related to that, in that it can cause some frame to not have debug info (i.e., an associated compile unit), but it's not directly related as there are other things that can cause a frame to not have debug info. And those things are easier to reproduce in a test.

@kuilpd
Copy link
Contributor Author

kuilpd commented Jul 10, 2025

Added a test without debug info and you're right, it fails without this fix.

@labath
Copy link
Collaborator

labath commented Jul 10, 2025

@cmtice

Copy link
Collaborator

@labath labath left a comment

Choose a reason for hiding this comment

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

Cool, thanks.

I'm not totally sure about our ability to break on symbols with no debug info on all platforms (particularly, windows). If this fails somewhere, you can use work around this by putting a breakpoint in a file with debug info, then doing an up to a frame/file with no debug info, and evaluating the expressions there. You can use this syntax to build a single file with no debug info:

@kuilpd kuilpd merged commit 09fb20e into llvm:main Jul 10, 2025
9 checks passed
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.

3 participants