Re: Not so minor a fix for --enable-targets=all

[Hans-Peter Nilsson <hp at bitrange dot com>]

On Sat, 26 Jan 2013, Alan Modra wrote:
> On Fri, Jan 25, 2013 at 08:45:54AM +1030, Alan Modra wrote:
> > So the real problem is left-over crud from a previous target match.
> > Now in the case of this particular objdump call, assuming som.c is
> > modified to tolerate this, we'll just exit with
> >
> > 	objdump: Matching formats: a.out-newsos3 som
> >
> > but it's possible for bfd_check_format_matches to return without an
> > error when more than one target matches.  If that happens the bfd will
> > be in a right royal mess.
>
> After diving into the rathole of BFD for quite a while, this is how
> I've fixed the problem.  My first reasonably successful attempt simply
> re-ran _bfd_check_format after exit from the target matching loop in
> bfd_check_format_matches.  That caused wholesale failures for mmix due
> to segfaults in qsort, fixed by the mmo.c change.

Hm, I guess due to being called with half-way initialized bfd
contents, right?   Thanks, but...

>  I also had a couple
> of arm-linuxeabi test failures due to stubs in a different order than
> the test expected.  That was because the stubs are emitted by a hash
> table traverse, stub names include section->id, and the section ids
> were different.  Arrgh!  section->id is unique for all sections
> created, set from a static var incremented each time a section is
> created.  The first successful _bfd_check_format call created
> sections, affecting the second _bfd_check_format section ids.  Calling
> _bfd_check_format twice isn't a good idea, and in any case does
> unnecessary work.
>
> Well, we've used bfd_preserve_save/restore to undo changes in bfd state
> for the ELF object_p functions for quite a while.  That was done so
> that a failing elf_object_p didn't destroy a previous match, a
> peculiarity of the ELF object_p functions.  Other target object_p
> function don't alter bfd state significantly unless finding a match.
> However, this solution failed to handle cases where we had multiple
> matches, a quite likely scenario for a.out targets.  So I move the
> bfd_preserve_save/restore out of all the existing object_p function
> and into their caller.  Why didn't I do it that way the first time?
>
> I kept the mmo.c change from my first attempt even though it isn't
> needed with this final patch.

> 	* mmo.c (mmo_scan): Clear abfd->symcount.

...shouldn't this rather be done in generic code?  Just thinking
that the backend code should be able to assume the bfd is a
clean slate and this seems like having to initialize seemingly
random fields.

brgds, H-P