Closed Strum355 closed 3 years ago
@beyang @efritz could one (or both) of ye try this out on OSX please. Heres a sample snippet for causing a sigsegv (from beyangs commit)
Put it into https://github.com/sourcegraph/lsif-clang/blob/llvmorg-10.0.0-lsif-clang/src/indexer/IndexerMain.cpp
void baz() {
int *foo = (int*)-1; // make a bad pointer
printf("%d\n", *foo); // causes segfault
}
int main(int argc, const char **argv) {
//sys::PrintStackTraceOnErrorSignal(argv[0]);
baz();
// ...
}
Make sure binutils is installed and let me know if theres a good backtrace with code snippet : )
Command to build ~= cmake -B build && make -C build -j<cores>
if ye dont have it on-hand
With that latest commit pls ^ forgot to push
Further toying around shows that this isnt an absolute success. My suspicions are it doesnt apply to any of the worker threads that are spun up while indexing. The original pastebin was in the serial emitting of LSIF data on the main thread. Probably still worth merging this and seeing how it can be expanded upon
@Strum355 Curious before diving in: have you looked into writing our own ToolExecutor? Currently using the AllTUsToolExecutor but the implementation is rly smol and it seems like there'd be an obvious insertion point for what we want (Sourcegraph snippet widget when), but idk how backtraces/multiprocessing works in cpp any more than u do
@gbrik ah that's an interesting thought, hadnt clicked that the ToolExecutors were pluggable/abstract classes. Im in the same knowledge boat as you here, I didnt uncover anything about backtrace-cpp with multithreading, was under the impression that it should just work:tm: but apparently not? Im sure theres probably something obvious (or less so) that Im missing here.
If its an issue with threads, would subprocessing work I wonder?
@Strum355 I have binutils installed but I don't get a pretty stack trace:
Stack trace (most recent call last):
#3 Object "[0xffffffffffffffff]", at 0xffffffffffffffff, in
#2 Object "bin/lsif-clang", at 0x5610aa26ebad, in
#1 Object "/lib/x86_64-linux-gnu/libc.so.6", at 0x7fa96b7370b2, in __libc_start_main
#0 Object "bin/lsif-clang", at 0x5610aa272473, in
Segmentation fault (Address not mapped to object [0xffffffffffffffff])
zsh: segmentation fault (core dumped) bin/lsif-clang
This is just doing the baz()
with bad pointer from above.
Just to confirm, do you have binutils-dev specifically? Im not sure if just binutils will do, as theyre two separate packages and the latter doesnt install the former afaik
Recompiled with dev and it works \o/
Do you have a reliable way to repro segfault in the worker threads?
On Wed, Jan 13, 2021 at 12:08 PM Noah Santschi-Cooney < notifications@github.com> wrote:
Just to confirm, do you have binutils-dev specifically? Im not sure if just binutils will do, as theyre two separate packages and the latter doesnt install the former afaik
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/sourcegraph/lsif-clang/pull/32#issuecomment-759710014, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA2A6AZEFVYXRNPKB7BUE3TSZX4TLANCNFSM4T3SSB2A .
--
Garo Brik (they/them)
Code intelligence engineer
Simplest way: src/index/SymbolCollector.cpp#602 add the baz function, and call it inside *SymbolCollector::addDeclaration immediately following it @gbrik
If I put the following:
const Symbol *SymbolCollector::addDeclaration(const NamedDecl &ND, SymbolID ID,
bool IsMainFileOnly) {
backward::SignalHandling sh;
baz();
I think I get a pretty stack trace (abbrev'd):
...
343: // If OriginalDecl is preferred, replace the existing canonical
344: // declaration (e.g. a class forward declaration). There should be at most
#0 | Source "/home/arrow/sourcegraph/lsif-clang/src/index/SymbolCollector.cpp", line 611, in clang::clangd::SymbolCollector::addDeclaration(clang::NamedDecl const&, clang::clangd::SymbolID, bool)
| 609: bool IsMainFileOnly) {
| 610: backward::SignalHandling sh;
| > 611: baz();
| 612: auto &Ctx = ND.getASTContext();
| 613: auto &SM = Ctx.getSourceManager();
| Source "/home/arrow/sourcegraph/lsif-clang/src/index/SymbolCollector.cpp", line 605, in clang::clangd::baz()
| 603: void baz() {
| 604: int *foo = (int *)-1; // make a bad pointer
| > 605: printf("%d\n", *foo); // causes segfault
| 606: }
Source "/usr/include/x86_64-linux-gnu/bits/stdio2.h", line 107, in clang::clangd::SymbolCollector::addDeclaration(clang::NamedDecl const&, clang::clangd::SymbolID, bool) [0x5643050dc05e]
104: __fortify_function int
105: printf (const char *__restrict __fmt, ...)
106: {
> 107: return __printf_chk (__USE_FORTIFY_LEVEL - 1, __fmt, __va_arg_pack ());
108: }
109: # elif !defined __cplusplus
110: # define printf(...) \
Segmentation fault (Address not mapped to object [0xffffffffffffffff])
#1 Source "#1 Source "/home/arrow/sourcegraph/lsif-clang/src/index/SymbolCollector.cpp", line 341, in clang::clangd::SymbolCollector::handleDeclOccurrence(clang::Decl const*, unsigned int, llvm::ArrayRef<clang::index::SymbolRelation>, clang::SourceLocation, clang::index::IndexDataConsumer::ASTNodeInfo) [/home/arrow/sourcegraph/lsif-clang/src/index/SymbolCollector.cpp", line 341, in clang::clangd::SymbolCollector::handleDeclOccurrence(clang::Decl const*, unsigned int, llvm::ArrayRef<clang::index::SymbolRelation>, clang::SourceLocation, clang::index::IndexDataConsumer::ASTNodeInfo) [0x5643050dd598]
0x5643050dd598]
339: const Symbol *BasicSymbol = Symbols.find(*ID);
340: if (!BasicSymbol) // Regardless of role, ND is the canonical declaration.
> 341: BasicSymbol = addDeclaration(*ND, std::move(*ID), IsMainFileOnly);
342: else if (isPreferredDeclaration(*OriginalDecl, Roles))
343: // If OriginalDecl is preferred, replace the existing canonical
344: // declaration (e.g. a class forward declaration). There should be at most
339: const Symbol *BasicSymbol = Symbols.find(*ID);
340: if (!BasicSymbol) // Regardless of role, ND is the canonical declaration.
> 341: BasicSymbol = addDeclaration(*ND, std::move(*ID), IsMainFileOnly);
342: else if (isPreferredDeclaration(*OriginalDecl, Roles))
343: // If OriginalDecl is preferred, replace the existing canonical
344: // declaration (e.g. a class forward declaration). There should be at most
#1 Source "/home/arrow/sourcegraph/lsif-clang/src/index/SymbolCollector.cpp", line 341, in clang::cla
ngd::SymbolCollector::handleDeclOccurrence(clang::Decl const*, unsigned int, llvm::ArrayRef<clang::index::SymbolRelation>, clang::SourceLocation, clang::index::IndexDataConsumer::ASTNodeInfo) [0x5643050dd598]
339: const Symbol *BasicSymbol = Symbols.find(*ID);
340: if (!BasicSymbol) // Regardless of role, ND is the canonical declaration.
> 341: BasicSymbol = addDeclaration(*ND, std::move(*ID), IsMainFileOnly);
342: else if (isPreferredDeclaration(*OriginalDecl, Roles))
343: // If OriginalDecl is preferred, replace the existing canonical
344: // declaration (e.g. a class forward declaration). There should be at most
#1 Source "/home/arrow/sourcegraph/lsif-clang/src/index/SymbolCollector.cpp", line 341, in clang::clangd::SymbolCollector::handleDeclOccurrence(clang::Decl const*, unsigned int, llvm::ArrayRef<clang::index::SymbolRelation>, clang::SourceLocation, clang::index::IndexDataConsumer::ASTNodeInfo) [0x5643050dd598]
zsh: segmentation fault (core dumped) bin/lsif-clang compile_commands.json
so maybe try adding that line to a copypasta of AllTUs executor when it kicks off the threads?
Efforts to get stacktraces from within the executor workers was unsuccessful, so I explored how to utilize coredumps for debugging the times when backwards-cpp fails. I wrote a doc at https://github.com/sourcegraph/lsif-clang/pull/32/files#diff-8739bb7905639474aa523485b68fd8d6ced0fe5aec3707e8ba738896e598c12e to guide users on how to enable coredumps that we can use
@olafurpg to get this into the hands of a customer to further debug an issue, Im moving the segfault doc into a separate PR to unblock this PR
https://github.com/bombela/backward-cpp
Example: https://pastebin.ubuntu.com/p/p957KYbgKt/