[cfe-dev] [llvm] r193937 - When LLVM is embedded in a larger application, it's not OK for LLVM to intercept crashes. LLVM already has
Filip Pizlo
fpizlo at apple.com
Sat Nov 2 21:40:41 PDT 2013
On Nov 2, 2013, at 9:26 PM, Alp Toker <alp at nuanti.com> wrote:
> On 03/11/2013 04:18, Filip Pizlo wrote:
>> On Nov 2, 2013, at 9:15 PM, Alp Toker <alp at nuanti.com
>> <mailto:alp at nuanti.com>> wrote:
>>
>>> Brilliant. Could you revert the API addition of
>>> LLVMDisablePrettyStackTrace() in r193937 while we figure this out?
>>
>> I'd rather not. Without this function, it's impossible for a C API
>> client to embed LLVM.
>>
>> I agree that there is a better solution out there, but I'd rather have
>> this as a stop-gap until then. Is there any real harm to having this
>> in the tree in the meantime? Because there is clear harm on my end
>> from removing it - namely, the usual crash recording machinery for OS
>> X applications doesn't work as soon as LLVM is embedded.
>
> I'm concerned that with this function now in the C API and documented,
> others tracking SVN including other LLVM subprojects will start using
> it, adding a burden of support and making it difficult to implement a
> proper fix.
I believe that in between LLVM releases, the C API is expected to be unstable. We can change our minds about it.
>
> After all this clearly should have been the default all along :-)
>
> So either let's amend the comment in the C header and work on the Enable
> solution quickly or let's revert for that reason.
I will amend the comment.
>
> FWIW we've also been patching LLVM for a long time to fix this, so
> couldn't you hold on a day or two longer to get the right fix in place
> for OS X?
I certainly could. I'm just playing out the trade-offs and I don't see the upside of reverting. I think we agree that we will replace this API with something more thought-out. So, we agree that this new function of mine will have a short shelf-life and should not make it into a release. We can either leave my horrible function in svn for the next few days, or we can revert it. Leaving it in has the downside that maybe others will take a liking to it. Reverting it means more hassle for C API clients, like me. Seems like leaving it in is the lesser evil?
-Filip
>
> Alp.
>
>
>
>>
>> -Filip
>>
>>
>>>
>>> The new approach will be a step towards cleaning up libclang as well:
>>>
>>> tools/libclang/CIndex.cpp- // Disable pretty stack trace functionality,
>>> which will otherwise be a very
>>> tools/libclang/CIndex.cpp- // poor citizen of the world and set up all
>>> sorts of signal handlers.
>>> tools/libclang/CIndex.cpp: llvm::DisablePrettyStackTrace = true;
>>>
>>> Thanks for noticing the issue and starting on a solution!
>>>
>>> Alp.
>>>
>>>
>>> On 03/11/2013 04:10, Filip Pizlo wrote:
>>>>
>>>> On Nov 2, 2013, at 9:07 PM, Alp Toker <alp at nuanti.com
>>>> <mailto:alp at nuanti.com>
>>>> <mailto:alp at nuanti.com>> wrote:
>>>>
>>>>> Could we make this the default, then provide
>>>>> LLVMEnablePrettyStackTrace() instead?
>>>>
>>>> I like this.
>>>>
>>>>>
>>>>> By definition the C API is for embedding, and embedders don't expect
>>>>> each API they use to install global signal handlers.
>>>>
>>>> Right, and we could also do the same thing for the fatal error
>>>> handler. The default fatal error handler does exit(1), which is
>>>> clearly not right for embedders.
>>>>
>>>> It seems like both the pretty stack traces and the fatal error handler
>>>> could be manually enabled by those clients that depend on it.
>>>>
>>>>>
>>>>> Alp.
>>>>>
>>>>>
>>>>> On 03/11/2013 00:29, Filip Pizlo wrote:
>>>>>> Author: fpizlo
>>>>>> Date: Sat Nov 2 19:29:47 2013
>>>>>> New Revision: 193937
>>>>>>
>>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=193937&view=rev
>>>>>> Log:
>>>>>> When LLVM is embedded in a larger application, it's not OK for LLVM
>>>>>> to intercept crashes. LLVM already has
>>>>>> the ability to disable this functionality. This patch exposes it
>>>>>> via the C API.
>>>>>>
>>>>>>
>>>>>> Modified:
>>>>>> llvm/trunk/include/llvm-c/Core.h
>>>>>> llvm/trunk/lib/Support/PrettyStackTrace.cpp
>>>>>>
>>>>>> Modified: llvm/trunk/include/llvm-c/Core.h
>>>>>> URL:
>>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm-c/Core.h?rev=193937&r1=193936&r2=193937&view=diff
>>>>>> ==============================================================================
>>>>>> --- llvm/trunk/include/llvm-c/Core.h (original)
>>>>>> +++ llvm/trunk/include/llvm-c/Core.h Sat Nov 2 19:29:47 2013
>>>>>> @@ -434,6 +434,12 @@ void LLVMInstallFatalErrorHandler(LLVMFa
>>>>>> void LLVMResetFatalErrorHandler(void);
>>>>>>
>>>>>> /**
>>>>>> + * Disable LLVM's built-in stack trace code. This must be called
>>>>>> before any
>>>>>> + * other LLVM APIs; otherwise the results are undefined.
>>>>>> + */
>>>>>> +void LLVMDisablePrettyStackTrace(void);
>>>>>> +
>>>>>> +/**
>>>>>> * @defgroup LLVMCCoreContext Contexts
>>>>>> *
>>>>>> * Contexts are execution states for the core LLVM IR system.
>>>>>>
>>>>>> Modified: llvm/trunk/lib/Support/PrettyStackTrace.cpp
>>>>>> URL:
>>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/PrettyStackTrace.cpp?rev=193937&r1=193936&r2=193937&view=diff
>>>>>> ==============================================================================
>>>>>> --- llvm/trunk/lib/Support/PrettyStackTrace.cpp (original)
>>>>>> +++ llvm/trunk/lib/Support/PrettyStackTrace.cpp Sat Nov 2
>>>>>> 19:29:47 2013
>>>>>> @@ -20,6 +20,7 @@
>>>>>> #include "llvm/Support/ThreadLocal.h"
>>>>>> #include "llvm/Support/Watchdog.h"
>>>>>> #include "llvm/Support/raw_ostream.h"
>>>>>> +#include "llvm-c/Core.h"
>>>>>>
>>>>>> #ifdef HAVE_CRASHREPORTERCLIENT_H
>>>>>> #include <CrashReporterClient.h>
>>>>>> @@ -147,3 +148,7 @@ void PrettyStackTraceProgram::print(raw_
>>>>>> OS << ArgV[i] << ' ';
>>>>>> OS << '\n';
>>>>>> }
>>>>>> +
>>>>>> +void LLVMDisablePrettyStackTrace() {
>>>>>> + DisablePrettyStackTrace = true;
>>>>>> +}
>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> llvm-commits mailing list
>>>>>> llvm-commits at cs.uiuc.edu
>>>>>> <mailto:llvm-commits at cs.uiuc.edu> <mailto:llvm-commits at cs.uiuc.edu>
>>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>>>
>>>>> --
>>>>> http://www.nuanti.com <http://www.nuanti.com/> <http://www.nuanti.com/>
>>>>> the browser experts
>>>>
>>>
>>> --
>>> http://www.nuanti.com <http://www.nuanti.com/>
>>> the browser experts
>>
>
> --
> http://www.nuanti.com
> the browser experts
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20131102/eef44bfa/attachment.html>
More information about the cfe-dev
mailing list