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

@@ -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 <typename T>
-	void destroy(Deallocator *dealloc, T *obj)
-	{
-		if (!obj)
-			return;
-
-		/* call destructors */
-		obj->~T();
-
-		/* free memory at the allocator */
-		dealloc->free(obj, sizeof(T));
-	}
+	template <typename T, typename DEALLOC> 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 <typename T, typename DEALLOC>
+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_ */

base/run/new_delete.run

@@ -0,0 +1,79 @@
+build "core init test/new_delete"
+
+create_boot_directory
+
+install_config {
+	<config>
+		<parent-provides>
+			<service name="LOG"/>
+			<service name="RM"/>
+		</parent-provides>
+		<default-route>
+			<any-service> <parent/> </any-service>
+		</default-route>
+		<start name="test-new_delete">
+			<resource name="RAM" quantum="10M"/>
+		</start>
+	</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"

base/src/base/cxx/new_delete.cc

@@ -13,9 +13,12 @@
 
 #include <base/printf.h>
 #include <base/allocator.h>
+#include <base/sleep.h>
 
 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

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 <base/printf.h>
+#include <base/env.h>
+
+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"); }
+}

base/src/test/new_delete/target.mk

@@ -0,0 +1,3 @@
+TARGET = test-new_delete
+SRC_CC = main.cc
+LIBS   = base