On Wed, Sep 25, 2019 at 11:00:13AM +0100, Ben Dooks wrote: > +static const char *get_printf_fmt(struct symbol *fn, struct expression_list *head) > +{ > + struct expression *expr; > + const char *fmt_string = NULL; > + > + expr = get_expression_n(head, fn->ctype.printf_msg-1); > + if (!expr) > + return NULL; > + if (expr->string && expr->string->length) expr->string is only valid if expr->type == EXPR_STRING, so this must be checked first. > + fmt_string = expr->string->data; > + if (!fmt_string) { > + struct symbol *sym = evaluate_expression(expr); evaluate_expression() only returns the type of the expression, not the underlying symbol (if there is one). But yes, confusingly, both are struct symbol *. You need to check if expr->type == EXPR_SYMBOL and then simply use expr->symbol. > + > + /* attempt to find initialiser for this */ > + if (sym && sym->initializer && sym->initializer->string) Same here with ->string & EXPR_STRING. > + fmt_string = sym->initializer->string->data; > + } I think the code here above should be replaced by something like: if (!evaluate_expression(expr)) return NULL; if (expr->type == EXPR_PREOP && expr->op == '*') expr = expr->unop; if (expr->type == EXPR_SYMBOL) { struct symbol *sym = expr->symbol; if (sym && sym->initializer) expr = sym->initializer; } if (expr->type == EXPR_STRING) return expr->string->data; return NULL; This is not tested but it solves a nasty situation with a test like: void __attribute__((format(printf, 1, 2))) prt(char *, ...); static int x; static inline void fun(void) { prt("%08x\n", x); } static void foo(void) { fun; fun(); } Don't ask the details about this specific test, I don't fully understand it myself, but it's a reduced case for a failure on the kernel code where a warning "not a format string?" was wrongly issued. Please add this one in your tests. -- Lu