[qfcc] Warn when super dealloc invocation is missing

Forgetting to invoke [super dealloc] in a derived class's -dealloc
method has caused me to waste far too much time chasing down the
resulting memory leaks and crashes. This is actually the main focus of
issue #24, but I want to take care of multiple paths before I consider
the issue to be done.

However, as a bonus, four cases were found :)

libs/video/renderer/vulkan/vkgen/vkfieldcustom.r

@@ -27,6 +27,7 @@
 {
 	str_free (pltype);
 	str_free (parser);
+	[super dealloc];
 }
 
 -writeParseData

libs/video/renderer/vulkan/vkgen/vkfielddef.r

@@ -68,6 +68,7 @@
 {
 	str_free (struct_name);
 	str_free (field_name);
+	[super dealloc];
 }
 
 -writeParseData

libs/video/renderer/vulkan/vkgen/vkfieldtype.r

@@ -32,6 +32,7 @@ parseItemType (PLItem *item)
 	str_free (type);
 	str_free (parser);
 	str_free (parse_type);
+	[super dealloc];
 }
 
 -initWithItem:(PLItem *)item

ruamoko/cl_menu/CvarObject.r

@@ -20,5 +20,6 @@
 -(void)dealloc
 {
 	str_free (name);
+	[super dealloc];
 }
 @end

tools/qfcc/include/expr.h

@@ -586,6 +586,13 @@ int expr_integral (expr_t *e) __attribute__((pure));
 */
 int is_constant (expr_t *e) __attribute__((pure));
 
+/** Check if the expression refers to a selector
+
+	\param e		The expression to check.
+	\return			True if the expression is a selector.
+*/
+int is_selector (expr_t *e) __attribute__((pure));
+
 /**	Return a value expression representing the constant stored in \a e.
 
 	If \a e does not represent a constant, or \a e is already a value or

tools/qfcc/source/expr.c

@@ -887,6 +887,12 @@ is_constant (expr_t *e)
 	return 0;
 }
 
+int
+is_selector (expr_t *e)
+{
+	return e->type == ex_selector;
+}
+
 expr_t *
 constant_expr (expr_t *e)
 {

tools/qfcc/source/statements.c

@@ -45,6 +45,7 @@
 #include "QF/mathlib.h"
 #include "QF/va.h"
 
+#include "tools/qfcc/include/class.h"
 #include "tools/qfcc/include/dags.h"
 #include "tools/qfcc/include/diagnostic.h"
 #include "tools/qfcc/include/dot.h"
@@ -1002,6 +1003,7 @@ expr_call (sblock_t *sblock, expr_t *call, operand_t **op)
 	const char *pref = "";
 	statement_t *s;
 
+	// function arguments are in reverse order
 	for (a = args; a; a = a->next)
 		count++;
 	ind = count;
@@ -2027,6 +2029,42 @@ remove_dead_blocks (sblock_t *blocks)
 	return did_anything;
 }
 
+static void
+search_for_super_dealloc (sblock_t *sblock)
+{
+	while (sblock) {
+		for (statement_t *st = sblock->statements; st; st = st->next) {
+			if (!statement_is_call (st)) {
+				continue;
+			}
+			if (st->opa->op_type != op_def
+				|| strcmp (st->opa->o.def->name, "obj_msgSend_super") != 0) {
+				continue;
+			}
+			// FIXME this is messy (calls should have their own expression
+			// type)
+			// have effectively checked e1 above
+			if (st->expr->type != ex_expr) {
+				continue;
+			}
+			// function arguments are in reverse order, and the selector
+			// is the second argument (or second last in the list)
+			expr_t     *arg;
+			for (arg = st->expr->e.expr.e2;
+				 arg && arg->next && arg->next->next; arg = arg->next) {
+			}
+			if (arg && arg->next && is_selector (arg)) {
+				selector_t *sel = get_selector (st->expr->e.expr.e2);
+				if (sel && strcmp (sel->name, "dealloc") == 0) {
+					return;
+				}
+			}
+		}
+		sblock = sblock->next;
+	}
+	warning (0, "Derived class -dealloc does not call [super dealloc]");
+}
+
 static void
 check_final_block (sblock_t *sblock)
 {
@@ -2071,6 +2109,7 @@ make_statements (expr_t *e)
 	sblock_t   *sblock = new_sblock ();
 	int         did_something;
 	int         pass = 0;
+	class_t    *class;
 
 	if (options.block_dot.expr)
 		dump_dot ("expr", e, dump_dot_expr);
@@ -2088,6 +2127,16 @@ make_statements (expr_t *e)
 		pass++;
 	} while (did_something);
 	check_final_block (sblock);
+	if (current_class && (class = extract_class (current_class))) {
+		// If the class is a root class, then it is not possible for there
+		// to be a call to [super dealloc] so do not check. However, any
+		// derived class implementeing -dealloc must call [super dealloc]
+		// (or some other deallocator (FIXME: later))
+		// FIXME better check for dealloc?
+		if (class->super_class && !strcmp (current_func->name, "-dealloc")) {
+			search_for_super_dealloc (sblock);
+		}
+	}
 	if (options.block_dot.final)
 		dump_dot ("final", sblock, dump_dot_sblock);
 

tools/qfcc/test/Makemodule.am

@@ -65,6 +65,7 @@ fail_progs_dat=
 test_build_errors=\
 	tools/qfcc/test/classarray.r \
 	tools/qfcc/test/dealloc-warn.r \
+	tools/qfcc/test/dealloc-warn2.r \
 	tools/qfcc/test/double-demote-float.r \
 	tools/qfcc/test/double-demote-float-ainit.r \
 	tools/qfcc/test/double-demote-float-ginit.r \
@@ -231,6 +232,9 @@ tools/qfcc/test/classarray.run$(EXEEXT): tools/qfcc/test/classarray.r $(qfcc_fai
 tools/qfcc/test/dealloc-warn.run$(EXEEXT): tools/qfcc/test/dealloc-warn.r $(qfcc_fail_run_deps)
 	@$(top_srcdir)/tools/qfcc/test/build-compile-fail-run $@ $(QFCC) $(QCFLAGS) $<
 
+tools/qfcc/test/dealloc-warn2.run$(EXEEXT): tools/qfcc/test/dealloc-warn2.r $(qfcc_fail_run_deps)
+	@$(top_srcdir)/tools/qfcc/test/build-compile-fail-run $@ $(QFCC) $(QCFLAGS) $<
+
 tools/qfcc/test/double-demote-int.run$(EXEEXT): tools/qfcc/test/double-demote-int.r $(qfcc_fail_run_deps)
 	@$(top_srcdir)/tools/qfcc/test/build-compile-fail-run $@ $(QFCC) $(QCFLAGS) $<
 

tools/qfcc/test/dealloc-warn2.r

@@ -0,0 +1,44 @@
+@interface Object
+{
+	Class		isa;
+}
+-(void)dealloc;
+-(void)release;
+@end
+
+@interface derived : Object
+{
+	id          something;
+}
+@end
+
+@implementation Object
+-(void) dealloc
+{
+	// this is the root of the hierarchy, so no super to call, thus
+	// must not check for [super dealloc]
+}
+-(void) release
+{
+}
+@end
+
+@implementation derived
+-(void) dealloc
+{
+	// as this is a derived class, failure to call [super dealloc] will
+	// result in a memory leak (yes, there could be special allocators
+	// involved, in which case something will be needed to inform the
+	// compiler)
+	[super release];
+	[something dealloc];
+}
+@end
+
+void __obj_exec_class (struct obj_module *msg) = #0;
+id obj_msgSend_super (Super *class, SEL op, ...) = #0;
+
+int main ()
+{
+	return 1;	// test fails if compile succeeds (with -Werror)
+}