Re: [PATCH] Re: [Xen-staging] [xen-unstable] Explicitly tag every anonymous aggregate in the public headers.

[Alex Williamson <alex.williamson@hp.com>]

On Fri, 2008-03-28 at 21:47 +0000, Keir Fraser wrote:
> On 28/3/08 21:04, "Alex Williamson" <alex.williamson@hp.com> wrote:
> 
> >    My previous fix allowed this to build on ia64, but it turns out
> > there's still a boot issue that I don't understand.  As is, we take a
> > nested dtlb fault on boot, which hg bisect determines is caused by this
> > patch.  From a simple test program, I can verify that only the outermost
> > __extension__ is necessary to include code w/ -std=c99.  So embedding an
> > __extension__ within an __extension__ isn't necessary, but I don't know
> > why it actually changes the behavior of the code.  The patch below
> > reverts a few chucks of this cset and gets us booting again.  FWIW, I'm
> > using gcc-4.1.2.  Thanks,
> 
> Playing around with this a bit I can't see what changes for having nested
> usage of __extension__. I'm using gcc 4.1 but on x86_64.
> 
> Could you play around with sizeof() and offsetof() and find out exactly
> what's getting laid out differently? Maybe we'll have to do some of the
> reversion that you suggest (or just revert the whole lot and assert __GNUC__
> and !__STRICT_ANSI__ at the top of the file). But it'd be nice to know a bit
> more about what's going on here.

   Ok, here's what I found out:

# struct xen_ia64_opt_feature with __anonymous_struct:

sizeof(struct xen_ia64_opt_feature): 24
offsetof(struct xen_ia64_opt_feature, pgprot): 16

# without:

sizeof(struct xen_ia64_opt_feature): 32
offsetof(struct xen_ia64_opt_feature, pgprot): 16

The sizeof the struct changes, and it can't possible be 24 bytes if an 8
byte pgprot starts at offset 16 and is followed by another 8 bytes, so
the size is wrong with the __extension__ attribute.

--

# struct mapped_regs with __anonymous_struct:

sizeof(struct mapped_regs): 4096
offsetof(struct mapped_regs, dcr): 2048
offsetof(struct mapped_regs, rsv6): 2048
offsetof(struct mapped_regs, precover_ifs): 3072
offsetof(struct mapped_regs, tmp): 3072

# without:

sizeof(struct mapped_regs): 4096
offsetof(struct mapped_regs, dcr): 2048
offsetof(struct mapped_regs, rsv6): 2704
offsetof(struct mapped_regs, precover_ifs): 3072
offsetof(struct mapped_regs, tmp): 3280

This time the sizeof is correct, and the offset of the first entries in
the struct are correct, but the last entry in the struct is at the same
offset as the first, which is obviously unintended.

It seems pretty clear that in nesting __extension__ attributes in this
manner, the struct is actually ignored and the compiler is treating the
everything that was in the struct as a separate member of the union.  In
the mapped_regs example, the structure size remains correct only because
the union is full padded out using the full size arrays.  Please apply
the patch I sent previously to revert the nested __extension__
attributes.  Thanks,

	Alex

-- 
Alex Williamson                             HP Open Source & Linux Org.