[coreboot] GCC update broke AMD Fam10h boot

[Aaron Durbin]

On Thu, Mar 19, 2015 at 5:29 PM, Julius Werner <jwerner at chromium.org> wrote:
> Sounds like this could've been solved with a simple ALIGN(8) in the
> ldscript, right? I don't know what made the compiler think that it
> would have to align i386 pointers to 8 byte (which seems to be what
> happened), but if it makes that decision then it should also conclude
> that sizeof(struct boot_state_init_entry) == 0x20 and the array
> traversal should still work. If those structures need to be writable
> we could simply move them from .rodata to .data in the ldscript.

You are right that it would work, but back solving for which alignment
the compiler decided is hard. It's definitely whack-a-mole in that
case. It could have very well decided 16 too. Without any insight as
to why that breaks down. Or you go the route of putting alignments on
structures just for that behavior which I consider not the best
approach.

They do need to be writable since our linked list is within them.

>
> Aaron, do you have a definitive source for the "no two symbols may be
> the same" rule? Because that's a pretty common pattern, I would be
> surprised if it was incorrect (especially since there's things like
> __attribute__((alias)) that explicitly allow you to do it even in C).
> I can't find anything in a cursory glance at the standard... in fact,
> 6.5.9.6 (of some weird 2007 draft version I found, at least) says:

I don't  have a pointer. I only remember clang doing something because
of that behavior. I think Patrick may remember the details.

>
> "Two pointers compare equal if and only if [...] or one is a pointer
> to one past the end of an array object and the other is a pointer to
> the start of a different array object that happens to immediately
> follow the first array object in the address space."
>
> From that I'd conclude that if you did:
>
> extern struct boot_state_init_entry _bs_init_begin[];
> extern struct boot_state_init_entry _bs_init_end[];
>
> for (cur = _bs_init_begin; cur < _bs_init_end; cur++) ...
>
> the compiler shouldn't be able to guarantee that the first array
> wasn't empty and therefore pointing to "one after the end" which is
> the start of the next array in the first iteration.

I think you are right on that point -- using '<' instead of equality.
But those aren't pointer types. Not sure if the spec things arrays
like that are poitners or a separate type. Either way, hopefully there
isn't some language in the spec that suggests comparisons like that
can be optimized away.

-Aaron