On 03/04/2019 21:46, Luc Van Oostenryck wrote:
On Wed, Apr 03, 2019 at 04:35:48PM +0100, Ben Dooks wrote:
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.
s/.../encountered/
Signed-off-by: Ben Dooks <ben.dooks@xxxxxxxxxxxxxxx>
---
Fixes since v1:
- moved to using ctype in base_type to store infromation
- fixed formatting issues
- updated check for bad format arguments
- reduced the line count to unsigned short to save space
Fixes since v2:
- correctly use the decl->ctype to store printf information
- fixed issues with checking format positions for va_list code
- parse but ignore scanf formatting for now
Fixes since v4:
- deal with function pointers losing format info
---
parse.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-
symbol.h | 2 ++
2 files changed, 103 insertions(+), 1 deletion(-)
diff --git a/parse.c b/parse.c
index f291e24..42ba457 100644
--- a/parse.c
+++ b/parse.c
@@ -136,6 +136,10 @@ static void asm_modifier_inline(struct token *token, unsigned long *mods)
asm_modifier(token, mods, MOD_INLINE);
}
+enum {
+ FmtPrintf = 0, FmtScanf,
It would be good to add a small comment explaining the purpose.
Something like: "for the different kinds of 'format' attributes".
ok, fixed
@@ -1172,6 +1194,74 @@ static struct token *attribute_address_space(struct token *token, struct symbol
return token;
}
+static int invalid_printf_format_args(long long start, long long at)
+{
+ return start < 0 || at < 0 || (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;
+ int argc = 0;
+
+ /* expecting format ( type, start, va_args at) */
+
+ token = expect(token, '(', "after format attribute");
+ while (!match_op(token, ')')) {
+ struct expression *expr = NULL;
+
+ if (argc == 0) {
+ if (token_type(token) == TOKEN_IDENT)
+ fmt_sym = lookup_keyword(token->ident, NS_KEYWORD);
+
+ if (!fmt_sym || !fmt_sym->op ||
+ fmt_sym->op->type != KW_FORMAT) {
+ sparse_error(token->pos,
+ "unknown format type '%s'\n",
+ show_ident(token->ident));
For now, it's better to just ignore unknown format type.
+ fmt_sym = NULL;
+ }
+ }
+
+ token = conditional_expression(token, &expr);
+ if (!expr)
+ break;
+ if (argc < 3)
+ args[argc++] = expr;
+ if (!match_op(token, ','))
+ break;
+ token = token->next;
+ }
+
+ if (argc != 3 || !fmt_sym) {
+ warning(token->pos, "incorrect format attribute");
Since there is no optional argument, I think it would be clearer
to not use a loop but instead do something like:
token = expect(token, '(', "after format attribute");
... check for the KW_FORMAT ...
token = expect(token, ',', ...
token = conditional_expression(token, &expr1);
token = expect(token, ',', ...
token = conditional_expression(token, &expr2);
token = expect(token, ')', ...
ok, i'll change that
@@ -2981,8 +3071,18 @@ 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.printf_msg = decl->ctype.printf_msg;
+ base_type->ctype.printf_va_start = decl->ctype.printf_va_start;
} else if (base_type == &void_ctype && !(decl->ctype.modifiers & MOD_EXTERN)) {
sparse_error(token->pos, "void declaration");
+ } else if (base_type && base_type->type == SYM_PTR) {
+ // think this is correct to do //
+ struct symbol *ptr_base = get_base_type(base_type);
+ if (ptr_base) {
+ ptr_base->ctype.printf_msg = decl->ctype.printf_msg;
+ ptr_base->ctype.printf_va_start = decl->ctype.printf_va_start;
+ }
I'll need to check this.
What situation is this supposed to cover?
definition of pointers to functions, ie:
int (*ptr)(args..) __attribute__((format...));
diff --git a/symbol.h b/symbol.h
index ac43b31..7bb6f29 100644
--- a/symbol.h
+++ b/symbol.h
@@ -103,6 +104,7 @@ struct ctype {
struct context_list *contexts;
struct ident *as;
struct symbol *base_type;
+ unsigned short printf_va_start, printf_msg;
};
Note (mainly to myself): it's unfortunate that this will make
struct ctype bigger but without some generic solution for attributes'
paramaters, this is OK.
ok, thanks.
--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
https://www.codethink.co.uk/privacy.html
