[libclang] Expose abort()-ing fatal error handler

[arphaman]

Is there any way to test this, or is it pretty much hopeless?

[jkorous]

How to minimize a patch to nothing:

1. Write unit test (EXPECT_DEATH during parsing of #pragma clang __debug parser_crash).
2. Hit CrashRecoveryContext.
3. Find clang_toggleCrashRecovery.

I guess what we were looking for is clang_toggleCrashRecovery(0);.

[arphaman]

In D66775#1646188, @arphaman wrote:

Is there any way to test this, or is it pretty much hopeless?

In D66775#1649966, @jkorous wrote:

How to minimize a patch to nothing:

1. Write unit test (EXPECT_DEATH during parsing of #pragma clang __debug parser_crash).
2. Hit CrashRecoveryContext.
3. Find clang_toggleCrashRecovery.

I guess what we were looking for is clang_toggleCrashRecovery(0);.

Crash recovery is completely orthogonal! It just happens in your constructed test case, not all APIs go through crash recovery. Please re-open the patch.

[jkorous]

Added a proper test.

[jkorous]

It turned out that recovery context matters as it messes with signals.

[jkorous]

There are actually couple other fatal-error-handling strategies. But all the other I've seen raise a signal (mostly SIGABRT; SIGILL in case of LLVM_BUILTIN_TRAP). For the purpose of having a way how to ask libclang to crash in a way that might lead to core dump this seems fine. Uninstalling the handler doesn't lead to libclang just calling exit in case of any crash though. Maybe I should rename the functions? Open to suggestions...

[arphaman]

clang/include/clang-c/FatalErrorHandler.h

20 ↗

(On Diff #218541)

What is the purpose of this FIXME? Will it be needed to install fatal error handler for other kinds of LLVM traps/calls?

[jkorous]

clang/include/clang-c/FatalErrorHandler.h

20 ↗

(On Diff #218541)

Sorry, I didn't make it clear. It's related to what I wrote in the change description:

There are actually couple other fatal-error-handling strategies. But all the other I've seen raise a signal (mostly SIGABRT; SIGILL in case of LLVM_BUILTIN_TRAP). For the purpose of having a way how to ask libclang to crash in a way that might lead to core dump this seems fine. Uninstalling the handler doesn't lead to libclang just calling exit in case of any crash though. Maybe I should rename the functions? Open to suggestions...

In other words - I am not too excited about creating an interface whose only function is to expose "detail of an implementation detail". On the other hand - I don't see a practical way how to implement a libclang-wide error handling strategy. The best I can imagine is naming these functions in weird-enough way that nobody would assume they promise more than they actually do. WDYT?

[arphaman]

clang/include/clang-c/FatalErrorHandler.h

20 ↗

(On Diff #218541)

Yes, unfortunately it's not an ideal interface. There's really no way for us to know what the client wants though, so I think this would have to be a reasonable compromise :(

I think it might be better to rename it to clang_install_aborting_llvm_fatal_error_handler to make it more clear that it's only for the LLVM fatal errors.

LGTM otherwise, thanks!

[• ddunbar]

clang/include/clang-c/FatalErrorHandler.h

20 ↗

(On Diff #218541)

Makes sense to me.

clang/tools/libclang/CIndex.cpp

3249 ↗

(On Diff #218541)

I don't get it. Per the ManagedStatic below, it looks like libclang was already trying to always install the handler. But based on the behavior I observed, it looks like some uses cases weren't necessarily triggering it.

It feels strange to add these new APIs when the ManagedStatic below will I think mean that the installation automatically happens?

[jkorous]

clang/tools/libclang/CIndex.cpp

3249 ↗

(On Diff #218541)

IIUC you need to call operator*() on managed ManagedStatic to have it construct an instance of the underlying type.

https://llvm.org/doxygen/classllvm_1_1ManagedStatic.html
"This transparently changes the behavior of global statics to be lazily constructed on demand"

https://llvm.org/doxygen/ManagedStatic_8h_source.html#l00083

That does happens in clang_createIndex:

// Look through the managed static to trigger construction of the managed
// static which registers our fatal error handler. This ensures it is only
// registered once.
(void)*RegisterFatalErrorHandlerOnce;

but not in other parts of the API. Now, some other functions expects CXIndex as an input which would imply the handler is set up before you can call them but that's still just a subset of the whole libclang API.

[jkorous]

• renamed functions to suggest their limitations
• added one more test

[• ddunbar]

clang/tools/libclang/CIndex.cpp

3249 ↗

(On Diff #218541)

Got it, that makes sense now.

[davezarzycki]

This breaks one of the clang unit tests on Fedora 30 (x86_64). Can we revert or fix this?

FAIL: Clang-Unit :: libclang/CrashTests/./libclangCrashTests/LibclangParseTest.UninstallAbortingLLVMFatalErrorHandler (19745 of 51413)
******************** TEST 'Clang-Unit :: libclang/CrashTests/./libclangCrashTests/LibclangParseTest.UninstallAbortingLLVMFatalErrorHandler' FAILED ********************
Note: Google Test filter = LibclangParseTest.UninstallAbortingLLVMFatalErrorHandler
[==========] Running 1 test from 1 test case.
[----------] Global test environment set-up.
[----------] 1 test from LibclangParseTest
[ RUN      ] LibclangParseTest.UninstallAbortingLLVMFatalErrorHandler
LLVM ERROR: #pragma clang __debug llvm_fatal_error

********************
Testing Time: 86.35s
********************
Failing Tests (1):
    Clang-Unit :: libclang/CrashTests/./libclangCrashTests/LibclangParseTest.UninstallAbortingLLVMFatalErrorHandler

  Expected Passes    : 40666
  Expected Failures  : 84
  Unsupported Tests  : 10662
  Unexpected Failures: 1

[thakis]

cfe/trunk/unittests/libclang/CrashTests/CMakeLists.txt

1

Can you add a comment on why this isn't just in libclangTests? I'm guessing it's to make sure the global error handler state doesn't accidentally link into the other tests in libclangTests, but I'm not sure.

[aaronpuchert]

cfe/trunk/tools/libclang/CMakeLists.txt

47

Why is this needed?

113

The support library is already added here.

[jessipete]

Hello everyone…we do not need to go through complications trying to hack our partner’s phone,hacking job is best done by proffessionals…when i needed to hack my partner’swhatsapp,i contacted wisetechhacker @gmail.com or whatsapp him via:[phone] …he did a perfect job for me within 48hrs…you can also contact him for all sorts of hacking job..he is fast and reliable..tell him clara reffered you..he would be willing to help.

cfe/trunk/include/clang-c/FatalErrorHandler.h

+/*===-- clang-c/FatalErrorHandler.h - Fatal Error Handling --------*- C -*-===*\
+|*                                                                            *|
+|* Part of the LLVM Project, under the Apache License v2.0 with LLVM          *|
+|* Exceptions.                                                                *|
+|* See https://llvm.org/LICENSE.txt for license information.                  *|
+|* SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception                    *|
+|*                                                                            *|
+\*===----------------------------------------------------------------------===*/
+
+#ifndef LLVM_CLANG_C_FATAL_ERROR_HANDLER_H
+#define LLVM_CLANG_C_FATAL_ERROR_HANDLER_H
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/**
+ * Installs error handler that prints error message to stderr and calls abort().
+ * Replaces currently installed error handler (if any).
+ */
+void clang_install_aborting_llvm_fatal_error_handler();
+
+/**
+ * Removes currently installed error handler (if any).
+ * If no error handler is intalled, the default strategy is to print error
+ * message to stderr and call exit(1).
+ */
+void clang_uninstall_llvm_fatal_error_handler();
+
+#ifdef __cplusplus
+}
+#endif
+#endif

cfe/trunk/tools/libclang/CIndex.cpp

 #include "CIndexer.h"
 #include "CLog.h"
 #include "CXCursor.h"
 #include "CXSourceLocation.h"
 #include "CXString.h"
 #include "CXTranslationUnit.h"
 #include "CXType.h"
 #include "CursorVisitor.h"
+#include "clang-c/FatalErrorHandler.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/Mangle.h"
 #include "clang/AST/StmtVisitor.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/DiagnosticCategories.h"
 #include "clang/Basic/DiagnosticIDs.h"
 #include "clang/Basic/Stack.h"
 #include "clang/Basic/TargetInfo.h"
RefNamePieces buildPieces(unsigned NameFlags, bool IsMemberRefExpr,
   return Pieces;  
 }
 }
 
 //===----------------------------------------------------------------------===//
 // Misc. API hooks.
 //===----------------------------------------------------------------------===//               
 
-static void fatal_error_handler(void *user_data, const std::string& reason,
-                                bool gen_crash_diag) {
-  // Write the result out to stderr avoiding errs() because raw_ostreams can
-  // call report_fatal_error.
-  fprintf(stderr, "LIBCLANG FATAL ERROR: %s\n", reason.c_str());
-  ::abort();
-}
-
 namespace {
 struct RegisterFatalErrorHandler {
   RegisterFatalErrorHandler() {
-    llvm::install_fatal_error_handler(fatal_error_handler, nullptr);
+    clang_install_aborting_llvm_fatal_error_handler();
   }
 };
 }
 
 static llvm::ManagedStatic<RegisterFatalErrorHandler> RegisterFatalErrorHandlerOnce;
 
 CXIndex clang_createIndex(int excludeDeclarationsFromPCH,
                           int displayDiagnostics) {

cfe/trunk/tools/libclang/CMakeLists.txt

set(SOURCES
   CXIndexDataConsumer.cpp
   CXCompilationDatabase.cpp
   CXLoadedDiagnostic.cpp
   CXSourceLocation.cpp
   CXStoredDiagnostic.cpp
   CXString.cpp
   CXType.cpp
   Indexing.cpp
+  FatalErrorHandler.cpp
 
   ADDITIONAL_HEADERS
   CIndexDiagnostic.h
   CIndexer.h
   CXCursor.h
   CXLoadedDiagnostic.h
   CXSourceLocation.h
   CXString.h
   CXTranslationUnit.h
   CXType.h
   Index_Internal.h
   ../../include/clang-c/Index.h
   )
 
 set(LIBS
   clangAST
   clangBasic
   clangDriver
   clangFrontend
   clangIndex
   clangLex
   clangSema
   clangSerialization
   clangTooling
+  LLVMSupport
 )
 
 if (CLANG_ENABLE_ARCMT)
   list(APPEND LIBS clangARCMigrate)
 endif ()
 
 if (TARGET clangTidyPlugin)
   add_definitions(-DCLANG_TOOL_EXTRA_BUILD)
add_clang_library(libclang ${ENABLE_SHARED} ${ENABLE_STATIC} INSTALL_WITH_TOOLCHAIN
   clang-resource-headers
 
   LINK_LIBS
   ${LIBS}
 
   LINK_COMPONENTS
   ${LLVM_TARGETS_TO_BUILD}
   Core
   Support
   )
 
 if(ENABLE_SHARED)
   if(WIN32)
     set_target_properties(libclang
       PROPERTIES
       VERSION ${LIBCLANG_LIBRARY_VERSION}
       DEFINE_SYMBOL _CINDEX_LIB_)

cfe/trunk/tools/libclang/FatalErrorHandler.cpp

+/*===-- clang-c/FatalErrorHandler.cpp - Fatal Error Handling ------*- C -*-===*\
+|*                                                                            *|
+|* Part of the LLVM Project, under the Apache License v2.0 with LLVM          *|
+|* Exceptions.                                                                *|
+|* See https://llvm.org/LICENSE.txt for license information.                  *|
+|* SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception                    *|
+|*                                                                            *|
+\*===----------------------------------------------------------------------===*/
+
+#include "clang-c/FatalErrorHandler.h"
+#include "llvm/Support/ErrorHandling.h"
+
+static void aborting_fatal_error_handler(void *, const std::string &reason,
+                                         bool) {
+  // Write the result out to stderr avoiding errs() because raw_ostreams can
+  // call report_fatal_error.
+  fprintf(stderr, "LIBCLANG FATAL ERROR: %s\n", reason.c_str());
+  ::abort();
+}
+
+void clang_install_aborting_llvm_fatal_error_handler() {
+  llvm::remove_fatal_error_handler();
+  llvm::install_fatal_error_handler(aborting_fatal_error_handler, nullptr);
+}
+
+void clang_uninstall_llvm_fatal_error_handler() {
+  llvm::remove_fatal_error_handler();
+}

cfe/trunk/tools/libclang/libclang.exports

 clang_EvalResult_getAsUnsigned
 clang_EvalResult_isUnsignedInt
 clang_EvalResult_getAsDouble
 clang_EvalResult_getAsStr
 clang_EvalResult_dispose
 clang_PrintingPolicy_getProperty
 clang_PrintingPolicy_setProperty
 clang_PrintingPolicy_dispose
+clang_install_aborting_llvm_fatal_error_handler
+clang_uninstall_llvm_fatal_error_handler

cfe/trunk/unittests/libclang/CMakeLists.txt

 add_clang_unittest(libclangTests
   LibclangTest.cpp
   )
 
 target_link_libraries(libclangTests
   PRIVATE
   libclang
   )
+
+add_subdirectory(CrashTests)

cfe/trunk/unittests/libclang/CrashTests/CMakeLists.txt

+add_clang_unittest(libclangCrashTests
+  LibclangCrashTest.cpp
+  )
+
+target_link_libraries(libclangCrashTests
+  PRIVATE
+  libclang
+  )

cfe/trunk/unittests/libclang/CrashTests/LibclangCrashTest.cpp

+//===- unittests/libclang/LibclangCrashTest.cpp --- libclang tests --------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "../TestUtils.h"
+#include "clang-c/FatalErrorHandler.h"
+#include "gtest/gtest.h"
+#include <string>
+
+TEST_F(LibclangParseTest, InstallAbortingLLVMFatalErrorHandler) {
+  clang_toggleCrashRecovery(0);
+  clang_install_aborting_llvm_fatal_error_handler();
+
+  std::string Main = "main.h";
+  WriteFile(Main, "#pragma clang __debug llvm_fatal_error");
+
+  EXPECT_DEATH(clang_parseTranslationUnit(Index, Main.c_str(), nullptr, 0,
+                                          nullptr, 0, TUFlags),
+               "");
+}
+
+TEST_F(LibclangParseTest, UninstallAbortingLLVMFatalErrorHandler) {
+  clang_toggleCrashRecovery(0);
+  clang_install_aborting_llvm_fatal_error_handler();
+  clang_uninstall_llvm_fatal_error_handler();
+
+  std::string Main = "main.h";
+  WriteFile(Main, "#pragma clang __debug llvm_fatal_error");
+
+  EXPECT_NO_FATAL_FAILURE(clang_parseTranslationUnit(
+      Index, Main.c_str(), nullptr, 0, nullptr, 0, TUFlags));
+}