From aa02fb825675daca6eb9c89fd848b7bef7688404 Mon Sep 17 00:00:00 2001 From: Christian Helmuth Date: Thu, 23 Jan 2014 10:26:04 +0100 Subject: [PATCH] Revise delete with allocators Delete operators with additional allocator reference/pointer parameters are needed if the constructor of an 'new(allocator)' allocated object throws an exception. Also, destroy now uses the operator to free memory and provides variants with allocator reference and pointer. The commit includes a simple test scripts 'run/new_delete', which exercises the several 'delete' cases. Related to #1030. --- base/include/base/allocator.h | 49 +++++++----- base/run/new_delete.run | 79 ++++++++++++++++++++ base/src/base/cxx/new_delete.cc | 23 ++++-- base/src/test/new_delete/main.cc | 115 +++++++++++++++++++++++++++++ base/src/test/new_delete/target.mk | 3 + 5 files changed, 244 insertions(+), 25 deletions(-) create mode 100644 base/run/new_delete.run create mode 100644 base/src/test/new_delete/main.cc create mode 100644 base/src/test/new_delete/target.mk diff --git a/base/include/base/allocator.h b/base/include/base/allocator.h index 55a6da8c99..db4afd75ee 100644 --- a/base/include/base/allocator.h +++ b/base/include/base/allocator.h @@ -204,28 +204,21 @@ namespace Genode { /** * Destroy object * - * For destroying an object, we need to specify the allocator - * that was used by the object. Because we cannot pass the - * allocator directly to the delete operator, we mimic the - * delete operator using this template function. + * For destroying an object, we need to specify the allocator that was used + * by the object. Because we cannot pass the allocator directly to the + * delete expression, we mimic the expression by using this template + * function. The function explicitly calls the object destructor and + * operator delete afterwards. * - * \param T implicit object type + * For details see https://github.com/genodelabs/genode/issues/1030. * - * \param alloc allocator from which the object was allocated - * \param obj object to destroy + * \param T implicit object type + * + * \param dealloc reference or pointer to allocator from which the object + * was allocated + * \param obj object to destroy */ - template - void destroy(Deallocator *dealloc, T *obj) - { - if (!obj) - return; - - /* call destructors */ - obj->~T(); - - /* free memory at the allocator */ - dealloc->free(obj, sizeof(T)); - } + template void destroy(DEALLOC dealloc, T *obj); } void *operator new (Genode::size_t, Genode::Allocator *); @@ -260,6 +253,22 @@ void *operator new [] (Genode::size_t, Genode::Allocator &); * 'free()' function for the allocation of objects that may throw exceptions * at their construction time! */ -void operator delete (void *, Genode::Allocator *); +void operator delete (void *, Genode::Deallocator *); +void operator delete (void *, Genode::Deallocator &); + + +/* implemented here as it needs the special delete operators */ +template +void Genode::destroy(DEALLOC dealloc, T *obj) +{ + if (!obj) + return; + + /* call destructors */ + obj->~T(); + + /* free memory at the allocator */ + operator delete (obj, dealloc); +} #endif /* _INCLUDE__BASE__ALLOCATOR_H_ */ diff --git a/base/run/new_delete.run b/base/run/new_delete.run new file mode 100644 index 0000000000..f70cd4bb98 --- /dev/null +++ b/base/run/new_delete.run @@ -0,0 +1,79 @@ +build "core init test/new_delete" + +create_boot_directory + +install_config { + + + + + + + + + + + + +} + +build_boot_image "core init test-new_delete" + +append qemu_args "-nographic -m 64" + +run_genode_until {child exited with exit value 0} 15 + +grep_output {^\[init -> test-new_delete\]} + +compare_output_to { + [init -> test-new_delete] Allocator::alloc() + [init -> test-new_delete] A + [init -> test-new_delete] C + [init -> test-new_delete] B + [init -> test-new_delete] D + [init -> test-new_delete] E + [init -> test-new_delete] ~E + [init -> test-new_delete] ~D + [init -> test-new_delete] ~B + [init -> test-new_delete] ~C + [init -> test-new_delete] ~A + [init -> test-new_delete] Allocator::free() + [init -> test-new_delete] Allocator::alloc() + [init -> test-new_delete] A + [init -> test-new_delete] C + [init -> test-new_delete] B + [init -> test-new_delete] D + [init -> test-new_delete] E + [init -> test-new_delete] throw exception + [init -> test-new_delete] ~D + [init -> test-new_delete] ~B + [init -> test-new_delete] ~C + [init -> test-new_delete] ~A + [init -> test-new_delete] exception caught + [init -> test-new_delete] Allocator::alloc() + [init -> test-new_delete] A + [init -> test-new_delete] C + [init -> test-new_delete] B + [init -> test-new_delete] D + [init -> test-new_delete] E + [init -> test-new_delete] ~E + [init -> test-new_delete] ~D + [init -> test-new_delete] ~B + [init -> test-new_delete] ~C + [init -> test-new_delete] ~A + [init -> test-new_delete] Allocator::free() + [init -> test-new_delete] Allocator::alloc() + [init -> test-new_delete] A + [init -> test-new_delete] C + [init -> test-new_delete] B + [init -> test-new_delete] D + [init -> test-new_delete] E + [init -> test-new_delete] throw exception + [init -> test-new_delete] ~D + [init -> test-new_delete] ~B + [init -> test-new_delete] ~C + [init -> test-new_delete] ~A + [init -> test-new_delete] exception caught +} + +#puts "Test succeeded" diff --git a/base/src/base/cxx/new_delete.cc b/base/src/base/cxx/new_delete.cc index aea3749095..2145c02b77 100644 --- a/base/src/base/cxx/new_delete.cc +++ b/base/src/base/cxx/new_delete.cc @@ -13,9 +13,12 @@ #include #include +#include using Genode::size_t; using Genode::Allocator; +using Genode::Deallocator; +using Genode::sleep_forever; static void *try_alloc(Allocator *alloc, size_t size) @@ -33,17 +36,27 @@ void *operator new (size_t s, Allocator &a) { return a.alloc(s); } void *operator new [] (size_t s, Allocator &a) { return a.alloc(s); } -void operator delete (void *ptr, Allocator *alloc) +static void try_dealloc(void *ptr, Deallocator &dealloc) { /* - * Warn on the attempt to use an allocator that relies on the size - * argument. + * Log error and block on the attempt to use an allocator that relies on + * the size argument. */ - if (alloc->need_size_for_free()) - PERR("C++ runtime: delete called with unsafe allocator, leaking memory"); + if (dealloc.need_size_for_free()) { + PERR("C++ runtime: delete called with allocator, which needs " + "'size' on free. Blocking before leaking memory..."); + sleep_forever(); + } + + /* size not required, so call with dummy size */ + dealloc.free(ptr, 0); } +void operator delete (void *ptr, Deallocator *dealloc) { try_dealloc(ptr, *dealloc); } +void operator delete (void *ptr, Deallocator &dealloc) { try_dealloc(ptr, dealloc); } + + /* * The 'delete (void *)' operator gets referenced by compiler generated code, * so it must be publicly defined in the 'cxx' library. These compiler diff --git a/base/src/test/new_delete/main.cc b/base/src/test/new_delete/main.cc new file mode 100644 index 0000000000..4c4a0972e8 --- /dev/null +++ b/base/src/test/new_delete/main.cc @@ -0,0 +1,115 @@ +/* + * \brief Test dynamic memory management in C++ + * \author Christian Helmuth + * \date 2014-01-20 + * + */ + +/* + * Copyright (C) 2012-2014 Genode Labs GmbH + * + * This file is part of the Genode OS framework, which is distributed + * under the terms of the GNU General Public License version 2. + */ + +#include +#include + +using Genode::printf; +using Genode::destroy; + + +#define L() printf(" %s\n", __func__) + + +struct A { int a; A() { L(); } virtual ~A() { L(); } }; +struct B { int b; B() { L(); } virtual ~B() { L(); } }; +struct C : A { int c; C() { L(); } virtual ~C() { L(); } }; +struct D : B { int d; D() { L(); } virtual ~D() { L(); } }; + + +struct E : C, D +{ + int e; + + E(bool thro) { L(); if (thro) { printf("throw exception\n"); throw 1; } } + + virtual ~E() { L(); } +}; + + +struct Allocator : Genode::Allocator +{ + Genode::Allocator &heap = *Genode::env()->heap(); + + Allocator() { } + virtual ~Allocator() { } + + Genode::size_t consumed() override + { + return heap.consumed(); + } + + Genode::size_t overhead(Genode::size_t size) override + { + return heap.overhead(size); + } + + bool need_size_for_free() const override + { + return heap.need_size_for_free(); + } + + bool alloc(Genode::size_t size, void **p) override + { + *p = heap.alloc(size); + + printf("Allocator::alloc()\n"); + + return *p != 0; + } + + void free(void *p, Genode::size_t size) override + { + printf("Allocator::free()\n"); + heap.free(p, size); + } +}; + + +int main() +{ + Allocator a; + + /*********************** + ** Allocator pointer ** + ***********************/ + + /* test successful allocation / successful construction */ + { + E *e = new (&a) E(false); + destroy(&a, e); + } + + /* test successful allocation / exception in construction */ + try { + E *e = new (&a) E(true); + destroy(&a, e); + } catch (...) { printf("exception caught\n"); } + + /************************* + ** Allocator reference ** + *************************/ + + /* test successful allocation / successful construction */ + { + E *e = new (a) E(false); + destroy(&a, e); + } + + /* test successful allocation / exception in construction */ + try { + E *e = new (a) E(true); + destroy(&a, e); + } catch (...) { printf("exception caught\n"); } +} diff --git a/base/src/test/new_delete/target.mk b/base/src/test/new_delete/target.mk new file mode 100644 index 0000000000..edc4f8dd32 --- /dev/null +++ b/base/src/test/new_delete/target.mk @@ -0,0 +1,3 @@ +TARGET = test-new_delete +SRC_CC = main.cc +LIBS = base