Re: Designated initializers for fields in anonymous structs and unions

[Linus Torvalds <torvalds@linux-foundation.org>]

[-- Attachment #1: Type: text/plain, Size: 1650 bytes --]

On Thu, Jul 31, 2014 at 7:41 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Hmm. It exposes other problems.
>
> For example, the fact that we look up the identifier now exposes the
> fact that "first_subobject()" creates this dummy "EXPR_IDENTIFIER"
> expression that has no identifier. That will then cause
> "find_identifier()" to fail, which in turn causes convert_ident() to
> fail. Resulting in the parse tree having remaining EXPR_IDENTIFIER
> nodes, which will later result in "unknown expression (25,0)" errors.
>
> That's easy enough to fix with just adding the proper initializer, ie
>
>     new->expr_ident = field->ident;
>
> to first_subobject().

Actually, sadly, while that worked for my test-cases (and for Josh's
case), it turns out to really *not* work in general: the dummy
EXPR_IDENTIFIER that we create may end up referring to a unnamed
union, which doesn't *have* an ident associated with it.

This only really triggers when you mix named initializers with then
relying on the whole "initialize the next one without a name", but
that does happen. And it fundamentally breaks that whole "use
find_identifier()" approach.

So convert_ident() really cannot use find_indent(), and really does
have to use "sym->offset". But that doesn't work for anonymous union
members, because that offset was the offset within the anonymous
union, not the parent structure.

However, the fix is "simple". We can just say that anonymous
union/structure member offsets have to be the offsets within the
parent union instead, and fix things up. So how about a patch like
this *instead* of the previous patch..

                 Linus

[-- Attachment #2: patch.diff --]
[-- Type: text/plain, Size: 1551 bytes --]

 evaluate.c |  2 +-
 symbol.c   | 16 ++++++++++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/evaluate.c b/evaluate.c
index 03992d03ae1d..78150f83a19f 100644
--- a/evaluate.c
+++ b/evaluate.c
@@ -1898,7 +1898,7 @@ static struct symbol *find_identifier(struct ident *ident, struct symbol_list *_
 				sub = find_identifier(ident, ctype->symbol_list, offset);
 				if (!sub)
 					continue;
-				*offset += sym->offset;
+				*offset = sym->offset;
 				return sub;
 			}	
 		}
diff --git a/symbol.c b/symbol.c
index a2731348a011..32af64050a64 100644
--- a/symbol.c
+++ b/symbol.c
@@ -116,6 +116,18 @@ static int bitfield_base_size(struct symbol *sym)
 	return sym->bit_size;
 }
 
+static void update_symbol_offsets(struct symbol *sym, unsigned int offset)
+{
+	struct symbol *field;
+
+	if (sym->type != SYM_STRUCT && sym->type != SYM_UNION)
+		return;
+	FOR_EACH_PTR(sym->symbol_list, field) {
+		field->offset += offset;
+		update_symbol_offsets(field, offset);
+	} END_FOR_EACH_PTR(field);
+}
+
 /*
  * Structures are a bit more interesting to lay out
  */
@@ -174,6 +186,10 @@ static void lay_out_struct(struct symbol *sym, struct struct_union_info *info)
 	bit_size = (bit_size + align_bit_mask) & ~align_bit_mask;
 	sym->offset = bits_to_bytes(bit_size);
 
+	/* If it's an unnamed struct or union, update the offsets of the sub-members */
+	if (sym->offset && !sym->ident)
+		update_symbol_offsets(sym, sym->offset);
+
 	info->bit_size = bit_size + base_size;
 	// warning (sym->pos, "regular: offset=%d", sym->offset);
 }