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

[rnk]

Summary

We would like to introduce a new type of llvm error handler for handling bad alloc fault situations.
LLVM already provides a fatal error handler for serious non-recoverable error situations which by
default writes some error information to stderr and calls exit(1) at the end (functions are marked as
'noreturn').

For long running processes (e.g. a server application), exiting the process is not an acceptable option,
especially not when the system is in a temporary resource bottleneck with a good chance to recover from
this fault situation. In such a situation you would rather throw an exception to stop the current
compilation and try to overcome the resource bottleneck. The user should be aware of the problem of throwing
an exception in bad alloc situations, e.g. you must not do any allocations in the unwind chain. This is especially
true when adding exceptions in existing unfamiliar code (as already stated in the comment of the current fatal error
handler)

So the new handler can also be used to distinguish from general fatal error situations where recovering is no option.
It should be used in cases where a clean unwind after the allocation is guaranteed.

This patch contains:

• A report_bad_alloc function which calls a user defined bad alloc error handler. If no user handler is registered the report_fatal_error function is called. This function is not marked as 'noreturn'.
• A install/restore_bad_alloc_error_handler to install/restore the bad alloc handler.
• An example (in Mutex.cpp) where the report_bad_alloc function is called in case of a malloc returns a nullptr.

If this patch gets accepted we would create similar patches to fix corresponding malloc/calloc usages in the llvm code.

Patch by Klaus Kretzschmar

[MatzeB]

In general: This may warrant an RFC to llvm-dev to discuss the scope of these patches. I feel that making all of LLVM abort gracefully in out of memory situations will be hard/unmaintainable. I believe the trick would be to limit the scope. Remembering your EuroLLVM talk it seemed that you would only need IR construction in the long running server process so you can hand over that IR to shorter lived/restartable worker processes doing the actual code generation. So the scope may be limited to this use case avoiding the need to fix all the transformation passes to recover gracefully.

Specifically about this patch: So a failing malloc would now call the handler, but a failing new would throw an exception (or more likely abort() in the default no-exceptions configuration of LLVM. Maybe we should rather: Use new instead of malloc wherever possible. I'm not sure why this particular code uses malloc at all? For the cases that still use malloc (only good reason I can think of would be using realloc later) maybe we can trigger the same C++ exception as a failing new?

[kkretzsch]

In D34753#799802, @MatzeB wrote:

In general: This may warrant an RFC to llvm-dev to discuss the scope of these patches. I feel that making all of LLVM abort gracefully in out of memory situations will be hard/unmaintainable. I believe the trick would be to limit the scope. Remembering your EuroLLVM talk it seemed that you would only need IR construction in the long running server process so you can hand over that IR to shorter lived/restartable worker processes doing the actual code generation. So the scope may be limited to this use case avoiding the need to fix all the transformation passes to recover gracefully.

Yes, I agree limiting the scope is necessary and has to be discussed with the people. Howver, on short term we would like to start with small patches that fixes some obvious flaws, like exchanging mallocs by raw new operators as you suggest

! In D34753#799802, @MatzeB wrote:

Specifically about this patch: So a failing malloc would now call the handler, but a failing new would throw an exception (or more likely abort() in the default no-exceptions configuration of LLVM. Maybe we should rather: Use new instead of malloc wherever possible. I'm not sure why this particular code uses malloc at all? For the cases that still use malloc (only good reason I can think of would be using realloc later) maybe we can trigger the same C++ exception as a failing new?

Good idea. I'll run through our malloc patches and check where I can replace the mallocs . So a new bad alloc handler is not necessary anymore, so I'm closing this patch request.

[kkretzsch]

! In D34753#799802, @MatzeB wrote:

Specifically about this patch: So a failing malloc would now call the handler, but a failing new would throw an exception (or more likely abort() in the default no-exceptions configuration of LLVM. Maybe we should rather: Use new instead of malloc wherever possible. I'm not sure why this particular code uses malloc at all? For the cases that still use malloc (only good reason I can think of would be using realloc later) maybe we can trigger the same C++ exception as a failing new?

Good idea. I'll run through our malloc patches and check where I can replace the mallocs . So a new bad alloc handler is not necessary anymore, so I'm closing this patch request.

SmallPtrsSet and SmallVector uses realloc which means that the mallocs and frees in that code cannot be replaced with global operator new/delete. Coming back to your original comment, overriding the global ::operator new in a way that it calls the bad alloc handler in case of OOM would create a consistent behaviour between the malloc/realloc/calloc familiy and global ::operator new.

[rnk]

Given that LLVM will contain calls to malloc and it may return null, I'm in favor of this. Even for users that don't install the handler, it turns an OOM crash into a fatal error. That seems good.

include/llvm/Support/ErrorHandling.h

100 ↗

(On Diff #104418)

use clang-format to align the wrapped line

lib/Support/ErrorHandling.cpp

130 ↗

(On Diff #104418)

Add a space after ','

[kkretzsch]

Thanks Reid for reviewing. I updated this patch according to your review comments. In addition I noticed that the report_fatal_error function does allocations inside and is therefore not suitable to be called in the report_bad_alloc_error function. So I changed the default behavior to call an assert instead.

I do not have commit rights to push this change to llvm trunk. Therefore I need someone who is doing this for me.

Thanks,
Klaus

[MatzeB]

So how about

#include <new>
...
x = malloc(...);
if (x == nullptr)
  throw std::bad_alloc();

that way running out of memory in malloc and new would behave the same way.

include/llvm/Support/ErrorHandling.h

82 ↗

(On Diff #105847)

Do not repeat the name of the function in the documentation like this (you see the pattern a lot in older code, but we avoid it in new code).

lib/Support/ErrorHandling.cpp

130 ↗

(On Diff #105847)

How about:

#ifdef __EXCEPTIONS
  throw std::bad_alloc();
#else
  report_fatal_error("Unable to allocate memory.\n");
#endif

(__EXCEPTIONS is defined by clang/gcc when exceptions are enabled. An actual patch would probably better put that into llvm/Support/Compiler.h and define some LLVM specific constant like LLVM_ENABLE_EXCEPTIONS so we can add the msvc specific thing later).

lib/Support/Mutex.cpp

52–54 ↗

(On Diff #105847)

We omit {} in llvm in simple if and loop statements.

[MatzeB]

Please, ignore the first part in my last reply that proposes immediately throwing an exception. I realized that we cannot compile that line while -fno-exceptions is enabled. Hence I proposed a different solution that could go as part of llvm::report_bad_alloc_error() (as I proposed later in the review).

[kkretzsch]

Hi Matthias,
thanks for reviewing. I have updated the patch.

Klaus

[rnk]

I made some minor modifications. Taking the revision to upload them so you can see them.

[rnk]

• minor NFC changes

[kkretzsch]

In 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.

[rnk]

In D34753#806164, @kkretzsch wrote:

In 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.

[rnk]

Hopefully rL307827 addresses the memory allocation consideration.

[kkretzsch]

In D34753#806738, @rnk wrote:

In D34753#806164, @kkretzsch wrote:

In 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 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 D34753#806906, @rnk wrote:

Hopefully rL307827 addresses the memory allocation consideration.

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

llvm/trunk/include/llvm/Support/Compiler.h

 #define LLVM_THREAD_LOCAL __thread
 #endif
 #else // !LLVM_ENABLE_THREADS
 // If threading is disabled entirely, this compiles to nothing and you get
 // a normal global variable.
 #define LLVM_THREAD_LOCAL
 #endif
 
+/// \macro LLVM_ENABLE_EXCEPTIONS
+/// \brief Whether LLVM is built with exception support.
+#if __has_feature(cxx_exceptions)
+#define LLVM_ENABLE_EXCEPTIONS 1
+#elif defined(__GNUC__) && defined(__EXCEPTIONS)
+#define LLVM_ENABLE_EXCEPTIONS 1
+#elif defined(_MSC_VER) && defined(_CPPUNWIND)
+#define LLVM_ENABLE_EXCEPTIONS 1
+#endif
+
 #endif

llvm/trunk/include/llvm/Support/ErrorHandling.h

LLVM_ATTRIBUTE_NORETURN void report_fatal_error(const char *reason,
                                                 bool gen_crash_diag = true);
 LLVM_ATTRIBUTE_NORETURN void report_fatal_error(const std::string &reason,
                                                 bool gen_crash_diag = true);
 LLVM_ATTRIBUTE_NORETURN void report_fatal_error(StringRef reason,
                                                 bool gen_crash_diag = true);
 LLVM_ATTRIBUTE_NORETURN void report_fatal_error(const Twine &reason,
                                                 bool gen_crash_diag = true);
 
-  /// This function calls abort(), and prints the optional message to stderr.
-  /// Use the llvm_unreachable macro (that adds location info), instead of
-  /// calling this function directly.
-  LLVM_ATTRIBUTE_NORETURN void
-  llvm_unreachable_internal(const char *msg=nullptr, const char *file=nullptr,
-                            unsigned line=0);
+/// Installs a new bad alloc error handler that should be used whenever a
+/// bad alloc error, e.g. failing malloc/calloc, is encountered by LLVM.
+///
+/// The user can install a bad alloc handler, in order to define the behavior
+/// in case of failing allocations, e.g. throwing an exception. Note that this
+/// handler must not trigger any additional allocations itself.
+///
+/// If no error handler is installed the default is to print the error message
+/// to stderr, and call exit(1).  If an error handler is installed then it is
+/// the handler's responsibility to log the message, it will no longer be
+/// printed to stderr.  If the error handler returns, then exit(1) will be
+/// called.
+///
+///
+/// \param user_data - An argument which will be passed to the installed error
+/// handler.
+void install_bad_alloc_error_handler(fatal_error_handler_t handler,
+                                     void *user_data = nullptr);
+
+/// Restores default bad alloc error handling behavior.
+void remove_bad_alloc_error_handler();
+
+/// Reports a bad alloc error, calling any user defined bad alloc
+/// error handler. In contrast to the generic 'report_fatal_error'
+/// functions, this function is expected to return, e.g. the user
+/// defined error handler throws an exception.
+///
+/// Note: When throwing an exception in the bad alloc handler, make sure that
+/// the following unwind succeeds, e.g. do not trigger additional allocations
+/// in the unwind chain.
+///
+/// If no error handler is installed (default), then a bad_alloc exception
+/// is thrown if LLVM is compiled with exception support, otherwise an assertion
+/// is called.
+void report_bad_alloc_error(const char *Reason, bool GenCrashDiag = true);
+
+llvm_unreachable_internal(const char *msg = nullptr, const char *file = nullptr,
+                          unsigned line = 0);
 }
 
 /// Marks that the current location is not supposed to be reachable.
 /// In !NDEBUG builds, prints the message and location info to stderr.
 /// In NDEBUG builds, becomes an optimizer hint that the current location
 /// is not supposed to be reachable.  On compilers that don't support
 /// such hints, prints a reduced message instead.
 ///

llvm/trunk/lib/Support/ErrorHandling.cpp

 #include "llvm/Support/Mutex.h"
 #include "llvm/Support/MutexGuard.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/Threading.h"
 #include "llvm/Support/WindowsError.h"
 #include "llvm/Support/raw_ostream.h"
 #include <cassert>
 #include <cstdlib>
+#include <new>
 
 #if defined(HAVE_UNISTD_H)
 # include <unistd.h>
 #endif
 #if defined(_MSC_VER)
 # include <io.h>
 # include <fcntl.h>
 #endif
 
 using namespace llvm;
 
 static fatal_error_handler_t ErrorHandler = nullptr;
 static void *ErrorHandlerUserData = nullptr;
-
 static ManagedStatic<sys::Mutex> ErrorHandlerMutex;
 
+static fatal_error_handler_t BadAllocErrorHandler = nullptr;
+static void *BadAllocErrorHandlerUserData = nullptr;
+static ManagedStatic<sys::Mutex> BadAllocErrorHandlerMutex;
+
 void llvm::install_fatal_error_handler(fatal_error_handler_t handler,
                                        void *user_data) {
   llvm::MutexGuard Lock(*ErrorHandlerMutex);
   assert(!ErrorHandler && "Error handler already registered!\n");
   ErrorHandler = handler;
   ErrorHandlerUserData = user_data;
 }
 
void llvm::report_fatal_error(const Twine &Reason, bool GenCrashDiag) {
   // If we reached here, we are failing ungracefully. Run the interrupt handlers
   // to make sure any special cleanups get done, in particular that we remove
   // files registered with RemoveFileOnSignal.
   sys::RunInterruptHandlers();
 
   exit(1);
 }
 
+void llvm::install_bad_alloc_error_handler(fatal_error_handler_t handler,
+                                           void *user_data) {
+  MutexGuard Lock(*BadAllocErrorHandlerMutex);
+  assert(!ErrorHandler && "Bad alloc error handler already registered!\n");
+  BadAllocErrorHandler = handler;
+  BadAllocErrorHandlerUserData = user_data;
+}
+
+void llvm::remove_bad_alloc_error_handler() {
+  MutexGuard Lock(*BadAllocErrorHandlerMutex);
+  BadAllocErrorHandler = nullptr;
+  BadAllocErrorHandlerUserData = nullptr;
+}
+
+void llvm::report_bad_alloc_error(const char *Reason, bool GenCrashDiag) {
+  fatal_error_handler_t Handler = nullptr;
+  void *HandlerData = nullptr;
+  {
+    // Only acquire the mutex while reading the handler, so as not to invoke a
+    // user-supplied callback under a lock.
+    MutexGuard Lock(*BadAllocErrorHandlerMutex);
+    Handler = BadAllocErrorHandler;
+    HandlerData = BadAllocErrorHandlerUserData;
+  }
+
+  if (Handler) {
+    Handler(HandlerData, Reason, GenCrashDiag);
+    llvm_unreachable("bad alloc handler should not return");
+  }
+
+#ifdef LLVM_ENABLE_EXCEPTIONS
+  // If exceptions are enabled, make OOM in malloc look like OOM in new.
+  throw std::bad_alloc();
+#else
+  // Otherwise, fall back to the normal fatal error handler.
+  report_fatal_error("out of memory: " + Twine(Reason));
+#endif
+}
+
 void llvm::llvm_unreachable_internal(const char *msg, const char *file,
                                      unsigned line) {
   // This code intentionally doesn't call the ErrorHandler callback, because
   // llvm_unreachable is intended to be used to indicate "impossible"
   // situations, and not legitimate runtime errors.
   if (msg)
     dbgs() << msg << "\n";
   dbgs() << "UNREACHABLE executed";

llvm/trunk/lib/Support/Mutex.cpp

 //===- Mutex.cpp - Mutual Exclusion Lock ------------------------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements the llvm::sys::Mutex class.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Support/Mutex.h"
+#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Config/config.h"
 
 //===----------------------------------------------------------------------===//
 //=== WARNING: Implementation here must contain only TRULY operating system
 //===          independent code.
 //===----------------------------------------------------------------------===//
 
 #if !defined(LLVM_ENABLE_THREADS) || LLVM_ENABLE_THREADS == 0

 
 // Construct a Mutex using pthread calls
 MutexImpl::MutexImpl( bool recursive)
   : data_(nullptr)
 {
   // Declare the pthread_mutex data structures
   pthread_mutex_t* mutex =
     static_cast<pthread_mutex_t*>(malloc(sizeof(pthread_mutex_t)));
+
+  if (mutex == nullptr)
+    report_bad_alloc_error("Mutex allocation failed");
+
   pthread_mutexattr_t attr;
 
   // Initialize the mutex attributes
   int errorcode = pthread_mutexattr_init(&attr);
   assert(errorcode == 0); (void)errorcode;
 
   // Initialize the mutex as a recursive mutex, if requested, or normal
   // otherwise.