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
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
