Re: debug-early branch merged into mainline

[Aldy Hernandez <aldyh at redhat dot com>]

On 06/08/2015 09:30 AM, Richard Biener wrote:
> On Mon, Jun 8, 2015 at 2:05 PM, Aldy Hernandez <aldyh@redhat.com> wrote:
> > On 06/08/2015 04:26 AM, Richard Biener wrote:
> > > On Mon, Jun 8, 2015 at 3:23 AM, Aldy Hernandez <aldyh@redhat.com> wrote:
> > > > On 06/07/2015 02:33 PM, Richard Biener wrote:
> > > > > On June 7, 2015 6:00:05 PM GMT+02:00, Aldy Hernandez <aldyh@redhat.com>
> > > > > wrote:
> > > > > > On 06/07/2015 11:25 AM, Richard Biener wrote:
> > > > > > > On June 7, 2015 5:03:30 PM GMT+02:00, Aldy Hernandez
> > > > > >
> > > > > >
> > > > > > <aldyh@redhat.com> wrote:
> > > > > > > > On 06/06/2015 05:49 AM, Andreas Schwab wrote:
> > > > > > > > > Bootstrap fails on aarch64:
> > > > > > > > >
> > > > > > > > > Comparing stages 2 and 3
> > > > > > > > > warning: gcc/cc1objplus-checksum.o differs
> > > > > > > > > warning: gcc/cc1obj-checksum.o differs
> > > > > > > > > warning: gcc/cc1plus-checksum.o differs
> > > > > > > > > warning: gcc/cc1-checksum.o differs
> > > > > > > > > Bootstrap comparison failure!
> > > > > > > > > gcc/ira-costs.o differs
> > > > > > > > > gcc/tree-sra.o differs
> > > > > > > > > gcc/tree-parloops.o differs
> > > > > > > > > gcc/tree-vect-data-refs.o differs
> > > > > > > > > gcc/java/jcf-io.o differs
> > > > > > > > > gcc/ipa-inline-analysis.o differs
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > The bootstrap comparison failure on ppc64le, aarch64, and possibly
> > > > > > > > others is due to the order of some sections being in a different
> > > > > >
> > > > > >
> > > > > > order
> > > > > > > > with and without debugging.
> > > > > > > >
> > > > > > > > Stage2 is being compiled with no debugging due to -gtoggle, and
> > > > > >
> > > > > >
> > > > > > stage3
> > > > > > > > is being compiled with debugging.
> > > > > > > >
> > > > > > > > For ira-costs.o on ppc64le we have:
> > > > > > > >
> > > > > > > > -Disassembly of section
> > > > > >
> > > > > >
> > > > > > .rodata._ZN10hash_tableI19cost_classes_hasher11xcallocatorE6expandEv.str1.8:
> > > > > > > > +Disassembly of section
> > > > > >
> > > > > >
> > > > > > .rodata._ZN10hash_tableI19cost_classes_hasher11xcallocatorE26find_empty_slot_for_expandEj.str1.8:
> > > > > > > > ...
> > > > > > > >
> > > > > > > > -Disassembly of section
> > > > > >
> > > > > >
> > > > > > .rodata._ZN10hash_tableI19cost_classes_hasher11xcallocatorE26find_empty_slot_for_expandEj.str1.8:
> > > > > > > > +Disassembly of section
> > > > > >
> > > > > >
> > > > > > .rodata._ZN10hash_tableI19cost_classes_hasher11xcallocatorE6expandEv.str1.8:
> > > > > > > > There is no semantic difference between the objects, just the
> > > > > >
> > > > > >
> > > > > > ordering.
> > > > > > > > I assume it's the same problem for the rest of the objects and
> > > > > > > > architectures.
> > > > > > > >
> > > > > > > > I will look into this, unless someone beats me to it, or has an idea
> > > > > > > > right off the bat.
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > Check whether the symbol table walkers are walking hash tables.  I
> > > > > >
> > > > > >
> > > > > > assume the above are emitted via the symbol removal handling for debug
> > > > > > stuff?
> > > > > >
> > > > > > Ughh, indeed.  These sections are being outputted from
> > > > > > output_object_blocks which traverses a hash table:
> > > > > >
> > > > > > void
> > > > > > output_object_blocks (void)
> > > > > > {
> > > > > >     object_block_htab->traverse<void *, output_object_block_htab>
> > > > > > (NULL);
> > > > > > }
> > > > > >
> > > > > > Perhaps we should sort them by some deterministic field and then call
> > > > > > output_object_block() on each member of the resulting list?
> > > > >
> > > > >
> > > > >
> > > > > Yes, that would be the usual fix. Maybe sth has an UID already, is the
> > > > > 'object' a decl by chance?
> > > >
> > > >
> > > >
> > > > The attached patch fixes the bootstrap failure on ppc64le, and
> > > > theoretically
> > > > the aarch64 problem as well, but I haven't checked.
> > > >
> > > > Tested on ppc64le linux by bootstrapping, and regtesting C/C++ against
> > > > pre
> > > > debug-early merge sources.  Also tested by a full bootstrap and regtest
> > > > on
> > > > x86-64 Linux.
> > > >
> > > > OK for mainline?
> > >
> > >
> > > Please use FOR_EACH_HASH_TABLE_ELEMENT to put elements on the
> > > vector instead of the htab traversal.
> > >
> > > The compare function looks like we will end up having many equal elements
> > > (and thus random ordering on hosts where qsort doesn't behave "sane"
> > > here, like Solaris IIRC).  Unless all sections are named (which it looks
> > > like)
> >
> >
> > Some sections are not named.
> >
> > How about we sort the named sections and output them, but call
> > output_object_block() on the rest of the sections on whatever order they
> > were in?  This solves the bootstrap problem as well.
> >
> > Attached patch tested on x86-64 and ppc64le Linux.
> >
> > OK?
>
> No, but hash_section suggests to sort after sect->common.flags if
> the section is not named.  Conveniently flags is just an 'int' ...

What about if the comparison routine gets a named section and an unnamed 
section?  How to compare?  That's why I was giving priority to one over 
the other originally, but I didn't know about problematic qsort 
implementations.

Aldy