[PATCH] D64505: [Support] Move the static initializer install_out_memory_new_handler to InitLLVM

Wed Jul 10 19:16:38 PDT 2019

MaskRay added a comment.

In D64505#1579593 <https://reviews.llvm.org/D64505#1579593>, @jfb wrote:

> In D64505#1579592 <https://reviews.llvm.org/D64505#1579592>, @rnk wrote:
>
> > In D64505#1579577 <https://reviews.llvm.org/D64505#1579577>, @jfb wrote:
> >
> > > So you're saying: maybe it's a bug if we install one but there was already one installed?
> >
> >
> > Yes. And the user can call llvm_shutdown without setting creating an `InitLLVM` object, so llvm_shutdown shouldn't assume that LLVM's new handler is in use.
>

Yes. I could check `std::get_new_handler`, but I think that is not really necessary. Beside manging argv on Windows, InitLLVM installs signal handlers, which may not be desired in application code. I agree that InitLLVM should only be used by LLVM tools.

>> So, I think this change is fine as is:
>> 
>> - LLVM tools want this OOM handler, and they use InitLLVM to set up crash handlers
>> - Users of LLVM as a library should avoid InitLLVM and set their own OOM handler if they want one
> 
> Is there a diagnostic that we can add to catch misuses for `InitLLVM`?

I can imagine some out-of-tree LLVM tools don't bother doing their own signal handlers or new handler. I don't know how such tools (where LLVM is an integral part of them) can be differentiated from other tools (e.g. the AMD GPU driver that revealed the problem). So, I don't think a diagnostic is possible...

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64505/new/

https://reviews.llvm.org/D64505