From: Alan Modra <amodra@gmail.com>
Date: Mon, 4 Aug 2025 05:59:22 +0000 (+0930)
Subject: Make bfd_check_format better respect given target
X-Git-Tag: gdb-17-branchpoint~282
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=28daddd33accbfc519a2663a4e1bfaa66eafcef0;p=thirdparty%2Fbinutils-gdb.git

Make bfd_check_format better respect given target

bfd_check_format currently does not take much notice of the target
set in its abfd arg, merely checking that target first if
target_defaulted is false.  As the comment says this was due to
complications with archive handling which I think was fixed in
commit f832531609d0.  It should now be possible to just check the
given target, except for linker input files.  This will speed checking
the target of the first file in archives, which is done in the process
of matching archive targets.

This will no doubt expose some misuse of target options, as found in
the binutils "efi app" tests.

The stricter checking also exposed some errors.  Cris targets gave
"FAIL: objcopy decompress debug sections in archive (reason: unexpected output)"
"FAIL: objcopy decompress debug sections in archive with zlib-gabi (reason: unexpected output)"
without the bfd_generic_archive_p fix.  cris has a default target of
cris-aout which doesn't match objects for any of the cris-elf or
cris-linux targets.  The problem here was that checking the first
object file in archives didn't match but a logic error there meant
bfd_error_wrong_object_format wasn't returned as it should be.  There
was also a possibility of bfd_error_wrong_object_format persisting
from one target check to the next.

bfd/
	* archive.c (bfd_generic_archive_p): Correct object in archive
	target test.
	* format.c (bfd_check_format_matches_lto): Try to match given
	target first even when target_defaulted is set.  Don't try
	other targets if !target_defaulted except for linker input.
	Clear bfd_error before attempted target matches.
	* targets.c (bfd_find_target): Set target_defaulted for
	plugin target.
binutils/
	* bucomm.h (set_plugin_target): Set target_defaulted.
	* testsuite/binutils-all/aarch64/pei-aarch64-little.d: Don't
	wrongly specify both input and output target, just specify
	output.
	* testsuite/binutils-all/loongarch64/pei-loongarch64.d: Likewise.
	* testsuite/binutils-all/riscv/pei-riscv64.d: Likewise.
---

diff --git a/bfd/archive.c b/bfd/archive.c
index be96bb25759..16b725b9958 100644
--- a/bfd/archive.c
+++ b/bfd/archive.c
@@ -950,8 +950,8 @@ bfd_generic_archive_p (bfd *abfd)
       if (first != NULL)
 	{
 	  first->target_defaulted = false;
-	  if (bfd_check_format (first, bfd_object)
-	      && first->xvec != abfd->xvec)
+	  if (!bfd_check_format (first, bfd_object)
+	      || first->xvec != abfd->xvec)
 	    bfd_set_error (bfd_error_wrong_object_format);
 	  bfd_close (first);
 	}
diff --git a/bfd/format.c b/bfd/format.c
index b74c6240f23..4dbe863e742 100644
--- a/bfd/format.c
+++ b/bfd/format.c
@@ -513,38 +513,47 @@ bfd_check_format_matches (bfd *abfd, bfd_format format, char ***matching)
   if (!bfd_preserve_save (abfd, &preserve, NULL))
     goto err_ret;
 
-  /* If the target type was explicitly specified, just check that target.  */
+  /* First try matching the current target.  The current target may
+     have been set due to a user option, or due to the linker trying
+     optimistically to load input files for the same target as the
+     output, or due to the plugin support setting "plugin", or failing
+     any of those bfd_find_target will have chosen a default target.
+     target_defaulted will be set in the last case, or when "plugin"
+     is the target (even if chosen by user option).  Note that
+     bfd_plugin_no excluding the plugin target condition is an
+     optimisation, and can be removed if desired.  */
   fail_targ = NULL;
-  if (!abfd->target_defaulted
 #if BFD_SUPPORTS_PLUGINS
-      && !(abfd->plugin_format == bfd_plugin_no
-	   && bfd_plugin_target_p (save_targ))
+  if (!(abfd->plugin_format == bfd_plugin_no
+	&& bfd_plugin_target_p (save_targ)))
 #endif
-      )
     {
-      if (bfd_seek (abfd, 0, SEEK_SET) != 0)	/* rewind! */
+      if (bfd_seek (abfd, 0, SEEK_SET) != 0)
 	goto err_ret;
 
+      bfd_set_error (bfd_error_no_error);
       cleanup = BFD_SEND_FMT (abfd, _bfd_check_format, (abfd));
-
       if (cleanup)
-	goto ok_ret;
-
-      /* For a long time the code has dropped through to check all
-	 targets if the specified target was wrong.  I don't know why,
-	 and I'm reluctant to change it.  However, in the case of an
-	 archive, it can cause problems.  If the specified target does
-	 not permit archives (e.g., the binary target), then we should
-	 not allow some other target to recognize it as an archive, but
-	 should instead allow the specified target to recognize it as an
-	 object.  When I first made this change, it broke the PE target,
-	 because the specified pei-i386 target did not recognize the
-	 actual pe-i386 archive.  Since there may be other problems of
-	 this sort, I changed this test to check only for the binary
-	 target.  */
-      if (format == bfd_archive && save_targ == &binary_vec)
-	goto err_unrecog;
-      fail_targ = save_targ;
+	{
+	  if (abfd->format != bfd_archive
+	      /* An archive with object files matching the archive
+		 target is OK.  Other archives should be further
+		 tested.  */
+	      || (bfd_has_map (abfd)
+		  && bfd_get_error () != bfd_error_wrong_object_format)
+	      /* Empty archives can match the current target.
+		 Attempting to read the armap will result in a file
+		 truncated error.  */
+	      || (!bfd_has_map (abfd)
+		  && bfd_get_error () == bfd_error_file_truncated))
+	    goto ok_ret;
+	}
+      else
+	{
+	  if (!abfd->target_defaulted && !abfd->is_linker_input)
+	    goto err_unrecog;
+	  fail_targ = save_targ;
+	}
     }
 
   /* Check all targets in the hope that one will be recognized.  */
@@ -606,6 +615,7 @@ bfd_check_format_matches (bfd *abfd, bfd_format format, char ***matching)
       if (bfd_seek (abfd, 0, SEEK_SET) != 0)
 	goto err_ret;
 
+      bfd_set_error (bfd_error_no_error);
       cleanup = BFD_SEND_FMT (abfd, _bfd_check_format, (abfd));
       if (cleanup)
 	{
diff --git a/bfd/targets.c b/bfd/targets.c
index c2ee9179f37..a7b0450a005 100644
--- a/bfd/targets.c
+++ b/bfd/targets.c
@@ -1572,7 +1572,11 @@ bfd_find_target (const char *target_name, bfd *abfd)
     }
 
   if (abfd)
-    abfd->target_defaulted = false;
+    /* Treating "plugin" specially here is due to the fact that some
+       of the binutils magically supply a "plugin" target.  That
+       really is a defaulted target, but unfortunately we can't
+       distinguish it from a user supplied "plugin" target.  */
+    abfd->target_defaulted = strcmp (targname, "plugin") == 0;
 
   target = find_target (targname);
   if (target == NULL)
diff --git a/binutils/bucomm.h b/binutils/bucomm.h
index 8b41d42f5f0..54f54d74286 100644
--- a/binutils/bucomm.h
+++ b/binutils/bucomm.h
@@ -94,9 +94,8 @@ set_plugin_target (bfd *abfd, const struct bfd_target *targ)
   if (abfd->my_archive && targ)
     {
       abfd->xvec = targ;
-      /* Arrange for the plugin target to be tried first in
-	 bfd_check_format.  */
-      abfd->target_defaulted = false;
+      /* Don't fail if the element isn't recognised by the plugin.  */
+      abfd->target_defaulted = true;
     }
 }
 
diff --git a/binutils/testsuite/binutils-all/aarch64/pei-aarch64-little.d b/binutils/testsuite/binutils-all/aarch64/pei-aarch64-little.d
index 27cb6e17db3..22f664929c5 100644
--- a/binutils/testsuite/binutils-all/aarch64/pei-aarch64-little.d
+++ b/binutils/testsuite/binutils-all/aarch64/pei-aarch64-little.d
@@ -1,7 +1,7 @@
 #skip: aarch64_be-*-*
 #ld: -e0
 #PROG: objcopy
-#objcopy: -j .text -j .sdata -j .data -j .dynamic -j .dynsym -j .rel -j .rela -j .rel.* -j .rela.* -j .rel* -j .rela* -j .reloc --target=efi-app-aarch64
+#objcopy: -j .text -j .sdata -j .data -j .dynamic -j .dynsym -j .rel -j .rela -j .rel.* -j .rela.* -j .rel* -j .rela* -j .reloc --output-target=efi-app-aarch64
 #objdump: -h -f
 #name: Check if efi app format is recognized
 
diff --git a/binutils/testsuite/binutils-all/loongarch64/pei-loongarch64.d b/binutils/testsuite/binutils-all/loongarch64/pei-loongarch64.d
index 574b3e5448d..61b026d59ca 100644
--- a/binutils/testsuite/binutils-all/loongarch64/pei-loongarch64.d
+++ b/binutils/testsuite/binutils-all/loongarch64/pei-loongarch64.d
@@ -1,6 +1,6 @@
 #ld: -e0
 #PROG: objcopy
-#objcopy: -j .text -j .sdata -j .data -j .dynamic -j .dynsym -j .rel -j .rela -j .rel.* -j .rela.* -j .rel* -j .rela* -j .reloc --target=pei-loongarch64
+#objcopy: -j .text -j .sdata -j .data -j .dynamic -j .dynsym -j .rel -j .rela -j .rel.* -j .rela.* -j .rel* -j .rela* -j .reloc --output-target=pei-loongarch64
 #objdump: -h -f
 #name: Check if efi app format is recognized
 
diff --git a/binutils/testsuite/binutils-all/riscv/pei-riscv64.d b/binutils/testsuite/binutils-all/riscv/pei-riscv64.d
index 189b0162ba5..43164147c3d 100644
--- a/binutils/testsuite/binutils-all/riscv/pei-riscv64.d
+++ b/binutils/testsuite/binutils-all/riscv/pei-riscv64.d
@@ -1,7 +1,7 @@
 #as: -march=rv64gc -mabi=lp64d
 #ld: -m elf64lriscv -e0
 #PROG: objcopy
-#objcopy: -j .text -j .sdata -j .data -j .dynamic -j .dynsym -j .rel -j .rela -j .rel.* -j .rela.* -j .rel* -j .rela* -j .reloc --target=pei-riscv64-little
+#objcopy: -j .text -j .sdata -j .data -j .dynamic -j .dynsym -j .rel -j .rela -j .rel.* -j .rela.* -j .rel* -j .rela* -j .reloc --output-target=pei-riscv64-little
 #objdump: -h -f
 #name: Check if efi app format is recognized