[PATCH 0/6] improve handling of function attributes

[PATCH 0/6] improve handling of function attributes

[Luc Van Oostenryck]

Small improvements in the handling of function attributes as
a preparatory step for more functional changes.


Luc Van Oostenryck (6):
  attribute: sort the table of modifier names
  attribute: add helper apply_mod() and use it
  attribute: allow some attribute to be present multiple times
  attribute: add support for unused
  attribute: separate modifiers into type/declaration
  attribute: teach sparse about attribute((gnu_inline))

 parse.c                      | 34 +++++++++++++++++++++-------------
 show-parse.c                 | 28 +++++++++++++++-------------
 symbol.h                     | 11 ++++++++---
 validation/cond_expr2.c      |  2 +-
 validation/function-redecl.c |  6 +++---
 5 files changed, 48 insertions(+), 33 deletions(-)


base-commit: b8fad4bcd02210912de8cbdefcd77e9ca743a575
-- 
2.26.2

[PATCH 1/6] attribute: sort the table of modifier names

[Luc Van Oostenryck]

It easier to search an item if sorted and this avoid needless
conflict when new items are always added at the end of the table.

So, sort the table but keep the storage modifers first so
that show_typename() & friends still display types as usual.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 show-parse.c                 | 26 +++++++++++++-------------
 validation/cond_expr2.c      |  2 +-
 validation/function-redecl.c |  6 +++---
 3 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/show-parse.c b/show-parse.c
index 8a145b887914..0c4b9ec27645 100644
--- a/show-parse.c
+++ b/show-parse.c
@@ -119,28 +119,28 @@ const char *modifier_string(unsigned long mod)
 
 	static struct mod_name mod_names[] = {
 		{MOD_AUTO,		"auto"},
+		{MOD_EXTERN,		"extern"},
 		{MOD_REGISTER,		"register"},
 		{MOD_STATIC,		"static"},
-		{MOD_EXTERN,		"extern"},
+		{MOD_INLINE,		"inline"},
 		{MOD_CONST,		"const"},
-		{MOD_VOLATILE,		"volatile"},
 		{MOD_RESTRICT,		"restrict"},
-		{MOD_ATOMIC,		"[atomic]"},
-		{MOD_SIGNED,		"[signed]"},
-		{MOD_UNSIGNED,		"[unsigned]"},
-		{MOD_TLS,		"[tls]"},
-		{MOD_INLINE,		"inline"},
+		{MOD_VOLATILE,		"volatile"},
 		{MOD_ADDRESSABLE,	"[addressable]"},
+		{MOD_ASSIGNED,		"[assigned]"},
+		{MOD_ATOMIC,		"[atomic]"},
+		{MOD_BITWISE,		"[bitwise]"},
+		{MOD_EXPLICITLY_SIGNED,	"[explicitly-signed]"},
 		{MOD_NOCAST,		"[nocast]"},
 		{MOD_NODEREF,		"[noderef]"},
-		{MOD_TOPLEVEL,		"[toplevel]"},
-		{MOD_ASSIGNED,		"[assigned]"},
-		{MOD_SAFE,		"[safe]"},
-		{MOD_USERTYPE,		"[usertype]"},
 		{MOD_NORETURN,		"[noreturn]"},
-		{MOD_EXPLICITLY_SIGNED,	"[explicitly-signed]"},
-		{MOD_BITWISE,		"[bitwise]"},
 		{MOD_PURE,		"[pure]"},
+		{MOD_SAFE,		"[safe]"},
+		{MOD_SIGNED,		"[signed]"},
+		{MOD_TLS,		"[tls]"},
+		{MOD_TOPLEVEL,		"[toplevel]"},
+		{MOD_UNSIGNED,		"[unsigned]"},
+		{MOD_USERTYPE,		"[usertype]"},
 	};
 
 	for (i = 0; i < ARRAY_SIZE(mod_names); i++) {
diff --git a/validation/cond_expr2.c b/validation/cond_expr2.c
index 5e974cfaffb3..a5d965764a6f 100644
--- a/validation/cond_expr2.c
+++ b/validation/cond_expr2.c
@@ -16,7 +16,7 @@ cond_expr2.c:6:11: warning: incorrect type in assignment (different modifiers)
 cond_expr2.c:6:11:    expected void volatile *extern [addressable] [toplevel] q
 cond_expr2.c:6:11:    got void const volatile *
 cond_expr2.c:8:11: warning: incorrect type in assignment (different modifiers)
-cond_expr2.c:8:11:    expected int volatile *extern [addressable] [toplevel] [assigned] r
+cond_expr2.c:8:11:    expected int volatile *extern [addressable] [assigned] [toplevel] r
 cond_expr2.c:8:11:    got int const volatile *
  * check-error-end
  */
diff --git a/validation/function-redecl.c b/validation/function-redecl.c
index 0315135932ab..d0d844a5422a 100644
--- a/validation/function-redecl.c
+++ b/validation/function-redecl.c
@@ -51,11 +51,11 @@ void arg_vararg(int a, ...) { }			/* check-should-fail */
 function-redecl.c:5:6: error: symbol 'ret_type' redeclared with different type (different base types):
 function-redecl.c:5:6:    void extern [addressable] [toplevel] ret_type( ... )
 function-redecl.c:4:5: note: previously declared as:
-function-redecl.c:4:5:    int extern [signed] [addressable] [toplevel] ret_type( ... )
+function-redecl.c:4:5:    int extern [addressable] [signed] [toplevel] ret_type( ... )
 function-redecl.c:9:11: error: symbol 'ret_const' redeclared with different type (different modifiers):
-function-redecl.c:9:11:    int extern const [signed] [addressable] [toplevel] ret_const( ... )
+function-redecl.c:9:11:    int extern const [addressable] [signed] [toplevel] ret_const( ... )
 function-redecl.c:8:5: note: previously declared as:
-function-redecl.c:8:5:    int extern [signed] [addressable] [toplevel] ret_const( ... )
+function-redecl.c:8:5:    int extern [addressable] [signed] [toplevel] ret_const( ... )
 function-redecl.c:13:13: error: symbol 'ret_as' redeclared with different type (different address spaces):
 function-redecl.c:13:13:    void <asn:1> *extern [addressable] [toplevel] ret_as( ... )
 function-redecl.c:12:6: note: previously declared as:
-- 
2.26.2

[PATCH 2/6] attribute: add helper apply_mod() and use it

[Luc Van Oostenryck]

to avoid duplicated code checking for ... duplicated modifiers!

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 parse.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/parse.c b/parse.c
index a29c67c8cf41..974ff7a1961c 100644
--- a/parse.c
+++ b/parse.c
@@ -1127,11 +1127,16 @@ static struct token *attribute_aligned(struct token *token, struct symbol *attr,
 	return token;
 }
 
+static void apply_mod(struct position *pos, unsigned long *mods, unsigned long mod)
+{
+	if (*mods & mod)
+		warning(*pos, "duplicate %s", modifier_string(mod));
+	*mods |= mod;
+}
+
 static void apply_qualifier(struct position *pos, struct ctype *ctx, unsigned long qual)
 {
-	if (ctx->modifiers & qual)
-		warning(*pos, "duplicate %s", modifier_string(qual));
-	ctx->modifiers |= qual;
+	apply_mod(pos, &ctx->modifiers, qual);
 }
 
 static struct token *attribute_modifier(struct token *token, struct symbol *attr, struct decl_state *ctx)
@@ -1142,10 +1147,7 @@ static struct token *attribute_modifier(struct token *token, struct symbol *attr
 
 static struct token *attribute_function(struct token *token, struct symbol *attr, struct decl_state *ctx)
 {
-	unsigned long mod = attr->ctype.modifiers;
-	if (ctx->f_modifiers & mod)
-		warning(token->pos, "duplicate %s", modifier_string(mod));
-	ctx->f_modifiers |= mod;
+	apply_mod(&token->pos, &ctx->f_modifiers, attr->ctype.modifiers);
 	return token;
 }
 
-- 
2.26.2

[PATCH 3/6] attribute: allow some attribute to be present multiple times

[Luc Van Oostenryck]

A warning is issued when a qualifier or another modifier
is present more than once in a declaration.

This is fine but in the kernel some attributes, for example
'unused' & 'gnu_inline', are sometimes present multiple times
in the same declaration (mainly they are added in the define
used for 'inline'). This then creates a lot of useless noise.

So, use a (now empty) white list to not warn when these attributes
are present multiple times in the same declaration.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 parse.c  | 2 +-
 symbol.h | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/parse.c b/parse.c
index 974ff7a1961c..c1573b396840 100644
--- a/parse.c
+++ b/parse.c
@@ -1129,7 +1129,7 @@ static struct token *attribute_aligned(struct token *token, struct symbol *attr,
 
 static void apply_mod(struct position *pos, unsigned long *mods, unsigned long mod)
 {
-	if (*mods & mod)
+	if (*mods & mod & ~MOD_DUP_OK)
 		warning(*pos, "duplicate %s", modifier_string(mod));
 	*mods |= mod;
 }
diff --git a/symbol.h b/symbol.h
index 18476582aa8d..9ba764b596da 100644
--- a/symbol.h
+++ b/symbol.h
@@ -261,6 +261,8 @@ struct symbol {
 #define MOD_FUN_ATTR	(MOD_PURE|MOD_NORETURN)
 /* like cvr-qualifiers but 'reversed' (OK: source <= target) */
 #define MOD_REV_QUAL	(MOD_PURE|MOD_NORETURN)
+/* do not warn when these are duplicated */
+#define MOD_DUP_OK	0
 
 
 /* Current parsing/evaluation function */
-- 
2.26.2

[PATCH 4/6] attribute: add support for unused

[Luc Van Oostenryck]

Add support for the attribute 'unused' (and its double underscore
variant. There is no semantic attached to it but it's now at least
parsed and added to the modifiers.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 parse.c      | 2 ++
 show-parse.c | 1 +
 symbol.h     | 6 +++---
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/parse.c b/parse.c
index c1573b396840..281140bf2ee4 100644
--- a/parse.c
+++ b/parse.c
@@ -571,6 +571,8 @@ static struct init_keyword {
 	{ "__noderef__",NS_KEYWORD,	MOD_NODEREF,	.op = &attr_mod_op },
 	{ "safe",	NS_KEYWORD,	MOD_SAFE, 	.op = &attr_mod_op },
 	{ "__safe__",	NS_KEYWORD,	MOD_SAFE, 	.op = &attr_mod_op },
+	{ "unused",	NS_KEYWORD,	MOD_UNUSED,	.op = &attr_mod_op },
+	{ "__unused__",	NS_KEYWORD,	MOD_UNUSED,	.op = &attr_mod_op },
 	{ "force",	NS_KEYWORD,	.op = &attr_force_op },
 	{ "__force__",	NS_KEYWORD,	.op = &attr_force_op },
 	{ "bitwise",	NS_KEYWORD,	MOD_BITWISE,	.op = &attr_bitwise_op },
diff --git a/show-parse.c b/show-parse.c
index 0c4b9ec27645..f4c7de30da2c 100644
--- a/show-parse.c
+++ b/show-parse.c
@@ -140,6 +140,7 @@ const char *modifier_string(unsigned long mod)
 		{MOD_TLS,		"[tls]"},
 		{MOD_TOPLEVEL,		"[toplevel]"},
 		{MOD_UNSIGNED,		"[unsigned]"},
+		{MOD_UNUSED,		"[unused]"},
 		{MOD_USERTYPE,		"[usertype]"},
 	};
 
diff --git a/symbol.h b/symbol.h
index 9ba764b596da..b594eb36ced0 100644
--- a/symbol.h
+++ b/symbol.h
@@ -235,8 +235,8 @@ struct symbol {
      // MOD UNUSED		0x00080000
      // MOD UNUSED		0x00100000
      // MOD UNUSED		0x00200000
-     // MOD UNUSED		0x00400000
 
+#define MOD_UNUSED		0x00400000
 #define MOD_SAFE		0x00800000	// non-null/non-trapping pointer
 #define MOD_PURE		0x01000000
 #define MOD_BITWISE		0x02000000
@@ -252,7 +252,7 @@ struct symbol {
 #define MOD_ESIGNED	(MOD_SIGNED | MOD_EXPLICITLY_SIGNED)
 #define MOD_SIGNEDNESS	(MOD_SIGNED | MOD_UNSIGNED | MOD_EXPLICITLY_SIGNED)
 #define MOD_SPECIFIER	MOD_SIGNEDNESS
-#define MOD_IGNORE	(MOD_STORAGE | MOD_ACCESS | MOD_USERTYPE | MOD_EXPLICITLY_SIGNED | MOD_EXT_VISIBLE)
+#define MOD_IGNORE	(MOD_STORAGE | MOD_ACCESS | MOD_USERTYPE | MOD_EXPLICITLY_SIGNED | MOD_EXT_VISIBLE | MOD_UNUSED)
 #define MOD_QUALIFIER	(MOD_CONST | MOD_VOLATILE | MOD_RESTRICT)
 #define MOD_PTRINHERIT	(MOD_QUALIFIER | MOD_ATOMIC | MOD_NODEREF | MOD_NORETURN | MOD_NOCAST)
 /* modifiers preserved by typeof() operator */
@@ -262,7 +262,7 @@ struct symbol {
 /* like cvr-qualifiers but 'reversed' (OK: source <= target) */
 #define MOD_REV_QUAL	(MOD_PURE|MOD_NORETURN)
 /* do not warn when these are duplicated */
-#define MOD_DUP_OK	0
+#define MOD_DUP_OK	(MOD_UNUSED)
 
 
 /* Current parsing/evaluation function */
-- 
2.26.2

[PATCH 5/6] attribute: separate modifiers into type/declaration

[Luc Van Oostenryck]

When parsing a declaration, type specifiers, qualifiers and other
modifiers are handled by declaration_specifiers(). Some of these
are part of the type being declared but some others only concern
the object in itself. For example, the storage specifiers pertains
to the objects being declared but not their type. Because of this
the storage specifiers need to be processed separately in order to
be correctly applied to the object node. This is done via the helper:
storage_modifier().

However, some attributes are exactly in the same situation (an
obvious example is something like the section attribute).
These attributes should also be moved to the declaration and it's
only because they are currently ignored/without effect that they're
not causing problem in the type.

So generalize storage_modifiers() into decl_modifiers() to extract
all modifiers not pertaining to the type of the declared object.

The modifiers currently concerned are the attributes:
  - unused
  - pure
  - noreturn
  - externally_visible

Note: currently this change shouldn't have any effects other
      than not showing anymore the "[unused]" when displaying
      the type differences in diagnostics.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 parse.c  | 14 ++++++++------
 symbol.h |  3 +++
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/parse.c b/parse.c
index 281140bf2ee4..96e6fda59663 100644
--- a/parse.c
+++ b/parse.c
@@ -1386,7 +1386,7 @@ static const char *storage_class[] =
 	[SForced] = "[force]"
 };
 
-static unsigned long storage_modifiers(struct decl_state *ctx)
+static unsigned long decl_modifiers(struct decl_state *ctx)
 {
 	static unsigned long mod[SMax] =
 	{
@@ -1395,9 +1395,11 @@ static unsigned long storage_modifiers(struct decl_state *ctx)
 		[SStatic] = MOD_STATIC,
 		[SRegister] = MOD_REGISTER
 	};
+	unsigned long mods = ctx->ctype.modifiers & MOD_DECLARE;
+	ctx->ctype.modifiers &= ~MOD_DECLARE;
 	return mod[ctx->storage_class] | (ctx->is_inline ? MOD_INLINE : 0)
 		| (ctx->is_tls ? MOD_TLS : 0)
-		| (ctx->is_ext_visible ? MOD_EXT_VISIBLE : 0);
+		| (ctx->is_ext_visible ? MOD_EXT_VISIBLE : 0) | mods;
 }
 
 static void set_storage_class(struct position *pos, struct decl_state *ctx, int class)
@@ -1674,7 +1676,7 @@ static struct token *declaration_specifiers(struct token *token, struct decl_sta
 			}
 		}
 		token = token->next;
-		if (s->op->declarator)
+		if (s->op->declarator)	// Note: this eats attributes
 			token = s->op->declarator(token, ctx);
 		if (s->op->type & KW_EXACT) {
 			ctx->ctype.base_type = s->ctype.base_type;
@@ -2001,7 +2003,7 @@ static struct token *declaration_list(struct token *token, struct symbol_list **
 	unsigned long mod;
 
 	token = declaration_specifiers(token, &ctx);
-	mod = storage_modifiers(&ctx);
+	mod = decl_modifiers(&ctx);
 	saved = ctx.ctype;
 	for (;;) {
 		struct symbol *decl = alloc_symbol(token->pos, SYM_NODE);
@@ -2054,7 +2056,7 @@ static struct token *parameter_declaration(struct token *token, struct symbol *s
 	token = handle_attributes(token, &ctx, KW_ATTRIBUTE);
 	apply_modifiers(token->pos, &ctx);
 	sym->ctype = ctx.ctype;
-	sym->ctype.modifiers |= storage_modifiers(&ctx);
+	sym->ctype.modifiers |= decl_modifiers(&ctx);
 	sym->endpos = token->pos;
 	sym->forced_arg = ctx.storage_class == SForced;
 	return token;
@@ -2957,7 +2959,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 = decl_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 b594eb36ced0..4bce78d55e54 100644
--- a/symbol.h
+++ b/symbol.h
@@ -263,6 +263,9 @@ struct symbol {
 #define MOD_REV_QUAL	(MOD_PURE|MOD_NORETURN)
 /* do not warn when these are duplicated */
 #define MOD_DUP_OK	(MOD_UNUSED)
+/* must be part of the declared symbol, not its type */
+#define MOD_DECLARE	(MOD_STORAGE|MOD_UNUSED|MOD_PURE|MOD_NORETURN|MOD_EXT_VISIBLE)
+
 
 
 /* Current parsing/evaluation function */
-- 
2.26.2

[PATCH 6/6] attribute: teach sparse about attribute((gnu_inline))

[Luc Van Oostenryck]

But for the moment do nothing special with it.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 parse.c      | 2 ++
 show-parse.c | 1 +
 symbol.h     | 8 ++++----
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/parse.c b/parse.c
index 96e6fda59663..9e7b74f98638 100644
--- a/parse.c
+++ b/parse.c
@@ -590,6 +590,8 @@ static struct init_keyword {
 	{"const",	NS_KEYWORD,	MOD_PURE,	.op = &attr_fun_op },
 	{"__const",	NS_KEYWORD,	MOD_PURE,	.op = &attr_fun_op },
 	{"__const__",	NS_KEYWORD,	MOD_PURE,	.op = &attr_fun_op },
+	{"gnu_inline",	NS_KEYWORD,	MOD_GNU_INLINE,	.op = &attr_fun_op },
+	{"__gnu_inline__",NS_KEYWORD,	MOD_GNU_INLINE,	.op = &attr_fun_op },
 	{"externally_visible",	NS_KEYWORD,	.op = &ext_visible_op },
 	{"__externally_visible__",	NS_KEYWORD,	.op = &ext_visible_op },
 
diff --git a/show-parse.c b/show-parse.c
index f4c7de30da2c..eb71b6504be4 100644
--- a/show-parse.c
+++ b/show-parse.c
@@ -131,6 +131,7 @@ const char *modifier_string(unsigned long mod)
 		{MOD_ATOMIC,		"[atomic]"},
 		{MOD_BITWISE,		"[bitwise]"},
 		{MOD_EXPLICITLY_SIGNED,	"[explicitly-signed]"},
+		{MOD_GNU_INLINE,	"[gnu_inline]"},
 		{MOD_NOCAST,		"[nocast]"},
 		{MOD_NODEREF,		"[noderef]"},
 		{MOD_NORETURN,		"[noreturn]"},
diff --git a/symbol.h b/symbol.h
index 4bce78d55e54..7241f13df4e4 100644
--- a/symbol.h
+++ b/symbol.h
@@ -229,7 +229,7 @@ struct symbol {
 #define MOD_UNSIGNED		0x00004000
 #define MOD_EXPLICITLY_SIGNED	0x00008000
 
-     // MOD UNUSED		0x00010000
+#define MOD_GNU_INLINE		0x00010000
 #define MOD_USERTYPE		0x00020000
      // MOD UNUSED		0x00040000
      // MOD UNUSED		0x00080000
@@ -252,7 +252,7 @@ struct symbol {
 #define MOD_ESIGNED	(MOD_SIGNED | MOD_EXPLICITLY_SIGNED)
 #define MOD_SIGNEDNESS	(MOD_SIGNED | MOD_UNSIGNED | MOD_EXPLICITLY_SIGNED)
 #define MOD_SPECIFIER	MOD_SIGNEDNESS
-#define MOD_IGNORE	(MOD_STORAGE | MOD_ACCESS | MOD_USERTYPE | MOD_EXPLICITLY_SIGNED | MOD_EXT_VISIBLE | MOD_UNUSED)
+#define MOD_IGNORE	(MOD_STORAGE | MOD_ACCESS | MOD_USERTYPE | MOD_EXPLICITLY_SIGNED | MOD_EXT_VISIBLE | MOD_UNUSED | MOD_GNU_INLINE)
 #define MOD_QUALIFIER	(MOD_CONST | MOD_VOLATILE | MOD_RESTRICT)
 #define MOD_PTRINHERIT	(MOD_QUALIFIER | MOD_ATOMIC | MOD_NODEREF | MOD_NORETURN | MOD_NOCAST)
 /* modifiers preserved by typeof() operator */
@@ -262,9 +262,9 @@ struct symbol {
 /* like cvr-qualifiers but 'reversed' (OK: source <= target) */
 #define MOD_REV_QUAL	(MOD_PURE|MOD_NORETURN)
 /* do not warn when these are duplicated */
-#define MOD_DUP_OK	(MOD_UNUSED)
+#define MOD_DUP_OK	(MOD_UNUSED|MOD_GNU_INLINE)
 /* must be part of the declared symbol, not its type */
-#define MOD_DECLARE	(MOD_STORAGE|MOD_UNUSED|MOD_PURE|MOD_NORETURN|MOD_EXT_VISIBLE)
+#define MOD_DECLARE	(MOD_STORAGE|MOD_GNU_INLINE|MOD_UNUSED|MOD_PURE|MOD_NORETURN|MOD_EXT_VISIBLE)
 
 
 
-- 
2.26.2