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