| Message ID | 20191115004913.53104-1-luc.vanoostenryck@gmail.com (mailing list archive) |
|---|---|
| State | Mainlined, archived |
| Headers | show |
| Series | function attributes apply to the function declaration | expand |
On 15/11/2019 00:49, Luc Van Oostenryck wrote: > Function attributes relate to the function declaration they > appear in. Sparse ignore most these attributes but a few ones > have a semantic value: 'pure', 'noreturn' & 'externally_visible'. > > Due to how Sparse parse attributes and how these attributes > are stored for functions, the attributes 'pure' & 'noreturn' > are applied not to the function itself but its return type > if the function returns a pointer. > > Fix this by extracting these attributes from the declaration > context and ensure they're applied to the declarator. > > Reported-by: John Levon <john.levon@joyent.com> > Reported-by: Alex Kogan <alex.kogan@oracle.com> > Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com> > --- > parse.c | 17 ++++++++++++++++- > symbol.h | 2 ++ > validation/function-attribute.c | 19 +++++++++++++++++++ > 3 files changed, 37 insertions(+), 1 deletion(-) > create mode 100644 validation/function-attribute.c > Hi Luc, Just a quick heads up (since I can't look at this much for a few days now ...) that the current 'master' branch, when applied to git, causes 8 additional warnings. I have created a cut-down version of the code, thus: $ cat -n git-noreturn.c 1 #include <stdarg.h> 2 #include <unistd.h> 3 #include <stdlib.h> 4 5 #define NORETURN __attribute__((__noreturn__)) 6 #define NORETURN_PTR __attribute__((__noreturn__)) 7 8 void set_die_routine(NORETURN_PTR void (*routine)(const char *err, va_list params)); 9 10 static void NORETURN child_die_fn(const char *err, va_list params) 11 { 12 _exit(2); 13 } 14 15 static void somefunc(void) 16 { 17 set_die_routine(child_die_fn); 18 } 19 20 static NORETURN void die_builtin(const char *err, va_list params) 21 { 22 exit(128); 23 } 24 25 static NORETURN_PTR void (*die_routine)(const char *err, va_list params) = die_builtin; $ ... which causes the following warnings: $ ./sparse git-noreturn.c git-noreturn.c:17:25: warning: incorrect type in argument 1 (different modifiers) git-noreturn.c:17:25: expected void ( [noreturn] *routine )( ... ) git-noreturn.c:17:25: got void ( [noreturn] * )( ... ) git-noreturn.c:25:76: warning: incorrect type in initializer (different modifiers) git-noreturn.c:25:76: expected void ( *static [toplevel] [noreturn] die_routine )( ... ) git-noreturn.c:25:76: got void ( [noreturn] * )( ... ) $ The explanation of the two NORETUN macros is given in commit 18660bc96ec (add NORETURN_PTR for function pointers, 2009-09-30), which reads: commit 18660bc96ec0419cc096a53998d3197f2b905e8a Author: Erik Faye-Lund <kusmabite@gmail.com> Date: Wed Sep 30 18:05:50 2009 +0000 add NORETURN_PTR for function pointers Some compilers (including at least MSVC and ARM RVDS) supports NORETURN on function declarations, but not on function pointers. This patch makes it possible to define NORETURN for these compilers, by splitting the NORETURN macro into two - one for function declarations and one for function pointers. Signed-off-by: Erik Faye-Lund <kusmabite@gmail.com> Signed-off-by: Jeff King <peff@peff.net> ... Sorry to just dump and run ... (hopefully, the above is useful information - I will add the test file as an attachment). ATB, Ramsay Jones
On Sat, Nov 16, 2019 at 12:26:15AM +0000, Ramsay Jones wrote: > Hi Luc, > > Just a quick heads up (since I can't look at this much for > a few days now ...) that the current 'master' branch, when > applied to git, causes 8 additional warnings. > > I have created a cut-down version of the code, thus: ... > ... which causes the following warnings: > > $ ./sparse git-noreturn.c > git-noreturn.c:17:25: warning: incorrect type in argument 1 (different modifiers) > git-noreturn.c:17:25: expected void ( [noreturn] *routine )( ... ) > git-noreturn.c:17:25: got void ( [noreturn] * )( ... ) > git-noreturn.c:25:76: warning: incorrect type in initializer (different modifiers) > git-noreturn.c:25:76: expected void ( *static [toplevel] [noreturn] die_routine )( ... ) > git-noreturn.c:25:76: got void ( [noreturn] * )( ... ) > $ > > Sorry to just dump and run ... (hopefully, the above is useful > information - I will add the test file as an attachment). Hi Ramsay, Thank you very much for reporting this and, yes, the information above is very useful. I've looked a bit at this and all I can say for the moment is that it's a quite intricated problem. I fact there are several problems which interact with each other more or less subltly. -- Luc
diff --git a/parse.c b/parse.c index fa92fae68..37ffede72 100644 --- a/parse.c +++ b/parse.c @@ -2900,6 +2900,21 @@ static struct token *toplevel_asm_declaration(struct token *token, struct symbol return token; } +static unsigned long declaration_modifiers(struct decl_state *ctx) +{ + unsigned long mods; + + // Storage modifiers only relates to the declaration + mods = storage_modifiers(ctx); + + // Function attributes also only relates to the declaration + // and must not be present in the function/return type. + mods |= ctx->ctype.modifiers & MOD_FUN_ATTR; + ctx->ctype.modifiers &=~ MOD_FUN_ATTR; + + return mods; +} + struct token *external_declaration(struct token *token, struct symbol_list **list, validate_decl_t validate_decl) { @@ -2920,7 +2935,7 @@ struct token *external_declaration(struct token *token, struct symbol_list **lis /* Parse declaration-specifiers, if any */ token = declaration_specifiers(token, &ctx); - mod = storage_modifiers(&ctx); + mod = declaration_modifiers(&ctx); decl = alloc_symbol(token->pos, SYM_NODE); /* Just a type declaration? */ if (match_op(token, ';')) { diff --git a/symbol.h b/symbol.h index 4e7e437bf..516b61361 100644 --- a/symbol.h +++ b/symbol.h @@ -251,6 +251,8 @@ struct symbol { #define MOD_PTRINHERIT (MOD_QUALIFIER | MOD_NODEREF | MOD_NORETURN | MOD_NOCAST) /* modifiers preserved by typeof() operator */ #define MOD_TYPEOF (MOD_QUALIFIER | MOD_NOCAST | MOD_SPECIFIER) +/* modifiers for funtion attributes */ +#define MOD_FUN_ATTR (MOD_PURE|MOD_NORETURN) /* Current parsing/evaluation function */ diff --git a/validation/function-attribute.c b/validation/function-attribute.c new file mode 100644 index 000000000..0f2c75922 --- /dev/null +++ b/validation/function-attribute.c @@ -0,0 +1,19 @@ +#define __pure __attribute__((pure)) + +struct s { + int x; +}; + +static __pure struct s *grab(struct s *ptr) +{ + return ptr; +} + +static void foo(struct s *ptr) +{ + struct s *ptr = grab(ptr); +} + +/* + * check-name: function-attribute + */
Function attributes relate to the function declaration they appear in. Sparse ignore most these attributes but a few ones have a semantic value: 'pure', 'noreturn' & 'externally_visible'. Due to how Sparse parse attributes and how these attributes are stored for functions, the attributes 'pure' & 'noreturn' are applied not to the function itself but its return type if the function returns a pointer. Fix this by extracting these attributes from the declaration context and ensure they're applied to the declarator. Reported-by: John Levon <john.levon@joyent.com> Reported-by: Alex Kogan <alex.kogan@oracle.com> Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com> --- parse.c | 17 ++++++++++++++++- symbol.h | 2 ++ validation/function-attribute.c | 19 +++++++++++++++++++ 3 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 validation/function-attribute.c