labath marked an inline comment as done.
labath added inline comments.
================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:1044
for (const llvm::DWARFDebugLine::Sequence &seq : line_table->Sequences) {
+ if (!list.ContainsFileAddressRange(seq.LowPC, seq.HighPC - seq.LowPC))
+ continue;
----------------
clayborg wrote:
> dblaikie wrote:
> > Could you specifically look for/propagate tombstones here, to reduce the
> > risk of large functions overlapping with the valid address range, even when
> > they're tombstoned to zero? (because zero+large function size could still
> > end up overlapping with valid code)
> >
> > To do that well, I guess it'd have to be implemented at a lower-layer,
> > inside the line table state machine - essentially dropping all lines
> > derived from a "set address" operation that specifies a tombstone.
> Just checking if the section lists contains an address doesn't help weed out
> addresses that were tombstoned to zero since our PT_LOAD[0] will almost
> always contain zero for shared libraries. It might be nice to make a list of
> addresses that come from sections with read + execute permissions and store
> that in SymbolFileDWARF one time at startup. Then these searches will be
> faster as we are looking in less ranges, and most likely will not contain
> address zero. This code will catch the -1 and -2 tombstones, but most linkers
> I have run into use zero and the tombstone.
>
> If our algorithm only checks sections with no subsections and then makes a
> list of file addresses for and section ranges for those, we should have a
> great list. The entire PT_LOAD[0] will usually be mapped read + execute, so
> we can't just check top level sections for ELF. Mach-o also has this issue
> __TEXT in mac is also mapped read + execute and usually contains zero for
> shared libraries, but since the sections must come after the mach-o header,
> the sections within the __TEXT segment have correct permissions and would
> work, just like they would for ELF.
You're right -- this would not handle shared libraries with base zero.
I am slightly uneasy about requiring executable permissions for all line
tables. While it does not seem terribly useful to have line tables for
non-executable code, if someone does have a line table for it for whatever
reason (maybe he wants to make it executable at runtime?) it would be a shame
not to display it. Also the choice of using section rather than segment
permissions feels slightly arbitrary (although I could make a case for it), as
it's the segment permissions which will actually define the runtime memory
permissions.
Since this is really about filtering out (near) zero addresses, how about we
make that explicit? Find the lowest executable (section) address and reject
anything below that? Additionally, I'd also reject all addresses which are
completely outside of the module range, as those not going to get used for
anything, and they are generating bogus line-table dumps.
What do you think?
David: The -1 tombstones are already sort of handled in llvm (and in lldb since
D83957). They are "handled" in the sense that the all sequences with and end PC
lower than the start PC are rejected (and line sequences starting with
(unsigned)-1 will definitely wrap). This is trying to do something about the
zero tombstones.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84402/new/
https://reviews.llvm.org/D84402
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
