Re: [PATCH]: sh-hms: set target_defaulted in bfd_generic_archive_p

[Nick Clifton]

Hi Joern,

> With current bfd, ld complains about the first file in libgcc.a:
> :_ashiftrt.o: File format is ambiguous
> :Matching formats: coff-shl coff-shl-small

Right - there does not appear to be a way to detect if a object file
is in coff-shl or coff-shl-small format, so the code has to rely upon
the user explicitly selecting the coff-shl-small format.


> As far as I can see, there are only three places where
> target_defaulted is used (other than copied): binary.c, coff-sh.c,
> and ppcboot.c.

Of course it is also used in format.c:bfd_check_format_matches().


> binary.c and ppcboot.c say that they should only be used as output
> formats; does that mean they should not be present in archives?

Effectively yes.  It means that they should never be selected as the
output format by default, but only if the user has explicitly
requested that format.  Thus they should never be considered to be the
format of an element in an archive unless the user has explicitly said
that this archive should be considered to be a archive of "binary"
format elements (or "ppcboot" format elements).  And since it is not
possible for the user to specify this, it can never happen.

See also the comment and code in format.c starting at line 178.


> The following patch fixes the sh-hms port:
> 
> Sun Sep 30 21:27:03 2001  J"orn Rennecke <address@hidden>
> 
>       * archive.c (bfd_generic_archive_p):
>       Start out with target_defaulted = true.

I think that this patch is a good idea, although it ought to include a
comment explaining why target_defaulted is set to true, and it also
ought to be applied to ecoff.c.  See below for my suggested version of
the patch.

It is also worth noting that the ambiguous recognition problem is not
fully resolved for the these files, since if you build a toolchain
with --enable-targets=all configured you will get the following
errors:

  In archive /local/nickc/builds/sh-coff-egcs/gcc/ml/libgcc.a:
  binutils/objdump: _ashiftrt.o: File format is ambiguous
  binutils/objdump: Matching formats: coff-shl pe-shl
  binutils/objdump: _ashiftrt_n.o: File format is ambiguous
  binutils/objdump: Matching formats: coff-shl pe-shl
  binutils/objdump: _ashiftlt.o: File format is ambiguous
  ....

Still I doubt if that is something that you are worried about.

Cheers
        Nick


2001-10-02  J"orn Rennecke <address@hidden>
            Nick Clifton  <address@hidden>

        * archive.c (bfd_generic_archive_p): Start out with
        target_defaulted = true.  Remove redundant variable 'fail'.
        * ecoff.c (_bfd_ecoff_archive_p): Clone code from
        bfd_generic_archive_p.

Index: bfd/archive.c
===================================================================
RCS file: /cvs/src/src/bfd/archive.c,v
retrieving revision 1.18
diff -p -r1.18 archive.c
*** archive.c   2001/09/19 05:33:11     1.18
--- archive.c   2001/10/02 17:27:33
*************** bfd_generic_archive_p (abfd)
*** 671,680 ****
        first = bfd_openr_next_archived_file (abfd, (bfd *) NULL);
        if (first != NULL)
        {
!         boolean fail;
  
!         first->target_defaulted = false;
!         fail = false;
          if (bfd_check_format (first, bfd_object)
              && first->xvec != abfd->xvec)
            {
--- 671,694 ----
        first = bfd_openr_next_archived_file (abfd, (bfd *) NULL);
        if (first != NULL)
        {
!         /* We used to set target_defaulted to false.  This was a problem
!            for the coff-sh format archives, since coff-shl and
!            coff-shl-small targets look the same, and if target_defaulted is
!            false then the coff-shl-small target is selectable. Setting
!            target_defaulted to true solves this problem.
  
!            Setting target_defaulted to true only changes the code slightly.
!            It now means that the "ppcboot" format is no longer a potentially
!            selectable archive format.  (It should never be in an archive
!            anyway).  The "binary" format was already excluded by some
!            special code in bfd_check_format_matches().
! 
!            Setting target_defaulted to true also makes sense, since what we
!            are effectively doing here is using bfd_check_format to choose a
!            format for us, and then checking to see if this format matches
!            the format of the BFD.  */
!         first->target_defaulted = true;
! 
          if (bfd_check_format (first, bfd_object)
              && first->xvec != abfd->xvec)
            {
Index: bfd/ecoff.c
===================================================================
RCS file: /cvs/src/src/bfd/ecoff.c,v
retrieving revision 1.12
diff -p -r1.12 ecoff.c
*** ecoff.c     2001/09/29 12:07:00     1.12
--- ecoff.c     2001/10/02 17:27:35
*************** _bfd_ecoff_write_armap (abfd, elength, m
*** 3304,3310 ****
  }
  
  /* See whether this BFD is an archive.  If it is, read in the armap
!    and the extended name table.  */
  
  const bfd_target *
  _bfd_ecoff_archive_p (abfd)
--- 3304,3311 ----
  }
  
  /* See whether this BFD is an archive.  If it is, read in the armap
!    and the extended name table.  This is a copy of the function
!    bfd_generic_archive_p() with ecoff specific customisations.  */
  
  const bfd_target *
  _bfd_ecoff_archive_p (abfd)
*************** _bfd_ecoff_archive_p (abfd)
*** 3370,3391 ****
        first = bfd_openr_next_archived_file (abfd, (bfd *) NULL);
        if (first != NULL)
        {
!         boolean fail;
  
-         first->target_defaulted = false;
-         fail = false;
          if (bfd_check_format (first, bfd_object)
              && first->xvec != abfd->xvec)
            {
              (void) bfd_close (first);
              bfd_release (abfd, bfd_ardata (abfd));
              abfd->tdata.aout_ar_data = tdata_hold;
              bfd_set_error (bfd_error_wrong_format);
              return NULL;
            }
! 
!         /* We ought to close first here, but we can't, because we
!              have no way to remove it from the archive cache.  FIXME.  */
        }
      }
  
--- 3371,3395 ----
        first = bfd_openr_next_archived_file (abfd, (bfd *) NULL);
        if (first != NULL)
        {
!         /* See comment in archive.c:bfd_generic_archive_p().  */
!         first->target_defaulted = true;
  
          if (bfd_check_format (first, bfd_object)
              && first->xvec != abfd->xvec)
            {
+ #if 0
+             /* We ought to close `first' here, but we can't, because
+                we have no way to remove it from the archive cache.
+                It's close to impossible to figure out when we can
+                release bfd_ardata.  FIXME.  */
              (void) bfd_close (first);
              bfd_release (abfd, bfd_ardata (abfd));
              abfd->tdata.aout_ar_data = tdata_hold;
+ #endif
              bfd_set_error (bfd_error_wrong_format);
              return NULL;
            }
!         /* And we ought to close `first' here too.  */
        }
      }