diff mbox

[2/2] Use any previous initializer to size a symbol

Message ID alpine.LFD.2.11.1403301020530.16374@i7.linux-foundation.org (mailing list archive)
State Mainlined, archived
Headers show

Commit Message

Linus Torvalds March 30, 2014, 5:22 p.m. UTC
From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Sun, 30 Mar 2014 10:13:54 -0700
Subject: [PATCH 2/2] 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@xs4all.nl>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---

This seems to be the simplest approach. Chris, what was your concern about 
scoping? The "same_symbol" list should already take scoping into account.

 symbol.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

Comments

Christopher Li March 31, 2014, 5:23 a.m. UTC | #1
On Sun, Mar 30, 2014 at 10:22 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> From: Linus Torvalds <torvalds@linux-foundation.org>
> Date: Sun, 30 Mar 2014 10:13:54 -0700
> Subject: [PATCH 2/2] 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@xs4all.nl>
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

This patch looks good to me. Will apply.


> ---
>
> This seems to be the simplest approach. Chris, what was your concern about
> scoping? The "same_symbol" list should already take scoping into account.


This bug is a special case of the more general sparse bug in merging
declaration types.

The scoping I am concern about is some thing like this:
(BTW, your patch does not have the scoping issue).

extern void foo(void) __attribute__((X));
for (;;) {
    extern void foo(void) __attribute__((Y));
    /* function foo()  should have both attribute X and Y here */

}

/* foo() should have only attribute X here */

Merging the initializer is a special case of this more general
bug. Obviously, this patch fix the merging of initialization,  should
be applied.

Chris
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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;