LLVM tools are not playing Visual Studio debugger friendly

[Quuxplusone]

Bugzilla Link	PR33854
Status	NEW
Importance	P enhancement
Reported by	jujjyl@gmail.com
Reported on	2017-07-19 13:54:59 -0700
Last modified on	2017-07-20 07:35:21 -0700
Version	unspecified
Hardware	PC Windows NT
CC	geek4civic@gmail.com, greg.bedwell@sony.com, llvm-bugs@lists.llvm.org, paul_robinson@playstation.sony.com, rnk@google.com
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also

It looks like the various tools in the Clang toolchain are set to internally
capture all crash signals and generate crash minidumps from them. This prevents
from being able to attach a Visual Studio debugger at the time of crash. By
default if applications do not register any signal handlers, the VC runtime
will pop up a debug handler dialog asking to attach a VS debugger in Debug
builds.

Would it be possible to make it so that Debug CMake config build would by
default not attach crash handlers, so that VC runtime would pop up the debugger
dialog? Additionally, it would be good to hook onto an environment variable,
say LLVM_DISABLE_CRASH_HANDLERS=1, which if set, would cause LLVM to hand off
to Visual Studio debugging.

Now I notice I need to do custom builds that comment out the crash handlers,
e.g. for llc.exe, in llc.cpp:

int main(int argc, char **argv) {
//  sys::PrintStackTraceOnErrorSignal(argv[0]);

It would be nice for LLVM to behave debugger friendly out of the box.

[Quuxplusone]

I'd like to disable minidumps by default as well. They serve no purpose for most users and consume disk space.

[Quuxplusone]

(In reply to Reid Kleckner from comment #1)
> I'd like to disable minidumps by default as well. They serve no purpose for
> most users and consume disk space.

I think this a subtly different issue as the minidump writing is just part of
what LLVMUnhandledExceptionFilter does.  In our case our users occasionally
provide us with dmp files for intermittent crashes or at times when they're
unable to provide us with preprocessed files (if they're working on an
unannounced project for example) so we'd need a way for us to opt-in to this by
default for all our builds via a CMake flag or similar.  Users can currently
opt-out of minidump generation with the LLVM_DISABLE_CRASH_REPORT environment
variable but still get the rest of the exception handler behaviour such as
writing out a stack trace.

With regard to the dialog box, we currently go to some lengths to prevent them
from appearing as the last thing you want on your headless Buildbot/Jenkins
agent is a command line tool throwing up a modal dialog and that blocks
everything until clicked on.

Looking at the current clang (RelWithDebInfo + asserts) behaviour with a couple
of testcases:

#pragma clang __debug parser_crash // crash.cpp
and
#pragma clang __debug assert // assert.cpp

By default we get a tty stack trace and no dialog for each case.

If I stub out sys::PrintStackTraceOnErrorSignal I still get the stack trace and
no dialog for crash.cpp, but I now get the dialog but no stack trace for
assert.cpp.  The only affect of stubbing that function out seems to be that
sys::DisableSystemDialogsOnCrash is no longer called which seems to be
preventing the dialog for abort() but not for hard crashes.

The reason stubbing out  out sys::PrintStackTraceOnErrorSignal isn't enough on
its own is because RegisterHandler is also being called from
sys::RemoveFileOnSignal (and sys::DontRemoveFileOnSignal).  I don't really
understand how these APIs work though so I don't know what they're doing.

FWIW, the following patch seems to give us both the exception handler and the
debug dialog (with a sensible looking callstack view in VS2017) in both cases.
Obviously this is not commitable as it adds the modal dialog back in all cases,
but maybe it can provide a starting point:

diff --git a/lib/Support/Windows/Signals.inc b/lib/Support/Windows/Signals.inc
index 1ef5188..03b559b 100644
--- a/lib/Support/Windows/Signals.inc
+++ b/lib/Support/Windows/Signals.inc
@@ -506,7 +506,7 @@ void sys::PrintStackTraceOnErrorSignal(StringRef Argv0,
   if (DisableCrashReporting || getenv("LLVM_DISABLE_CRASH_REPORT"))
     Process::PreventCoreFiles();

-  DisableSystemDialogsOnCrash();
+  //DisableSystemDialogsOnCrash();
   RegisterHandler();
   LeaveCriticalSection(&CriticalSection);
 }
@@ -811,6 +811,7 @@ static LONG WINAPI
LLVMUnhandledExceptionFilter(LPEXCEPTION_POINTERS ep) {
   PrintStackTraceForThread(llvm::errs(), hProcess, hThread, StackFrame,
                            ep->ContextRecord);

+  LLVM_BUILTIN_TRAP;
   _exit(ep->ExceptionRecord->ExceptionCode);
 }