labath added a comment. Looks pretty good. I might have split the record parsing functions into a separate patch, but this is not bad either.
Just one comment about error handling. ================ Comment at: lldb/source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp:313-314 + BlockSP block_sp = std::make_shared<Block>(It.GetBookmark().offset); + FileSpec callsite_file = (*m_files)[record->CallSiteFileNum]; + llvm::StringRef name = (*m_inline_origins)[record->OriginNum]; + Declaration callsite(callsite_file, record->CallSiteLineNum); ---------------- In lldb, we treat (or at least try to) debug info [[ https://lldb.llvm.org/resources/contributing.html#error-handling-and-use-of-assertions-in-lldb | as "user input"]], meaning invalid/inconsistent debug info should not crash the debugger. While the situation is not as critical for breakpad as it is for dwarf (due to it not having as many producers), I think the principle remains. It doesn't really matter much how you handle invalid records (like, it'd be fine if you just throw away all inline info for the affected function), but it'd be nice if it does not crash lldb. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113330/new/ https://reviews.llvm.org/D113330 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
