[Lldb-commits] [lldb] Fix call site breakpoint patch (PR #114158)
via lldb-commits
lldb-commits at lists.llvm.org
Mon Nov 4 11:00:01 PST 2024
jimingham wrote:
There were two issues here, one very minor, and then one that mattered... This was a little thinko, I forgot that we build into the same SymbolContextList over all the CU iterations, so I needed to check "did I add to the SC list" not "is there anything in the SC list" to see if looking for a call site found anything:
diff --git a/lldb/source/Symbol/CompileUnit.cpp b/lldb/source/Symbol/CompileUnit.cpp
index 73389b2e8479..d7df6ee1f221 100644
--- a/lldb/source/Symbol/CompileUnit.cpp
+++ b/lldb/source/Symbol/CompileUnit.cpp
@@ -326,16 +326,18 @@ void CompileUnit::ResolveSymbolContext(
// the function containing the PC of the line table match. That way we can
// limit the call site search to that function.
// We will miss functions that ONLY exist as a call site entry.
-
+
if (line_entry.IsValid() &&
- (line_entry.line != line || line_entry.column != column_num) &&
- resolve_scope & eSymbolContextLineEntry && check_inlines) {
+ (line_entry.line != line || (column_num != 0 && line_entry.column != column_num))
+ && (resolve_scope & eSymbolContextLineEntry) && check_inlines) {
// We don't move lines over function boundaries, so the address in the
// line entry will be the in function that contained the line that might
// be a CallSite, and we can just iterate over that function to find any
// inline records, and dig up their call sites.
Address start_addr = line_entry.range.GetBaseAddress();
Function *function = start_addr.CalculateSymbolContextFunction();
+ // Record the size of the list to see if we added to it:
+ size_t old_sc_list_size = sc_list.GetSize();
Declaration sought_decl(file_spec, line, column_num);
// We use this recursive function to descend the block structure looking
@@ -417,7 +419,7 @@ void CompileUnit::ResolveSymbolContext(
// FIXME: Should I also do this for "call site line exists between the
// given line number and the later line we found in the line table"? That's
// a closer approximation to our general sliding algorithm.
- if (sc_list.GetSize())
+ if (sc_list.GetSize() > old_sc_list_size)
return;
}
But I'm kind of surprised that nothing in our test suite was sensitive to this. I'll work up this example into a test, and then submit this.
Thanks for the example!
Jim
> On Nov 4, 2024, at 9:36 AM, Jim Ingham ***@***.***> wrote:
>
> Give me a bit to look at this. The intention of this patch was just to add more locations, it shouldn't be reducing the number of breakpoints. There's likely some simple goof here.
>
> Jim
>
>
>> On Nov 4, 2024, at 2:44 AM, Pavel Labath ***@***.***> wrote:
>>
>>
>> So it sounds like the problem is that lldb no longer looks for all compile units with the given name when setting a breakpoint. Changing that doesn't seem like it was the intention of this patch. Jim, is there an easy fix for this or should we revert the patch for now?
>>
>> —
>> Reply to this email directly, view it on GitHub <https://github.com/llvm/llvm-project/pull/114158#issuecomment-2454373743>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADUPVW3EAWSSOK7BI5AW6ZDZ65FYTAVCNFSM6AAAAABQ27K63WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINJUGM3TGNZUGM>.
>> You are receiving this because you were mentioned.
>>
>
https://github.com/llvm/llvm-project/pull/114158
More information about the lldb-commits
mailing list