zequanwu 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);
   }
 
----------------
labath wrote:
> 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.
These two commands `image lookup -n ...` and `image show-unwind -n ..` also 
failed to give any information if the name is from FUNC records.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113163/new/

https://reviews.llvm.org/D113163

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to