labath added inline comments.
================ Comment at: lldb/source/Plugins/SymbolFile/JSON/SymbolFileJSON.cpp:112 + symtab.AddSymbol(Symbol( + /*symID*/ 0, Mangled(symbol.name), eSymbolTypeCode, + /*is_global*/ true, /*is_debug*/ false, ---------------- clayborg wrote: > JDevlieghere wrote: > > mib wrote: > > > Should we increment the `symID` here ? > > The IDs are arbitrary, and if we start at zero, we'll have conflicts with > > the ones already in the symbol table (e.g. the lldb_unnamed_symbols for > > stripped binaries). We could check the size of the symtab and continue > > counting from there. Or we can use 0 like we did here to indicate that > > these values are "special". I went with the latter approach mostly because > > that's what SymbolFileBreakpad does too. > regardless of where the code goes that converts ObjectFileJSON::Symbol to > lldb_private::Symbol, I would vote that we include enough information in > ObjectFileJSON::Symbol so that we don't have to make up symbols IDs, or set > all symbol IDs to zero. LLDB relies on having unique symbol IDs in each > lldb_private::Symbol instance. Speaking of breakpad, have you considered using SymbolFileBreakpad directly? I think it serves pretty much the same purpose, and it also supports other, more advanced functionality (e.g. line tables and unwind info). It sounds like you don't need that now, but if you ever did, you wouldn't need to reinvent that logic... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145180/new/ https://reviews.llvm.org/D145180 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
