This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


In my last patch to bfd_check_format_matches, I added an assert.  It
turns out that the assert can trigger on a corner case, I think.  When
an archive matches but objects within the archive are for a different
target, bfd_generic_archive_p currently returns NULL.
bfd_check_format_matches records this partial match, and will use it
if no better match is found.  If it's necessary to redo the
_bfd_check_format call (due to another partial match), then that call
will also return NULL.

In looking at this, I noticed that the bfd_ardata, stored in
bfd->tdata.aout_ar_data, is lost when bfd_generic_archive_p returns
NULL.  That's quite normal for the object_p and archive_p functions.
If returning NULL, ie. not a match, restore the bfd. (*)  However, if
we then try to link such an archive we'll attempt to use bfd_ardata
and segfault.  I think the linker ought to treat archives with unknown
target objects files exactly like it does plain unknown target
object files.  Which requires bfd_ardata be kept, and
bfd_generic_archive_p return the target.

	* archive.c (bfd_generic_archive_p): Return target and keep
	ardata on partial matches.
	* format.c (bfd_check_format_matches): Adjust for above
	change.  Remove bfd_error_file_ambiguously_recognized dead
	code.

(*) This is old behaviour, not needed now that their caller does the
restoring.

Index: bfd/archive.c
===================================================================
RCS file: /cvs/src/src/bfd/archive.c,v
retrieving revision 1.94
diff -u -p -r1.94 archive.c
--- bfd/archive.c	10 Jan 2013 20:03:52 -0000	1.94
+++ bfd/archive.c	28 Jan 2013 05:21:52 -0000
@@ -852,11 +852,7 @@ bfd_generic_archive_p (bfd *abfd)
 	  first->target_defaulted = FALSE;
 	  if (bfd_check_format (first, bfd_object)
 	      && first->xvec != abfd->xvec)
-	    {
-	      bfd_set_error (bfd_error_wrong_object_format);
-	      bfd_ardata (abfd) = tdata_hold;
-	      return NULL;
-	    }
+	    bfd_set_error (bfd_error_wrong_object_format);
 	  /* And we ought to close `first' here too.  */
 	}
     }
Index: bfd/format.c
===================================================================
RCS file: /cvs/src/src/bfd/format.c,v
retrieving revision 1.31
diff -u -p -r1.31 format.c
--- bfd/format.c	26 Jan 2013 02:08:01 -0000	1.31
+++ bfd/format.c	28 Jan 2013 05:21:53 -0000
@@ -279,7 +279,6 @@ bfd_check_format_matches (bfd *abfd, bfd
   for (target = bfd_target_vector; *target != NULL; target++)
     {
       const bfd_target *temp;
-      bfd_error_type err;
 
       /* Don't check the default target twice.  */
       if (*target == &binary_vec
@@ -310,47 +309,47 @@ bfd_check_format_matches (bfd *abfd, bfd
 	  match_targ = temp;
 	  if (preserve.marker != NULL)
 	    bfd_preserve_finish (abfd, &preserve);
-	}
-
-      if (temp && (abfd->format != bfd_archive || bfd_has_map (abfd)))
-	{
-	  /* This format checks out as ok!  */
-	  right_targ = temp;
 
-	  /* If this is the default target, accept it, even if other
-	     targets might match.  People who want those other targets
-	     have to set the GNUTARGET variable.  */
-	  if (temp == bfd_default_vector[0])
-	    goto ok_ret;
-
-	  if (matching_vector)
-	    matching_vector[match_count] = temp;
-	  match_count++;
+	  if (abfd->format != bfd_archive
+	      || (bfd_has_map (abfd)
+		  && bfd_get_error () != bfd_error_wrong_object_format))
+	    {
+	      /* This format checks out as ok!  */
+	      right_targ = temp;
 
-	  if (temp->match_priority < best_match)
+	      /* If this is the default target, accept it, even if
+		 other targets might match.  People who want those
+		 other targets have to set the GNUTARGET variable.  */
+	      if (temp == bfd_default_vector[0])
+		goto ok_ret;
+
+	      if (matching_vector)
+		matching_vector[match_count] = temp;
+	      match_count++;
+
+	      if (temp->match_priority < best_match)
+		{
+		  best_match = temp->match_priority;
+		  best_count = 0;
+		}
+	      best_count++;
+	    }
+	  else
 	    {
-	      best_match = temp->match_priority;
-	      best_count = 0;
+	      /* An archive with no armap or objects of the wrong
+		 type.  We want this target to match if we get no
+		 better matches.  */
+	      if (ar_right_targ != bfd_default_vector[0])
+		ar_right_targ = *target;
+	      if (matching_vector)
+		matching_vector[ar_match_index] = *target;
+	      ar_match_index++;
 	    }
-	  best_count++;
-	}
-      else if (temp
-	       || (err = bfd_get_error ()) == bfd_error_wrong_object_format
-	       || err == bfd_error_file_ambiguously_recognized)
-	{
-	  /* An archive with no armap or objects of the wrong type,
-	     or an ambiguous match.  We want this target to match
-	     if we get no better matches.  */
-	  if (ar_right_targ != bfd_default_vector[0])
-	    ar_right_targ = *target;
-	  if (matching_vector)
-	    matching_vector[ar_match_index] = *target;
-	  ar_match_index++;
-	}
-      else if (err != bfd_error_wrong_format)
-	goto err_ret;
 
-      if (temp && !bfd_preserve_save (abfd, &preserve))
+	  if (!bfd_preserve_save (abfd, &preserve))
+	    goto err_ret;
+	}
+      else if (bfd_get_error () != bfd_error_wrong_format)
 	goto err_ret;
     }
 

-- 
Alan Modra
Australia Development Lab, IBM


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]