Enabled Backtraces with just ENABLE_BACKTRACES

[jmittert]

Summary

Some platforms, e.g. Windows, support backtraces but don't have
BACKTRACE. Checking for BACKTRACE prevents Windows from having
backtraces.

[compnerd]

This seems right, the Unix side will provide the same interfaces that do nothing in the case that the target doesn't support the backtrace API, and on Windows we provide the same interfaces using DbgHelp if we have access to it.

[jmittert]

Thanks! Can you commit this for me?

[compnerd]

SVN r354951

[yifeihe]

Hi！
This move will call AddSignalHandler and enable SetUnhandledExceptionFilter in llvm\lib\Support\Windows\signals.inc. In our application, llvm's component is called as a library , and the Unhandled Exception handler set in our application is over written by the llvm.
I think the application, not one of its components, should be responsible for application-wide error handling and reporting policy. Any libraries called from application should not permanently change global exception handler. This is reserved for application code (not the library code).
The llvm called as a library may change the exception handler temporarily, but must restore the handler before call finishes.
To sum up my point of view, permanently changing UnhandledExceptionFilter in library code is a bug that should be fixed

[yifeihe]

Hi @compnerd !
This patch will enable exception filter in llvm, which is not a thing llvm should do, and introduced bugs in our application, I think we should revert or modify this patch.

[compnerd]

@yifeihe, you should be able to build with backtraces disabled to get the previous behaviour. The fact that the behaviour was a certain way previously was just an accident, not an explicit design point.

lib/Support/PrettyStackTrace.cpp

 #include <CrashReporterClient.h>
 #endif
 
 using namespace llvm;
 
 // If backtrace support is not enabled, compile out support for pretty stack
 // traces.  This has the secondary effect of not requiring thread local storage
 // when backtrace support is disabled.
-#if defined(HAVE_BACKTRACE) && ENABLE_BACKTRACES
+#if ENABLE_BACKTRACES
 
 // We need a thread local pointer to manage the stack of our stack trace
 // objects, but we *really* cannot tolerate destructors running and do not want
 // to pay any overhead of synchronizing. As a consequence, we use a raw
 // thread-local variable.
 static LLVM_THREAD_LOCAL PrettyStackTraceEntry *PrettyStackTraceHead = nullptr;
 
 namespace llvm {
#elif HAVE_CRASHREPORTER_INFO
     __crashreporter_info__ = strdup(TmpStr.c_str());
 #endif
     errs() << TmpStr.str();
   }
 
 #endif
 }
 
-// defined(HAVE_BACKTRACE) && ENABLE_BACKTRACES
+// ENABLE_BACKTRACES
 #endif
 
 PrettyStackTraceEntry::PrettyStackTraceEntry() {
-#if defined(HAVE_BACKTRACE) && ENABLE_BACKTRACES
+#if ENABLE_BACKTRACES
   // Link ourselves.
   NextEntry = PrettyStackTraceHead;
   PrettyStackTraceHead = this;
 #endif
 }
 
 PrettyStackTraceEntry::~PrettyStackTraceEntry() {
-#if defined(HAVE_BACKTRACE) && ENABLE_BACKTRACES
+#if ENABLE_BACKTRACES
   assert(PrettyStackTraceHead == this &&
          "Pretty stack trace entry destruction is out of order");
   PrettyStackTraceHead = NextEntry;
 #endif
 }
 
 void PrettyStackTraceString::print(raw_ostream &OS) const { OS << Str << "\n"; }
 

 void PrettyStackTraceProgram::print(raw_ostream &OS) const {
   OS << "Program arguments: ";
   // Print the argument list.
   for (unsigned i = 0, e = ArgC; i != e; ++i)
     OS << ArgV[i] << ' ';
   OS << '\n';
 }
 
-#if defined(HAVE_BACKTRACE) && ENABLE_BACKTRACES
+#if ENABLE_BACKTRACES
 static bool RegisterCrashPrinter() {
   sys::AddSignalHandler(CrashHandler, nullptr);
   return false;
 }
 #endif
 
 void llvm::EnablePrettyStackTrace() {
-#if defined(HAVE_BACKTRACE) && ENABLE_BACKTRACES
+#if ENABLE_BACKTRACES
   // The first time this is called, we register the crash printer.
   static bool HandlerRegistered = RegisterCrashPrinter();
   (void)HandlerRegistered;
 #endif
 }
 
 const void *llvm::SavePrettyStackState() {
-#if defined(HAVE_BACKTRACE) && ENABLE_BACKTRACES
+#if ENABLE_BACKTRACES
   return PrettyStackTraceHead;
 #else
   return nullptr;
 #endif
 }
 
 void llvm::RestorePrettyStackState(const void *Top) {
-#if defined(HAVE_BACKTRACE) && ENABLE_BACKTRACES
+#if ENABLE_BACKTRACES
   PrettyStackTraceHead =
       static_cast<PrettyStackTraceEntry *>(const_cast<void *>(Top));
 #endif
 }
 
 void LLVMEnablePrettyStackTrace() {
   EnablePrettyStackTrace();
 }