On Thu, Jul 31, 2014 at 11:10 AM, <josh@xxxxxxxxxxxxxxxx> wrote:
> Test case:
>
> struct S {
> int a;
> union {
> int b;
> int c;
> };
> };
>
> static struct S s = {
> .a = 0,
> .b = 0,
> };
So sparse fails on this test-case (differently from what Josh
reported, because apparently Josh is running 0.5.0) with:
t.c:10:6: warning: Initializer entry defined twice
t.c:11:6: also defined here
because the offsets are miscomputed. The conversion from
EXPR_IDENTIFIER (an initializer entry with a named identifier) to
EXPR_POS (an initializer entry with a byte offset) got this wrong.
We need to use "find_identifier()" that does the proper recursive
lookup and updates the offset, but the required structure type wasn't
passed down to convert_designators() and convert_ident(), so the patch
is a bit bigger than just changing a couple of lines.
I'm quite sure we still screw complex intiializers up . But this makes
at least Josh's test-case pass, and looks superficially sane.
Hmm?
Linus
From 7670d933e82917ea28a7e5a5df09bcc9744dba34 Mon Sep 17 00:00:00 2001
From: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
Date: Thu, 31 Jul 2014 19:01:36 -0700
Subject: [PATCH] Make 'convert_ident()' use 'find_identifier()' to get the
proper member offset
The convert_ident() function used to do
struct symbol *sym = e->field;
e->init_offset = sym->offset;
which is wrong for embedded unnamed unions. It really wants to do
"find_identifier()", which looks up the field name in the structure or
union type, and properly adds the offsets when it looks things up in an
anonymous structure or union.
However, we didn't pass in the right symbol type, so convert_ident()
couldn't be just trivially converted with a couple of lines.
This changes the code to pass in the struct/union type, and also
re-organizes find_identifier() to have a nicer calling convention. We
now pass in the type, not the name-list, which makes it possible for
find_identifier() to do the peeling of the SYM_NODE if required,
simplifying the callers (including the recursive call for the anonymous
case).
Reported-by: Just Triplett <josh@xxxxxxxxxxxxxxxx>
Signed-off-by: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
---
evaluate.c | 47 ++++++++++++++++++++++++++---------------------
1 file changed, 26 insertions(+), 21 deletions(-)
diff --git a/evaluate.c b/evaluate.c
index 03992d03ae1d..5195decd1b2e 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -1872,13 +1872,19 @@ static struct symbol *evaluate_preop(struct expression *expr)
return ctype;
}
-static struct symbol *find_identifier(struct ident *ident, struct symbol_list *_list, int *offset)
+static struct symbol *find_identifier(struct ident *ident, struct symbol *ctype, int *offset)
{
- struct ptr_list *head = (struct ptr_list *)_list;
- struct ptr_list *list = head;
+ struct ptr_list *head, *list;
+ if (ctype->type == SYM_NODE)
+ ctype = ctype->ctype.base_type;
+ if (ctype->type != SYM_UNION && ctype->type != SYM_STRUCT)
+ return NULL;
+
+ head = (struct ptr_list *)ctype->symbol_list;
if (!head)
return NULL;
+ list = head;
do {
int i;
for (i = 0; i < list->nr; i++) {
@@ -1889,13 +1895,8 @@ static struct symbol *find_identifier(struct ident *ident, struct symbol_list *_
*offset = sym->offset;
return sym;
} else {
- struct symbol *ctype = sym->ctype.base_type;
struct symbol *sub;
- if (!ctype)
- continue;
- if (ctype->type != SYM_UNION && ctype->type != SYM_STRUCT)
- continue;
- sub = find_identifier(ident, ctype->symbol_list, offset);
+ sub = find_identifier(ident, sym, offset);
if (!sub)
continue;
*offset += sym->offset;
@@ -1965,7 +1966,7 @@ static struct symbol *evaluate_member_dereference(struct expression *expr)
return NULL;
}
offset = 0;
- member = find_identifier(ident, ctype->symbol_list, &offset);
+ member = find_identifier(ident, ctype, &offset);
if (!member) {
const char *type = ctype->type == SYM_STRUCT ? "struct" : "union";
const char *name = "<unnamed>";
@@ -2220,23 +2221,27 @@ static void convert_index(struct expression *e)
e->init_expr = child;
}
-static void convert_ident(struct expression *e)
+static void convert_ident(struct expression *e, struct symbol *ctype)
{
+ int offset;
struct expression *child = e->ident_expression;
- struct symbol *sym = e->field;
+
+ ctype = find_identifier(e->expr_ident, ctype, &offset);
+ if (!ctype)
+ return;
e->type = EXPR_POS;
- e->init_offset = sym->offset;
+ e->init_offset = offset;
e->init_nr = 1;
e->init_expr = child;
}
-static void convert_designators(struct expression *e)
+static void convert_designators(struct expression *e, struct symbol *ctype)
{
while (e) {
if (e->type == EXPR_INDEX)
convert_index(e);
else if (e->type == EXPR_IDENTIFIER)
- convert_ident(e);
+ convert_ident(e, ctype);
else
break;
e = e->init_expr;
@@ -2322,7 +2327,7 @@ static struct expression *check_designators(struct expression *e,
err = "field name not in struct or union";
break;
}
- ctype = find_identifier(e->expr_ident, ctype->symbol_list, &offset);
+ ctype = find_identifier(e->expr_ident, ctype, &offset);
if (!ctype) {
err = "unknown field name in";
break;
@@ -2392,7 +2397,7 @@ static struct expression *next_designators(struct expression *old,
if (!copy) {
field = old->field->next_subobject;
if (!field) {
- convert_ident(old);
+ convert_ident(old, ctype);
return NULL;
}
copy = e;
@@ -2406,7 +2411,7 @@ static struct expression *next_designators(struct expression *old,
new->expr_ident = field->ident;
new->ident_expression = copy;
new->ctype = field;
- convert_ident(old);
+ convert_ident(old, ctype);
}
return new;
}
@@ -2465,7 +2470,7 @@ static void handle_list_initializer(struct expression *expr,
top = next;
/* deeper than one designator? */
jumped = top != e;
- convert_designators(last);
+ convert_designators(last, ctype);
last = e;
}
@@ -2497,7 +2502,7 @@ found:
} END_FOR_EACH_PTR(e);
- convert_designators(last);
+ convert_designators(last, ctype);
expr->ctype = ctype;
}
@@ -2901,7 +2906,7 @@ static struct symbol *evaluate_offsetof(struct expression *expr)
return NULL;
}
- field = find_identifier(expr->ident, ctype->symbol_list, &offset);
+ field = find_identifier(expr->ident, ctype, &offset);
if (!field) {
expression_error(expr, "unknown member");
return NULL;
--
2.1.0.rc0.52.gaa544bf
