[PATCH 00/13] format-check: add specific type checking

[PATCH 00/13] format-check: add specific type checking

[Luc Van Oostenryck]

These patches improve/complete the parsing of the lenght and
the type modifiers and, more importantly, add specific checking
functions for all the types, replacing the use of
check_assignment_types() which don't do the kind of type checking
needed here (and which evaluate expressions, something not
needed and which complicate the interface a lot).

These checking functions are not yet complete but are a solid
base for some further improvements. They already allow to
use 'sparse -Wformat' on the kernel without false positive
and can detect of a few errors.

The series is available for review & testing at:
  git://github.com/lucvoo/sparse-dev.git format-check


Luc Van Oostenryck (13):
  format-check: void * is not OK for strings, fix the test
  format-check: more complete parsing of the length & type modifiers
  format-check: add helper type_class()
  format-check: merge 'fmt_string' & 'string'
  format-check: remove unneeded member: target
  format-check: add a function to check to type of strings
  format-check: add a function to check to type of 'n' arguments
  format-check: add a function to check to type of pointers
  format-check: remove printf_fmt_print_pointer()
  format-check: add a function to check the type of floats
  format-check: add a function to check the type of integers
  format-check: remove wrappers around type checking methods
  format-check: simplify calling of parse_printf_get_fmt()

 validation/varargs-format-addrspace1.c |  12 +-
 verify-format.c                        | 496 ++++++++++++++++---------
 2 files changed, 323 insertions(+), 185 deletions(-)

-- 
2.28.0

[PATCH 01/13] format-check: void * is not OK for strings, fix the test

[Luc Van Oostenryck]

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 validation/varargs-format-addrspace1.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/validation/varargs-format-addrspace1.c b/validation/varargs-format-addrspace1.c
index 3370ac67849c..53c210b78ae0 100644
--- a/validation/varargs-format-addrspace1.c
+++ b/validation/varargs-format-addrspace1.c
@@ -4,8 +4,8 @@ extern int variadic2(char *msg, int , ...) __attribute__((format (printf, 1, 3))
 extern int variadic3(int, char *msg,  ...) __attribute__((format (printf, 2, 3)));
 
 static void test(void) {
-	void __attribute__((noderef, address_space(1))) *a;
-	void *b;
+	const char __attribute__((noderef, address_space(1))) *a;
+	const char *b;
 
 	variadic("%s\n", a);
 	variadic("%s\n", b);
@@ -22,15 +22,15 @@ static void test(void) {
  * check-error-start
 varargs-format-addrspace1.c:10:26: warning: incorrect type in argument 2 (different address spaces)
 varargs-format-addrspace1.c:10:26:    expected char const *
-varargs-format-addrspace1.c:10:26:    got void [noderef] <asn:1> *a
+varargs-format-addrspace1.c:10:26:    got char const [noderef] <asn:1> *a
 varargs-format-addrspace1.c:12:32: warning: incorrect type in argument 3 (different address spaces)
 varargs-format-addrspace1.c:12:32:    expected char const *
-varargs-format-addrspace1.c:12:32:    got void [noderef] <asn:1> *a
+varargs-format-addrspace1.c:12:32:    got char const [noderef] <asn:1> *a
 varargs-format-addrspace1.c:13:36: warning: incorrect type in argument 4 (different address spaces)
 varargs-format-addrspace1.c:13:36:    expected char const *
-varargs-format-addrspace1.c:13:36:    got void [noderef] <asn:1> *a
+varargs-format-addrspace1.c:13:36:    got char const [noderef] <asn:1> *a
 varargs-format-addrspace1.c:14:36: warning: incorrect type in argument 4 (different address spaces)
 varargs-format-addrspace1.c:14:36:    expected char const *
-varargs-format-addrspace1.c:14:36:    got void [noderef] <asn:1> *a
+varargs-format-addrspace1.c:14:36:    got char const [noderef] <asn:1> *a
  * check-error-end
  */
-- 
2.28.0

[PATCH 02/13] format-check: more complete parsing of the length & type modifiers

[Luc Van Oostenryck]

Reorganize the parsing the length modifiers to follow more closely
the specs and handle some missing cases like 'hh' or '%[ieEaA]'.
Also, treats 'L' and 'll' synonymously as done by GCC.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 verify-format.c | 265 +++++++++++++++++++++++++++---------------------
 1 file changed, 151 insertions(+), 114 deletions(-)

diff --git a/verify-format.c b/verify-format.c
index 4b4730285237..ae5bb2e6e985 100644
--- a/verify-format.c
+++ b/verify-format.c
@@ -44,6 +44,14 @@
 #include "expression.h"
 #include "verify-format.h"
 
+enum length_mod {
+	LEN_none,
+	LEN_hh, LEN_h,
+	LEN_l, LEN_ll,
+	LEN_L,
+	LEN_j, LEN_z, LEN_t,
+};
+
 struct format_type {
 	const char	*format;
 	int		(*test)(struct format_type *fmt,
@@ -113,140 +121,169 @@ static struct format_type printf_fmt_ptr_ref = {
 	.test = printf_fmt_pointer,
 };
 
-static int is_float_spec(char t)
-{
-	return t == 'f' || t == 'g' || t == 'F' || t == 'G';
-}
-
 static struct format_type *parse_printf_get_fmt(struct format_type *type,
 						const char *msg, const char **msgout)
 {
 	const char *ptr = msg;
-	int szmod=0;
+	int szmod = LEN_none;
 
 	type->test = NULL;
 	*msgout = ptr;
 
-	if (*ptr == 's') {
-		ptr++;
-		type->test = printf_fmt_string;
-		type->type = &const_string_ctype;
-	} else if (*ptr == 'c') {
-		ptr++;
-		type->test = printf_fmt_numtype;
-		type->type = &char_ctype;
-	} else if (*ptr == 'p') {
-		ptr++;
-		type->test = printf_fmt_print_pointer;
-		/* check for pointer being printed as hex explicitly */
-		if (*ptr == 'x' || *ptr == 'X') {
+	switch (*ptr++) {
+	case 'h':
+		szmod = LEN_h;
+		if (*ptr == 'h') {
+			szmod = LEN_hh;
 			ptr++;
-		} else if (isalpha(*ptr)) {
-			/* probably some extra specifiers after %p */
+		}
+		break;
+	case 'l':
+		szmod = LEN_l;
+		if (*ptr == 'l') {
+			szmod = LEN_ll;
 			ptr++;
-			type->test = printf_fmt_pointer;
 		}
-	} else if (*ptr == 'z') {
-		// todo - we should construct pointer to int/etc //
+		break;
+	case 'q':
+		szmod = LEN_ll;
+		break;
+	case 'L':
+		szmod = LEN_L;
+		break;
+	case 'j':
+		szmod = LEN_j;
+		break;
+	case 'z':
+		szmod = LEN_z;
+		break;
+	case 't':
+		szmod = LEN_t;
+		break;
+	default:
+		ptr--;
+		break;
+	}
 
-		ptr++;
-		if (*ptr == 'd' || *ptr == 'i') {
-			ptr++;
-			type->test = printf_fmt_numtype;
+	switch (*ptr++) {
+	case 'd': case 'i':
+		type->test = printf_fmt_numtype;
+		switch (szmod) {
+		case LEN_hh:
+		case LEN_h:
+		case LEN_none:
+			type->type = &int_ctype;
+			break;
+		case LEN_l:
+			type->type = &long_ctype;
+			break;
+		case LEN_L:
+		case LEN_ll:
+			type->type = &llong_ctype;
+			break;
+		case LEN_j:
+			type->type = intmax_ctype;
+			break;
+		case LEN_z:
 			type->type = ssize_t_ctype;
-		} else if (*ptr == 'u' || *ptr == 'x' || *ptr == 'X' ||
-			   *ptr == 'o') {
-			ptr++;
-			type->test = printf_fmt_numtype;
-			type->type = size_t_ctype;
+			break;
+		case LEN_t:
+			type->type = ptrdiff_ctype;
+			break;
+		default:
+			type->test = NULL;
 		}
-	} else {
-		if (*ptr == 'l') {
-			szmod++;
-			ptr++;
-			if (*ptr == 'l') {
-				szmod++;
-				ptr++;
-			}
-		} else if (*ptr == 'L') {
-			szmod++;
-			ptr++;
-		} else {
-			if (*ptr == 'h') { // short/char to int
-				szmod = -1;
-				ptr++;
-				if (*ptr == 'h')  // promotion from char
-					ptr++;
-			}
-			if (*ptr == 't') {  // ptrdiff_t
-				szmod = 2;
-				ptr++;
-			}
-			if (*ptr == 'j') { // intmax_t
-				// todo - replace iwth intmax_ctype when added
-				szmod = 1;
-				ptr++;
-			}
+		break;
+	case 'u': case 'o': case 'x': case 'X':
+		type->test = printf_fmt_numtype;
+		switch (szmod) {
+		case LEN_hh:
+		case LEN_h:
+		case LEN_none:
+			type->type = &uint_ctype;
+			break;
+		case LEN_l:
+			type->type = &ulong_ctype;
+			break;
+		case LEN_L:
+		case LEN_ll:
+			type->type = &ullong_ctype;
+			break;
+		case LEN_j:
+			type->type = uintmax_ctype;
+			break;
+		case LEN_z:
+			type->type = size_t_ctype;
+			break;
+		case LEN_t:
+			type->type = ptrdiff_ctype;
+			break;
+		default:
+			type->test = NULL;
 		}
-
-		if (*ptr == 'x' || *ptr == 'X' || *ptr == 'u' || *ptr == 'o') {
-			ptr++;
-			type->test = printf_fmt_numtype;
-			switch (szmod) {
-			case -1:
-				type->type = &ushort_ctype;
-				break;
-			case 0:
-				type->type = &uint_ctype;
-				break;
-			case 1:
-				type->type = &ulong_ctype;
-				break;
-			case 2:
-				type->type = &ullong_ctype;
-				break;
-			default:
-				type->test = NULL;
-			}
-		} else if (*ptr == 'i' || *ptr == 'd') {
-			ptr++;
-			type->test = printf_fmt_numtype;
-			switch (szmod) {
-			case -1:
-				type->type = &short_ctype;
-				break;
-			case 0:
-				type->type = &int_ctype;
-				break;
-			case 1:
-				type->type = &long_ctype;
-				break;
-			case 2:
-				type->type = &llong_ctype;
-				break;
-			default:
-				type->test = NULL;
-			}
-		} else if (*ptr == 'L' && is_float_spec(ptr[1])) {
-			type->test = printf_fmt_numtype;
+		break;
+	case 'e': case 'E': case 'f': case 'F': case 'g': case 'G':
+	case 'a': case 'A':
+		type->test = printf_fmt_numtype;
+		switch (szmod) {
+		case LEN_none:
+			type->type = &double_ctype;
+			break;
+		case LEN_L:
 			type->type = &ldouble_ctype;
-			ptr += 2;
-		} else if (is_float_spec(*ptr)) {
-			type->test = printf_fmt_numtype;
-			type->type = szmod == 1 ? &ldouble_ctype :  &double_ctype;
-			ptr++;
-		} else if (*ptr == 's') {
-			type->test = printf_fmt_string;
+			break;
+		default:
+			break;
+		}
+		break;
+	case 'c':
+		type->test = printf_fmt_numtype;
+		switch (szmod) {
+		case LEN_none:
+			type->type = &int_ctype;
+			break;
+		case LEN_L:
+		case LEN_l:
+			type->type = wint_ctype;
+			break;
+		default:
+			break;
+		}
+		break;
+	case 's':
+		type->test = printf_fmt_string;
+		switch (szmod) {
+		case LEN_none:
+			type->type = &const_string_ctype;
+			break;
+		case LEN_L:
+		case LEN_l:
 			type->type = &const_wstring_ctype;
+			break;
+		default:
+			break;
+		}
+		break;
+	case 'p':
+		type->test = printf_fmt_print_pointer;
+		/* check for pointer being printed as hex explicitly */
+		if (*ptr == 'x' || *ptr == 'X') {
 			ptr++;
-		} else if (*ptr == 'n') {	/* pointer to an de-referenced int/etc */
-			// todo - we should construct pointer to int/etc //
-			// also should not have any flags or widths for this
-			type->test = printf_fmt_pointer;
+		} else if (isalpha(*ptr)) {
+			/* probably some extra specifiers after %p */
 			ptr++;
-		} else {
-			// anything else here?
+			type->test = printf_fmt_pointer;
 		}
+		break;
+	case 'n':
+		/* pointer to an de-referenced int/etc */
+		// todo - we should construct pointer to int/etc //
+		// also should not have any flags or widths for this
+		type->test = printf_fmt_pointer;
+		break;
+	default:
+		// anything else here?
+		break;
 	}
 
 	if (type->test == NULL)
-- 
2.28.0

[PATCH 03/13] format-check: add helper type_class()

[Luc Van Oostenryck]

This will help the next patch to verify the types.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 verify-format.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/verify-format.c b/verify-format.c
index ae5bb2e6e985..fd5a9ed821e1 100644
--- a/verify-format.c
+++ b/verify-format.c
@@ -70,6 +70,35 @@ struct format_state {
 	unsigned int		used_position: 1;
 };
 
+enum {
+	CLASS_OTHER,
+	CLASS_INT,
+	CLASS_BITWISE,
+	CLASS_FLOAT,
+	CLASS_PTR,
+};
+
+static inline int type_class(struct symbol *type, struct symbol **base)
+{
+	if (type->type == SYM_NODE)
+		type = type->ctype.base_type;
+	if (type->type == SYM_ENUM)
+		type = type->ctype.base_type;
+	*base = type;
+	if (type->type == SYM_BASETYPE) {
+		struct symbol *kind = type->ctype.base_type;
+		if (kind == &int_type)
+			return CLASS_INT;
+		if (kind == &fp_type)
+			return CLASS_FLOAT;
+	}
+	if (type->type == SYM_PTR)
+		return CLASS_PTR;
+	if (type->type == SYM_RESTRICT)
+		return CLASS_BITWISE;
+	return CLASS_OTHER;
+}
+
 static int printf_fmt_numtype(struct format_type *fmt,
 			      struct expression **expr,
 			      struct symbol *ctype,
-- 
2.28.0

[PATCH 04/13] format-check: merge 'fmt_string' & 'string'

[Luc Van Oostenryck]

Those are 2 variables for the same things. Merge them.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 verify-format.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/verify-format.c b/verify-format.c
index fd5a9ed821e1..99a36c8eef5f 100644
--- a/verify-format.c
+++ b/verify-format.c
@@ -524,7 +524,7 @@ void verify_format_attribute(struct symbol *fn, struct expression_list *args)
 	struct format_state state = { };
 	struct expression *expr;
 	struct expression *init;
-	const char *fmt_string;
+	const char *string;
 
 	if (!fn || !Wformat)
 		return;
@@ -540,16 +540,15 @@ void verify_format_attribute(struct symbol *fn, struct expression_list *args)
 	init = expr->symbol->initializer;
 	if (!init || init->type != EXPR_STRING)
 		return;			// not a string
-	fmt_string = init->string->data;
+	string = init->string->data;
 
 	state.expr = expr;
 	state.first = fn->ctype.format.first;
 	state.arg_index = fn->ctype.format.first;
 
-	if (!fmt_string) {
+	if (!string) {
 		warning(expr->pos, "not a format string?");
 	} else {
-		const char *string = fmt_string;
 		int fail = 0;
 
 		while (string[0]) {
-- 
2.28.0

[PATCH 05/13] format-check: remove unneeded member: target

[Luc Van Oostenryck]

The signature of the checking method allow to return the target
type. But this is never needed as it is always statically known.

So, remove this argument.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 verify-format.c | 33 +++++++++++++--------------------
 1 file changed, 13 insertions(+), 20 deletions(-)

diff --git a/verify-format.c b/verify-format.c
index 99a36c8eef5f..0ef2cb863ae9 100644
--- a/verify-format.c
+++ b/verify-format.c
@@ -57,7 +57,6 @@ struct format_type {
 	int		(*test)(struct format_type *fmt,
 				struct expression **expr,
 				struct symbol *ctype,
-				struct symbol **target,
 				const char **typediff);
 	struct symbol	*type;
 };
@@ -102,41 +101,35 @@ static inline int type_class(struct symbol *type, struct symbol **base)
 static int printf_fmt_numtype(struct format_type *fmt,
 			      struct expression **expr,
 			      struct symbol *ctype,
-			      struct symbol **target, const char **typediff)
+			      const char **typediff)
 {
-	struct symbol *type = fmt->type;
-	*target = type;
-	return check_assignment_types(*target, expr, typediff);
+	return check_assignment_types(fmt->type, expr, typediff);
 }
 
 static int printf_fmt_string(struct format_type *fmt,
 			     struct expression **expr,
 			     struct symbol *ctype,
-			     struct symbol **target, const char **typediff)
+			     const char **typediff)
 {
-	*target = fmt->type;
-	return check_assignment_types(*target, expr, typediff);
+	return check_assignment_types(fmt->type, expr, typediff);
 }
 
 static int printf_fmt_pointer(struct format_type *fmt,
 			      struct expression **expr,
 			      struct symbol *ctype,
-			      struct symbol **target, const char **typediff)
+			      const char **typediff)
 {
-	*target = &const_ptr_ctype;
-	return check_assignment_types(*target, expr, typediff);
+	return check_assignment_types(fmt->type, expr, typediff);
 }
 
 static int printf_fmt_print_pointer(struct format_type *fmt,
 				    struct expression **expr,
 				    struct symbol *ctype,
-				    struct symbol **target,
 				    const char **typediff)
 {
 	// TODO TODO: fix this here!!!
 	int ret;
-	*target = &const_ptr_ctype;
-	ret = check_assignment_types(*target, expr, typediff);
+	ret = check_assignment_types(fmt->type, expr, typediff);
 	if (ret == 0) {
 		/* if just printing, ignore address-space mismatches */
 		if (strcmp(*typediff, "different address spaces") == 0)
@@ -148,6 +141,7 @@ static int printf_fmt_print_pointer(struct format_type *fmt,
 static struct format_type printf_fmt_ptr_ref = {
 	.format = "p",
 	.test = printf_fmt_pointer,
+	.type = &const_ptr_ctype,
 };
 
 static struct format_type *parse_printf_get_fmt(struct format_type *type,
@@ -295,6 +289,7 @@ static struct format_type *parse_printf_get_fmt(struct format_type *type,
 		break;
 	case 'p':
 		type->test = printf_fmt_print_pointer;
+		type->type = &const_ptr_ctype;
 		/* check for pointer being printed as hex explicitly */
 		if (*ptr == 'x' || *ptr == 'X') {
 			ptr++;
@@ -309,6 +304,7 @@ static struct format_type *parse_printf_get_fmt(struct format_type *type,
 		// todo - we should construct pointer to int/etc //
 		// also should not have any flags or widths for this
 		type->test = printf_fmt_pointer;
+		type->type = &const_ptr_ctype;
 		break;
 	default:
 		// anything else here?
@@ -474,7 +470,7 @@ static int parse_format_printf(const char **fmtstring,
 		type = &printf_fmt_ptr_ref;	/* probably some extension */
 
 	if (type) {
-		struct symbol *ctype, *target = NULL;
+		struct symbol *ctype;
 		const char *typediff = "different types";
 		int ret;
 
@@ -490,13 +486,10 @@ static int parse_format_printf(const char **fmtstring,
 		if (!ctype)
 			return -3;
 
-		ret = type->test(type, &expr, ctype, &target, &typediff);
-		if (!target)	/* shouldn't happen, but catch anyway */
-			return -4;
-
+		ret = type->test(type, &expr, ctype, &typediff);
 		if (ret == 0) {
 			warning(expr->pos, "incorrect type in argument %d (%s)", pos, typediff);
-			info(expr->pos, "   expected %s", show_typename(target));
+			info(expr->pos, "   expected %s", show_typename(type->type));
 			info(expr->pos, "   got %s", show_typename(ctype));
 		}
 	} else {
-- 
2.28.0

[PATCH 06/13] format-check: add a function to check to type of strings

[Luc Van Oostenryck]

This checking is currently done by check_assignment_types()
but this is not adequate, for example because it allows
a void pointer for the string argument while a warning
should be issued.

So, add a custom checking function.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 verify-format.c | 34 +++++++++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/verify-format.c b/verify-format.c
index 0ef2cb863ae9..90fe0ede0431 100644
--- a/verify-format.c
+++ b/verify-format.c
@@ -106,12 +106,44 @@ static int printf_fmt_numtype(struct format_type *fmt,
 	return check_assignment_types(fmt->type, expr, typediff);
 }
 
+// For 's' & 'S' specifiers
+static const char *check_printf_string(struct format_type *fmt, struct symbol *source)
+{
+	const char *typediff = "different base types";
+	struct symbol *target = fmt->type;
+
+	examine_pointer_target(target);
+	examine_pointer_target(source);
+
+	if (type_class(source, &source) != CLASS_PTR)
+		return typediff;
+	if (source->ctype.as)
+		return "different address spaces";
+
+	// get the base type
+	type_class(source->ctype.base_type, &source);
+	type_class(source, &source);
+
+	if (target == &const_string_ctype) {
+		if (source == &char_ctype)
+			return NULL;
+		if (source == &uchar_ctype)
+			return NULL;
+		if (source == &schar_ctype)
+			return NULL;
+	} if (target == &const_wstring_ctype) {
+		if (source == wchar_ctype)
+			return NULL;
+	}
+	return typediff;
+}
+
 static int printf_fmt_string(struct format_type *fmt,
 			     struct expression **expr,
 			     struct symbol *ctype,
 			     const char **typediff)
 {
-	return check_assignment_types(fmt->type, expr, typediff);
+	return !(*typediff = check_printf_string(fmt, ctype));
 }
 
 static int printf_fmt_pointer(struct format_type *fmt,
-- 
2.28.0

[PATCH 07/13] format-check: add a function to check to type of 'n' arguments

[Luc Van Oostenryck]

This checking is currently done by check_assignment_types()
but this is not adequate, for example because it allows
a void pointer while a pointer of the specific type is needed.

So, add a custom checking function.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 verify-format.c | 37 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/verify-format.c b/verify-format.c
index 90fe0ede0431..1b40b2c796a2 100644
--- a/verify-format.c
+++ b/verify-format.c
@@ -146,6 +146,41 @@ static int printf_fmt_string(struct format_type *fmt,
 	return !(*typediff = check_printf_string(fmt, ctype));
 }
 
+
+#define MOD_IGN (MOD_QUALIFIER | MOD_FUN_ATTR)
+static inline const char *compare_pointer(struct symbol *target, struct symbol *source)
+{
+	unsigned long modt = target->ctype.modifiers & MOD_IGN & ~MOD_REV_QUAL;
+	unsigned long mods = source->ctype.modifiers & MOD_IGN &  MOD_REV_QUAL;
+
+	return type_difference(&target->ctype, &source->ctype, mods, modt);
+}
+
+// For 'n' specifier
+static const char *check_printf_length(struct format_type *fmt, struct symbol *source)
+{
+	const char *typediff = "different base types";
+	struct symbol *target = fmt->type;
+
+	examine_pointer_target(target);
+	examine_pointer_target(source);
+
+	if (type_class(source, &source) != CLASS_PTR)
+		return typediff;
+	if (source->ctype.as)
+		return "different address spaces";
+
+	return compare_pointer(target, source);
+}
+
+static int printf_fmt_length(struct format_type *fmt,
+			      struct expression **expr,
+			      struct symbol *ctype,
+			      const char **typediff)
+{
+	return !(*typediff = check_printf_length(fmt, ctype));
+}
+
 static int printf_fmt_pointer(struct format_type *fmt,
 			      struct expression **expr,
 			      struct symbol *ctype,
@@ -335,7 +370,7 @@ static struct format_type *parse_printf_get_fmt(struct format_type *type,
 		/* pointer to an de-referenced int/etc */
 		// todo - we should construct pointer to int/etc //
 		// also should not have any flags or widths for this
-		type->test = printf_fmt_pointer;
+		type->test = printf_fmt_length;
 		type->type = &const_ptr_ctype;
 		break;
 	default:
-- 
2.28.0

[PATCH 08/13] format-check: add a function to check to type of pointers

[Luc Van Oostenryck]

This checking is currently done by check_assignment_types()
but as an extension, specific pointer types will be checked.

So, add a custom checking function, currently accepting any
pointer type.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 verify-format.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/verify-format.c b/verify-format.c
index 1b40b2c796a2..cd55a49e0676 100644
--- a/verify-format.c
+++ b/verify-format.c
@@ -181,12 +181,24 @@ static int printf_fmt_length(struct format_type *fmt,
 	return !(*typediff = check_printf_length(fmt, ctype));
 }
 
+// For 'p' specifiers
+static const char *check_printf_pointer(struct format_type *fmt, struct symbol *source)
+{
+	const char *typediff = "different base types";
+	struct symbol *base;
+
+	if (type_class(source, &base) != CLASS_PTR)
+		return typediff;
+	// FIXME: check or ignore address spaces
+	return NULL;
+}
+
 static int printf_fmt_pointer(struct format_type *fmt,
 			      struct expression **expr,
 			      struct symbol *ctype,
 			      const char **typediff)
 {
-	return check_assignment_types(fmt->type, expr, typediff);
+	return !(*typediff = check_printf_pointer(fmt, ctype));
 }
 
 static int printf_fmt_print_pointer(struct format_type *fmt,
-- 
2.28.0

[PATCH 09/13] format-check: remove printf_fmt_print_pointer()

[Luc Van Oostenryck]

This is currently not needed, so remove it.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 verify-format.c | 19 +------------------
 1 file changed, 1 insertion(+), 18 deletions(-)

diff --git a/verify-format.c b/verify-format.c
index cd55a49e0676..34c3db96fe3a 100644
--- a/verify-format.c
+++ b/verify-format.c
@@ -201,22 +201,6 @@ static int printf_fmt_pointer(struct format_type *fmt,
 	return !(*typediff = check_printf_pointer(fmt, ctype));
 }
 
-static int printf_fmt_print_pointer(struct format_type *fmt,
-				    struct expression **expr,
-				    struct symbol *ctype,
-				    const char **typediff)
-{
-	// TODO TODO: fix this here!!!
-	int ret;
-	ret = check_assignment_types(fmt->type, expr, typediff);
-	if (ret == 0) {
-		/* if just printing, ignore address-space mismatches */
-		if (strcmp(*typediff, "different address spaces") == 0)
-			ret = 1;
-	}
-	return ret;
-}
-
 static struct format_type printf_fmt_ptr_ref = {
 	.format = "p",
 	.test = printf_fmt_pointer,
@@ -367,7 +351,7 @@ static struct format_type *parse_printf_get_fmt(struct format_type *type,
 		}
 		break;
 	case 'p':
-		type->test = printf_fmt_print_pointer;
+		type->test = printf_fmt_pointer;
 		type->type = &const_ptr_ctype;
 		/* check for pointer being printed as hex explicitly */
 		if (*ptr == 'x' || *ptr == 'X') {
@@ -375,7 +359,6 @@ static struct format_type *parse_printf_get_fmt(struct format_type *type,
 		} else if (isalpha(*ptr)) {
 			/* probably some extra specifiers after %p */
 			ptr++;
-			type->test = printf_fmt_pointer;
 		}
 		break;
 	case 'n':
-- 
2.28.0

[PATCH 10/13] format-check: add a function to check the type of floats

[Luc Van Oostenryck]

This checking is currently done by check_assignment_types()
for all numeric types but this is not adequate because we
want to have a better control over what is allowed or not

So, add a custom checking function, currently just checking
if the argument is one of the floating-point types.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 verify-format.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/verify-format.c b/verify-format.c
index 34c3db96fe3a..6bcbfdfef1b4 100644
--- a/verify-format.c
+++ b/verify-format.c
@@ -106,6 +106,27 @@ static int printf_fmt_numtype(struct format_type *fmt,
 	return check_assignment_types(fmt->type, expr, typediff);
 }
 
+static const char *check_printf_float(struct format_type *fmt, struct symbol *source)
+{
+	const char *typediff = "different base types";
+	struct symbol *target = fmt->type;
+	struct symbol *base;
+
+	if (type_class(source, &base) != CLASS_FLOAT)
+		return typediff;
+	if (base == target)
+		return NULL;
+	return typediff;
+}
+
+static int printf_fmt_float(struct format_type *fmt,
+			      struct expression **expr,
+			      struct symbol *ctype,
+			      const char **typediff)
+{
+	return !(*typediff = check_printf_float(fmt, ctype));
+}
+
 // For 's' & 'S' specifiers
 static const char *check_printf_string(struct format_type *fmt, struct symbol *source)
 {
@@ -310,7 +331,7 @@ static struct format_type *parse_printf_get_fmt(struct format_type *type,
 		break;
 	case 'e': case 'E': case 'f': case 'F': case 'g': case 'G':
 	case 'a': case 'A':
-		type->test = printf_fmt_numtype;
+		type->test = printf_fmt_float;
 		switch (szmod) {
 		case LEN_none:
 			type->type = &double_ctype;
-- 
2.28.0

[PATCH 11/13] format-check: add a function to check the type of integers

[Luc Van Oostenryck]

This checking is currently done by check_assignment_types()
for all numeric types but this is not adequate because we
want to have a better control over what is allowed or not

So, add a custom checking function:
*) checking if the argument is one of the integer types.
*) currently accepting bitwise types too
*) easily extended to handle -Wformat-signedness

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 verify-format.c | 74 +++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 69 insertions(+), 5 deletions(-)

diff --git a/verify-format.c b/verify-format.c
index 6bcbfdfef1b4..fdfe9c22e858 100644
--- a/verify-format.c
+++ b/verify-format.c
@@ -98,12 +98,76 @@ static inline int type_class(struct symbol *type, struct symbol **base)
 	return CLASS_OTHER;
 }
 
-static int printf_fmt_numtype(struct format_type *fmt,
+static struct symbol *normalize_sint(struct symbol *s)
+{
+	if (s == &sint_ctype)
+		return &int_ctype;
+	if (s == &slong_ctype)
+		return &long_ctype;
+	if (s == &sllong_ctype)
+		return &llong_ctype;
+	if (s == &sint128_ctype)
+		return &int128_ctype;
+	return s;
+}
+
+static struct symbol *normalize_uint(struct symbol *s)
+{
+	if (s == &uint_ctype)
+		return &int_ctype;
+	if (s == &ulong_ctype)
+		return &long_ctype;
+	if (s == &ullong_ctype)
+		return &llong_ctype;
+	if (s == &uint128_ctype)
+		return &int128_ctype;
+	return s;
+}
+
+static const char *check_printf_int(struct format_type *fmt, struct symbol *source, int sign)
+{
+	const char *typediff = "different base types";
+	struct symbol *target = fmt->type;
+	int class = type_class(source, &source);
+
+	if (class == CLASS_BITWISE)
+		class = type_class(source->ctype.base_type, &source);
+	if (class != CLASS_INT)
+		return typediff;
+
+	// For now, ignore the signedness
+	target = normalize_uint(target);
+	source = normalize_uint(source);
+	source = normalize_sint(source);
+	if (source == target)
+		return NULL;
+	return typediff;
+}
+
+static const char *check_printf_sint(struct format_type *fmt, struct symbol *source)
+{
+	return check_printf_int(fmt, source, 1);
+}
+
+static int printf_fmt_sint(struct format_type *fmt,
+			      struct expression **expr,
+			      struct symbol *ctype,
+			      const char **typediff)
+{
+	return !(*typediff = check_printf_sint(fmt, ctype));
+}
+
+static const char *check_printf_uint(struct format_type *fmt, struct symbol *source)
+{
+	return check_printf_int(fmt, source, 0);
+}
+
+static int printf_fmt_uint(struct format_type *fmt,
 			      struct expression **expr,
 			      struct symbol *ctype,
 			      const char **typediff)
 {
-	return check_assignment_types(fmt->type, expr, typediff);
+	return !(*typediff = check_printf_uint(fmt, ctype));
 }
 
 static const char *check_printf_float(struct format_type *fmt, struct symbol *source)
@@ -274,7 +338,7 @@ static struct format_type *parse_printf_get_fmt(struct format_type *type,
 
 	switch (*ptr++) {
 	case 'd': case 'i':
-		type->test = printf_fmt_numtype;
+		type->test = printf_fmt_sint;
 		switch (szmod) {
 		case LEN_hh:
 		case LEN_h:
@@ -302,7 +366,7 @@ static struct format_type *parse_printf_get_fmt(struct format_type *type,
 		}
 		break;
 	case 'u': case 'o': case 'x': case 'X':
-		type->test = printf_fmt_numtype;
+		type->test = printf_fmt_uint;
 		switch (szmod) {
 		case LEN_hh:
 		case LEN_h:
@@ -344,7 +408,7 @@ static struct format_type *parse_printf_get_fmt(struct format_type *type,
 		}
 		break;
 	case 'c':
-		type->test = printf_fmt_numtype;
+		type->test = printf_fmt_sint;	// FIXME: need its own check?
 		switch (szmod) {
 		case LEN_none:
 			type->type = &int_ctype;
-- 
2.28.0

[PATCH 12/13] format-check: remove wrappers around type checking methods

[Luc Van Oostenryck]

Now that all types have their own checking functions, the
signature of the methods can be simplified and the wrappers
removed.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 verify-format.c | 86 +++++++------------------------------------------
 1 file changed, 12 insertions(+), 74 deletions(-)

diff --git a/verify-format.c b/verify-format.c
index fdfe9c22e858..753e59c167ce 100644
--- a/verify-format.c
+++ b/verify-format.c
@@ -54,10 +54,7 @@ enum length_mod {
 
 struct format_type {
 	const char	*format;
-	int		(*test)(struct format_type *fmt,
-				struct expression **expr,
-				struct symbol *ctype,
-				const char **typediff);
+	const char*	(*test)(struct format_type *fmt, struct symbol *source);
 	struct symbol	*type;
 };
 
@@ -149,27 +146,11 @@ static const char *check_printf_sint(struct format_type *fmt, struct symbol *sou
 	return check_printf_int(fmt, source, 1);
 }
 
-static int printf_fmt_sint(struct format_type *fmt,
-			      struct expression **expr,
-			      struct symbol *ctype,
-			      const char **typediff)
-{
-	return !(*typediff = check_printf_sint(fmt, ctype));
-}
-
 static const char *check_printf_uint(struct format_type *fmt, struct symbol *source)
 {
 	return check_printf_int(fmt, source, 0);
 }
 
-static int printf_fmt_uint(struct format_type *fmt,
-			      struct expression **expr,
-			      struct symbol *ctype,
-			      const char **typediff)
-{
-	return !(*typediff = check_printf_uint(fmt, ctype));
-}
-
 static const char *check_printf_float(struct format_type *fmt, struct symbol *source)
 {
 	const char *typediff = "different base types";
@@ -183,14 +164,6 @@ static const char *check_printf_float(struct format_type *fmt, struct symbol *so
 	return typediff;
 }
 
-static int printf_fmt_float(struct format_type *fmt,
-			      struct expression **expr,
-			      struct symbol *ctype,
-			      const char **typediff)
-{
-	return !(*typediff = check_printf_float(fmt, ctype));
-}
-
 // For 's' & 'S' specifiers
 static const char *check_printf_string(struct format_type *fmt, struct symbol *source)
 {
@@ -223,15 +196,6 @@ static const char *check_printf_string(struct format_type *fmt, struct symbol *s
 	return typediff;
 }
 
-static int printf_fmt_string(struct format_type *fmt,
-			     struct expression **expr,
-			     struct symbol *ctype,
-			     const char **typediff)
-{
-	return !(*typediff = check_printf_string(fmt, ctype));
-}
-
-
 #define MOD_IGN (MOD_QUALIFIER | MOD_FUN_ATTR)
 static inline const char *compare_pointer(struct symbol *target, struct symbol *source)
 {
@@ -258,14 +222,6 @@ static const char *check_printf_length(struct format_type *fmt, struct symbol *s
 	return compare_pointer(target, source);
 }
 
-static int printf_fmt_length(struct format_type *fmt,
-			      struct expression **expr,
-			      struct symbol *ctype,
-			      const char **typediff)
-{
-	return !(*typediff = check_printf_length(fmt, ctype));
-}
-
 // For 'p' specifiers
 static const char *check_printf_pointer(struct format_type *fmt, struct symbol *source)
 {
@@ -278,20 +234,6 @@ static const char *check_printf_pointer(struct format_type *fmt, struct symbol *
 	return NULL;
 }
 
-static int printf_fmt_pointer(struct format_type *fmt,
-			      struct expression **expr,
-			      struct symbol *ctype,
-			      const char **typediff)
-{
-	return !(*typediff = check_printf_pointer(fmt, ctype));
-}
-
-static struct format_type printf_fmt_ptr_ref = {
-	.format = "p",
-	.test = printf_fmt_pointer,
-	.type = &const_ptr_ctype,
-};
-
 static struct format_type *parse_printf_get_fmt(struct format_type *type,
 						const char *msg, const char **msgout)
 {
@@ -338,7 +280,7 @@ static struct format_type *parse_printf_get_fmt(struct format_type *type,
 
 	switch (*ptr++) {
 	case 'd': case 'i':
-		type->test = printf_fmt_sint;
+		type->test = check_printf_sint;
 		switch (szmod) {
 		case LEN_hh:
 		case LEN_h:
@@ -366,7 +308,7 @@ static struct format_type *parse_printf_get_fmt(struct format_type *type,
 		}
 		break;
 	case 'u': case 'o': case 'x': case 'X':
-		type->test = printf_fmt_uint;
+		type->test = check_printf_uint;
 		switch (szmod) {
 		case LEN_hh:
 		case LEN_h:
@@ -387,6 +329,7 @@ static struct format_type *parse_printf_get_fmt(struct format_type *type,
 			type->type = size_t_ctype;
 			break;
 		case LEN_t:
+			type->test = check_printf_sint;
 			type->type = ptrdiff_ctype;
 			break;
 		default:
@@ -395,7 +338,7 @@ static struct format_type *parse_printf_get_fmt(struct format_type *type,
 		break;
 	case 'e': case 'E': case 'f': case 'F': case 'g': case 'G':
 	case 'a': case 'A':
-		type->test = printf_fmt_float;
+		type->test = check_printf_float;
 		switch (szmod) {
 		case LEN_none:
 			type->type = &double_ctype;
@@ -408,7 +351,7 @@ static struct format_type *parse_printf_get_fmt(struct format_type *type,
 		}
 		break;
 	case 'c':
-		type->test = printf_fmt_sint;	// FIXME: need its own check?
+		type->test = check_printf_sint;
 		switch (szmod) {
 		case LEN_none:
 			type->type = &int_ctype;
@@ -422,7 +365,7 @@ static struct format_type *parse_printf_get_fmt(struct format_type *type,
 		}
 		break;
 	case 's':
-		type->test = printf_fmt_string;
+		type->test = check_printf_string;
 		switch (szmod) {
 		case LEN_none:
 			type->type = &const_string_ctype;
@@ -436,7 +379,7 @@ static struct format_type *parse_printf_get_fmt(struct format_type *type,
 		}
 		break;
 	case 'p':
-		type->test = printf_fmt_pointer;
+		type->test = check_printf_pointer;
 		type->type = &const_ptr_ctype;
 		/* check for pointer being printed as hex explicitly */
 		if (*ptr == 'x' || *ptr == 'X') {
@@ -450,7 +393,7 @@ static struct format_type *parse_printf_get_fmt(struct format_type *type,
 		/* pointer to an de-referenced int/etc */
 		// todo - we should construct pointer to int/etc //
 		// also should not have any flags or widths for this
-		type->test = printf_fmt_length;
+		type->test = check_printf_length;
 		type->type = &const_ptr_ctype;
 		break;
 	default:
@@ -612,14 +555,9 @@ static int parse_format_printf(const char **fmtstring,
 	}
 
 	type = parse_printf_get_fmt(&ftype, fmt, &fmtpost);
-
-	if (!type && fmt[0] == 'p')
-		type = &printf_fmt_ptr_ref;	/* probably some extension */
-
 	if (type) {
 		struct symbol *ctype;
-		const char *typediff = "different types";
-		int ret;
+		const char *typediff;
 
 		*fmtstring = fmtpost;
 		expr = get_nth_expression(args, pos-1);
@@ -633,8 +571,8 @@ static int parse_format_printf(const char **fmtstring,
 		if (!ctype)
 			return -3;
 
-		ret = type->test(type, &expr, ctype, &typediff);
-		if (ret == 0) {
+		typediff = ftype.test(&ftype, ctype);
+		if (typediff) {
 			warning(expr->pos, "incorrect type in argument %d (%s)", pos, typediff);
 			info(expr->pos, "   expected %s", show_typename(type->type));
 			info(expr->pos, "   got %s", show_typename(ctype));
-- 
2.28.0

[PATCH 13/13] format-check: simplify calling of parse_printf_get_fmt()

[Luc Van Oostenryck]

The function parse_printf_get_fmt() was allowed to return a
customized version of its format_type argument but this is
not really needed. So simplify the calling of this function.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 verify-format.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/verify-format.c b/verify-format.c
index 753e59c167ce..1f5a2798dce4 100644
--- a/verify-format.c
+++ b/verify-format.c
@@ -234,8 +234,7 @@ static const char *check_printf_pointer(struct format_type *fmt, struct symbol *
 	return NULL;
 }
 
-static struct format_type *parse_printf_get_fmt(struct format_type *type,
-						const char *msg, const char **msgout)
+static int parse_printf_get_fmt(struct format_type *type, const char *msg, const char **msgout)
 {
 	const char *ptr = msg;
 	int szmod = LEN_none;
@@ -401,11 +400,8 @@ static struct format_type *parse_printf_get_fmt(struct format_type *type,
 		break;
 	}
 
-	if (type->test == NULL)
-		return NULL;
-
 	*msgout = ptr;
-	return type;
+	return type->test == NULL;
 }
 
 static int is_printf_flag(char ch)
@@ -505,7 +501,6 @@ static int parse_format_printf(const char **fmtstring,
 			       struct expression_list *args)
 {
 	struct format_type ftype;	/* temp storage for format info */
-	struct format_type *type;	/* type found from the parse */
 	struct expression *expr;
 	const char *fmt = *fmtstring;	/* pointer to parse position */
 	const char *fmtpost = NULL;	/* moved to end of the parsed format */
@@ -554,8 +549,8 @@ static int parse_format_printf(const char **fmtstring,
 			error++;
 	}
 
-	type = parse_printf_get_fmt(&ftype, fmt, &fmtpost);
-	if (type) {
+	ret = parse_printf_get_fmt(&ftype, fmt, &fmtpost);
+	if (!ret) {
 		struct symbol *ctype;
 		const char *typediff;
 
@@ -574,7 +569,7 @@ static int parse_format_printf(const char **fmtstring,
 		typediff = ftype.test(&ftype, ctype);
 		if (typediff) {
 			warning(expr->pos, "incorrect type in argument %d (%s)", pos, typediff);
-			info(expr->pos, "   expected %s", show_typename(type->type));
+			info(expr->pos, "   expected %s", show_typename(ftype.type));
 			info(expr->pos, "   got %s", show_typename(ctype));
 		}
 	} else {
-- 
2.28.0