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

[Igor Kudrin via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL283531: Recommit r282692: [libc++abi] Use fallback_malloc to 
allocate __cxa_eh_globals… (authored by ikudrin).

Changed prior to commit:
  https://reviews.llvm.org/D17815?vs=72951&id=73893#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D17815

Files:
  libcxxabi/trunk/src/CMakeLists.txt
  libcxxabi/trunk/src/cxa_exception.cpp
  libcxxabi/trunk/src/cxa_exception_storage.cpp
  libcxxabi/trunk/src/fallback_malloc.cpp
  libcxxabi/trunk/src/fallback_malloc.h
  libcxxabi/trunk/src/fallback_malloc.ipp
  libcxxabi/trunk/test/test_exception_storage_nodynmem.pass.cpp
  libcxxabi/trunk/test/test_fallback_malloc.pass.cpp

Index: libcxxabi/trunk/src/cxa_exception.cpp
===
--- libcxxabi/trunk/src/cxa_exception.cpp
+++ libcxxabi/trunk/src/cxa_exception.cpp
@@ -15,13 +15,10 @@
 #include "cxxabi.h"
 
 #include // for std::terminate
-#include   // for malloc, free
 #include   // for memset
-#ifndef _LIBCXXABI_HAS_NO_THREADS
-#  include   // 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 |
@@ -104,20 +101,6 @@
 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 
@@ -158,7 +141,8 @@
 //  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 *>(__malloc_with_fallback(actual_size));
 if (NULL == exception_header)
 std::terminate();
 std::memset(exception_header, 0, actual_size);
@@ -168,16 +152,16 @@
 
 //  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));
+__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 = __malloc_with_fallback(actual_size);
 if (NULL == ptr)
 std::terminate();
 std::memset(ptr, 0, actual_size);
@@ -188,7 +172,7 @@
 //  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);
+__free_with_fallback(dependent_exception);
 }
 
 
Index: libcxxabi/trunk/src/fallback_malloc.cpp
===
--- libcxxabi/trunk/src/fallback_malloc.cpp
+++ libcxxabi/trunk/src/fallback_malloc.cpp
@@ -0,0 +1,226 @@
+//=== fallback_malloc.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 "fallback_malloc.h"
+
+#include "config.h"
+
+#include  // for malloc, calloc, free
+#include  // for memset
+
+#ifndef _LIBCXXABI_HAS_NO_THREADS
+#include  // for mutexes
+#endif
+
+//  A small, simple heap manager based (loosely) on
+//  the startup heap manager from FreeBSD, optimized for space.
+//
+//  Manages a fixed-size memory pool, supports malloc and free only.
+//  No support for realloc.
+//
+//  Allocates chunks in multiples of four bytes, with a four byte header
+//  for each chunk. The overhead of each chunk is kept low by keeping pointers
+//  as two byte offsets within the heap, rather than (4 or 8 byte) pointers.
+
+namespace {
+
+// When POSIX threads are not available, make the mu

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

[Igor Kudrin via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL282692: [libc++abi] Use fallback_malloc to allocate 
__cxa_eh_globals in case of… (authored by ikudrin).

Changed prior to commit:
  https://reviews.llvm.org/D17815?vs=54189&id=72951#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D17815

Files:
  libcxxabi/trunk/src/CMakeLists.txt
  libcxxabi/trunk/src/cxa_exception.cpp
  libcxxabi/trunk/src/cxa_exception_storage.cpp
  libcxxabi/trunk/src/fallback_malloc.cpp
  libcxxabi/trunk/src/fallback_malloc.h
  libcxxabi/trunk/src/fallback_malloc.ipp
  libcxxabi/trunk/test/test_exception_storage_nodynmem.pass.cpp
  libcxxabi/trunk/test/test_fallback_malloc.pass.cpp

Index: libcxxabi/trunk/src/fallback_malloc.cpp
===
--- libcxxabi/trunk/src/fallback_malloc.cpp
+++ libcxxabi/trunk/src/fallback_malloc.cpp
@@ -0,0 +1,226 @@
+//=== fallback_malloc.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 "fallback_malloc.h"
+
+#include "config.h"
+
+#include  // for malloc, calloc, free
+#include  // for memset
+
+#ifndef _LIBCXXABI_HAS_NO_THREADS
+#include  // for mutexes
+#endif
+
+//  A small, simple heap manager based (loosely) on
+//  the startup heap manager from FreeBSD, optimized for space.
+//
+//  Manages a fixed-size memory pool, supports malloc and free only.
+//  No support for realloc.
+//
+//  Allocates chunks in multiples of four bytes, with a four byte header
+//  for each chunk. The overhead of each chunk is kept low by keeping pointers
+//  as two byte offsets within the heap, rather than (4 or 8 byte) pointers.
+
+namespace {
+
+// When POSIX threads are not available, make the mutex operations a nop
+#ifndef _LIBCXXABI_HAS_NO_THREADS
+static pthread_mutex_t heap_mutex = PTHREAD_MUTEX_INITIALIZER;
+#else
+static void * heap_mutex = 0;
+#endif
+
+class mutexor {
+public:
+#ifndef _LIBCXXABI_HAS_NO_THREADS
+mutexor ( pthread_mutex_t *m ) : mtx_(m) { pthread_mutex_lock ( mtx_ ); }
+~mutexor () { pthread_mutex_unlock ( mtx_ ); }
+#else
+mutexor ( void * ) {}
+~mutexor () {}
+#endif
+private:
+mutexor ( const mutexor &rhs );
+mutexor & operator = ( const mutexor &rhs );
+#ifndef _LIBCXXABI_HAS_NO_THREADS
+pthread_mutex_t *mtx_;
+#endif
+};
+
+
+static const size_t HEAP_SIZE = 512;
+char heap [ HEAP_SIZE ] __attribute__((aligned));
+
+typedef unsigned short heap_offset;
+typedef unsigned short heap_size;
+
+struct heap_node {
+heap_offset next_node;  // offset into heap
+heap_size   len;// size in units of "sizeof(heap_node)"
+};
+
+static const heap_node *list_end = (heap_node *) ( &heap [ HEAP_SIZE ] );   // one past the end of the heap
+static heap_node *freelist = NULL;
+
+heap_node *node_from_offset ( const heap_offset offset )
+{ return (heap_node *) ( heap + ( offset * sizeof (heap_node))); }
+
+heap_offset offset_from_node ( const heap_node *ptr )
+{ return static_cast(static_cast(reinterpret_cast(ptr) - heap)  / sizeof (heap_node)); }
+
+void init_heap () {
+freelist = (heap_node *) heap;
+freelist->next_node = offset_from_node ( list_end );
+freelist->len = HEAP_SIZE / sizeof (heap_node);
+}
+
+//  How big a chunk we allocate
+size_t alloc_size (size_t len)
+{ return (len + sizeof(heap_node) - 1) / sizeof(heap_node) + 1; }
+
+bool is_fallback_ptr ( void *ptr )
+{ return ptr >= heap && ptr < ( heap + HEAP_SIZE ); }
+
+void *fallback_malloc(size_t len) {
+heap_node *p, *prev;
+const size_t nelems = alloc_size ( len );
+mutexor mtx ( &heap_mutex );
+
+if ( NULL == freelist )
+init_heap ();
+
+//  Walk the free list, looking for a "big enough" chunk
+for (p = freelist, prev = 0;
+p && p != list_end; prev = p, p = node_from_offset ( p->next_node)) {
+
+if (p->len > nelems) {  //  chunk is larger, shorten, and return the tail
+heap_node *q;
+
+p->len = static_cast(p->len - nelems);
+q = p + p->len;
+q->next_node = 0;
+q->len = static_cast(nelems);
+return (void *) (q + 1);
+}
+
+if (p->len == nelems) { // exact size match
+if (prev == 0)
+freelist = node_from_offset(p->next_node);
+else
+prev->next_node = p->next_node;
+p->next_node = 0;
+return (void *) (p + 1);
+}
+}
+return NULL;// couldn't find a spot big enough
+}
+
+//  Return the start of the next block
+heap_node *after ( struct heap_node *p ) { return p + p->len; }
+
+void fallback_free (void *ptr) {
+struct heap_node *cp = ((str

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

[Eric Fiselier via cfe-commits]

EricWF accepted this revision.
EricWF added a comment.
This revision is now accepted and ready to land.

LGTM after addressing the inline comments.



Comment at: src/fallback_malloc.cpp:58
@@ +57,3 @@
+
+#define HEAP_SIZE   512
+char heap [ HEAP_SIZE ];

A static const variable would be better here.


Comment at: src/fallback_malloc.cpp:59
@@ +58,3 @@
+#define HEAP_SIZE   512
+char heap [ HEAP_SIZE ];
+

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


Comment at: src/fallback_malloc.h:20
@@ +19,3 @@
+// Allocate some memory from _somewhere_
+void * __cxa_malloc_with_fallback(size_t size);
+

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.


https://reviews.llvm.org/D17815



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Igor Kudrin via cfe-commits]

Hi Eric,

Ping...

On 19-Sep-16 13:57, Eric Fiselier wrote:


I'll look at these first thing next week. If you remember please ping 
me. (It's Cppcon this week)


Sorry I haven't reviewed this already. Thank you for working on this.


On Sep 18, 2016 11:55 PM, "Igor Kudrin" > wrote:


ikudrin added a comment.

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?


https://reviews.llvm.org/D17815 





___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Eric Fiselier via cfe-commits]

I'll look at these first thing next week. If you remember please ping me.
(It's Cppcon this week)

Sorry I haven't reviewed this already. Thank you for working on this.

On Sep 18, 2016 11:55 PM, "Igor Kudrin"  wrote:

> ikudrin added a comment.
>
> 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?
>
>
> https://reviews.llvm.org/D17815
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Igor Kudrin via cfe-commits]

ikudrin added a comment.

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?


https://reviews.llvm.org/D17815



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Igor Kudrin via cfe-commits]

ikudrin added a comment.

Ping...


https://reviews.llvm.org/D17815



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Igor Kudrin via cfe-commits]

ikudrin added a comment.

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?


https://reviews.llvm.org/D17815



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Igor Kudrin via cfe-commits]

ikudrin added a comment.

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?


http://reviews.llvm.org/D17815



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Igor Kudrin via cfe-commits]

ikudrin added a comment.

Ping.


http://reviews.llvm.org/D17815



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Igor Kudrin via cfe-commits]

ikudrin added a comment.

Ping?..


http://reviews.llvm.org/D17815



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Igor Kudrin via cfe-commits]

ikudrin added a comment.

Ping...


http://reviews.llvm.org/D17815



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Igor Kudrin via cfe-commits]

ikudrin updated this revision to Diff 54189.
ikudrin added a comment.

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


http://reviews.llvm.org/D17815

Files:
  src/CMakeLists.txt
  src/cxa_exception.cpp
  src/cxa_exception_storage.cpp
  src/fallback_malloc.cpp
  src/fallback_malloc.h
  src/fallback_malloc.ipp
  test/test_exception_storage_nodynmem.pass.cpp
  test/test_fallback_malloc.pass.cpp

Index: test/test_fallback_malloc.pass.cpp
===
--- test/test_fallback_malloc.pass.cpp
+++ test/test_fallback_malloc.pass.cpp
@@ -16,7 +16,7 @@
 
 // #define  DEBUG_FALLBACK_MALLOC
 #define INSTRUMENT_FALLBACK_MALLOC
-#include "../src/fallback_malloc.ipp"
+#include "../src/fallback_malloc.cpp"
 
 container alloc_series ( size_t sz ) {
 container ptrs;
Index: test/test_exception_storage_nodynmem.pass.cpp
===
--- /dev/null
+++ test/test_exception_storage_nodynmem.pass.cpp
@@ -0,0 +1,32 @@
+//===--- 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 
+#include 
+
+static bool OverwrittenCallocCalled = false;
+
+// Override calloc to simulate exhaustion of dynamic memory
+void *calloc(size_t, size_t) {
+OverwrittenCallocCalled = true;
+return 0;
+}
+
+int main(int argc, char *argv[]) {
+// Run the test a couple of times
+// to ensure that fallback memory doesn't leak.
+for (int I = 0; I < 1000; ++I)
+try {
+throw 42;
+} catch (...) {
+}
+
+assert(OverwrittenCallocCalled);
+return 0;
+}
Index: src/fallback_malloc.ipp
===
--- src/fallback_malloc.ipp
+++ /dev/null
@@ -1,188 +0,0 @@
-//=== 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.
-//
-//  
-//  This file implements the "Exception Handling APIs"
-//  http://mentorembedded.github.io/cxx-abi/abi-eh.html
-//  
-//===--===//
-
-#include "config.h"
-
-//  A small, simple heap manager based (loosely) on 
-//  the startup heap manager from FreeBSD, optimized for space.
-//
-//  Manages a fixed-size memory pool, supports malloc and free only.
-//  No support for realloc.
-//
-//  Allocates chunks in multiples of four bytes, with a four byte header
-//  for each chunk. The overhead of each chunk is kept low by keeping pointers
-//  as two byte offsets within the heap, rather than (4 or 8 byte) pointers.
-
-namespace {
-
-// When POSIX threads are not available, make the mutex operations a nop
-#if LIBCXXABI_HAS_NO_THREADS
-static void * heap_mutex = 0;
-#else
-static pthread_mutex_t heap_mutex = PTHREAD_MUTEX_INITIALIZER;
-#endif
-
-class mutexor {
-public:
-#if LIBCXXABI_HAS_NO_THREADS
-mutexor ( void * ) {}
-~mutexor () {}
-#else
-mutexor ( pthread_mutex_t *m ) : mtx_(m) { pthread_mutex_lock ( mtx_ ); }
-~mutexor () { pthread_mutex_unlock ( mtx_ ); }
-#endif
-private:
-mutexor ( const mutexor &rhs );
-mutexor & operator = ( const mutexor &rhs );
-#if !LIBCXXABI_HAS_NO_THREADS
-pthread_mutex_t *mtx_;
-#endif
-};
-
-
-#define HEAP_SIZE   512
-char heap [ HEAP_SIZE ];
-
-typedef unsigned short heap_offset;
-typedef unsigned short heap_size;
-
-struct heap_node {
-heap_offset next_node;  // offset into heap
-heap_size   len;// size in units of "sizeof(heap_node)"
-};
-
-static const heap_node *list_end = (heap_node *) ( &heap [ HEAP_SIZE ] );   // one past the end of the heap
-static heap_node *freelist = NULL;
-
-heap_node *node_from_offset ( const heap_offset offset )
-{ return (heap_node *) ( heap + ( offset * sizeof (heap_node))); }
-
-heap_offset offset_from_node ( const heap

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

[Eric Fiselier via cfe-commits]

EricWF added a comment.

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.



Comment at: src/fallback_malloc.cpp:23
@@ +22,3 @@
+namespace __cxxabiv1 {
+
+void * __cxa_malloc_with_fallback(size_t size) {

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


Comment at: test/test_exception_storage_nodynmem.pass.cpp:13
@@ +12,3 @@
+// Override calloc to simulate exhaustion of dynamic memory
+void *calloc(size_t, size_t) { return 0; }
+

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


Comment at: test/test_exception_storage_nodynmem.pass.cpp:16
@@ +15,3 @@
+int main(int argc, char *argv[]) {
+  try {
+throw 42;

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.


http://reviews.llvm.org/D17815



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Igor Kudrin via cfe-commits]

ikudrin updated this revision to Diff 54151.
ikudrin added a comment.

Regenerated the patch against project's root.


http://reviews.llvm.org/D17815

Files:
  src/CMakeLists.txt
  src/cxa_exception.cpp
  src/cxa_exception_storage.cpp
  src/fallback_malloc.cpp
  src/fallback_malloc.h
  test/test_exception_storage_nodynmem.pass.cpp

Index: test/test_exception_storage_nodynmem.pass.cpp
===
--- /dev/null
+++ test/test_exception_storage_nodynmem.pass.cpp
@@ -0,0 +1,21 @@
+//===--- 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 
+
+// 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;
+}
Index: src/fallback_malloc.h
===
--- /dev/null
+++ src/fallback_malloc.h
@@ -0,0 +1,29 @@
+//===- 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
Index: src/fallback_malloc.cpp
===
--- /dev/null
+++ src/fallback_malloc.cpp
@@ -0,0 +1,49 @@
+//=== 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  // for malloc, calloc, free
+#include  // for memset
+
+#if !LIBCXXABI_HAS_NO_THREADS
+#include  // 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
Index: src/cxa_exception_storage.cpp
===
--- src/cxa_exception_storage.cpp
+++ src/cxa_exception_storage.cpp
@@ -45,8 +45,8 @@
 #else
 
 #include 
-#include   // 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
@@ -58,7 +58,7 @@
 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()");
 }
@@ -77,7 +77,7 @@
 //  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 ) )
Index: src/cxa_exception.cpp
===
--- src/cxa_exception.cpp
+++ src/cxa_

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

[Eric Fiselier via cfe-commits]

EricWF added a comment.

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


http://reviews.llvm.org/D17815



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Igor Kudrin via cfe-commits]

ikudrin added a comment.

Ping! Could anyone check this patch, please?


http://reviews.llvm.org/D17815



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Igor Kudrin via cfe-commits]

ikudrin added a comment.

Ping...


http://reviews.llvm.org/D17815



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Igor Kudrin via cfe-commits]

ikudrin added a comment.

Ping...


http://reviews.llvm.org/D17815



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Igor Kudrin via cfe-commits]

ikudrin updated the summary for this revision.
ikudrin updated this revision to Diff 51782.
ikudrin added a comment.

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


http://reviews.llvm.org/D17815

Files:
  libcxxabi/trunk/src/CMakeLists.txt
  libcxxabi/trunk/src/cxa_exception.cpp
  libcxxabi/trunk/src/cxa_exception_storage.cpp
  libcxxabi/trunk/src/fallback_malloc.cpp
  libcxxabi/trunk/src/fallback_malloc.h
  libcxxabi/trunk/test/test_exception_storage_nodynmem.pass.cpp

Index: libcxxabi/trunk/test/test_exception_storage_nodynmem.pass.cpp
===
--- /dev/null
+++ libcxxabi/trunk/test/test_exception_storage_nodynmem.pass.cpp
@@ -0,0 +1,21 @@
+//===--- 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 
+
+// 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;
+}
Index: libcxxabi/trunk/src/fallback_malloc.h
===
--- /dev/null
+++ libcxxabi/trunk/src/fallback_malloc.h
@@ -0,0 +1,29 @@
+//===- 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
Index: libcxxabi/trunk/src/fallback_malloc.cpp
===
--- /dev/null
+++ libcxxabi/trunk/src/fallback_malloc.cpp
@@ -0,0 +1,49 @@
+//=== 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  // for malloc, calloc, free
+#include  // for memset
+
+#if !LIBCXXABI_HAS_NO_THREADS
+#include  // 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
Index: libcxxabi/trunk/src/cxa_exception_storage.cpp
===
--- libcxxabi/trunk/src/cxa_exception_storage.cpp
+++ libcxxabi/trunk/src/cxa_exception_storage.cpp
@@ -45,8 +45,8 @@
 #else
 
 #include 
-#include   // 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
@@ -58,7 +58,7 @@
 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()");
 }
@@ -77,7 +77,7 @@
 //  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

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

[Igor Kudrin via cfe-commits]

ikudrin added a comment.

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


http://reviews.llvm.org/D17815



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Eric Fiselier via cfe-commits]

EricWF added a comment.

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.


http://reviews.llvm.org/D17815



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Ben Craig via cfe-commits]

bcraig added a subscriber: bcraig.
bcraig added a comment.

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.


http://reviews.llvm.org/D17815



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Igor Kudrin via cfe-commits]

ikudrin added a comment.

Ping?..


http://reviews.llvm.org/D17815



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Igor Kudrin via cfe-commits]

ikudrin added a comment.

Ping.


http://reviews.llvm.org/D17815



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Igor Kudrin via cfe-commits]

ikudrin created this revision.
ikudrin added reviewers: mclow.lists, howard.hinnant, EricWF.
ikudrin added a subscriber: cfe-commits.

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

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

http://reviews.llvm.org/D17815

Files:
  libcxxabi/trunk/src/cxa_exception_storage.cpp
  libcxxabi/trunk/test/test_exception_storage_nodynmem.pass.cpp

Index: libcxxabi/trunk/test/test_exception_storage_nodynmem.pass.cpp
===
--- /dev/null
+++ libcxxabi/trunk/test/test_exception_storage_nodynmem.pass.cpp
@@ -0,0 +1,21 @@
+//===--- 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 
+
+// 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;
+}
Index: libcxxabi/trunk/src/cxa_exception_storage.cpp
===
--- libcxxabi/trunk/src/cxa_exception_storage.cpp
+++ libcxxabi/trunk/src/cxa_exception_storage.cpp
@@ -46,19 +46,38 @@
 
 #include 
 #include   // for calloc, free
+#include   // for memset
 #include "abort_message.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 {
+
+#include "fallback_malloc.ipp"
+
+//  Allocate some memory from _somewhere_
+static void *do_calloc(size_t count, size_t size) {
+void *ptr = std::calloc(count, size);
+if (NULL == 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;
+}
+
+static void do_free(void *ptr) {
+is_fallback_ptr(ptr) ? fallback_free(ptr) : std::free(ptr);
+}
+
 namespace {
 pthread_key_t  key_;
 pthread_once_t flag_ = PTHREAD_ONCE_INIT;
 
 void destruct_ (void *p) {
-std::free ( p );
+do_free ( p );
 if ( 0 != ::pthread_setspecific ( key_, NULL ) ) 
 abort_message("cannot zero out thread value for 
__cxa_get_globals()");
 }
@@ -77,7 +96,7 @@
 //  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)));
+(do_calloc (1, sizeof (__cxa_eh_globals)));
 if ( NULL == retVal )
 abort_message("cannot allocate __cxa_eh_globals");
 if ( 0 != pthread_setspecific ( key_, retVal ) )


Index: libcxxabi/trunk/test/test_exception_storage_nodynmem.pass.cpp
===
--- /dev/null
+++ libcxxabi/trunk/test/test_exception_storage_nodynmem.pass.cpp
@@ -0,0 +1,21 @@
+//===--- 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 
+
+// 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;
+}
Index: libcxxabi/trunk/src/cxa_exception_storage.cpp
===
--- libcxxabi/trunk/src/cxa_exception_storage.cpp
+++ libcxxabi/trunk/src/cxa_exception_storage.cpp
@@ -46,19 +46,38 @@
 
 #include 
 #include   // for calloc, free
+#include   // for memset
 #include "abort_message.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 {
+
+#include "fallback_malloc.ipp"
+
+//  Allocate some memory from _somewhere_
+static void *do_calloc(size_t count, size_t size) {
+void *ptr = std::calloc(count, size);
+if (NULL == ptr) { // if calloc fails, fall back to emergency stash
+ptr = fallback_malloc(size * count);
+if (NULL != ptr)
+std::memset