[PATCH] D34753: [Support] - Add bad alloc error handler for handling allocation malfunctions

[Klaus Kretzschmar via Phabricator via llvm-commits]

kkretzsch added a comment.

In https://reviews.llvm.org/D34753#806738, @rnk wrote:

> In https://reviews.llvm.org/D34753#806164, @kkretzsch wrote:
>
> > In https://reviews.llvm.org/D34753#805362, @rnk wrote:
> >
> > > I made some minor modifications. Taking the revision to upload them so you can see them.
> >
> >
> > Thanks for commiting my change. One comment on your changes. The reason why I changed the fallback behavior to calling an assertion instead of  the report_fatal_error function was that 'report_fatal_error' itself does some allocation, e.g. constucts a  MutexGuard.  This is problematic in an OOM situation.
>
>
> Hm. We probably don't want assert(false), though, since that's compiled out in a release build. exit(1) or abort() would be more appropriate.


I missed that point, you are right.

>>! In https://reviews.llvm.org/D34753#806738, @rnk wrote:
>  The MutexGuard I added around the read of BadAllocHandler is also problematic, though. It's not really the MutexGuard that's problematic, though, it's the ManagedStatic<Mutex>, which conditionally allocates memory if we haven't touched the mutex before. The whole design there is troublesome. What we really need is a global statically initialized mutex.
> 
> In theory, std::mutex has a constexpr default constructor, so it fits the bill, but I assume there are still portability issues there. It might be worth addressing them. I think at this point, the only supported way to build LLVM with an STL that lacks <mutex> is to disable threading support (cmake -DLLVM_ENABLE_THREADS=OFF), so we can try using std::mutex. That said, we still have to take it on faith that std::mutex::lock doesn't allocate memory. Does that sound reasonable? Certainly pthread_mutex_lock and EnterCriticalSection don't look like they need it.



In https://reviews.llvm.org/D34753#806906, @rnk wrote:

> Hopefully https://reviews.llvm.org/rL307827 addresses the memory allocation consideration.


Yes, using the std::lock_guard on a static mutex variable should work, Thanks!


Repository:
  rL LLVM

https://reviews.llvm.org/D34753