Check struct sizes on the CI (in code) instead of during compile time (using the preprocessor)

Signed-off-by: DL6ER <dl6er@dl6er.de>

src/CMakeLists.txt

@@ -147,7 +147,8 @@ set(sources
         shmem.h
         signals.c
         signals.h
-        static_assert.h
+        struct_size.c
+        struct_size.h
         timers.c
         timers.h
         vector.c

src/args.c

@@ -354,6 +354,12 @@ void parse_args(int argc, char* argv[])
 			exit(EXIT_SUCCESS);
 		}
 
+		// Return number of errors on this undocumented flag
+		if(strcmp(argv[i], "--check-structs") == 0)
+		{
+			exit(check_struct_sizes());
+		}
+
 		// Complain if invalid options have been found
 		if(!ok)
 		{

src/args.h

@@ -24,4 +24,7 @@ const char *cli_done(void) __attribute__ ((const));
 const char *cli_bold(void) __attribute__ ((const));
 const char *cli_normal(void) __attribute__ ((const));
 
+// defined in dnsmasq_interface.c
+int check_struct_sizes(void);
+
 #endif //ARGS_H

src/config.h

@@ -17,10 +17,12 @@
 #include <sys/types.h>
 // typedef uni32_t
 #include <stdint.h>
-// assert_sizeof
-#include "static_assert.h"
 // struct in_addr, in6_addr
 #include <netinet/in.h>
+// type bool
+#include <stdbool.h>
+// type FILE
+#include <stdio.h>
 
 void init_config_mutex(void);
 void getLogFilePath(void);
@@ -91,7 +93,6 @@ typedef struct {
 		} ip_blocking;
 	} reply_addr;
 } ConfigStruct;
-ASSERT_SIZEOF(ConfigStruct, 112, 104, 104);
 
 typedef struct {
 	const char* conf;

src/datastructure.h

@@ -15,8 +15,9 @@
 
 // enum privacy_level
 #include "enums.h"
-// assert_sizeof
-#include "static_assert.h"
+
+// Definitions like OVERTIME_SLOT
+#include "FTL.h"
 
 extern const char *querytypes[TYPE_MAX];
 
@@ -51,9 +52,6 @@ typedef struct {
 	} flags;
 } queriesData;
 
-// ARM needs extra padding at the end
-ASSERT_SIZEOF(queriesData, 56, 44, 44);
-
 typedef struct {
 	unsigned char magic;
 	bool new;
@@ -64,7 +62,6 @@ typedef struct {
 	size_t namepos;
 	time_t lastQuery;
 } upstreamsData;
-ASSERT_SIZEOF(upstreamsData, 640, 624, 624);
 
 typedef struct {
 	unsigned char magic;
@@ -91,7 +88,6 @@ typedef struct {
 	time_t lastQuery;
 	time_t firstSeen;
 } clientsData;
-ASSERT_SIZEOF(clientsData, 696, 668, 668);
 
 typedef struct {
 	unsigned char magic;
@@ -100,7 +96,6 @@ typedef struct {
 	uint32_t domainhash;
 	size_t domainpos;
 } domainsData;
-ASSERT_SIZEOF(domainsData, 24, 20, 20);
 
 typedef struct {
 	unsigned char magic;
@@ -111,7 +106,6 @@ typedef struct {
 	int clientID;
 	int black_regex_idx;
 } DNSCacheData;
-ASSERT_SIZEOF(DNSCacheData, 16, 16, 16);
 
 void strtolower(char *str);
 uint32_t hashStr(const char *s) __attribute__((const));

src/dnsmasq_interface.c

@@ -48,6 +48,10 @@
 #include "api/api_helper.h"
 // logg_rate_limit_message()
 #include "database/message-table.h"
+// type struct sqlite3_stmt_vec
+#include "vector.h"
+// check_one_struct()
+#include "struct_size.h"
 
 // Private prototypes
 static void print_flags(const unsigned int flags);
@@ -3335,3 +3339,28 @@ static void _query_set_dnssec(queriesData *query, const enum dnssec_status dnsse
 	// Set DNSSEC status
 	query->dnssec = dnssec;
 }
+
+// Check sizes of all important in-memory objects. This routine returns the number of
+// errors found (i.e., a return value of 0 is what we want and expect)
+int check_struct_sizes(void)
+{
+	int result = 0;
+	result += check_one_struct("ConfigStruct", sizeof(ConfigStruct), 112, 104, 104);
+	result += check_one_struct("queriesData", sizeof(queriesData), 56, 44, 44);
+	result += check_one_struct("upstreamsData", sizeof(upstreamsData), 640, 624, 624);
+	result += check_one_struct("clientsData", sizeof(clientsData), 696, 668, 668);
+	result += check_one_struct("domainsData", sizeof(domainsData), 24, 20, 20);
+	result += check_one_struct("DNSCacheData", sizeof(DNSCacheData), 16, 16, 16);
+	result += check_one_struct("ednsData", sizeof(ednsData), 72, 72, 72);
+	result += check_one_struct("overTimeData", sizeof(overTimeData), 32, 24, 24);
+	result += check_one_struct("regexData", sizeof(regexData), 56, 44, 44);
+	result += check_one_struct("SharedMemory", sizeof(SharedMemory), 24, 12, 12);
+	result += check_one_struct("ShmSettings", sizeof(ShmSettings), 12, 12, 12);
+	result += check_one_struct("countersStruct", sizeof(countersStruct), 240, 240, 240);
+	result += check_one_struct("sqlite3_stmt_vec", sizeof(sqlite3_stmt_vec), 32, 16, 16);
+
+	if(result == 0)
+		printf("All okay\n");
+
+	return result;
+}

src/edns0.h

@@ -10,9 +10,6 @@
 #ifndef EDNS0_HEADER
 #define EDNS0_HEADER
 
-// assert_sizeof
-#include "static_assert.h"
-
 typedef struct {
 	bool client_set;
 	bool mac_set;
@@ -20,7 +17,6 @@ typedef struct {
 	char mac_byte[6];
 	char mac_text[18];
 } ednsData;
-ASSERT_SIZEOF(ednsData, 72, 72, 72);
 
 void FTL_parse_pseudoheaders(struct dns_header *header, size_t n, union mysockaddr *peer, ednsData *edns);
 

src/overTime.h

@@ -14,9 +14,6 @@
 // TYPE_MAX
 #include "datastructure.h"
 
-// assert_sizeof
-#include "static_assert.h"
-
 void initOverTime(void);
 
 #define getOverTimeID(timestamp) _getOverTimeID(timestamp, __FILE__, __LINE__)
@@ -38,7 +35,6 @@ typedef struct {
 	int forwarded;
 	time_t timestamp;
 } overTimeData;
-ASSERT_SIZEOF(overTimeData, 32, 24, 24);
 
 extern overTimeData *overTime;
 

src/regex_r.h

@@ -24,9 +24,6 @@ extern const char *regextype[];
 #include <regex.h>
 #endif
 
-// assert_sizeof
-#include "static_assert.h"
-
 #include <netinet/in.h>
 
 typedef struct {
@@ -46,8 +43,6 @@ typedef struct {
 	regex_t regex;
 } regexData;
 
-ASSERT_SIZEOF(regexData, 56, 44, 44);
-
 unsigned int get_num_regex(const enum regex_type regexid) __attribute__((pure));
 int match_regex(const char *input, DNSCacheData* dns_cache, const int clientID,
                 const enum regex_type regexid, const bool regextest);

src/shmem.h

@@ -18,22 +18,17 @@
 // TYPE_MAX
 #include "datastructure.h"
 
-// assert_sizeof
-#include "static_assert.h"
-
 typedef struct {
     const char *name;
     size_t size;
     void *ptr;
 } SharedMemory;
-ASSERT_SIZEOF(SharedMemory, 24, 12, 12);
 
 typedef struct {
 	int version;
 	unsigned int global_shm_counter;
 	unsigned int next_str_pos;
 } ShmSettings;
-ASSERT_SIZEOF(ShmSettings, 12, 12, 12);
 
 typedef struct {
 	int queries;
@@ -54,7 +49,6 @@ typedef struct {
 	int status[QUERY_STATUS_MAX];
 	int reply[QUERY_REPLY_MAX];
 } countersStruct;
-ASSERT_SIZEOF(countersStruct, 240, 240, 240);
 
 extern countersStruct *counters;
 

src/static_assert.h

@@ -1,27 +0,0 @@
-/* Pi-hole: A black hole for Internet advertisements
-*  (c) 2021 Pi-hole, LLC (https://pi-hole.net)
-*  Network-wide ad blocking via your own hardware.
-*
-*  FTL Engine
-*  Struct size assertion tool
-*
-*  This file is copyright under the latest version of the EUPL.
-*  Please see LICENSE file for your rights under this license. */
-
-#include <assert.h>
-
-
-#define STATIC_ASSERT(OBJECT, EXPECTED)   \
-  static_assert(sizeof(OBJECT) == EXPECTED , "Expected size of " #OBJECT " is " #EXPECTED " on this architecture.");
-
-// Check based on detected architecture
-#if defined(__x86_64__) || defined(__aarch64__)
-#define ASSERT_SIZEOF(OBJECT, SIZE64, SIZE32, SIZEARM) \
-	STATIC_ASSERT(OBJECT, SIZE64)
-#elif defined(__i386__) || defined(__mips__) // issue #290
-#define ASSERT_SIZEOF(OBJECT, SIZE64, SIZE32, SIZEARM) \
-	STATIC_ASSERT(OBJECT, SIZE32)
-#elif defined(__arm__)
-#define ASSERT_SIZEOF(OBJECT, SIZE64, SIZE32, SIZEARM) \
-	STATIC_ASSERT(OBJECT, SIZEARM)
-#endif

src/struct_size.c

@@ -0,0 +1,44 @@
+
+/* Pi-hole: A black hole for Internet advertisements
+*  (c) 2022 Pi-hole, LLC (https://pi-hole.net)
+*  Network-wide ad blocking via your own hardware.
+*
+*  FTL Engine
+*  Struct size checking routines
+*
+*  This file is copyright under the latest version of the EUPL.
+*  Please see LICENSE file for your rights under this license. */
+
+#include "struct_size.h"
+#include <stdio.h>
+
+int check_one_struct(const char *struct_name, const long found_size, const long size64, const long size32, const long sizeARM)
+{
+#if defined(__x86_64__)
+	const long expected_size = size64;
+	const char *arch = "x86_64";
+#elif defined(__aarch64__)
+	const long expected_size = size64;
+	const char *arch = "aarch64";
+#elif defined(__i386__)
+	const long expected_size = size32;
+	const char *arch = "i386";
+#elif defined(__mips__) // issue #290
+	const long expected_size = size32;
+	const char *arch = "mips";
+#elif defined(__arm__)
+	const long expected_size = sizeARM;
+	const char *arch = "arm";
+#endif
+
+	// Check struct size meets expectation
+	if(found_size == expected_size)
+		return 0;
+
+	// Size mismatch
+	printf("WARNING: sizeof(%s) should be %lu on %s but is %lu\n",
+	       struct_name, expected_size, arch, found_size);
+	return 1;
+}
+
+

src/struct_size.h

@@ -0,0 +1,15 @@
+/* Pi-hole: A black hole for Internet advertisements
+*  (c) 2022 Pi-hole, LLC (https://pi-hole.net)
+*  Network-wide ad blocking via your own hardware.
+*
+*  FTL Engine
+*  Struct size checking prototypes
+*
+*  This file is copyright under the latest version of the EUPL.
+*  Please see LICENSE file for your rights under this license. */
+#ifndef STRUCT_SIZE_HEADER
+#define STRUCT_SIZE_HEADER
+
+int check_one_struct(const char *struct_name, const long found_size, const long size64, const long size32, const long sizeARM);
+
+#endif // STRUCT_SIZE_HEADER

src/vector.h

@@ -18,8 +18,6 @@
 #include <stdbool.h>
 // type sqlite3_stmt
 #include "database/sqlite3.h"
-// assert_sizeof
-#include "static_assert.h"
 
 #define VEC_ALLOC_STEP 10u
 
@@ -29,7 +27,6 @@ typedef struct sqlite3_stmt_vec {
 	sqlite3_stmt *(*get)(struct sqlite3_stmt_vec *, unsigned int);
 	void (*set)(struct sqlite3_stmt_vec *, unsigned int, sqlite3_stmt*);
 } sqlite3_stmt_vec;
-ASSERT_SIZEOF(sqlite3_stmt_vec, 32, 16, 16);
 
 sqlite3_stmt_vec *new_sqlite3_stmt_vec(unsigned int initial_size);
 void set_sqlite3_stmt_vec(sqlite3_stmt_vec *v, unsigned int index, sqlite3_stmt* item);

test/test_suite.bats

@@ -1097,6 +1097,12 @@
   [[ ${lines[0]} == *"using ${compiler_version}"* ]]
 }
 
+@test "Struct sizes are as expected" {
+  run bash -c './pihole-FTL --check-structs'
+  printf "%s\n" "${lines[@]}"
+  [[ $status == 0 ]]
+}
+
 @test "No errors on setting busy handlers for the databases" {
   run bash -c 'grep -c "Cannot set busy handler" /var/log/pihole-FTL.log'
   printf "%s\n" "${lines[@]}"