[libc++abi] Use fallback_malloc to allocate __cxa_eh_globals in case of dynamic memory exhaustion.

[ikudrin]

Summary

Throwing an exception for the first time may lead to calling calloc to allocate memory for __cxa_eh_globals. If the memory pool is exhausted at that moment, it results in abnormal termination of the program.

This patch addresses the issue by using fallback_malloc in that case.

[ikudrin]

Ping.

[ikudrin]

Ping?..

[bcraig]

LGTM. I don't know if that's good enough for a submit though :)

In my opinion, requiring heap allocation during an exception is one of the worst parts of the Itanium ABI. At least this particular path isn't triggered for platforms with true thread_local support.

[EricWF]

By including fallback_malloc.ipp in a new C++ file we now have 2 different OOM emergancy buffers. I think it would be preferable to use only one. This would require a fairly large restructuring of fallback_malloc.ipp though.

[ikudrin]

Thanks! I'll see what I can do about it.

[ikudrin]

Make cxa_exception.cpp and cxa_exception_storage.cpp to use the same emergency memory pool.

[ikudrin]

Ping...

[ikudrin]

Ping...

[ikudrin]

Ping! Could anyone check this patch, please?

[EricWF]

Could you generate a diff against the normal directory layout? Your's is against "libcxxabi/trunk/" where 'trunk' is the actual libcxxabi directory.

[ikudrin]

Regenerated the patch against project's root.

[EricWF]

It's almost there.

Please move "fallback_malloc.ipp" into "fallback_malloc.cpp" and then delete it all together. We can't have other files trying to include it.

src/fallback_malloc.cpp

23 ↗

(On Diff #54151)

I think we should have the pragma GCC visibility push(hidden) in this file as well, but I'm not 100% sure.

test/test_exception_storage_nodynmem.pass.cpp

13 ↗

(On Diff #54151)

Let's check that we actually replace calloc here and assert that our replacement has been called at the end of main.

16 ↗

(On Diff #54151)

Let's perform this test a couple of times so we're testing

• The memory get's returned properly
• The returned memory can be reused.

[ikudrin]

• Moved the content of fallback_malloc.ipp into falback_malloc.cpp.
• Removed fallback_malloc.ipp.
• Added pragma GCC visibility push(hidden) for the function's definitions.
• Added a check that the overwritten calloc is called to the test.
• Added a loop to run the test several times.

Please note that test_fallback_malloc.pass.cpp now has to #include fallback_malloc.cpp, because fallback_malloc.ipp was removed. I'd considered some other options, including declaring additional functions especially for testing purpose and even moving all fallback_malloc functionality into a separate class which could be tested independently. However, eventually, I found them more difficult and even much worse than the presented approach.

[ikudrin]

Ping...

[ikudrin]

Ping?..

[ikudrin]

Ping.

[ikudrin]

Ping...

This patch is under review for more than two months, and I believe I've addressed all the comments. Could someone take a look at it, please?

[ikudrin]

Hi,

Ping.

The issue in the current implementation of the libc++abi library looks like a time bomb. Even though its probability could be considered as very low, in fact, it depends on the type of an application. At least, we ran into it in our environment.

I'd guess that no one would expect their program to terminate when a legal exception is thrown, or when memory is allocated. I'd like this issue to be fixed in one or another way.

@EricWF, can you look at the patch again, please?

[ikudrin]

Ping...

[ikudrin]

Hi,

Ping.

The patch is under review for a long time and the described problem is still here. As we've seen the issue in practice, I'm sure that others may also run into it, and I do believe it should be fixed, one way or another.

@EricWF, can I improve the patch somehow to pass the review? Or maybe you prefer it to be fixed in some other way?

[EricWF]

LGTM after addressing the inline comments.

src/fallback_malloc.cpp

58 ↗

(On Diff #54189)

A static const variable would be better here.

59 ↗

(On Diff #54189)

This should have __attribute__((aligned)) on it.

src/fallback_malloc.h

20 ↗

(On Diff #54189)

Please don't use the __cxa prefix on these functions. It makes it seem like they are part of the Itanium spec but they're really internal details.

[rmaprath]

@ikudrin: Looks like you've reverted this soon after. I'm just about to commit D24864, which will affect this slightly. D24864 basically refactors all pthread dependencies behind a separate API. It would be pretty straightforward for you to update this patch though, just replacing pthread calls with ones in thread_support.h (perhaps adding anything missing).

Hope you don't mind me going first? If you are going to commit this soon, I can hold off D24864. Let me know.

Cheers,

/ Asiri

[ikudrin]

@rmaprath: There are some issues in the test which upset build bots and I need some time to fix them. You don't need to wait for me, I'll put my patch on the top of yours when my fix is ready.

libcxxabi/trunk/src/CMakeLists.txt

 # Get sources
 set(LIBCXXABI_SOURCES
   abort_message.cpp
   cxa_aux_runtime.cpp
   cxa_default_handlers.cpp
   cxa_demangle.cpp
   cxa_exception.cpp
   cxa_exception_storage.cpp
   cxa_guard.cpp
   cxa_handlers.cpp
   cxa_new_delete.cpp
   cxa_personality.cpp
   cxa_unexpected.cpp
   cxa_vector.cpp
   cxa_virtual.cpp
   exception.cpp
+  fallback_malloc.cpp
   private_typeinfo.cpp
   stdexcept.cpp
   typeinfo.cpp
 )
 
 if (UNIX AND NOT (APPLE OR CYGWIN))
   list(APPEND LIBCXXABI_SOURCES cxa_thread_atexit.cpp)
 endif()

libcxxabi/trunk/src/cxa_exception.cpp

 //  http://mentorembedded.github.io/cxx-abi/abi-eh.html
 //  
 //===----------------------------------------------------------------------===//
 
 #include "config.h"
 #include "cxxabi.h"
 
 #include <exception>        // for std::terminate
-#include <cstdlib>          // for malloc, free
 #include <cstring>          // for memset
-#if !LIBCXXABI_HAS_NO_THREADS
-#  include <pthread.h>      // for fallback_malloc.ipp's mutexes
-#endif
 #include "cxa_exception.hpp"
 #include "cxa_handlers.hpp"
+#include "fallback_malloc.h"
 
 // +---------------------------+-----------------------------+---------------+
 // | __cxa_exception           | _Unwind_Exception CLNGC++\0 | thrown object |
 // +---------------------------+-----------------------------+---------------+
 //                                                           ^
 //                                                           |
 //   +-------------------------------------------------------+
 //   |
static inline int incrementHandlerCount(__cxa_exception *exception) {
     return ++exception->handlerCount;
 }
 
 //  This does not need to be atomic
 static inline  int decrementHandlerCount(__cxa_exception *exception) {
     return --exception->handlerCount;
 }
 
-#include "fallback_malloc.ipp"
-
-//  Allocate some memory from _somewhere_
-static void *do_malloc(size_t size) {
-    void *ptr = std::malloc(size);
-    if (NULL == ptr) // if malloc fails, fall back to emergency stash
-        ptr = fallback_malloc(size);
-    return ptr;
-}
-
-static void do_free(void *ptr) {
-    is_fallback_ptr(ptr) ? fallback_free(ptr) : std::free(ptr);
-}
-
 /*
     If reason isn't _URC_FOREIGN_EXCEPTION_CAUGHT, then the terminateHandler
     stored in exc is called.  Otherwise the exceptionDestructor stored in 
     exc is called, and then the memory for the exception is deallocated.
 
     This is never called for a __cxa_dependent_exception.
 */
 static

 
 //  Allocate a __cxa_exception object, and zero-fill it.
 //  Reserve "thrown_size" bytes on the end for the user's exception
 //  object. Zero-fill the object. If memory can't be allocated, call
 //  std::terminate. Return a pointer to the memory to be used for the
 //  user's exception object.
 _LIBCXXABI_FUNC_VIS void *__cxa_allocate_exception(size_t thrown_size) throw() {
     size_t actual_size = cxa_exception_size_from_exception_thrown_size(thrown_size);
-    __cxa_exception* exception_header = static_cast<__cxa_exception*>(do_malloc(actual_size));
+    __cxa_exception *exception_header =
+        static_cast<__cxa_exception *>(__cxa_malloc_with_fallback(actual_size));
     if (NULL == exception_header)
         std::terminate();
     std::memset(exception_header, 0, actual_size);
     return thrown_object_from_cxa_exception(exception_header);
 }
 
 
 //  Free a __cxa_exception object allocated with __cxa_allocate_exception.
 _LIBCXXABI_FUNC_VIS void __cxa_free_exception(void *thrown_object) throw() {
-    do_free(cxa_exception_from_thrown_object(thrown_object));
+    __cxa_free_with_fallback(cxa_exception_from_thrown_object(thrown_object));
 }
 
 
 //  This function shall allocate a __cxa_dependent_exception and
 //  return a pointer to it. (Really to the object, not past its' end).
 //  Otherwise, it will work like __cxa_allocate_exception.
 void * __cxa_allocate_dependent_exception () {
     size_t actual_size = sizeof(__cxa_dependent_exception);
-    void *ptr = do_malloc(actual_size);
+    void *ptr = __cxa_malloc_with_fallback(actual_size);
     if (NULL == ptr)
         std::terminate();
     std::memset(ptr, 0, actual_size);
     return ptr;
 }
 
 
 //  This function shall free a dependent_exception.
 //  It does not affect the reference count of the primary exception.
 void __cxa_free_dependent_exception (void * dependent_exception) {
-    do_free(dependent_exception);
+    __cxa_free_with_fallback(dependent_exception);
 }
 
 
 // 2.4.3 Throwing the Exception Object
 /*
 After constructing the exception object with the throw argument value,
 the generated code calls the __cxa_throw runtime library routine. This
 routine never returns.

libcxxabi/trunk/src/cxa_exception_storage.cpp

extern "C" {
     __cxa_eh_globals * __cxa_get_globals      () { return __globals (); }
     __cxa_eh_globals * __cxa_get_globals_fast () { return __globals (); }
     }
 }
 
 #else
 
 #include <pthread.h>
-#include <cstdlib>          // for calloc, free
 #include "abort_message.h"
+#include "fallback_malloc.h"
 
 //  In general, we treat all pthread errors as fatal.
 //  We cannot call std::terminate() because that will in turn
 //  call __cxa_get_globals() and cause infinite recursion.
 
 namespace __cxxabiv1 {
 namespace {
     pthread_key_t  key_;
     pthread_once_t flag_ = PTHREAD_ONCE_INIT;
 
     void destruct_ (void *p) {
-        std::free ( p );
+        __cxa_free_with_fallback ( p );
         if ( 0 != ::pthread_setspecific ( key_, NULL ) ) 
             abort_message("cannot zero out thread value for __cxa_get_globals()");
         }
 
     void construct_ () {
         if ( 0 != pthread_key_create ( &key_, destruct_ ) )
             abort_message("cannot create pthread key for __cxa_get_globals()");
         }
 }   
 
 extern "C" {
     __cxa_eh_globals * __cxa_get_globals () {
     //  Try to get the globals for this thread
         __cxa_eh_globals* retVal = __cxa_get_globals_fast ();
     
     //  If this is the first time we've been asked for these globals, create them
         if ( NULL == retVal ) {
             retVal = static_cast<__cxa_eh_globals*>
-                        (std::calloc (1, sizeof (__cxa_eh_globals)));
+                        (__cxa_calloc_with_fallback (1, sizeof (__cxa_eh_globals)));
             if ( NULL == retVal )
                 abort_message("cannot allocate __cxa_eh_globals");
             if ( 0 != pthread_setspecific ( key_, retVal ) )
                abort_message("pthread_setspecific failure in __cxa_get_globals()");
            }
         return retVal;
         }
 

libcxxabi/trunk/src/fallback_malloc.h

+//===------------------------- fallback_malloc.h --------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef _FALLBACK_MALLOC_H
+#define _FALLBACK_MALLOC_H
+
+namespace __cxxabiv1 {
+
+#pragma GCC visibility push(hidden)
+
+// Allocate some memory from _somewhere_
+void * __cxa_malloc_with_fallback(size_t size);
+
+// Allocate and zero-initialize memory from _somewhere_
+void * __cxa_calloc_with_fallback(size_t count, size_t size);
+
+void __cxa_free_with_fallback(void *ptr);
+
+#pragma GCC visibility pop
+
+} // namespace __cxxabiv1
+
+#endif

libcxxabi/trunk/src/fallback_malloc.cpp

+//===------------------------ fallback_malloc.ipp -------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "config.h"
+
+#include <cstdlib> // for malloc, calloc, free
+#include <cstring> // for memset
+
+#if !LIBCXXABI_HAS_NO_THREADS
+#include <pthread.h> // for fallback_malloc.ipp's mutexes
+#endif
+
+#include "fallback_malloc.h"
+#include "fallback_malloc.ipp"
+
+namespace __cxxabiv1 {
+
+void * __cxa_malloc_with_fallback(size_t size) {
+    void *ptr = std::malloc(size);
+    if (NULL == ptr) // if malloc fails, fall back to emergency stash
+        ptr = fallback_malloc(size);
+    return ptr;
+}
+
+void * __cxa_calloc_with_fallback(size_t count, size_t size) {
+    void *ptr = std::calloc(count, size);
+    if (NULL != ptr)
+        return ptr;
+    // if calloc fails, fall back to emergency stash
+    ptr = fallback_malloc(size * count);
+    if (NULL != ptr)
+        std::memset(ptr, 0, size * count);
+    return ptr;
+}
+
+void __cxa_free_with_fallback(void *ptr) {
+    if (is_fallback_ptr(ptr))
+        fallback_free(ptr);
+    else
+        std::free(ptr);
+}
+
+} // namespace __cxxabiv1

libcxxabi/trunk/test/test_exception_storage_nodynmem.pass.cpp

+//===--------------- test_exception_storage_nodynmem.cpp ------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include <cstdlib>
+
+// Override calloc to simulate exhaustion of dynamic memory
+void *calloc(size_t, size_t) { return 0; }
+
+int main(int argc, char *argv[]) {
+  try {
+    throw 42;
+  } catch (...) {
+  }
+  return 0;
+}