On Thu, Jul 31, 2014 at 11:10 AM, <josh@xxxxxxxxxxxxxxxx> wrote:
> GCC 4.6 and newer support initializing designated initializers for fields in
> anonymous structs and unions. However, sparse does not.
I sent patches for this back in April, based on a report from Hans
Verkuil. My patches fixed the parsing part, but had a (known) bug with
getting the offsets right, so your test-case actually retulrs in
t.c:23:6: warning: Initializer entry defined twice
t.c:24:6: also defined here
because the offset calculation for "b" was wrong, and as a result
sparse thinks "a" and "b" overlap. John Keeping had a patch that tried
to fix that too.
I think they are all in
https://github.com/johnkeeping/sparse/
but not in the "official" sparse tree, which hasn't seen any work
since January afaik. Maybe there are other trees than John's..
I'm attaching my set of sparse patches from early April in case
anybody wants to look at them. No guarantees, and as mentioned there's
t least an offset bug, but they should be an improvement, at least.
Linus
From d245709b074c8d07dba61b4a475148d4549e3697 Mon Sep 17 00:00:00 2001
From: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
Date: Sun, 30 Mar 2014 09:49:13 -0700
Subject: [PATCH 1/4] Add warning about duplicate initializers
Noticed this while looking at an independent bug reported by Hans
Verkuil.
Signed-off-by: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
---
evaluate.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/evaluate.c b/evaluate.c
index 66556150ddac..8a53b3e884e0 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -3043,10 +3043,18 @@ static void check_duplicates(struct symbol *sym)
{
int declared = 0;
struct symbol *next = sym;
+ int initialized = sym->initializer != NULL;
while ((next = next->same_symbol) != NULL) {
const char *typediff;
evaluate_symbol(next);
+ if (initialized && next->initializer) {
+ sparse_error(sym->pos, "symbol '%s' has multiple initializers (originally initialized at %s:%d)",
+ show_ident(sym->ident),
+ stream_name(next->pos.stream), next->pos.line);
+ /* Only warn once */
+ initialized = 0;
+ }
declared++;
typediff = type_difference(&sym->ctype, &next->ctype, 0, 0);
if (typediff) {
--
2.0.1.427.gc47372d
From 17139e5d7a35c4b1793221f073a4f543f00b1cfa Mon Sep 17 00:00:00 2001
From: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
Date: Sun, 30 Mar 2014 10:13:54 -0700
Subject: [PATCH 2/4] Use any previous initializer to size a symbol
When we size a symbol, we only have one initializer per symbol, but we
may have multiple symbol declarations for the same symbol. So make sure
to walk the "same_symbol" chain to find the initializer, rather than
assuming it is attached to the current declaration.
Reported-by: Hans Verkuil <hverkuil@xxxxxxxxx>
Signed-off-by: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
---
symbol.c | 24 ++++++++++++++++++------
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/symbol.c b/symbol.c
index eb6e1215ee87..4b91abd8021e 100644
--- a/symbol.c
+++ b/symbol.c
@@ -354,6 +354,15 @@ static int count_array_initializer(struct symbol *t, struct expression *expr)
return nr;
}
+static struct expression *get_symbol_initializer(struct symbol *sym)
+{
+ do {
+ if (sym->initializer)
+ return sym->initializer;
+ } while ((sym = sym->same_symbol) != NULL);
+ return NULL;
+}
+
static struct symbol * examine_node_type(struct symbol *sym)
{
struct symbol *base_type = examine_base_type(sym);
@@ -376,12 +385,15 @@ static struct symbol * examine_node_type(struct symbol *sym)
sym->ctype.alignment = alignment;
/* Unsized array? The size might come from the initializer.. */
- if (bit_size < 0 && base_type->type == SYM_ARRAY && sym->initializer) {
- struct symbol *node_type = base_type->ctype.base_type;
- int count = count_array_initializer(node_type, sym->initializer);
-
- if (node_type && node_type->bit_size >= 0)
- bit_size = node_type->bit_size * count;
+ if (bit_size < 0 && base_type->type == SYM_ARRAY) {
+ struct expression *initializer = get_symbol_initializer(sym);
+ if (initializer) {
+ struct symbol *node_type = base_type->ctype.base_type;
+ int count = count_array_initializer(node_type, initializer);
+
+ if (node_type && node_type->bit_size >= 0)
+ bit_size = node_type->bit_size * count;
+ }
}
sym->bit_size = bit_size;
--
2.0.1.427.gc47372d
From 5e870f7fa535c03ac2c1ae78479327d112c2564e Mon Sep 17 00:00:00 2001
From: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
Date: Tue, 1 Apr 2014 10:18:59 -0700
Subject: [PATCH 3/4] Allow named initializers for anonymous union members
The initializer type evaluation didn't use the function that knew about
recursively looking for member names in anonymous unions and structs.
Signed-off-by: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
---
evaluate.c | 14 ++------------
1 file changed, 2 insertions(+), 12 deletions(-)
diff --git a/evaluate.c b/evaluate.c
index 8a53b3e884e0..5adfc1e3ff26 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -2171,17 +2171,6 @@ static int evaluate_arguments(struct symbol *f, struct symbol *fn, struct expres
return 1;
}
-static struct symbol *find_struct_ident(struct symbol *ctype, struct ident *ident)
-{
- struct symbol *sym;
-
- FOR_EACH_PTR(ctype->symbol_list, sym) {
- if (sym->ident == ident)
- return sym;
- } END_FOR_EACH_PTR(sym);
- return NULL;
-}
-
static void convert_index(struct expression *e)
{
struct expression *child = e->idx_expression;
@@ -2290,11 +2279,12 @@ static struct expression *check_designators(struct expression *e,
}
e = e->idx_expression;
} else if (e->type == EXPR_IDENTIFIER) {
+ int offset = 0;
if (ctype->type != SYM_STRUCT && ctype->type != SYM_UNION) {
err = "field name not in struct or union";
break;
}
- ctype = find_struct_ident(ctype, e->expr_ident);
+ ctype = find_identifier(e->expr_ident, ctype->symbol_list, &offset);
if (!ctype) {
err = "unknown field name in";
break;
--
2.0.1.427.gc47372d
From c011597ff67765da5dacb472ddbd69dc4b51416d Mon Sep 17 00:00:00 2001
From: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
Date: Tue, 1 Apr 2014 10:20:34 -0700
Subject: [PATCH 4/4] Fix scoping of extern symbols in block scope
We'd only match them with other symbols marked 'extern', but really, we
should match them with any top-level non-static symbol.
Signed-off-by: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
---
symbol.c | 14 +++++++++-----
1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/symbol.c b/symbol.c
index 4b91abd8021e..c5e4784734a8 100644
--- a/symbol.c
+++ b/symbol.c
@@ -576,11 +576,15 @@ void check_declaration(struct symbol *sym)
sym->same_symbol = next;
return;
}
- if (sym->ctype.modifiers & next->ctype.modifiers & MOD_EXTERN) {
- if ((sym->ctype.modifiers ^ next->ctype.modifiers) & MOD_INLINE)
- continue;
- sym->same_symbol = next;
- return;
+ /* Extern in block level matches a TOPLEVEL non-static symbol */
+ if (sym->ctype.modifiers & MOD_EXTERN) {
+ if ((next->ctype.modifiers & (MOD_TOPLEVEL|MOD_STATIC)) == MOD_TOPLEVEL) {
+ /* FIXME? Differs in 'inline' only? Why does that matter? */
+ if ((sym->ctype.modifiers ^ next->ctype.modifiers) & MOD_INLINE)
+ continue;
+ sym->same_symbol = next;
+ return;
+ }
}
if (!Wshadow || warned)
--
2.0.1.427.gc47372d
