From b35df4578b2ad1b94457cd4930123bcbeb1c04f7 Mon Sep 17 00:00:00 2001
From: Christian Helmuth <christian.helmuth@genode-labs.com>
Date: Tue, 14 Mar 2017 11:51:15 +0100
Subject: [PATCH] Check Registered base class provides virtual destructor

The base class of Registered must provide a virtual destructor to enable
safe deletion with just a base class pointer. This requirement can be
lifted by using Registered_no_delete in places where the deletion
property is not needed.

Fixes #2331
---
 repos/base/include/base/registry.h            | 25 +++++++++++++++++++
 repos/base/src/lib/cxx/guard.cc               |  6 ++---
 repos/os/src/app/cli_monitor/line_editor.h    |  2 --
 repos/os/src/app/cli_monitor/main.cc          |  1 -
 repos/os/src/server/input_filter/connection.h |  2 ++
 repos/os/src/test/timer/main.cc               |  2 ++
 6 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/repos/base/include/base/registry.h b/repos/base/include/base/registry.h
index 75af643adc..50561821e3 100644
--- a/repos/base/include/base/registry.h
+++ b/repos/base/include/base/registry.h
@@ -22,6 +22,7 @@ namespace Genode {
 	class Registry_base;
 	template <typename T> struct Registry;
 	template <typename T> class Registered;
+	template <typename T> class Registered_no_delete;
 }
 
 
@@ -157,9 +158,33 @@ class Genode::Registered : public T
 
 	public:
 
+		static_assert(__has_virtual_destructor(T), "registered object must have virtual destructor");
+
 		template <typename... ARGS>
 		Registered(Registry<Registered<T> > &registry, ARGS &&... args)
 		: T(args...), _element(registry, *this) { }
 };
 
+
+/**
+ * Variant of Registered that does not require a vtable in the base class
+ *
+ * The generic Registered convenience class requires the base class to provide
+ * a vtable resp. a virtual destructor for safe deletion of a base class
+ * pointer. By using Registered_no_delete this requirement can be lifted.
+ */
+template <typename T>
+class Genode::Registered_no_delete : public T
+{
+	private:
+
+		typename Registry<Registered_no_delete<T> >::Element _element;
+
+	public:
+
+		template <typename... ARGS>
+		Registered_no_delete(Registry<Registered_no_delete<T> > &registry, ARGS &&... args)
+		: T(args...), _element(registry, *this) { }
+};
+
 #endif /* _INCLUDE__BASE__REGISTRY_H_ */
diff --git a/repos/base/src/lib/cxx/guard.cc b/repos/base/src/lib/cxx/guard.cc
index 87cb6af98f..8f6b3a5bc1 100644
--- a/repos/base/src/lib/cxx/guard.cc
+++ b/repos/base/src/lib/cxx/guard.cc
@@ -17,7 +17,7 @@
 #include <cpu/atomic.h>
 
 
-static Genode::Registry<Genode::Registered<Genode::Semaphore> > blocked;
+static Genode::Registry<Genode::Registered_no_delete<Genode::Semaphore> > blocked;
 
 
 namespace __cxxabiv1 
@@ -54,7 +54,7 @@ namespace __cxxabiv1
 		if (!Genode::cmpxchg(in_init, INIT_NONE, IN_INIT)) {
 
 			/* register current thread for blocking */
-			Genode::Registered<Genode::Semaphore> block(blocked);
+			Genode::Registered_no_delete<Genode::Semaphore> block(blocked);
 
 			/* tell guard thread that current thread needs a wakeup */
 			while (!Genode::cmpxchg(in_init, *in_init, *in_init | WAITERS)) ;
@@ -88,7 +88,7 @@ namespace __cxxabiv1
 			return;
 
 		/* we had contention - wake all up */
-		blocked.for_each([](Genode::Registered<Genode::Semaphore> &wake) {
+		blocked.for_each([](Genode::Registered_no_delete<Genode::Semaphore> &wake) {
 			wake.up();
 		});
 	}
diff --git a/repos/os/src/app/cli_monitor/line_editor.h b/repos/os/src/app/cli_monitor/line_editor.h
index bcfa87e459..2068b4d0e7 100644
--- a/repos/os/src/app/cli_monitor/line_editor.h
+++ b/repos/os/src/app/cli_monitor/line_editor.h
@@ -16,7 +16,6 @@
 
 /* Genode includes */
 #include <base/printf.h>
-#include <base/registry.h>
 #include <terminal_session/connection.h>
 #include <util/string.h>
 #include <util/list.h>
@@ -26,7 +25,6 @@
 
 namespace Cli_monitor {
 
-	using Genode::Registry;
 	using Genode::List;
 	using Genode::max;
 	using Genode::strlen;
diff --git a/repos/os/src/app/cli_monitor/main.cc b/repos/os/src/app/cli_monitor/main.cc
index 398991ff3b..184ba3acb6 100644
--- a/repos/os/src/app/cli_monitor/main.cc
+++ b/repos/os/src/app/cli_monitor/main.cc
@@ -13,7 +13,6 @@
 
 /* Genode includes */
 #include <base/attached_rom_dataspace.h>
-#include <base/registry.h>
 #include <vfs/file_system_factory.h>
 #include <vfs/dir_file_system.h>
 #include <base/component.h>
diff --git a/repos/os/src/server/input_filter/connection.h b/repos/os/src/server/input_filter/connection.h
index a9a0c8dcc2..098913f98e 100644
--- a/repos/os/src/server/input_filter/connection.h
+++ b/repos/os/src/server/input_filter/connection.h
@@ -63,6 +63,8 @@ class Input_filter::Input_connection
 			_connection.sigh(_input_handler);
 		}
 
+		virtual ~Input_connection() { }
+
 		Session_label label() const { return _label; }
 
 		template <typename FUNC>
diff --git a/repos/os/src/test/timer/main.cc b/repos/os/src/test/timer/main.cc
index c809462fa5..cc2d0badda 100644
--- a/repos/os/src/test/timer/main.cc
+++ b/repos/os/src/test/timer/main.cc
@@ -79,6 +79,8 @@ struct Stress_test
 		: timer_handler(env.ep(), *this, &Slave::handle_timer),
 		  timer(env), us(ms * 1000) { timer.sigh(timer_handler); }
 
+		virtual ~Slave() { }
+
 		void handle_timer()
 		{
 			count++;