ar and foreign object files

Message ID Z3tTFW3ihetdn9-u@squeak.grove.modra.org
State New
Headers
Series ar and foreign object files |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm fail Patch failed to apply
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 fail Patch failed to apply

Commit Message

Alan Modra Jan. 6, 2025, 3:50 a.m. UTC
  ar is supposed to make archives containing any sort of file, and it
generally does that.  It also tries to make archives suited to target
object files stored.  Some targets have peculiar archives.

In one particular case we get into trouble trying to suit archives to
object files: where the target object file is recognised but that
target doesn't happen to support archives, and the default target has
a special archive format.  For example, we'll get failures on
rs6000-aix if trying to add tekhex objects to a new archive.  What
happens in that the tekhex object is recognised and its target vector
used to create an empty archive, ie. with _bfd_generic_mkarchive and
_bfd_write_archive_contents.  An attempt is then made to open the
newly created archive.  The tekhex target vector does not have a
check_format function to recognise generic archives, nor as it happens
do any of the xcoff or other targets built for rs6000-aix.

It seems to me the simplest fix is to not use any target vector to
create archives where that vector can't also recognise them.  That's
what this patch does, and to reinforce that I've removed target vector
support for creating empty archives from such targets.

bfd/
	* i386msdos.c (i386_msdos_vec): Remove support for creating
	empty archives.
	* ihex.c (ihex_vec): Likewise.
	* srec.c (srec_vec, symbolsrec_vec): Likewise.
	* tekhex.c (tekhex_vec): Likewise.
	* wasm-module.c (wasm_vec): Likewise.
	* ptrace-core.c (core_ptrace_vec): Tidy.
	* targets.c (bfd_target_supports_archives): New inline function.
	* bfd-in2.h: Regenerate.
binutils/
	* ar.c (open_inarch): Don't select a target from the first
	object file that can't read archives.  Set output_filename
	earlier.
	* testsuite/binutils-all/ar.exp (thin_archive_with_nested):
	Don't repeat --thin test using T.
	(foreign_object): New test.
	* testsuite/binutils-all/tek1.obj,
	* testsuite/binutils-all/tek2.obj: New files.
  

Patch

diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
index 511d2665ef5..3a5a6dd8a5f 100644
--- a/bfd/bfd-in2.h
+++ b/bfd/bfd-in2.h
@@ -8020,6 +8020,13 @@  bfd_keep_unused_section_symbols (const bfd *abfd)
   return abfd->xvec->keep_unused_section_symbols;
 }
 
+static inline bool
+bfd_target_supports_archives (const bfd *abfd)
+{
+  return (abfd->xvec->_bfd_check_format[bfd_archive]
+	  != abfd->xvec->_bfd_check_format[bfd_unknown]);
+}
+
 bool bfd_set_default_target (const char *name);
 
 const bfd_target *bfd_find_target (const char *target_name, bfd *abfd);
diff --git a/bfd/i386msdos.c b/bfd/i386msdos.c
index 6ac3dcf9a8c..ba58f0feabc 100644
--- a/bfd/i386msdos.c
+++ b/bfd/i386msdos.c
@@ -295,13 +295,13 @@  const bfd_target i386_msdos_vec =
     {
       _bfd_bool_bfd_false_error,
       msdos_mkobject,
-      _bfd_generic_mkarchive,
+      _bfd_bool_bfd_false_error,
       _bfd_bool_bfd_false_error,
     },
     {				/* bfd_write_contents */
       _bfd_bool_bfd_false_error,
       msdos_write_object_contents,
-      _bfd_write_archive_contents,
+      _bfd_bool_bfd_false_error,
       _bfd_bool_bfd_false_error,
     },
 
diff --git a/bfd/ihex.c b/bfd/ihex.c
index ce958bf6f33..96559749b91 100644
--- a/bfd/ihex.c
+++ b/bfd/ihex.c
@@ -997,13 +997,13 @@  const bfd_target ihex_vec =
   {
     _bfd_bool_bfd_false_error,
     ihex_mkobject,
-    _bfd_generic_mkarchive,
+    _bfd_bool_bfd_false_error,
     _bfd_bool_bfd_false_error,
   },
   {				/* bfd_write_contents.  */
     _bfd_bool_bfd_false_error,
     ihex_write_object_contents,
-    _bfd_write_archive_contents,
+    _bfd_bool_bfd_false_error,
     _bfd_bool_bfd_false_error,
   },
 
diff --git a/bfd/ptrace-core.c b/bfd/ptrace-core.c
index 54480eed5ef..1d44f9f5d02 100644
--- a/bfd/ptrace-core.c
+++ b/bfd/ptrace-core.c
@@ -194,12 +194,16 @@  const bfd_target core_ptrace_vec =
       ptrace_unix_core_file_p		/* a core file */
     },
     {				/* bfd_set_format */
-      _bfd_bool_bfd_false_error, bfd_false,
-      _bfd_bool_bfd_false_error, bfd_false
+      _bfd_bool_bfd_false_error,
+      _bfd_bool_bfd_false_error,
+      _bfd_bool_bfd_false_error,
+      _bfd_bool_bfd_false_error,
     },
     {				/* bfd_write_contents */
-      _bfd_bool_bfd_false_error, bfd_false,
-      _bfd_bool_bfd_false_error, bfd_false
+      _bfd_bool_bfd_false_error,
+      _bfd_bool_bfd_false_error,
+      _bfd_bool_bfd_false_error,
+      _bfd_bool_bfd_false_error,
     },
 
     BFD_JUMP_TABLE_GENERIC (_bfd_generic),
diff --git a/bfd/srec.c b/bfd/srec.c
index cef87c3bea1..e1d39e521fc 100644
--- a/bfd/srec.c
+++ b/bfd/srec.c
@@ -1307,13 +1307,13 @@  const bfd_target srec_vec =
   {
     _bfd_bool_bfd_false_error,
     srec_mkobject,
-    _bfd_generic_mkarchive,
+    _bfd_bool_bfd_false_error,
     _bfd_bool_bfd_false_error,
   },
   {				/* bfd_write_contents.  */
     _bfd_bool_bfd_false_error,
     srec_write_object_contents,
-    _bfd_write_archive_contents,
+    _bfd_bool_bfd_false_error,
     _bfd_bool_bfd_false_error,
   },
 
@@ -1364,13 +1364,13 @@  const bfd_target symbolsrec_vec =
   {
     _bfd_bool_bfd_false_error,
     srec_mkobject,
-    _bfd_generic_mkarchive,
+    _bfd_bool_bfd_false_error,
     _bfd_bool_bfd_false_error,
   },
   {				/* bfd_write_contents.  */
     _bfd_bool_bfd_false_error,
     symbolsrec_write_object_contents,
-    _bfd_write_archive_contents,
+    _bfd_bool_bfd_false_error,
     _bfd_bool_bfd_false_error,
   },
 
diff --git a/bfd/targets.c b/bfd/targets.c
index a6848c80552..18f5782daf7 100644
--- a/bfd/targets.c
+++ b/bfd/targets.c
@@ -667,6 +667,13 @@  to find an alternative output format that is suitable.
 .  return abfd->xvec->keep_unused_section_symbols;
 .}
 .
+.static inline bool
+.bfd_target_supports_archives (const bfd *abfd)
+.{
+.  return (abfd->xvec->_bfd_check_format[bfd_archive]
+.	   != abfd->xvec->_bfd_check_format[bfd_unknown]);
+.}
+.
 */
 
 /* All known xvecs (even those that don't compile on all systems).
diff --git a/bfd/tekhex.c b/bfd/tekhex.c
index 1498b0a6b10..01feeef8d4c 100644
--- a/bfd/tekhex.c
+++ b/bfd/tekhex.c
@@ -1012,13 +1012,13 @@  const bfd_target tekhex_vec =
   {
     _bfd_bool_bfd_false_error,
     tekhex_mkobject,
-    _bfd_generic_mkarchive,
+    _bfd_bool_bfd_false_error,
     _bfd_bool_bfd_false_error,
   },
   {				/* bfd_write_contents.  */
     _bfd_bool_bfd_false_error,
     tekhex_write_object_contents,
-    _bfd_write_archive_contents,
+    _bfd_bool_bfd_false_error,
     _bfd_bool_bfd_false_error,
   },
 
diff --git a/bfd/wasm-module.c b/bfd/wasm-module.c
index f3750860f1a..d5d186e07c8 100644
--- a/bfd/wasm-module.c
+++ b/bfd/wasm-module.c
@@ -823,13 +823,13 @@  const bfd_target wasm_vec =
   {
     _bfd_bool_bfd_false_error,
     wasm_mkobject,
-    _bfd_generic_mkarchive,
+    _bfd_bool_bfd_false_error,
     _bfd_bool_bfd_false_error,
   },
   {				/* bfd_write_contents.  */
     _bfd_bool_bfd_false_error,
     wasm_write_object_contents,
-    _bfd_write_archive_contents,
+    _bfd_bool_bfd_false_error,
     _bfd_bool_bfd_false_error,
   },
 
diff --git a/binutils/ar.c b/binutils/ar.c
index 3ad675c7bb4..40cad576b46 100644
--- a/binutils/ar.c
+++ b/binutils/ar.c
@@ -1006,12 +1006,16 @@  open_inarch (const char *archive_filename, const char *file)
 	  obj = bfd_openr (file, target);
 	  if (obj != NULL)
 	    {
-	      if (bfd_check_format (obj, bfd_object))
+	      if (bfd_check_format (obj, bfd_object)
+		  && bfd_target_supports_archives (obj))
 		target = bfd_get_target (obj);
 	      (void) bfd_close (obj);
 	    }
 	}
 
+      /* If we die creating a new archive, don't leave it around.  */
+      output_filename = xstrdup (archive_filename);
+
       /* Create an empty archive.  */
       arch = bfd_openw (archive_filename, target);
       if (arch == NULL
@@ -1020,9 +1024,6 @@  open_inarch (const char *archive_filename, const char *file)
 	bfd_fatal (archive_filename);
       else if (!silent_create)
         non_fatal (_("creating %s"), archive_filename);
-
-      /* If we die creating a new archive, don't leave it around.  */
-      output_filename = xstrdup (archive_filename);
     }
 
   arch = bfd_openr (archive_filename, target);
diff --git a/binutils/testsuite/binutils-all/ar.exp b/binutils/testsuite/binutils-all/ar.exp
index 8d346e4c6bd..d06fe90c8c5 100644
--- a/binutils/testsuite/binutils-all/ar.exp
+++ b/binutils/testsuite/binutils-all/ar.exp
@@ -309,13 +309,11 @@  proc thin_archive_with_nested { bfdtests } {
     if [is_remote host] {
 	set archive artest.a
 	set archive2 artest2.a
-	set archive3 artest3.a
 	set objfile [remote_download host tmpdir/bintest.${obj}]
 	remote_file host delete $archive
     } else {
 	set archive tmpdir/artest.a
 	set archive2 tmpdir/artest2.a
-	set archive3 tmpdir/artest3.a
 	set objfile tmpdir/bintest.${obj}
     }
 
@@ -329,15 +327,7 @@  proc thin_archive_with_nested { bfdtests } {
 
     remote_file build delete tmpdir/artest2.a
 
-    set got [binutils_run $AR "rcT $archive2 ${archive}"]
-    if ![string match "" $got] {
-	fail $testname
-	return
-    }
-
-    remote_file build delete tmpdir/artest3.a
-
-    set got [binutils_run $AR "rc --thin $archive3 ${archive}"]
+    set got [binutils_run $AR "rc --thin $archive2 ${archive}"]
     if ![string match "" $got] {
 	fail $testname
 	return
@@ -357,13 +347,6 @@  proc thin_archive_with_nested { bfdtests } {
 	    fail "$testname ($bfdtest)"
 	    return
 	}
-
-	set exec_output [binutils_run "$base_dir/$bfdtest" "$archive3"]
-	if ![string match "" $exec_output] {
-	    verbose -log $exec_output
-	    fail "$testname ($bfdtest)"
-	    return
-	}
     }
 
     set got [binutils_run $NM "--print-armap $archive"]
@@ -994,6 +977,54 @@  __.LIBDEP*" $got] {
     pass $testname
 }
 
+# Test creation of an archive containing object files not of the same
+# format as the target object files.
+
+proc foreign_object { } {
+    global AR
+    global NM
+    global srcdir
+    global subdir
+
+    set testname "ar foreign object"
+
+    set object1 $srcdir/$subdir/tek1.obj
+    set object2 $srcdir/$subdir/tek2.obj
+    if [is_remote host] {
+	set archive artest.a
+	set object1 [remote_download host $object1]
+	set object2 [remote_download host $object2]
+	remote_file host delete $archive
+    } else {
+	set archive tmpdir/artest.a
+    }
+
+    remote_file build delete tmpdir/artest.a
+
+    set got [binutils_run $AR "rc $archive $object1 $object2"]
+    if ![string match {} $got] {
+	fail $testname
+	return
+    }
+
+    set got [binutils_run $NM $archive]
+    if ![string match {
+tek1.obj:
+*: no symbols
+
+tek2.obj:
+00000001 D _binary_x_end
+00000001 A _binary_x_size
+00000000 D _binary_x_start
+} $got] {
+	fail $testname
+	return
+    }
+
+    pass $testname
+}
+
+
 # Run the tests.
 
 # Only run the bfdtest checks if the programs exist.  Since these
@@ -1030,3 +1061,5 @@  test_add_dependencies
 if { [is_elf_format] && [supports_gnu_unique] } {
     unique_symbol
 }
+
+foreign_object
diff --git a/binutils/testsuite/binutils-all/tek1.obj b/binutils/testsuite/binutils-all/tek1.obj
new file mode 100644
index 00000000000..7f828c5792e
--- /dev/null
+++ b/binutils/testsuite/binutils-all/tek1.obj
@@ -0,0 +1,2 @@ 
+%47615103000000000000000000000000000000000000000000000000000000000000000
+%0781010
diff --git a/binutils/testsuite/binutils-all/tek2.obj b/binutils/testsuite/binutils-all/tek2.obj
new file mode 100644
index 00000000000..da696d1773e
--- /dev/null
+++ b/binutils/testsuite/binutils-all/tek2.obj
@@ -0,0 +1,6 @@ 
+%47615103000000000000000000000000000000000000000000000000000000000000000
+%103E95.data11011
+%1E3FB5.data4F_binary_x_start10
+%1C3735.data4D_binary_x_end11
+%1D3135*ABS*2E_binary_x_size11
+%0781010