sammccall created this revision.
sammccall added reviewers: ioeric, ilya-biryukov.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay.
I think this was just copied from somewhere with the belief that it actually
did some crash handling.
Of course the question arises: *should* we set one? I don't think so:
- clangd used to crash a lot, now it's pretty stable, because we found and
fixed the crashes. I think the long-term effects of crashing hard are good.
- the implementation can't do any magic, it just uses longjmp to return without
running any destructors by default. This is unsafe in general (e.g. mutexes
won't unlock) and will certainly end up leaking memory. Whatever UB caused the
crash may still stomp all over global state, etc.
I think there's an argument for isolating the background indexer (autoindex)
because it's not directly under the user's control, the crash surface is larger,
and it doesn't particularly need to interact with the rest of clangd.
But there, fork() and communicate through the FS is safer.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53034
Files:
clangd/ClangdUnit.cpp
Index: clangd/ClangdUnit.cpp
===================================================================
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -31,7 +31,6 @@
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/SmallVector.h"
-#include "llvm/Support/CrashRecoveryContext.h"
#include "llvm/Support/raw_ostream.h"
#include <algorithm>
@@ -141,10 +140,6 @@
if (!Clang)
return llvm::None;
- // Recover resources if we crash before exiting this method.
- llvm::CrashRecoveryContextCleanupRegistrar<CompilerInstance> CICleanup(
- Clang.get());
-
auto Action = llvm::make_unique<ClangdFrontendAction>();
const FrontendInputFile &MainInput = Clang->getFrontendOpts().Inputs[0];
if (!Action->BeginSourceFile(*Clang, MainInput)) {
Index: clangd/ClangdUnit.cpp
===================================================================
--- clangd/ClangdUnit.cpp
+++ clangd/ClangdUnit.cpp
@@ -31,7 +31,6 @@
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/SmallVector.h"
-#include "llvm/Support/CrashRecoveryContext.h"
#include "llvm/Support/raw_ostream.h"
#include <algorithm>
@@ -141,10 +140,6 @@
if (!Clang)
return llvm::None;
- // Recover resources if we crash before exiting this method.
- llvm::CrashRecoveryContextCleanupRegistrar<CompilerInstance> CICleanup(
- Clang.get());
-
auto Action = llvm::make_unique<ClangdFrontendAction>();
const FrontendInputFile &MainInput = Clang->getFrontendOpts().Inputs[0];
if (!Action->BeginSourceFile(*Clang, MainInput)) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
