Linux-Sparse Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 00/13] format-check: add specific type checking
@ 2020-10-13 23:22 Luc Van Oostenryck
  2020-10-13 23:22 ` [PATCH 01/13] format-check: void * is not OK for strings, fix the test Luc Van Oostenryck
                   ` (12 more replies)
  0 siblings, 13 replies; 14+ messages in thread
From: Luc Van Oostenryck @ 2020-10-13 23:22 UTC (permalink / raw)
  To: linux-sparse; +Cc: Ben Dooks, 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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 01/13] format-check: void * is not OK for strings, fix the test
  2020-10-13 23:22 [PATCH 00/13] format-check: add specific type checking Luc Van Oostenryck
@ 2020-10-13 23:22 ` Luc Van Oostenryck
  2020-10-13 23:22 ` [PATCH 02/13] format-check: more complete parsing of the length & type modifiers Luc Van Oostenryck
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Luc Van Oostenryck @ 2020-10-13 23:22 UTC (permalink / raw)
  To: linux-sparse; +Cc: Ben Dooks, 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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 02/13] format-check: more complete parsing of the length & type modifiers
  2020-10-13 23:22 [PATCH 00/13] format-check: add specific type checking Luc Van Oostenryck
  2020-10-13 23:22 ` [PATCH 01/13] format-check: void * is not OK for strings, fix the test Luc Van Oostenryck
@ 2020-10-13 23:22 ` Luc Van Oostenryck
  2020-10-13 23:22 ` [PATCH 03/13] format-check: add helper type_class() Luc Van Oostenryck
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Luc Van Oostenryck @ 2020-10-13 23:22 UTC (permalink / raw)
  To: linux-sparse; +Cc: Ben Dooks, 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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 03/13] format-check: add helper type_class()
  2020-10-13 23:22 [PATCH 00/13] format-check: add specific type checking Luc Van Oostenryck
  2020-10-13 23:22 ` [PATCH 01/13] format-check: void * is not OK for strings, fix the test Luc Van Oostenryck
  2020-10-13 23:22 ` [PATCH 02/13] format-check: more complete parsing of the length & type modifiers Luc Van Oostenryck
@ 2020-10-13 23:22 ` Luc Van Oostenryck
  2020-10-13 23:22 ` [PATCH 04/13] format-check: merge 'fmt_string' & 'string' Luc Van Oostenryck
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Luc Van Oostenryck @ 2020-10-13 23:22 UTC (permalink / raw)
  To: linux-sparse; +Cc: Ben Dooks, 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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 04/13] format-check: merge 'fmt_string' & 'string'
  2020-10-13 23:22 [PATCH 00/13] format-check: add specific type checking Luc Van Oostenryck
                   ` (2 preceding siblings ...)
  2020-10-13 23:22 ` [PATCH 03/13] format-check: add helper type_class() Luc Van Oostenryck
@ 2020-10-13 23:22 ` Luc Van Oostenryck
  2020-10-13 23:22 ` [PATCH 05/13] format-check: remove unneeded member: target Luc Van Oostenryck
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Luc Van Oostenryck @ 2020-10-13 23:22 UTC (permalink / raw)
  To: linux-sparse; +Cc: Ben Dooks, 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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 05/13] format-check: remove unneeded member: target
  2020-10-13 23:22 [PATCH 00/13] format-check: add specific type checking Luc Van Oostenryck
                   ` (3 preceding siblings ...)
  2020-10-13 23:22 ` [PATCH 04/13] format-check: merge 'fmt_string' & 'string' Luc Van Oostenryck
@ 2020-10-13 23:22 ` Luc Van Oostenryck
  2020-10-13 23:22 ` [PATCH 06/13] format-check: add a function to check to type of strings Luc Van Oostenryck
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Luc Van Oostenryck @ 2020-10-13 23:22 UTC (permalink / raw)
  To: linux-sparse; +Cc: Ben Dooks, 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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 06/13] format-check: add a function to check to type of strings
  2020-10-13 23:22 [PATCH 00/13] format-check: add specific type checking Luc Van Oostenryck
                   ` (4 preceding siblings ...)
  2020-10-13 23:22 ` [PATCH 05/13] format-check: remove unneeded member: target Luc Van Oostenryck
@ 2020-10-13 23:22 ` Luc Van Oostenryck
  2020-10-13 23:22 ` [PATCH 07/13] format-check: add a function to check to type of 'n' arguments Luc Van Oostenryck
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Luc Van Oostenryck @ 2020-10-13 23:22 UTC (permalink / raw)
  To: linux-sparse; +Cc: Ben Dooks, 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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 07/13] format-check: add a function to check to type of 'n' arguments
  2020-10-13 23:22 [PATCH 00/13] format-check: add specific type checking Luc Van Oostenryck
                   ` (5 preceding siblings ...)
  2020-10-13 23:22 ` [PATCH 06/13] format-check: add a function to check to type of strings Luc Van Oostenryck
@ 2020-10-13 23:22 ` Luc Van Oostenryck
  2020-10-13 23:22 ` [PATCH 08/13] format-check: add a function to check to type of pointers Luc Van Oostenryck
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Luc Van Oostenryck @ 2020-10-13 23:22 UTC (permalink / raw)
  To: linux-sparse; +Cc: Ben Dooks, 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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 08/13] format-check: add a function to check to type of pointers
  2020-10-13 23:22 [PATCH 00/13] format-check: add specific type checking Luc Van Oostenryck
                   ` (6 preceding siblings ...)
  2020-10-13 23:22 ` [PATCH 07/13] format-check: add a function to check to type of 'n' arguments Luc Van Oostenryck
@ 2020-10-13 23:22 ` Luc Van Oostenryck
  2020-10-13 23:22 ` [PATCH 09/13] format-check: remove printf_fmt_print_pointer() Luc Van Oostenryck
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Luc Van Oostenryck @ 2020-10-13 23:22 UTC (permalink / raw)
  To: linux-sparse; +Cc: Ben Dooks, 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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 09/13] format-check: remove printf_fmt_print_pointer()
  2020-10-13 23:22 [PATCH 00/13] format-check: add specific type checking Luc Van Oostenryck
                   ` (7 preceding siblings ...)
  2020-10-13 23:22 ` [PATCH 08/13] format-check: add a function to check to type of pointers Luc Van Oostenryck
@ 2020-10-13 23:22 ` Luc Van Oostenryck
  2020-10-13 23:22 ` [PATCH 10/13] format-check: add a function to check the type of floats Luc Van Oostenryck
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Luc Van Oostenryck @ 2020-10-13 23:22 UTC (permalink / raw)
  To: linux-sparse; +Cc: Ben Dooks, 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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 10/13] format-check: add a function to check the type of floats
  2020-10-13 23:22 [PATCH 00/13] format-check: add specific type checking Luc Van Oostenryck
                   ` (8 preceding siblings ...)
  2020-10-13 23:22 ` [PATCH 09/13] format-check: remove printf_fmt_print_pointer() Luc Van Oostenryck
@ 2020-10-13 23:22 ` Luc Van Oostenryck
  2020-10-13 23:22 ` [PATCH 11/13] format-check: add a function to check the type of integers Luc Van Oostenryck
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 14+ messages in thread
From: Luc Van Oostenryck @ 2020-10-13 23:22 UTC (permalink / raw)
  To: linux-sparse; +Cc: Ben Dooks, 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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 11/13] format-check: add a function to check the type of integers
  2020-10-13 23:22 [PATCH 00/13] format-check: add specific type checking Luc Van Oostenryck
                   ` (9 preceding siblings ...)
  2020-10-13 23:22 ` [PATCH 10/13] format-check: add a function to check the type of floats Luc Van Oostenryck
@ 2020-10-13 23:22 ` Luc Van Oostenryck
  2020-10-13 23:22 ` [PATCH 12/13] format-check: remove wrappers around type checking methods Luc Van Oostenryck
  2020-10-13 23:22 ` [PATCH 13/13] format-check: simplify calling of parse_printf_get_fmt() Luc Van Oostenryck
  12 siblings, 0 replies; 14+ messages in thread
From: Luc Van Oostenryck @ 2020-10-13 23:22 UTC (permalink / raw)
  To: linux-sparse; +Cc: Ben Dooks, 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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 12/13] format-check: remove wrappers around type checking methods
  2020-10-13 23:22 [PATCH 00/13] format-check: add specific type checking Luc Van Oostenryck
                   ` (10 preceding siblings ...)
  2020-10-13 23:22 ` [PATCH 11/13] format-check: add a function to check the type of integers Luc Van Oostenryck
@ 2020-10-13 23:22 ` Luc Van Oostenryck
  2020-10-13 23:22 ` [PATCH 13/13] format-check: simplify calling of parse_printf_get_fmt() Luc Van Oostenryck
  12 siblings, 0 replies; 14+ messages in thread
From: Luc Van Oostenryck @ 2020-10-13 23:22 UTC (permalink / raw)
  To: linux-sparse; +Cc: Ben Dooks, 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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 13/13] format-check: simplify calling of parse_printf_get_fmt()
  2020-10-13 23:22 [PATCH 00/13] format-check: add specific type checking Luc Van Oostenryck
                   ` (11 preceding siblings ...)
  2020-10-13 23:22 ` [PATCH 12/13] format-check: remove wrappers around type checking methods Luc Van Oostenryck
@ 2020-10-13 23:22 ` Luc Van Oostenryck
  12 siblings, 0 replies; 14+ messages in thread
From: Luc Van Oostenryck @ 2020-10-13 23:22 UTC (permalink / raw)
  To: linux-sparse; +Cc: Ben Dooks, 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


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, back to index

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-13 23:22 [PATCH 00/13] format-check: add specific type checking Luc Van Oostenryck
2020-10-13 23:22 ` [PATCH 01/13] format-check: void * is not OK for strings, fix the test Luc Van Oostenryck
2020-10-13 23:22 ` [PATCH 02/13] format-check: more complete parsing of the length & type modifiers Luc Van Oostenryck
2020-10-13 23:22 ` [PATCH 03/13] format-check: add helper type_class() Luc Van Oostenryck
2020-10-13 23:22 ` [PATCH 04/13] format-check: merge 'fmt_string' & 'string' Luc Van Oostenryck
2020-10-13 23:22 ` [PATCH 05/13] format-check: remove unneeded member: target Luc Van Oostenryck
2020-10-13 23:22 ` [PATCH 06/13] format-check: add a function to check to type of strings Luc Van Oostenryck
2020-10-13 23:22 ` [PATCH 07/13] format-check: add a function to check to type of 'n' arguments Luc Van Oostenryck
2020-10-13 23:22 ` [PATCH 08/13] format-check: add a function to check to type of pointers Luc Van Oostenryck
2020-10-13 23:22 ` [PATCH 09/13] format-check: remove printf_fmt_print_pointer() Luc Van Oostenryck
2020-10-13 23:22 ` [PATCH 10/13] format-check: add a function to check the type of floats Luc Van Oostenryck
2020-10-13 23:22 ` [PATCH 11/13] format-check: add a function to check the type of integers Luc Van Oostenryck
2020-10-13 23:22 ` [PATCH 12/13] format-check: remove wrappers around type checking methods Luc Van Oostenryck
2020-10-13 23:22 ` [PATCH 13/13] format-check: simplify calling of parse_printf_get_fmt() Luc Van Oostenryck

Linux-Sparse Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-sparse/0 linux-sparse/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-sparse linux-sparse/ https://lore.kernel.org/linux-sparse \
		linux-sparse@vger.kernel.org
	public-inbox-index linux-sparse

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-sparse


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git