[libclang] Allow crash recovery with LIBCLANG_NOTHREADS

[nik]

Summary

Enabled crash recovery for some libclang operations on a calling thread even
when LIBCLANG_NOTHREAD is specified.

Previously it would only run under crash recovery if LIBCLANG_NOTHREAD is not
set. Moved handling of LIBCLANG_NOTHREAD env variable into RunSafely from its
call sites.

[yvvan]

This actually fixes the ability to run safely without threads. This happens because by default this solution leads to try/catch block instead of the direct function call which is implemented in deleted if blocks.

[nik]

Ping

[nik]

Ping 2

[yvvan]

ping 3

[klimek]

Adding Argyrios, who might have insight on how this is used.
I think this had the wrong list of reviewers so far :(

[nik]

Ping 4

[ilya-biryukov]

I think your change makes sense, but maybe asking for a better description.

It appears you:

• Enabled crash recovery for some libclang operations on a calling thread even when LIBCLANG_NOTHREAD is specified. Previously it would only run under crash recovery if LIBCLANG_NOTHREAD is not set.
• Moved handling of LIBCLANG_NOTHREAD env variable into RunSafely from its call sites.

That said, I am not familiar with the code you're changing, so can't really LGTM this. Hopefully, someone else can do that.

[nik]

Rebased and took over better wording/description from Ilya.

[nik]

Hmm, apparently "arc diff --update D37554" did not take the new commit message into account. Changed it manually with the web interface.

Ilya, I hope it's OK if I take your description :)

That said, I am not familiar with the code you're changing, so can't really LGTM this. Hopefully, someone else can do that.

So who would be the appropriate reviewer here? Manuel, any idea?

[ilya-biryukov]

In D37554#903401, @nik wrote:

Ilya, I hope it's OK if I take your description :)

Sure, no problem.

[nik]

...added some more reviewers that I've found with git blame. Ping to the new ones :)

[nik]

Ping...

[nik]

I don't get a review and I also don't know who should I add further to this change. What now?

[klimek]

LG.

[erikjv]

Committed as r318142.

tools/libclang/CIndex.cpp

enum CXErrorCode clang_parseTranslationUnit2FullArgv(
 
   CXErrorCode result = CXError_Failure;
   auto ParseTranslationUnitImpl = [=, &result] {
     result = clang_parseTranslationUnit_Impl(
         CIdx, source_filename, command_line_args, num_command_line_args,
         llvm::makeArrayRef(unsaved_files, num_unsaved_files), options, out_TU);
   };
 
-  if (getenv("LIBCLANG_NOTHREADS")) {
-    ParseTranslationUnitImpl();
-    return result;
-  }
-
   llvm::CrashRecoveryContext CRC;
 
   if (!RunSafely(CRC, ParseTranslationUnitImpl)) {
     fprintf(stderr, "libclang: crash detected during parsing: {\n");
     fprintf(stderr, "  'source_filename' : '%s'\n", source_filename);
     fprintf(stderr, "  'command_line_args' : [");
     for (int i = 0; i != num_command_line_args; ++i) {
       if (i)
int clang_saveTranslationUnit(CXTranslationUnit TU, const char *FileName,
   if (!CXXUnit->hasSema())
     return CXSaveError_InvalidTU;
 
   CXSaveError result;
   auto SaveTranslationUnitImpl = [=, &result]() {
     result = clang_saveTranslationUnit_Impl(TU, FileName, options);
   };
 
-  if (!CXXUnit->getDiagnostics().hasUnrecoverableErrorOccurred() ||
-      getenv("LIBCLANG_NOTHREADS")) {
+  if (!CXXUnit->getDiagnostics().hasUnrecoverableErrorOccurred()) {
     SaveTranslationUnitImpl();
 
     if (getenv("LIBCLANG_RESOURCE_USAGE"))
       PrintLibclangResourceUsage(TU);
 
     return result;
   }
 
if (num_unsaved_files && !unsaved_files)
     return CXError_InvalidArguments;
 
   CXErrorCode result;
   auto ReparseTranslationUnitImpl = [=, &result]() {
     result = clang_reparseTranslationUnit_Impl(
         TU, llvm::makeArrayRef(unsaved_files, num_unsaved_files), options);
   };
 
-  if (getenv("LIBCLANG_NOTHREADS")) {
-    ReparseTranslationUnitImpl();
-    return result;
-  }
-
   llvm::CrashRecoveryContext CRC;
 
   if (!RunSafely(CRC, ReparseTranslationUnitImpl)) {
     fprintf(stderr, "libclang: crash detected during reparsing\n");
     cxtu::getASTUnit(TU)->setUnsafeToFree(true);
     return CXError_Crashed;
   } else if (getenv("LIBCLANG_RESOURCE_USAGE"))
     PrintLibclangResourceUsage(TU);

 static unsigned SafetyStackThreadSize = 8 << 20;
 
 namespace clang {
 
 bool RunSafely(llvm::CrashRecoveryContext &CRC, llvm::function_ref<void()> Fn,
                unsigned Size) {
   if (!Size)
     Size = GetSafetyThreadStackSize();
-  if (Size)
+  if (Size && !getenv("LIBCLANG_NOTHREADS"))
     return CRC.RunSafelyOnThread(Fn, Size);
   return CRC.RunSafely(Fn);
 }
 
 unsigned GetSafetyThreadStackSize() {
   return SafetyStackThreadSize;
 }
 

tools/libclang/CIndexCodeCompletion.cpp

CXCodeCompleteResults *clang_codeCompleteAt(CXTranslationUnit TU,
 
   CXCodeCompleteResults *result;
   auto CodeCompleteAtImpl = [=, &result]() {
     result = clang_codeCompleteAt_Impl(
         TU, complete_filename, complete_line, complete_column,
         llvm::makeArrayRef(unsaved_files, num_unsaved_files), options);
   };
 
-  if (getenv("LIBCLANG_NOTHREADS")) {
-    CodeCompleteAtImpl();
-    return result;
-  }
-
   llvm::CrashRecoveryContext CRC;
 
   if (!RunSafely(CRC, CodeCompleteAtImpl)) {
     fprintf(stderr, "libclang: crash detected in code completion\n");
     cxtu::getASTUnit(TU)->setUnsafeToFree(true);
     return nullptr;
   } else if (getenv("LIBCLANG_RESOURCE_USAGE"))
     PrintLibclangResourceUsage(TU);

tools/libclang/Indexing.cpp

auto IndexSourceFileImpl = [=, &result]() {
     result = clang_indexSourceFile_Impl(
         idxAction, client_data, index_callbacks, index_callbacks_size,
         index_options, source_filename, command_line_args,
         num_command_line_args,
         llvm::makeArrayRef(unsaved_files, num_unsaved_files), out_TU,
         TU_options);
   };
 
-  if (getenv("LIBCLANG_NOTHREADS")) {
-    IndexSourceFileImpl();
-    return result;
-  }
-
   llvm::CrashRecoveryContext CRC;
 
   if (!RunSafely(CRC, IndexSourceFileImpl)) {
     fprintf(stderr, "libclang: crash detected during indexing source file: {\n");
     fprintf(stderr, "  'source_filename' : '%s'\n", source_filename);
     fprintf(stderr, "  'command_line_args' : [");
     for (int i = 0; i != num_command_line_args; ++i) {
       if (i)
int clang_indexTranslationUnit(CXIndexAction idxAction,
 
   CXErrorCode result;
   auto IndexTranslationUnitImpl = [=, &result]() {
     result = clang_indexTranslationUnit_Impl(
         idxAction, client_data, index_callbacks, index_callbacks_size,
         index_options, TU);
   };
 
-  if (getenv("LIBCLANG_NOTHREADS")) {
-    IndexTranslationUnitImpl();
-    return result;
-  }
-
   llvm::CrashRecoveryContext CRC;
 
   if (!RunSafely(CRC, IndexTranslationUnitImpl)) {
     fprintf(stderr, "libclang: crash detected during indexing TU\n");
     
     return 1;
   }