RamNalamothu added a comment. Thank you.
In D50304#3608740 <https://reviews.llvm.org/D50304#3608740>, @jingham wrote: > In D50304#3592849 <https://reviews.llvm.org/D50304#3592849>, @RamNalamothu > wrote: > >> In D50304#3586710 <https://reviews.llvm.org/D50304#3586710>, @jingham wrote: >> >>> For some reason I'm not getting mail notifications for review changes, >>> sorry about that. >>> >>> This is certainly better than the original implementation. Among other >>> things, if we find an exact match, we really shouldn't be doing any more >>> inexact matches, so after the first exact hit it should have switched exact >>> to true. >>> >>> But if you had line tables laid out like (this is in increasing order of >>> address: >>> >>> 10 >>> 20 >>> 30 >>> 10 >>> 16 >>> >>> and you did "thread until 15", this would find the inexact match at 20, >>> switch to an exact match for line 20 and find no other matches. But the >>> gap between 10 & 16 in the line table is maybe an even more plausible place >>> to put the line 15 until breakpoint, so maybe we did want to throw a >>> breakpoint there as well? >> >> @jingham >> Nope, with the current patch, we would find the inexact match at 16. > > Ah, my bad. FindLineEntry does end up in FindLineEntryByFileIndexImpl which > does the closest match. > >>> Regular breakpoint setting has to move inexact breakpoints in much the same >>> way. The code in the BreakpointResolverFileLine::SearchCallback ends up >>> calling CompileUnit::ResolveSymbolContext to get the "best" inexact match. >>> Maybe it would be better to not do this by hand here in the Until command, >>> but reuse the code that we use to move break points more generally? >> >> Be it CompileUnit::ResolveSymbolContext or CompileUnit::FindLineEntry, we >> end up calling LineTable::FindLineEntryIndexByFileIndex which finds "Exact >> match always wins. Otherwise try to find the closest line > the desired >> line." when we pass `exact = false` to it. >> Given that and we anyway have to extract address list, from what we get out >> of CompileUnit::ResolveSymbolContext, for the thread until thread plan to >> work with, probably the additional overhead to use >> CompileUnit::ResolveSymbolContext here does not make sense. Or am I missing >> something? > > Finding these addresses happens once in response to a user command, so I > highly doubt that the overhead matters one way or the other. Both end up > doing the closest match (I misremembered where that was done) which is the > main thing. ResolveSymbolContext does more work to handle inlined files, but > there's no way to do "thread until" from a main CU into inlined source so > that doesn't matter. Make sense. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D50304/new/ https://reviews.llvm.org/D50304 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
