labath added inline comments.
================
Comment at:
lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp:388-392
for (llvm::StringRef line : lines(Record::Func)) {
if (auto record = FuncRecord::parse(line))
add_symbol(record->Address, record->Size, record->Name);
}
----------------
zequanwu wrote:
> labath wrote:
> > Can you check if we can remove this now?
> >
> > I originally thought that we can remove this entire function, but I forgot
> > about PUBLIC records -- we don't have functions or compile units for those,
> > so they will have to stay.
> Removing it causes FUNC records not showing up in symtab when doing `image
> dump symtab ...` and fails some tests.
> The Breakpad doc says
> (https://chromium.googlesource.com/breakpad/breakpad/+/HEAD/docs/symbol_files.md#records-4):
>
> > If a given address is covered by both a PUBLIC record and a FUNC record,
> > the processor uses the FUNC data.
>
It's expected that the tests verifying symtab contents need updating after you
remove some things from it. It's also possible some other tests will need minor
tweaks (like the one in `line-table.test:54`) because of small differences in
output format.
Whether this is a reasonable change cannot be judged by failing tests alone.
You also need to evaluate the overall quality of the debugger output. That will
have to be a judgement call, but I'm hoping it won't be a hard one. For
example, the change in line-table.test was definitely for the better.
>> If a given address is covered by both a PUBLIC record and a FUNC record, the
>> processor uses the FUNC data.
And if an address is covered both by a Symtab Symbol, and an SymbolFile
Function, lldb will preferentially (in backtraces, for instance) display
information from the Function, so I think (hope) that this is going to work
exactly as desired.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D113163/new/
https://reviews.llvm.org/D113163
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
