Re: [PATCH-bfd] i386-mingw32-ld crash on x86_64 linux

[Dave Korn <dave dot korn dot cygwin at googlemail dot com>]

Peter O'Gorman wrote:

> Simply changing that one strcmp to strncmp eliminates the crash, but the
> output differs from the same link done on i686 linux (we don't have
> source code, or a way to test the output, so cmp is the only check we
> have).

  Um, isn't objdump useful for this?  And LD's "-Map" option?  That's how I
generally try and track down unexplained linking differences.  Source code
isn't necessary, it won't tell you anything about what the linker's doing
differently anyway.

> On x86_64 the union is 16 bytes long, because bfd_hostptr_t is an
> unsigned long. If we zero the _n_zeroes and _n_offset members before we
> do the memcpy, then the output on x86_64 is identical to that on i686.
> 
> Is this patch more acceptable?

  Well, the strcmp->strncmp bit is correct.  The changes to zero out _n_zeroes
and _n_offsets... no, not really.  I mean, they're not wrong, but I don't
think we should check in patches that we don't know how or why they work; that
way cargo-cult programming lies.  If the code requires more than one member of
a *union* to be initialised in order to work correctly, that means that
somewhere down the line we're probably actually doing something undefined or
invalid when it comes to accessing that union.

  Also there shouldn't have been any need to initialise _n_zeroes, only
_n_offset, because _n_zeros gets overwritten by the memcpy.  Since the section
names are eight chars, it must be the _n_offset field that is somehow making
the difference.

  We really do need to understand what is different better.  Can you try,
instead of zeroing the n_offset field, write a recognisable pattern into it
and see if/where it turns up later in the final linked exe?  Meanwhile, I'm
going to go through the sources looking at all the references to _n_offset and
see if I can figure out where it's coming back into relevance.  It might even
be that zeroing it actually is the correct thing to do, but we don't know yet.


    cheers,
      DaveK