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

[Reid Kleckner via Phabricator via llvm-commits]

rnk added a comment.

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.

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.


Repository:
  rL LLVM

https://reviews.llvm.org/D34753