* [PATCH 1/4] tests: add varargs printf format tests
2020-10-01 9:19 format updates for handing format attribute Ben Dooks
@ 2020-10-01 9:19 ` Ben Dooks
2020-10-01 9:19 ` [PATCH 2/4] parse: initial parsing of __attribute__((format)) Ben Dooks
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Ben Dooks @ 2020-10-01 9:19 UTC (permalink / raw)
To: linux-sparse; +Cc: thomas.preston, christopher.phang, Ben Dooks
Add some tests for the new printf format checking code.
Note, these do not all pass yet.
Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
validation/varargs-format-addrspace1.c | 36 ++++++++
validation/varargs-format-bad.c | 18 ++++
validation/varargs-format-checking.c | 21 +++++
validation/varargs-format-position.c | 32 +++++++
validation/varargs-format-prefix.c | 19 ++++
validation/varargs-format-tests.c | 55 ++++++++++++
validation/varargs-type-formattest.c | 117 +++++++++++++++++++++++++
7 files changed, 298 insertions(+)
create mode 100644 validation/varargs-format-addrspace1.c
create mode 100644 validation/varargs-format-bad.c
create mode 100644 validation/varargs-format-checking.c
create mode 100644 validation/varargs-format-position.c
create mode 100644 validation/varargs-format-prefix.c
create mode 100644 validation/varargs-format-tests.c
create mode 100644 validation/varargs-type-formattest.c
diff --git a/validation/varargs-format-addrspace1.c b/validation/varargs-format-addrspace1.c
new file mode 100644
index 00000000..3370ac67
--- /dev/null
+++ b/validation/varargs-format-addrspace1.c
@@ -0,0 +1,36 @@
+
+extern int variadic(char *msg, ...) __attribute__((format (printf, 1, 2)));
+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;
+
+ variadic("%s\n", a);
+ variadic("%s\n", b);
+ variadic("%s %s\n", b, a);
+ variadic2("%s %s\n", 1, b, a);
+ variadic3(1, "%s %s\n", b, a);
+ variadic3(1, "%s %p\n", b, a);
+}
+
+/*
+ * check-name: variadic formatting test with address-space to %s
+ * check-command: sparse -Wformat $file
+ *
+ * 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: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: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: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
+ * check-error-end
+ */
diff --git a/validation/varargs-format-bad.c b/validation/varargs-format-bad.c
new file mode 100644
index 00000000..82ae357c
--- /dev/null
+++ b/validation/varargs-format-bad.c
@@ -0,0 +1,18 @@
+
+extern int variadic(char *msg, ...) __attribute__((format (printf, 0, 0)));
+extern int variadic2(char *msg, int , ...) __attribute__((format (printf, 2, 2)));
+extern int variadic3(char *msg, int , ...) __attribute__((format (printf, 2, 1)));
+
+static void test(void) {
+}
+
+/*
+ * check-name: variadic formatting test with bad formatting parameters
+ * check-command: sparse -Wformat $file
+ *
+ * check-error-start
+varargs-format-bad.c:2:73: warning: bad format positions
+varargs-format-bad.c:3:80: warning: bad format positions
+varargs-format-bad.c:4:80: warning: format cannot be after va_args
+* check-error-end
+ */
diff --git a/validation/varargs-format-checking.c b/validation/varargs-format-checking.c
new file mode 100644
index 00000000..9f3e5ac2
--- /dev/null
+++ b/validation/varargs-format-checking.c
@@ -0,0 +1,21 @@
+
+extern void pf(char *msg, ...) __attribute__((format (printf, 1, 2)));
+
+static void test(void) {
+ pf("%u %lu %llu\n", 1U, 1UL, 1ULL);
+ pf("%d %ld %lld\n", 1, 1L, 1LL);
+ pf("%x %lx %llx\n", 1U, 1UL, 1ULL);
+ pf("%d %ld %lld\n", 1, 1L, 1L);
+}
+
+/*
+ * check-name: variadic formatting test type checking
+ * check-command: sparse -Wformat $file
+ * check-known-to-fail
+ *
+ * check-error-start
+varargs-format-checking.c:8:36: warning: incorrect type in argument 4 (different types)
+varargs-format-checking.c:8:36: expected long long
+varargs-format-checking.c:8:36: got long
+ * check-error-end
+ */
diff --git a/validation/varargs-format-position.c b/validation/varargs-format-position.c
new file mode 100644
index 00000000..88a4dbc2
--- /dev/null
+++ b/validation/varargs-format-position.c
@@ -0,0 +1,32 @@
+
+extern void pf(char *msg, ...) __attribute__((format (printf, 1, 2)));
+
+static void test(void) {
+ pf("%2$d %u\n", 1U, 1L);
+ pf("%3$d %2$u\n", 1U, 1);
+ pf("%1$d %2$d\n", 1L, 1);
+}
+
+/*
+ * check-name: variadic formatting test position checking
+ * check-command: sparse -Wformat $file
+ * check-known-to-fail
+ *
+ * check-error-start
+varargs-format-position.c:5:29: warning: incorrect type in argument 3 (different types)
+varargs-format-position.c:5:29: expected int
+varargs-format-position.c:5:29: got long
+varargs-format-position.c:5:12: warning: format 3: position: no position specified
+varargs-format-position.c:5:29: warning: incorrect type in argument 3 (different types)
+varargs-format-position.c:5:29: expected unsigned int
+varargs-format-position.c:5:29: got long
+varargs-format-position.c:6:12: warning: no argument at position '4'
+varargs-format-position.c:6:31: warning: incorrect type in argument 3 (different types)
+varargs-format-position.c:6:31: expected unsigned int
+varargs-format-position.c:6:31: got int
+varargs-format-position.c:7:27: warning: incorrect type in argument 2 (different types)
+varargs-format-position.c:7:27: expected int
+varargs-format-position.c:7:27: got long
+ * check-error-end
+ *
+ */
diff --git a/validation/varargs-format-prefix.c b/validation/varargs-format-prefix.c
new file mode 100644
index 00000000..8e2456e6
--- /dev/null
+++ b/validation/varargs-format-prefix.c
@@ -0,0 +1,19 @@
+
+extern int __attribute__((format (printf, 1, 2))) variadic(char *msg, ...);
+
+static int test(void) {
+ void __attribute__((noderef, address_space(1))) *a;
+
+ variadic("%s\n", a);
+}
+
+/*
+ * check-name: variadic formatting test prefix based __attribute__
+ * check-command: sparse -Wformat $file
+ *
+ * check-error-start
+varargs-format-prefix.c:7:26: warning: incorrect type in argument 2 (different address spaces)
+varargs-format-prefix.c:7:26: expected char const *
+varargs-format-prefix.c:7:26: got void [noderef] <asn:1> *a
+ * check-error-end
+ */
diff --git a/validation/varargs-format-tests.c b/validation/varargs-format-tests.c
new file mode 100644
index 00000000..659bbe94
--- /dev/null
+++ b/validation/varargs-format-tests.c
@@ -0,0 +1,55 @@
+
+extern void pf(char *msg, ...) __attribute__((format (printf, 1, 2)));
+
+static int test(void)
+{
+ pf("%*d\n", 5, 10); /* value 10, print width is 5 */
+ pf("%2$*1$d\n", 5, 10); /* value 10, print width is 5 */
+ pf("%3$*2$d\n", 1, 5, 10); /* ok, skipping the '1' */
+ pf("%3$-*2$d\n", 1, 5, 10); /* ok, skipping the '1' */
+ pf("%3$*2$-d\n", 1, 5, 10); /* bad, the "-" shouldn't be before the 'd' */
+ pf("%3$ *2$d\n", 1, 5, 10); /* ok, skipping the '1' */
+ pf("%3$+*2$d\n", 1, 5, 10); /* ok, skipping the '1' */
+ pf("%3$0+*2$d\n", 1, 5, 10); /* ok, skipping the '1' */
+ pf("%3$+0*2$d\n", 1, 5, 10); /* ok, skipping the '1' */
+ pf("%3$+#*2$d\n", 1, 5, 10); /* ok, skipping the '1' */
+ pf("%3$+#*2$.5d\n", 1, 5, 10); /* ok, skipping the '1' */
+
+ /* go with some precision as well as width strings */
+ pf("%2$+*1$.6d\n", 5, 10); /* ok */
+ pf("%2$+*1$.*3$d\n", 5, 10, 6); /* ok */
+ pf("%2$+*3$.*1$d\n", 6, 10, 5); /* ok */
+ pf("%2$+*1$.*d\n", 5, 10, 6); /* not ok */
+
+ pf("%s", "msg");
+ return 0;
+}
+
+static void test2(int x, int y, const void *p)
+{
+ pf("%02x%02x %8p\n", x, y, p);
+}
+
+static inline void fn(int x) { pf("%08x\n", x); }
+static void test3(int x)
+{
+ fn;
+ fn(x);
+}
+
+static void test4(int i, unsigned int u)
+{
+ pf("%d\n", i);
+ pf("%x\n", u);
+}
+
+/*
+ * check-name: variadic formatting tests for width/precisions
+ * check-command: sparse -Wformat $file
+ *
+ * check-error-start
+varargs-format-tests.c:10:12: warning: cannot evaluate type '%3$*2$-d'
+varargs-format-tests.c:10:12: warning: cannot evaluate format string
+varargs-format-tests.c:22:12: warning: format 3: position: no position specified
+ * check-error-end
+ */
diff --git a/validation/varargs-type-formattest.c b/validation/varargs-type-formattest.c
new file mode 100644
index 00000000..f01c6d89
--- /dev/null
+++ b/validation/varargs-type-formattest.c
@@ -0,0 +1,117 @@
+
+extern void pf1(char *msg, ...) __attribute__((format (printf, 1, 2)));
+extern void pf2(int m, char *msg, ...) __attribute__((format (printf, 2, 3)));
+
+/* run all the tests with both of these printf formatted types */
+#define pf(x...) do { pf1(x); pf2(1, x); } while(0);
+
+static void test(void) {
+ /* first two are valid */
+ pf("%*d", 5, 10); /* value 10, print width is 5 */
+ pf("%2$*1$d", 5, 10); /* value 10, print width is 5 */
+ pf("%2$*3$d", 5, 10); /* value 10, print width is ?? */
+
+ pf("%*d", 5, 10); /* value 10, print width is 5 */
+ pf("%*d", 5, 10L); /* value 10, print width is 5 (bad type) */
+ pf("%*d", 5UL, 10L); /* value 10, print width is 5 (bad type) */
+
+ pf("%3$*2$d", 1, 5, 10); /* ok, skipping the '1' */
+ pf("%3$*2$d", 1, 5, 10L); /* bad print type */
+ pf("%2$*3$d", 1UL, 10, 5); /* ok, try with swapping width/val */
+ pf("%2$*3$d", 1UL, 10L, 5); /* bad, try with swapping width/val */
+
+ /* and now try with precision specifiers */
+
+ pf("%*.6d", 5, 10); /* value 10, print width is 5 */
+ pf("%*.6d", 5, 10L); /* value 10, print width is 5 (bad type) */
+ pf("%*.6d", 5UL, 10L); /* value 10, print width is 5 (bad type) */
+
+ pf("%*.*d", 5, 6, 10); /* value 10, print width is 5 */
+ pf("%*.*d", 5, 6, 10L); /* value 10, print width is 5 (bad type) */
+ pf("%*.*d", 5UL, 6, 10L); /* value 10, print width is 5 (bad type) */
+ pf("%*.*d", 5, 6UL, 10); /* value 10, print width is 5 (bad type) */
+}
+
+/*
+ * check-name: variadic formatting test position checking types
+ * check-command: sparse -Wformat $file
+ * check-known-to-fail
+ *
+ * check-error-start
+varargs-type-formattest.c:12:9: warning: width: no argument at position 4
+varargs-type-formattest.c:12:9: warning: width: no argument at position 5
+varargs-type-formattest.c:15:9: warning: incorrect type in argument 3 (different types)
+varargs-type-formattest.c:15:9: expected int
+varargs-type-formattest.c:15:9: got long
+varargs-type-formattest.c:15:9: warning: incorrect type in argument 4 (different types)
+varargs-type-formattest.c:15:9: expected int
+varargs-type-formattest.c:15:9: got long
+varargs-type-formattest.c:16:9: warning: incorrect type for width argument 2
+varargs-type-formattest.c:16:9: expected int
+varargs-type-formattest.c:16:9: got unsigned long
+varargs-type-formattest.c:16:9: warning: incorrect type in argument 3 (different types)
+varargs-type-formattest.c:16:9: expected int
+varargs-type-formattest.c:16:9: got long
+varargs-type-formattest.c:16:9: warning: incorrect type for width argument 3
+varargs-type-formattest.c:16:9: expected int
+varargs-type-formattest.c:16:9: got unsigned long
+varargs-type-formattest.c:16:9: warning: incorrect type in argument 4 (different types)
+varargs-type-formattest.c:16:9: expected int
+varargs-type-formattest.c:16:9: got long
+varargs-type-formattest.c:19:9: warning: incorrect type in argument 4 (different types)
+varargs-type-formattest.c:19:9: expected int
+varargs-type-formattest.c:19:9: got long
+varargs-type-formattest.c:19:9: warning: incorrect type in argument 5 (different types)
+varargs-type-formattest.c:19:9: expected int
+varargs-type-formattest.c:19:9: got long
+varargs-type-formattest.c:21:9: warning: incorrect type in argument 3 (different types)
+varargs-type-formattest.c:21:9: expected int
+varargs-type-formattest.c:21:9: got long
+varargs-type-formattest.c:21:9: warning: incorrect type in argument 4 (different types)
+varargs-type-formattest.c:21:9: expected int
+varargs-type-formattest.c:21:9: got long
+varargs-type-formattest.c:26:9: warning: incorrect type in argument 3 (different types)
+varargs-type-formattest.c:26:9: expected int
+varargs-type-formattest.c:26:9: got long
+varargs-type-formattest.c:26:9: warning: incorrect type in argument 4 (different types)
+varargs-type-formattest.c:26:9: expected int
+varargs-type-formattest.c:26:9: got long
+varargs-type-formattest.c:27:9: warning: incorrect type for width argument 2
+varargs-type-formattest.c:27:9: expected int
+varargs-type-formattest.c:27:9: got unsigned long
+varargs-type-formattest.c:27:9: warning: incorrect type in argument 3 (different types)
+varargs-type-formattest.c:27:9: expected int
+varargs-type-formattest.c:27:9: got long
+varargs-type-formattest.c:27:9: warning: incorrect type for width argument 3
+varargs-type-formattest.c:27:9: expected int
+varargs-type-formattest.c:27:9: got unsigned long
+varargs-type-formattest.c:27:9: warning: incorrect type in argument 4 (different types)
+varargs-type-formattest.c:27:9: expected int
+varargs-type-formattest.c:27:9: got long
+varargs-type-formattest.c:30:9: warning: incorrect type in argument 4 (different types)
+varargs-type-formattest.c:30:9: expected int
+varargs-type-formattest.c:30:9: got long
+varargs-type-formattest.c:30:9: warning: incorrect type in argument 5 (different types)
+varargs-type-formattest.c:30:9: expected int
+varargs-type-formattest.c:30:9: got long
+varargs-type-formattest.c:31:9: warning: incorrect type for width argument 2
+varargs-type-formattest.c:31:9: expected int
+varargs-type-formattest.c:31:9: got unsigned long
+varargs-type-formattest.c:31:9: warning: incorrect type in argument 4 (different types)
+varargs-type-formattest.c:31:9: expected int
+varargs-type-formattest.c:31:9: got long
+varargs-type-formattest.c:31:9: warning: incorrect type for width argument 3
+varargs-type-formattest.c:31:9: expected int
+varargs-type-formattest.c:31:9: got unsigned long
+varargs-type-formattest.c:31:9: warning: incorrect type in argument 5 (different types)
+varargs-type-formattest.c:31:9: expected int
+varargs-type-formattest.c:31:9: got long
+varargs-type-formattest.c:32:9: warning: incorrect type for position argument 3
+varargs-type-formattest.c:32:9: expected int
+varargs-type-formattest.c:32:9: got unsigned long
+varargs-type-formattest.c:32:9: warning: incorrect type for position argument 4
+varargs-type-formattest.c:32:9: expected int
+varargs-type-formattest.c:32:9: got unsigned long
+ * check-error-end
+ *
+ */
--
2.28.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/4] parse: initial parsing of __attribute__((format))
2020-10-01 9:19 format updates for handing format attribute Ben Dooks
2020-10-01 9:19 ` [PATCH 1/4] tests: add varargs printf format tests Ben Dooks
@ 2020-10-01 9:19 ` Ben Dooks
2020-10-01 9:19 ` [PATCH 3/4] add -Wformat Ben Dooks
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Ben Dooks @ 2020-10-01 9:19 UTC (permalink / raw)
To: linux-sparse; +Cc: thomas.preston, christopher.phang, Ben Dooks
Add code to parse the __attribute__((format)) used to indicate that
a variadic function takes a printf-style format string and where
those are. Save the data in ctype ready for checking when such an
function is encoutered.
Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
---
parse.c | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
symbol.h | 10 +++++--
2 files changed, 88 insertions(+), 3 deletions(-)
diff --git a/parse.c b/parse.c
index 31ecef0f..1b021b87 100644
--- a/parse.c
+++ b/parse.c
@@ -85,7 +85,7 @@ static attr_t
attribute_address_space, attribute_context,
attribute_designated_init,
attribute_transparent_union, ignore_attribute,
- attribute_mode, attribute_force;
+ attribute_mode, attribute_force, attribute_format;
typedef struct symbol *to_mode_t(struct symbol *);
@@ -120,6 +120,12 @@ static void asm_modifier(struct token *token, unsigned long *mods, unsigned long
*mods |= mod;
}
+/* the types of formatting from __attribute__((format)) */
+enum {
+ FMT_PRINTF = 0,
+ FMT_SCANF,
+};
+
static struct symbol_op typedef_op = {
.type = KW_MODIFIER,
.declarator = storage_specifier,
@@ -377,6 +383,10 @@ static struct symbol_op attr_force_op = {
.attribute = attribute_force,
};
+static struct symbol_op attr_format_op = {
+ .attribute = attribute_format,
+};
+
static struct symbol_op address_space_op = {
.attribute = attribute_address_space,
};
@@ -436,6 +446,16 @@ static struct symbol_op mode_word_op = {
.to_mode = to_word_mode
};
+static struct symbol_op attr_printf_op = {
+ .type = KW_FORMAT,
+ .class = FMT_PRINTF,
+};
+
+static struct symbol_op attr_scanf_op = {
+ .type = KW_FORMAT,
+ .class = FMT_SCANF,
+};
+
/*
* Define the keyword and their effects.
* The entries in the 'typedef' and put in NS_TYPEDEF and
@@ -551,6 +571,9 @@ static struct init_keyword {
D("pure", &attr_fun_op, .mods = MOD_PURE),
A("const", &attr_fun_op, .mods = MOD_PURE),
D("gnu_inline", &attr_fun_op, .mods = MOD_GNU_INLINE),
+ D("format", &attr_format_op),
+ D("printf", &attr_printf_op),
+ D("scanf", &attr_scanf_op),
/* Modes */
D("mode", &mode_op),
@@ -1188,6 +1211,60 @@ static struct token *attribute_address_space(struct token *token, struct symbol
return token;
}
+static int invalid_format_args(long long start, long long at)
+{
+ return start < 0 || at < 0 || start > USHRT_MAX || at > USHRT_MAX ||
+ (start == at && start > 0) ||
+ (start == 0 && at == 0);
+}
+
+static struct token *attribute_format(struct token *token, struct symbol *attr, struct decl_state *ctx)
+{
+ struct expression *args[3];
+ struct symbol *fmt_sym = NULL;
+
+ /* expecting format ( type, start, va_args at) */
+
+ token = expect(token, '(', "after format attribute");
+ if (token_type(token) == TOKEN_IDENT)
+ fmt_sym = lookup_keyword(token->ident, NS_KEYWORD);
+ if (fmt_sym)
+ if (!fmt_sym->op || fmt_sym->op->type != KW_FORMAT)
+ fmt_sym = NULL;
+
+ token = conditional_expression(token, &args[0]);
+ token = expect(token, ',', "format attribute type");
+ token = conditional_expression(token, &args[1]);
+ token = expect(token, ',', "format attribute type position");
+ token = conditional_expression(token, &args[2]);
+ token = expect(token, ')', "format attribute arg position");
+
+ if (!fmt_sym || !args[0] || !args[1] || !args[2]) {
+ warning(token->pos, "incorrect format attribute");
+ } else if (fmt_sym->op->class != FMT_PRINTF) {
+ /* skip anything that isn't printf for the moment */
+ warning(token->pos, "only printf format attribute supported");
+ } else {
+ long long start, at;
+
+ start = get_expression_value(args[2]);
+ at = get_expression_value(args[1]);
+
+ if (invalid_format_args(start, at)) {
+ warning(token->pos, "bad format positions");
+ } else if (start == 0) {
+ /* nothing to do here, is va_list function */
+ } else if (start < at) {
+ warning(token->pos, "format cannot be after va_args");
+ } else {
+ ctx->ctype.format.index = at;
+ ctx->ctype.format.first = start;
+ }
+ }
+
+ return token;
+}
+
static struct symbol *to_QI_mode(struct symbol *ctype)
{
if (ctype->ctype.base_type != &int_type)
@@ -2957,6 +3034,8 @@ struct token *external_declaration(struct token *token, struct symbol_list **lis
if (!(decl->ctype.modifiers & MOD_STATIC))
decl->ctype.modifiers |= MOD_EXTERN;
+
+ base_type->ctype.format = decl->ctype.format;
} else if (base_type == &void_ctype && !(decl->ctype.modifiers & MOD_EXTERN)) {
sparse_error(token->pos, "void declaration");
}
diff --git a/symbol.h b/symbol.h
index a3ed9567..e8ebbb71 100644
--- a/symbol.h
+++ b/symbol.h
@@ -82,8 +82,8 @@ enum keyword {
KW_ASM = 1 << 5,
KW_MODE = 1 << 6,
KW_STATIC = 1 << 7,
- // KW UNUSED = 1 << 8,
- KW_EXACT = 1 << 9,
+ KW_EXACT = 1 << 8,
+ KW_FORMAT = 1 << 9,
};
struct context {
@@ -95,12 +95,18 @@ extern struct context *alloc_context(void);
DECLARE_PTR_LIST(context_list, struct context);
+struct attr_format {
+ unsigned short index; /* index in argument list for format string */
+ unsigned short first; /* where first variadic argument is */
+};
+
struct ctype {
struct symbol *base_type;
unsigned long modifiers;
unsigned long alignment;
struct context_list *contexts;
struct ident *as;
+ struct attr_format format;
};
struct decl_state {
--
2.28.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 3/4] add -Wformat
2020-10-01 9:19 format updates for handing format attribute Ben Dooks
2020-10-01 9:19 ` [PATCH 1/4] tests: add varargs printf format tests Ben Dooks
2020-10-01 9:19 ` [PATCH 2/4] parse: initial parsing of __attribute__((format)) Ben Dooks
@ 2020-10-01 9:19 ` Ben Dooks
2020-10-01 9:19 ` [PATCH 4/4] evaluate: check variadic argument types against formatting info Ben Dooks
2020-10-02 16:03 ` format updates for handing format attribute Luc Van Oostenryck
4 siblings, 0 replies; 6+ messages in thread
From: Ben Dooks @ 2020-10-01 9:19 UTC (permalink / raw)
To: linux-sparse; +Cc: thomas.preston, christopher.phang, Ben Dooks
Add option to enable/disable format checking (and default it to off)
Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
---
options.c | 2 ++
options.h | 1 +
sparse.1 | 10 ++++++++++
3 files changed, 13 insertions(+)
diff --git a/options.c b/options.c
index 294dfd3b..ffd11798 100644
--- a/options.c
+++ b/options.c
@@ -100,6 +100,7 @@ int Wdesignated_init = 1;
int Wdo_while = 0;
int Wenum_mismatch = 1;
int Wexternal_function_has_definition = 1;
+int Wformat = 0;
int Wimplicit_int = 1;
int Winit_cstring = 0;
int Wint_to_pointer_cast = 1;
@@ -840,6 +841,7 @@ static const struct flag warnings[] = {
{ "do-while", &Wdo_while },
{ "enum-mismatch", &Wenum_mismatch },
{ "external-function-has-definition", &Wexternal_function_has_definition },
+ { "format", &Wformat },
{ "implicit-int", &Wimplicit_int },
{ "init-cstring", &Winit_cstring },
{ "int-to-pointer-cast", &Wint_to_pointer_cast },
diff --git a/options.h b/options.h
index abdf0864..4e50db0b 100644
--- a/options.h
+++ b/options.h
@@ -99,6 +99,7 @@ extern int Wdesignated_init;
extern int Wdo_while;
extern int Wenum_mismatch;
extern int Wexternal_function_has_definition;
+extern int Wformat;
extern int Wimplicit_int;
extern int Winit_cstring;
extern int Wint_to_pointer_cast;
diff --git a/sparse.1 b/sparse.1
index 48dab7a9..860fde7c 100644
--- a/sparse.1
+++ b/sparse.1
@@ -257,6 +257,16 @@ Sparse issues these warnings by default. To turn them off, use
\fB\-Wno\-external\-function\-has\-definition\fR.
.
.TP
+.B \-Wformat
+Warn about parameter mismatch to any variadic function which specifies
+where the format string is specified with the
+.BI __attribute__((format( type, message, va_start )))
+attribute.
+
+Sparse does not issue these warnings by default. To turn them on, use
+\fB\-W-format\fR.
+.
+.TP
.B \-Winit\-cstring
Warn about initialization of a char array with a too long constant C string.
--
2.28.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 4/4] evaluate: check variadic argument types against formatting info
2020-10-01 9:19 format updates for handing format attribute Ben Dooks
` (2 preceding siblings ...)
2020-10-01 9:19 ` [PATCH 3/4] add -Wformat Ben Dooks
@ 2020-10-01 9:19 ` Ben Dooks
2020-10-02 16:03 ` format updates for handing format attribute Luc Van Oostenryck
4 siblings, 0 replies; 6+ messages in thread
From: Ben Dooks @ 2020-10-01 9:19 UTC (permalink / raw)
To: linux-sparse; +Cc: thomas.preston, christopher.phang, Ben Dooks
The variadic argumnet code did not check any of the variadic arguments
as it did not previously know the possible type. Now we have the possible
formatting information stored in the ctype, we can do some checks on the
printf formatting types.
Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
---
---
Makefile | 1 +
builtin.c | 2 +-
evaluate.c | 14 +-
evaluate.h | 10 +-
verify-format.c | 496 ++++++++++++++++++++++++++++++++++++++++++++++++
verify-format.h | 6 +
6 files changed, 523 insertions(+), 6 deletions(-)
create mode 100644 verify-format.c
create mode 100644 verify-format.h
diff --git a/Makefile b/Makefile
index 09726ebb..457d9853 100644
--- a/Makefile
+++ b/Makefile
@@ -90,6 +90,7 @@ LIB_OBJS += target-xtensa.o
LIB_OBJS += tokenize.o
LIB_OBJS += unssa.o
LIB_OBJS += utils.o
+LIB_OBJS += verify-format.o
PROGRAMS :=
PROGRAMS += compile
diff --git a/builtin.c b/builtin.c
index 26b612dc..ce9de28f 100644
--- a/builtin.c
+++ b/builtin.c
@@ -396,7 +396,7 @@ static int eval_sync_compare_and_swap(struct expression *expr)
if (!expr->ctype) // __sync_val_compare_and_swap()
expr->ctype = ctype;
- return evaluate_arguments(types, expr->args);
+ return evaluate_arguments(NULL, types, expr->args);
err:
sparse_error(arg->pos, "invalid type for argument %d:", n);
diff --git a/evaluate.c b/evaluate.c
index c1ef348a..fb3c0adb 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -42,6 +42,7 @@
#include "symbol.h"
#include "target.h"
#include "expression.h"
+#include "verify-format.h"
struct symbol *current_fn;
@@ -1411,8 +1412,8 @@ static int whitelist_pointers(struct symbol *t1, struct symbol *t2)
return !Wtypesign;
}
-static int check_assignment_types(struct symbol *target, struct expression **rp,
- const char **typediff)
+int check_assignment_types(struct symbol *target, struct expression **rp,
+ const char **typediff)
{
struct symbol *source = degenerate(*rp);
struct symbol *t, *s;
@@ -2344,7 +2345,8 @@ static struct symbol *evaluate_alignof(struct expression *expr)
return size_t_ctype;
}
-int evaluate_arguments(struct symbol_list *argtypes, struct expression_list *head)
+int evaluate_arguments(struct symbol *fn, struct symbol_list *argtypes,
+ struct expression_list *head)
{
struct expression *expr;
struct symbol *argtype;
@@ -2385,6 +2387,10 @@ int evaluate_arguments(struct symbol_list *argtypes, struct expression_list *hea
NEXT_PTR_LIST(argtype);
} END_FOR_EACH_PTR(expr);
FINISH_PTR_LIST(argtype);
+
+ if (fn && Wformat)
+ verify_format_attribute(fn, head);
+
return 1;
}
@@ -3208,7 +3214,7 @@ static struct symbol *evaluate_call(struct expression *expr)
if (!sym->op->args(expr))
return NULL;
} else {
- if (!evaluate_arguments(ctype->arguments, arglist))
+ if (!evaluate_arguments(ctype, ctype->arguments, arglist))
return NULL;
args = expression_list_size(expr->args);
fnargs = symbol_list_size(ctype->arguments);
diff --git a/evaluate.h b/evaluate.h
index a16e9703..3f51129d 100644
--- a/evaluate.h
+++ b/evaluate.h
@@ -28,8 +28,16 @@ void evaluate_symbol_list(struct symbol_list *list);
///
// evaluate the arguments of a function
+// @fn: the symbol of the prototype
// @argtypes: the list of the types in the prototype
// @args: the list of the effective arguments
-int evaluate_arguments(struct symbol_list *argtypes, struct expression_list *args);
+int evaluate_arguments(struct symbol *fn, struct symbol_list *argtypes, struct expression_list *args);
+///
+// check if assignment types are compatible
+// @target: the target assignment
+// @rp: the expression
+// @typediff: the resulant message if different type
+int check_assignment_types(struct symbol *target, struct expression **rp,
+ const char **typediff);
#endif
diff --git a/verify-format.c b/verify-format.c
new file mode 100644
index 00000000..2da9d206
--- /dev/null
+++ b/verify-format.c
@@ -0,0 +1,496 @@
+/*
+ * sparse/verify-format.c
+ *
+ * Copyright (C) 2019 Codethink Ltd.
+ * Written by Ben Dooks <ben.dooks@codethink.co.uk>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ *
+ * Verification code for format-attributes (currently printf)
+ */
+#include <stdlib.h>
+#include <stdarg.h>
+#include <stddef.h>
+#include <stdio.h>
+#include <string.h>
+#include <ctype.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <limits.h>
+
+#include "evaluate.h"
+#include "lib.h"
+#include "allocate.h"
+#include "parse.h"
+#include "token.h"
+#include "symbol.h"
+#include "target.h"
+#include "expression.h"
+#include "verify-format.h"
+
+struct format_type {
+ const char *format;
+ int (*test)(struct format_type *fmt,
+ struct expression **expr,
+ struct symbol *ctype,
+ struct symbol **target,
+ const char **typediff);
+ struct symbol *data;
+};
+
+struct format_state {
+ struct expression *expr;
+ unsigned int first;
+ unsigned int fmt_index;
+ unsigned int arg_index;
+ unsigned int used_position: 1;
+};
+
+static int printf_fmt_numtype(struct format_type *fmt,
+ struct expression **expr,
+ struct symbol *ctype,
+ struct symbol **target, const char **typediff)
+{
+ struct symbol *type = fmt->data;
+ *target = type;
+ return check_assignment_types(*target, expr, typediff);
+}
+
+static int printf_fmt_string(struct format_type *fmt,
+ struct expression **expr,
+ struct symbol *ctype,
+ struct symbol **target, const char **typediff)
+{
+ *target = &const_string_ctype;
+ return check_assignment_types(*target, expr, typediff);
+}
+
+static int printf_fmt_pointer(struct format_type *fmt,
+ struct expression **expr,
+ struct symbol *ctype,
+ struct symbol **target, const char **typediff)
+{
+ *target = &const_ptr_ctype;
+ return check_assignment_types(*target, 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);
+ 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,
+};
+
+static struct expression *get_nth_expression(struct expression_list *args, int nr)
+{
+ return ptr_list_nth_entry((struct ptr_list *)args, nr);
+}
+
+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;
+
+ type->test = NULL;
+ *msgout = ptr;
+
+ if (*ptr == 's') {
+ ptr++;
+ type->test = printf_fmt_string;
+ } else if (*ptr == 'c') {
+ ptr++;
+ type->test = printf_fmt_numtype;
+ type->data = &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') {
+ ptr++;
+ } else if (isalpha(*ptr)) {
+ /* probably some extra specifiers after %p */
+ ptr++;
+ type->test = printf_fmt_pointer;
+ }
+ } else if (*ptr == 'z') {
+ // todo - we should construct pointer to int/etc //
+
+ ptr++;
+ if (*ptr == 'd' || *ptr == 'i') {
+ ptr++;
+ type->test = printf_fmt_numtype;
+ type->data = ssize_t_ctype;
+ } else if (*ptr == 'u' || *ptr == 'x' || *ptr == 'X' ||
+ *ptr == 'o') {
+ ptr++;
+ type->test = printf_fmt_numtype;
+ type->data = size_t_ctype;
+ }
+ } else {
+ if (*ptr == 'l') {
+ szmod++;
+ ptr++;
+ 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++;
+ }
+ }
+
+ if (*ptr == 'x' || *ptr == 'X' || *ptr == 'u' || *ptr == 'o') {
+ ptr++;
+ type->test = printf_fmt_numtype;
+ switch (szmod) {
+ case -1:
+ type->data = &ushort_ctype;
+ break;
+ case 0:
+ type->data = &uint_ctype;
+ break;
+ case 1:
+ type->data = &ulong_ctype;
+ break;
+ case 2:
+ type->data = &ullong_ctype;
+ break;
+ default:
+ type->test = NULL;
+ }
+ } else if (*ptr == 'i' || *ptr == 'd') {
+ ptr++;
+ type->test = printf_fmt_numtype;
+ switch (szmod) {
+ case -1:
+ type->data = &short_ctype;
+ break;
+ case 0:
+ type->data = &int_ctype;
+ break;
+ case 1:
+ type->data = &long_ctype;
+ break;
+ case 2:
+ type->data = &llong_ctype;
+ break;
+ default:
+ type->test = NULL;
+ }
+ } else if (*ptr == 'L' && is_float_spec(ptr[1])) {
+ type->test = printf_fmt_numtype;
+ type->data = &ldouble_ctype;
+ ptr += 2;
+ } else if (is_float_spec(*ptr)) {
+ type->test = printf_fmt_numtype;
+ type->data = szmod == 1 ? &ldouble_ctype : &double_ctype;
+ 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;
+ ptr++;
+ } else {
+ // anything else here?
+ }
+ }
+
+ if (type->test == NULL)
+ return NULL;
+
+ *msgout = ptr;
+ return type;
+}
+
+static int is_printf_flag(char ch)
+{
+ return ch == '0' || ch == '+' || ch == '-' || ch == ' ' || ch == '#';
+}
+
+static int printf_check_position(const char **fmt)
+{
+ const char *ptr= *fmt;
+
+ if (!isdigit(*ptr))
+ return -1;
+ while (isdigit(*ptr))
+ ptr++;
+ if (*ptr == '$') {
+ const char *pos = *fmt;
+ *fmt = ptr+1;
+ return strtoul(pos, NULL, 10);
+ }
+ return -1;
+}
+
+static void parse_format_printf_checkpos(struct format_state *state, const char *which)
+{
+ if (state->used_position) {
+ warning(state->expr->pos,
+ "format %d: %s: no position specified",
+ state->arg_index-1, which);
+ }
+}
+
+static int parse_format_printf_argfield(const char **fmtptr,
+ struct format_state *state,
+ struct expression_list *args,
+ int *pos, const char *which)
+{
+ struct expression *expr;
+ struct symbol *ctype;
+ const char *fmt = *fmtptr;
+ int argpos = -1;
+
+ /* check for simple digit-string width/precision specifier first */
+ if (*fmt != '*') {
+ while (isdigit(*fmt))
+ fmt++;
+ *fmtptr = fmt;
+ return 0;
+ }
+
+ fmt++;
+ argpos = printf_check_position(&fmt);
+
+ if (argpos > 0) {
+ argpos += state->first - 1;
+ state->used_position = 1;
+ } else {
+ argpos = (*pos)++;
+ state->arg_index++;
+ parse_format_printf_checkpos(state, which);
+ }
+
+ *fmtptr = fmt;
+ expr = get_nth_expression(args, argpos-1);
+ if (!expr) {
+ warning(state->expr->pos, "%s: no argument at position %d", which, argpos);
+ return 1;
+ }
+
+ /* check the value we got was int/uint type */
+ ctype = expr->ctype;
+ if (ctype) {
+ struct symbol *target = &int_ctype;
+
+ if (ctype != &int_ctype && ctype != &uint_ctype) {
+ warning(expr->pos, "incorrect type for %s argument %d", which, argpos);
+ info(expr->pos, " expected %s", show_typename(target));
+ info(expr->pos, " got %s", show_typename(ctype));
+ }
+ }
+
+ return 0;
+}
+
+/*
+ * printf format parsing code
+ *
+ * this code currently does not:
+ * - check castable types (such as int vs long vs long long)
+ * - validate all arguments specified are also used...
+ */
+static int parse_format_printf(const char **fmtstring,
+ struct format_state *state,
+ 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 */
+ int pos = state->arg_index; /* position of the argument */
+ int error = 0;
+ int ret;
+
+ if (!fmt) {
+ warning(state->expr->pos, "no format string passed");
+ return -1;
+ }
+
+ /* trivial check for %% */
+ fmt++;
+ if (fmt[0] == '%') {
+ *fmtstring = fmt+1;
+ return 0;
+ }
+
+ state->arg_index++;
+ state->fmt_index++;
+
+ ret = printf_check_position(&fmt);
+ if (ret == 0) {
+ /* we got an invalid position argument */
+ error++;
+ } else if (ret < 0) {
+ parse_format_printf_checkpos(state, "position");
+ } else {
+ state->used_position = 1;
+ pos = ret + state->first - 1;
+ }
+
+ /* get rid of any formatting flag bits */
+ while (is_printf_flag(*fmt))
+ fmt++;
+
+ /* now there is the posibility of a width specifier */
+ if (parse_format_printf_argfield(&fmt, state, args, &pos, "width"))
+ error++;
+
+ /* now we might have the precision specifier */
+ if (*fmt == '.') {
+ fmt++;
+ if (parse_format_printf_argfield(&fmt, state, args, &pos, "position"))
+ error++;
+ }
+
+ 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, *target = NULL;
+ const char *typediff = "different types";
+ int ret;
+
+ *fmtstring = fmtpost;
+ expr = get_nth_expression(args, pos-1);
+ if (!expr) {
+ /* no argument, but otherwise valid argument string */
+ warning(state->expr->pos, "no argument at position '%d'", pos);
+ return 0;
+ }
+
+ ctype = expr->ctype;
+ if (!ctype)
+ return -3;
+
+ ret = type->test(type, &expr, ctype, &target, &typediff);
+ if (!target) /* shouldn't happen, but catch anyway */
+ return -4;
+
+ 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, " got %s", show_typename(ctype));
+ }
+ } else {
+ /* try and find the end of this format string by looking for a space*/
+ fmtpost = *fmtstring;
+ while (*fmtpost > ' ')
+ fmtpost++;
+ warning(state->expr->pos, "cannot evaluate type '%.*s'",
+ (int)(fmtpost - *fmtstring), *fmtstring);
+ *fmtstring += 1;
+ return -1;
+ }
+
+ return 1;
+}
+
+/*
+ * attempt to run through a printf format string and work out the types
+ * it specifies. The format is parsed from the __attribute__(format())
+ * in the parser code which stores the positions of the message and arg
+ * start in the ctype.
+ */
+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;
+
+ if (!fn->ctype.format.index)
+ return;
+
+ expr = get_nth_expression(args, fn->ctype.format.index-1);
+ if (!expr)
+ return;
+
+ if (expr->type != EXPR_SYMBOL || expr->symbol->ident)
+ return; // not a literal
+ init = expr->symbol->initializer;
+ if (!init || init->type != EXPR_STRING)
+ return; // not a string
+ fmt_string = init->string->data;
+
+ state.expr = expr;
+ state.first = fn->ctype.format.first;
+ state.arg_index = fn->ctype.format.first;
+
+ if (!fmt_string) {
+ warning(expr->pos, "not a format string?");
+ } else {
+ const char *string = fmt_string;
+ int fail = 0;
+
+ while (string[0]) {
+ if (string[0] != '%') {
+ /* strip anything before the '%' */
+ string++;
+ continue;
+ }
+
+ if (parse_format_printf(&string, &state, args) < 0)
+ fail++;
+ }
+
+ if (fail > 0)
+ /* format string may have '\n' etc embedded in it */
+ warning(expr->pos, "cannot evaluate format string");
+ }
+}
diff --git a/verify-format.h b/verify-format.h
new file mode 100644
index 00000000..4a7ef79d
--- /dev/null
+++ b/verify-format.h
@@ -0,0 +1,6 @@
+#ifndef VERIFY_FORMAT_H
+#define VERIFY_FORMAT_H
+
+void verify_format_attribute(struct symbol *fn, struct expression_list *args);
+
+#endif
--
2.28.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: format updates for handing format attribute
2020-10-01 9:19 format updates for handing format attribute Ben Dooks
` (3 preceding siblings ...)
2020-10-01 9:19 ` [PATCH 4/4] evaluate: check variadic argument types against formatting info Ben Dooks
@ 2020-10-02 16:03 ` Luc Van Oostenryck
4 siblings, 0 replies; 6+ messages in thread
From: Luc Van Oostenryck @ 2020-10-02 16:03 UTC (permalink / raw)
To: Ben Dooks; +Cc: linux-sparse, thomas.preston, christopher.phang
On Thu, Oct 01, 2020 at 10:19:19AM +0100, Ben Dooks wrote:
> Hi, this is the update set for the format attributes.
>
> I think the only thing I'm still thinking about is whether to
> just merged patches 3 and 4.
>
> branch:
>
> https://gitlab.com/CodethinkLabs/sparse.git bjdooks/printf-v062
This looks all good. Thanks.
I still need to do some runtime checks but this will be for later.
-- Luc
^ permalink raw reply [flat|nested] 6+ messages in thread