[Pdbg] [PATCH v2 01/11] libpdbg: Validate hardware units at build time

[Amitay Isaacs]

From: Alistair Popple <alistair at popple.id.au>

Hardware unit structs such as struct htm contain a struct pdbg_target
which can be passed around to create an abstraction. Currently there is
a bit of extra runtime code used to guard against developers not
declaring the struct pdbg_target member as the first member. If we can
enforce this at build time then a some runtime can be removed.

This patch enforces that the struct pdbg_target member of hardware
units is always the first member at build time.

The only caveat is that compiling with -O0 will result in a false
negative - the build succeeding when it should not. We should be using
litterally any other optimisation level so that isn't really a problem.

Signed-off-by: Cyril Bur <cyrilbur at gmail.com>
---
 Makefile.am      |  2 +-
 libpdbg/target.c | 10 ++++------
 libpdbg/target.h | 16 ++++++++++++----
 3 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 4156b21..099a035 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -8,7 +8,7 @@ include libfdt/Makefile.libfdt
 bin_PROGRAMS = pdbg
 
 ACLOCAL_AMFLAGS = -Im4
-AM_CFLAGS = -I$(top_srcdir)/ccan/array_size -Wall -Werror
+AM_CFLAGS = -I$(top_srcdir)/ccan/array_size -Wall -Werror -O2
 
 pdbg_SOURCES = \
 	src/main.c src/cfam.c src/scom.c src/reg.c src/mem.c src/thread.c \
diff --git a/libpdbg/target.c b/libpdbg/target.c
index 1eb85bb..07159d2 100644
--- a/libpdbg/target.c
+++ b/libpdbg/target.c
@@ -254,7 +254,7 @@ struct hw_unit_info *find_compatible_target(const char *compat)
 	struct pdbg_target *target;
 
 	for (p = &__start_hw_units; p < (struct hw_unit_info **) &__stop_hw_units; p++) {
-		target = (*p)->hw_unit + (*p)->struct_target_offset;
+		target = (*p)->hw_unit;
 		if (!strcmp(target->compatible, compat))
 			return *p;
 	}
@@ -268,7 +268,6 @@ void pdbg_targets_init(void *fdt)
 	const struct dt_property *p;
 	struct pdbg_target_class *target_class;
 	struct hw_unit_info *hw_unit_info;
-	void *new_hw_unit;
 	struct pdbg_target *new_target;
 	uint32_t index;
 
@@ -283,10 +282,9 @@ void pdbg_targets_init(void *fdt)
 		hw_unit_info = find_compatible_target(p->prop);
 		if (hw_unit_info) {
 			/* We need to allocate a new target */
-			new_hw_unit = malloc(hw_unit_info->size);
-			assert(new_hw_unit);
-			memcpy(new_hw_unit, hw_unit_info->hw_unit, hw_unit_info->size);
-			new_target = new_hw_unit + hw_unit_info->struct_target_offset;
+			new_target = malloc(hw_unit_info->size);
+			assert(new_target);
+			memcpy(new_target, hw_unit_info->hw_unit, hw_unit_info->size);
 			new_target->dn = dn;
 			dn->target = new_target;
 			index = dt_prop_get_u32_def(dn, "index", -1);
diff --git a/libpdbg/target.h b/libpdbg/target.h
index 0a618e2..2bc7dba 100644
--- a/libpdbg/target.h
+++ b/libpdbg/target.h
@@ -18,6 +18,7 @@
 
 #include <stdint.h>
 #include <ccan/list/list.h>
+#include <ccan/str/str.h>
 #include <ccan/container_of/container_of.h>
 #include "compiler.h"
 #include "device.h"
@@ -60,16 +61,23 @@ extern struct list_head empty_list;
 struct hw_unit_info {
 	void *hw_unit;
 	size_t size;
-	size_t struct_target_offset;
 };
 
 /* We can't pack the structs themselves directly into a special
  * section because there doesn't seem to be any standard way of doing
  * that due to alignment rules. So instead we pack pointers into a
- * special section. */
+ * special section.
+ *
+ * If this macro fails to compile for you, you've probably not
+ * declared the struct pdbg_target as the first member of the
+ * container struct. Not doing so will break other assumptions.
+ * */
 #define DECLARE_HW_UNIT(name)						\
-	const struct hw_unit_info __used name ##_hw_unit = \
-	{ .hw_unit = &name, .size = sizeof(name), .struct_target_offset = container_off(typeof(name), target) }; \
+	static inline void name ##_hw_unit_check(void) {		\
+		((void)sizeof(char[1 - 2 * (container_off(typeof(name), target) != 0)])); \
+	}								\
+	const struct hw_unit_info __used name ##_hw_unit =              \
+	{ .hw_unit = &name, .size = sizeof(name) }; 	                \
 	const struct hw_unit_info __used __section("hw_units") *name ##_hw_unit_p = &name ##_hw_unit
 
 struct htm {
-- 
2.14.3