On Sat, Aug 2, 2014 at 11:10 AM, Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:
>
>> Actually, the "find_identifier" call inside function "check_designators" already
>> calculate the correct offset. Only if we can pass that offset to
>> "convert_ident()".
>
> I actually tried that initially, and wanted to get rid of the whole
> "check_designators()" vs "convert_designators()", but couldn't really
> find a sane way to make that work. I'll look at it again, maybe I
> missed something.
No, I didn't miss anything.
The "trivial fix" is to just save off the offset in check_designators
(add a new field to the "EXPR_IDENTIFIER" part of "struct expression"
and then pick up that offset in "convert_ident()"
However, that has exactly the same issue with the whole fake
EXPR_IDENTIFIER created by "next_designators()". Now we need to
initialize the offset field there too. And, for the same reason as
before, the field that "next_designator()" picks may not *have* an
identifier, because the next designator may in fact be an anonymous
union.
Anyway, with all that said, maybe this really confusing and hacky
patch would work. It passes my tests. The magic offset calculation in
next_designators() has a big comment about what the heck it is doing,
and otherwise it's a fairly straightforward case of "save offset from
check_designators() time to be used by convert_ident()".
My stupid test-case is this incredibly hacky thing:
struct A {
int aa;
int bb;
};
struct S {
int a;
union {
int b;
int c;
} u[10];
int dummy;
union {
int d;
int e;
};
};
int fn(void)
{
struct S s = {
.a = 1,
.u[2].b = 2,
.dummy = 1,
{ 3 }
};
return s.dummy*1000 + s.d*100 + s.u[2].b*10 + s.a; // 1321
}
where I use "./test-linearize" to verify that the initializer layout
matches the code generation layout (so that 'fn' function *should*
linearize to just a simple "ret.32 $1321", and with this patch it
does).
But I bet this misses some case. However, the current state wrt
initializer offsets really is very broken, so maybe it's ok.
Can anybody see anything wrong with it? It's reasonably simple. Every
place that initializes "expr->field" for an EXPR_IDENTIFIER now also
initializes "expr->offset".
Linus
evaluate.c | 20 ++++++++++++++++++--
expression.h | 1 +
2 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/evaluate.c b/evaluate.c
index 03992d03ae1d..31a6a49329b8 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -2223,9 +2223,10 @@ static void convert_index(struct expression *e)
static void convert_ident(struct expression *e)
{
struct expression *child = e->ident_expression;
- struct symbol *sym = e->field;
+ int offset = e->offset;
+
e->type = EXPR_POS;
- e->init_offset = sym->offset;
+ e->init_offset = offset;
e->init_nr = 1;
e->init_expr = child;
}
@@ -2277,6 +2278,7 @@ static struct expression *first_subobject(struct symbol *ctype, int class,
new = alloc_expression(e->pos, EXPR_IDENTIFIER);
new->ident_expression = e;
new->field = new->ctype = field;
+ new->offset = field->offset;
}
*v = new;
return new;
@@ -2327,6 +2329,7 @@ static struct expression *check_designators(struct expression *e,
err = "unknown field name in";
break;
}
+ e->offset = offset;
e->field = e->ctype = ctype;
last = e;
if (!e->ident_expression) {
@@ -2386,6 +2389,7 @@ static struct expression *next_designators(struct expression *old,
} else if (old->type == EXPR_IDENTIFIER) {
struct expression *copy;
struct symbol *field;
+ int offset = 0;
copy = next_designators(old->ident_expression,
old->ctype, e, v);
@@ -2397,6 +2401,17 @@ static struct expression *next_designators(struct expression *old,
}
copy = e;
*v = new = alloc_expression(e->pos, EXPR_IDENTIFIER);
+ /*
+ * We can't necessarily trust "field->offset",
+ * because the field might be in an anonymous
+ * union, and the field offset is then the offset
+ * within that union.
+ *
+ * The "old->offset - old->field->offset"
+ * would be the offset of such an anonymous
+ * union.
+ */
+ offset = old->offset - old->field->offset;
} else {
field = old->field;
new = alloc_expression(e->pos, EXPR_IDENTIFIER);
@@ -2406,6 +2421,7 @@ static struct expression *next_designators(struct expression *old,
new->expr_ident = field->ident;
new->ident_expression = copy;
new->ctype = field;
+ new->offset = field->offset + offset;
convert_ident(old);
}
return new;
diff --git a/expression.h b/expression.h
index e31d14051e01..80b3be5f526f 100644
--- a/expression.h
+++ b/expression.h
@@ -149,6 +149,7 @@ struct expression {
struct expression_list *expr_list;
// EXPR_IDENTIFIER
struct /* ident_expr */ {
+ int offset;
struct ident *expr_ident;
struct symbol *field;
struct expression *ident_expression;
