[PATCH] Change pretty stack trace signal handler to being an opt-in feature rather than an opt-out feature

[Alp Toker]

Hello Filip,

Very tidy solution, thanks for putting in the time.


|Index: tools/libclang/CIndex.cpp||
||===================================================================||
||--- tools/libclang/CIndex.cpp    (revision 193960)||
||+++ tools/libclang/CIndex.cpp    (working copy)||
||@@ -2544,10 +2544,6 @@||
|| extern "C" {||
|| CXIndex clang_createIndex(int excludeDeclarationsFromPCH,||
||                           int displayDiagnostics) {||
||-  // Disable pretty stack trace functionality, which will otherwise
be a very||
||-  // poor citizen of the world and set up all sorts of signal handlers.||
||-  llvm::DisablePrettyStackTrace = true;||
||-||
||   // We use crash recovery to make some of our APIs more reliable,
implicitly||
||   // enable it.||
||   llvm::CrashRecoveryContext::Enable();|


You can remove the old PrettyStackTrace.h include here as well.

Also lldb will need patching:

|source/Expression/ClangExpressionParser.cpp:           
llvm::DisablePrettyStackTrace = true;||
|
Otherwise I think this is ready to go

Alp.


On 03/11/2013 17:17, Filip Pizlo wrote:
> Hi!
>
> Currently some large subset of LLVM embedders are expected to call to set DisablePrettyStackTrace to true to disable LLVM's pretty stack trace printing.
>
> Probably, it would be even better to flip this around: if you want pretty stack traces, you should ask for them explicitly.  This patch implements just such behavior, and does it in a way that I believe requires minimal changes from LLVM clients.
>
> Previously the pretty stack trace signal handler would be installed whenever a PrettyStackTraceEntry was constructed, unless the DisablePrettyStackTrace variable was set to true.  The C API would set this variable to true by calling LLVMDisablePrettyStackTrace().  Now, the signal handler is only installed if you call llvm::EnablePrettyStackTrace().
>
> At first glance this would seem to imply that any LLVM client that does not currently set DisablePrettyStackTrace to true should now add a call to llvm::EnablePrettyStackTrace().  But I believe this patch avoids this: instead, I've made PrettyStackTraceProgram's constructor call EnablePrettyStackTrace().  PrettyStackTraceProgram is never used in LLVM library code - only in LLVM tools and in clients (like clang).  I believe that this automatically gives us the behavior we want:
>
> - Existing LLVM-based tools continue to have pretty stack traces even though I didn't have to change any of their code.
>
> - LLVM embedders that use the C API will not get pretty stack traces anymore unless they call LLVMEnablePrettyStackTrace().  I believe that this is a good thing, since if embedders were getting pretty stack traces just by calling into the LLVM C API, then that was probably a bug.
>
> - LLVM embedders that use the C++ API will not get pretty stack traces unless they either use PrettyStackTraceProgram or call llvm::EnablePrettyStackTrace().
>
> I also include a corresponding clang patch, which removes CIndex's use of DisablePrettyStackTrace.
>
> -Filip
>
>

-- 
http://www.nuanti.com
the browser experts

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131103/1768a205/attachment.html>