public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
* Re: Keeping track of rs6000-coff archive element pointers
@ 2023-04-28  6:15 Alan Modra
  0 siblings, 0 replies; 3+ messages in thread
From: Alan Modra @ 2023-04-28  6:15 UTC (permalink / raw)
  To: binutils

Commit de7b90610e9e left a hole in the element checking, explained by
the comment added to _bfd_xcoff_openr_next_archived_file.  While
fixing this, tidy some types used to hold unsigned values so that
casts are not needed to avoid signed/unsigned comparison warnings.
Also tidy a few things in xcoff.h.

bfd/
	* coff-rs6000.c (_bfd_xcoff_openr_next_archived_file): Check
	that we aren't pointing back at the last element.  Make
	filestart a ufile_ptr.  Update for xcoff_artdata change.
	(_bfd_strntol, _bfd_strntoll): Return unsigned values.
	(_bfd_xcoff_slurp_armap): Make off a ufile_ptr.
	(add_ranges): Update for xcoff_artdata change.
	* libbfd-in.h (struct artdata): Make first_file_filepos a
	ufile_ptr.
	* libbfd.h: Regenerate.
include/
	* coff/xcoff.h (struct xcoff_artdata): Replace min_elt with
	ar_hdr_size.
	(xcoff_big_format_p): In the !SMALL_ARCHIVE case return true
	for anything but a small archive.
---
 bfd/coff-rs6000.c    | 35 ++++++++++++++++++++++++++---------
 bfd/libbfd-in.h      |  2 +-
 bfd/libbfd.h         |  2 +-
 include/coff/xcoff.h | 10 +++++-----
 4 files changed, 33 insertions(+), 16 deletions(-)

diff --git a/bfd/coff-rs6000.c b/bfd/coff-rs6000.c
index 3b451912df7..421dc8f7ee5 100644
--- a/bfd/coff-rs6000.c
+++ b/bfd/coff-rs6000.c
@@ -1294,7 +1294,7 @@ _bfd_xcoff_reloc_name_lookup (bfd *abfd ATTRIBUTE_UNUSED,
    take a maximum length as an additional parameter.  Also - just to save space,
    we omit the endptr return parameter, since we know that it is never used.  */
 
-static long
+static unsigned long
 _bfd_strntol (const char * nptr, int base, unsigned int maxlen)
 {
   char buf[24]; /* Should be enough.  */
@@ -1306,7 +1306,7 @@ _bfd_strntol (const char * nptr, int base, unsigned int maxlen)
   return strtol (buf, NULL, base);
 }
 
-static long long
+static unsigned long long
 _bfd_strntoll (const char * nptr, int base, unsigned int maxlen)
 {
   char buf[32]; /* Should be enough.  */
@@ -1338,7 +1338,7 @@ _bfd_strntoll (const char * nptr, int base, unsigned int maxlen)
 bool
 _bfd_xcoff_slurp_armap (bfd *abfd)
 {
-  file_ptr off;
+  ufile_ptr off;
   size_t namlen;
   bfd_size_type sz;
   bfd_byte *contents, *cend;
@@ -1636,7 +1636,8 @@ add_range (bfd *abfd, ufile_ptr start, ufile_ptr end)
     /* Overlap with another element.  */
     goto err;
 
-  unsigned min_elt = x_artdata (abfd)->min_elt;
+  /* A zero size element with a one char name is this big.  */
+  unsigned min_elt = x_artdata (abfd)->ar_hdr_size + 2 + SXCOFFARFMAG;
   if (start - lo->end < min_elt)
     {
       /* Merge into an existing range.  */
@@ -1757,7 +1758,7 @@ _bfd_xcoff_read_ar_hdr (bfd *abfd)
 bfd *
 _bfd_xcoff_openr_next_archived_file (bfd *archive, bfd *last_file)
 {
-  file_ptr filestart;
+  ufile_ptr filestart;
 
   if (x_artdata (archive) == NULL)
     {
@@ -1770,10 +1771,10 @@ _bfd_xcoff_openr_next_archived_file (bfd *archive, bfd *last_file)
       if (last_file == NULL)
 	{
 	  filestart = bfd_ardata (archive)->first_file_filepos;
-	  if (x_artdata (archive)->min_elt == 0)
+	  if (x_artdata (archive)->ar_hdr_size == 0)
 	    {
 	      x_artdata (archive)->ranges.end = SIZEOF_AR_FILE_HDR;
-	      x_artdata (archive)->min_elt = SIZEOF_AR_HDR + SXCOFFARFMAG + 2;
+	      x_artdata (archive)->ar_hdr_size = SIZEOF_AR_HDR;
 	    }
 	}
       else
@@ -1794,10 +1795,10 @@ _bfd_xcoff_openr_next_archived_file (bfd *archive, bfd *last_file)
       if (last_file == NULL)
 	{
 	  filestart = bfd_ardata (archive)->first_file_filepos;
-	  if (x_artdata (archive)->min_elt == 0)
+	  if (x_artdata (archive)->ar_hdr_size == 0)
 	    {
 	      x_artdata (archive)->ranges.end = SIZEOF_AR_FILE_HDR_BIG;
-	      x_artdata (archive)->min_elt = SIZEOF_AR_HDR_BIG + SXCOFFARFMAG + 2;
+	      x_artdata (archive)->ar_hdr_size = SIZEOF_AR_HDR_BIG;
 	    }
 	}
       else
@@ -1814,6 +1815,22 @@ _bfd_xcoff_openr_next_archived_file (bfd *archive, bfd *last_file)
 	}
     }
 
+  /* Check that we aren't pointing back at the last element.  This is
+     necessary depite the add_range checking in _bfd_xcoff_read_ar_hdr
+     because archive.c leaves the last element open and thus in the
+     archive element cache until the next element is opened.  */
+  if (last_file != NULL)
+    {
+      ufile_ptr laststart = last_file->proxy_origin;
+      laststart -= x_artdata (archive)->ar_hdr_size;
+      laststart -= arch_eltdata (last_file)->extra_size;
+      if (filestart == laststart)
+	{
+	  bfd_set_error (bfd_error_malformed_archive);
+	  return NULL;
+	}
+    }
+
   return _bfd_get_elt_at_filepos (archive, filestart, NULL);
 }
 
diff --git a/bfd/libbfd-in.h b/bfd/libbfd-in.h
index 68b5343fd2e..4305b8416ea 100644
--- a/bfd/libbfd-in.h
+++ b/bfd/libbfd-in.h
@@ -72,7 +72,7 @@ extern unsigned int _bfd_section_id ATTRIBUTE_HIDDEN;
 
 struct artdata
 {
-  file_ptr first_file_filepos;
+  ufile_ptr first_file_filepos;
   /* Speed up searching the armap */
   htab_t cache;
   carsym *symdefs;		/* The symdef entries.  */
diff --git a/bfd/libbfd.h b/bfd/libbfd.h
index bb7f2f1efcf..aceec4ab9c0 100644
--- a/bfd/libbfd.h
+++ b/bfd/libbfd.h
@@ -78,7 +78,7 @@ extern unsigned int _bfd_section_id ATTRIBUTE_HIDDEN;
 
 struct artdata
 {
-  file_ptr first_file_filepos;
+  ufile_ptr first_file_filepos;
   /* Speed up searching the armap */
   htab_t cache;
   carsym *symdefs;		/* The symdef entries.  */
diff --git a/include/coff/xcoff.h b/include/coff/xcoff.h
index 08afc000bf0..7e86cc37d5c 100644
--- a/include/coff/xcoff.h
+++ b/include/coff/xcoff.h
@@ -653,7 +653,7 @@ struct xcoff_artdata
   } u;
   struct ar_ranges ranges;
   /* Anything less than this size can't hold an archive element.  */
-  unsigned int min_elt;
+  unsigned int ar_hdr_size;
 };
 
 #define x_artdata(abfd) ((struct xcoff_artdata *) bfd_ardata (abfd)->tdata)
@@ -663,13 +663,13 @@ struct xcoff_artdata
 #ifndef SMALL_ARCHIVE
 /* Creates big archives by default */
 #define xcoff_big_format_p(abfd) \
-  (bfd_ardata (abfd) != NULL				\
-   && (x_artdata (abfd) == NULL			\
-       || x_artdata (abfd)->u.hdr.magic[1] == 'b'))
+  (bfd_ardata (abfd) == NULL			\
+   || x_artdata (abfd) == NULL			\
+   || x_artdata (abfd)->u.hdr.magic[1] != 'a')
 #else
 /* Creates small archives by default. */
 #define xcoff_big_format_p(abfd) \
-  (bfd_ardata (abfd) != NULL				\
+  (bfd_ardata (abfd) != NULL			\
    && x_artdata (abfd) != NULL			\
    && x_artdata (abfd)->u.hdr.magic[1] == 'b')
 #endif

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Keeping track of rs6000-coff archive element pointers
@ 2023-07-12  0:00 Alan Modra
  0 siblings, 0 replies; 3+ messages in thread
From: Alan Modra @ 2023-07-12  0:00 UTC (permalink / raw)
  To: binutils; +Cc: gdb-patches

See https://sourceware.org/pipermail/gdb-patches/2023-July/200772.html
thread.  I'm going to apply this to mainline and binutils-2.41.

bfd/
	* coff-rs6000.c (add_range): Revise comment, noting possible fail.
	(_bfd_xcoff_openr_next_archived_file): Start with clean ranges.
binutils/
	* bfdtest1.c: Enhance to catch errors on second scan.

diff --git a/bfd/coff-rs6000.c b/bfd/coff-rs6000.c
index 271a24fff69..a692c1ae474 100644
--- a/bfd/coff-rs6000.c
+++ b/bfd/coff-rs6000.c
@@ -1598,14 +1598,32 @@ _bfd_xcoff_archive_p (bfd *abfd)
 
 /* Track file ranges occupied by elements.  Add [START,END) to the
    list of ranges and return TRUE if there is no overlap between the
-   new and any other element or the archive file header.  Note that
-   this would seem to preclude calling _bfd_get_elt_at_filepos twice
-   for the same element, but we won't get to _bfd_xcoff_read_ar_hdr if
-   an element is read more than once.  See _bfd_get_elt_at_filepos use
-   of _bfd_look_for_bfd_in_cache.  Also, the xcoff archive code
-   doesn't call _bfd_read_ar_hdr when reading the armap, nor does it
-   need to use extended name tables.  So those other routines in
-   archive.c that call _bfd_read_ar_hdr are unused.  */
+   new and any other element or the archive file header.  This is
+   aimed at preventing infinite looping on malformed archives, for
+   "ar" and similar which typically use code like:
+   .  for (last = bfd_openr_next_archived_file (archive, NULL);
+   .       last;
+   .       last = next)
+   .    {
+   .      do_something_with (last);
+   .      next = bfd_openr_next_archived_file (archive, last);
+   .      bfd_close (last);
+   .    }
+   The check implemented here is only possible due to the fact that
+   for XCOFF archives bfd_openr_next_archived_file is the only code
+   path leading to _bfd_read_ar_hdr.  _bfd_read_ar_hdr is not called
+   when reading the armap, nor do XCOFF archives use the extended name
+   scheme implemented in archive.c.
+
+   Note that the check relies on the previous element being closed,
+   and there is one case where add_range might fail but I think it is
+   sufficently unusual that it doesn't warrant fixing:
+   If the loop body above called bfd_openr_next_archived_file twice
+   with the same arguments and the element returned is bfd_close'd
+   between those calls then we'll return false here for the second
+   call.  (For why this is so see _bfd_look_for_bfd_in_cache in
+   _bfd_get_elt_at_filepos, and know that bfd_close removes elements
+   from the cache.)  */
 
 static bool
 add_range (bfd *abfd, ufile_ptr start, ufile_ptr end)
@@ -1770,12 +1788,14 @@ _bfd_xcoff_openr_next_archived_file (bfd *archive, bfd *last_file)
     {
       if (last_file == NULL)
 	{
+	  /* If we are scanning over elements twice in an open archive,
+	     which can happen in gdb after a fork, ensure we start the
+	     second scan with clean ranges.  */
+	  x_artdata (archive)->ranges.start = 0;
+	  x_artdata (archive)->ranges.end = SIZEOF_AR_FILE_HDR;
+	  x_artdata (archive)->ranges.next = NULL;
+	  x_artdata (archive)->ar_hdr_size = SIZEOF_AR_HDR;
 	  filestart = bfd_ardata (archive)->first_file_filepos;
-	  if (x_artdata (archive)->ar_hdr_size == 0)
-	    {
-	      x_artdata (archive)->ranges.end = SIZEOF_AR_FILE_HDR;
-	      x_artdata (archive)->ar_hdr_size = SIZEOF_AR_HDR;
-	    }
 	}
       else
 	GET_VALUE_IN_FIELD (filestart, arch_xhdr (last_file)->nextoff, 10);
@@ -1794,12 +1814,11 @@ _bfd_xcoff_openr_next_archived_file (bfd *archive, bfd *last_file)
     {
       if (last_file == NULL)
 	{
+	  x_artdata (archive)->ranges.start = 0;
+	  x_artdata (archive)->ranges.end = SIZEOF_AR_FILE_HDR_BIG;
+	  x_artdata (archive)->ranges.next = NULL;
+	  x_artdata (archive)->ar_hdr_size = SIZEOF_AR_HDR_BIG;
 	  filestart = bfd_ardata (archive)->first_file_filepos;
-	  if (x_artdata (archive)->ar_hdr_size == 0)
-	    {
-	      x_artdata (archive)->ranges.end = SIZEOF_AR_FILE_HDR_BIG;
-	      x_artdata (archive)->ar_hdr_size = SIZEOF_AR_HDR_BIG;
-	    }
 	}
       else
 	GET_VALUE_IN_FIELD (filestart, arch_xhdr_big (last_file)->nextoff, 10);
diff --git a/binutils/bfdtest1.c b/binutils/bfdtest1.c
index 72355723479..b81f48b13fe 100644
--- a/binutils/bfdtest1.c
+++ b/binutils/bfdtest1.c
@@ -33,6 +33,7 @@ main (int argc, char **argv)
 {
   bfd *archive;
   bfd *last, *next;
+  int count;
 
   if (argc != 2)
     die ("usage: bfdtest1 <archive>");
@@ -47,12 +48,13 @@ main (int argc, char **argv)
       die ("bfd_check_format");
     }
 
-  for (last = bfd_openr_next_archived_file (archive, NULL);
+  for (count = 0, last = bfd_openr_next_archived_file (archive, NULL);
        last;
        last = next)
     {
       next = bfd_openr_next_archived_file (archive, last);
       bfd_close (last);
+      count++;
     }
 
   for (last = bfd_openr_next_archived_file (archive, NULL);
@@ -61,8 +63,12 @@ main (int argc, char **argv)
     {
       next = bfd_openr_next_archived_file (archive, last);
       bfd_close (last);
+      count--;
     }
 
+  if (count != 0)
+    die ("element count differs in second scan");
+
   if (!bfd_close (archive))
     die ("bfd_close");
 

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Keeping track of rs6000-coff archive element pointers
@ 2023-04-21  2:49 Alan Modra
  0 siblings, 0 replies; 3+ messages in thread
From: Alan Modra @ 2023-04-21  2:49 UTC (permalink / raw)
  To: binutils

rs6000-coff archives use a linked list of file offsets, where each
element points to the next element.  The idea is to allow updating of
large archives quickly without rewriting the whole archive.  (binutils
ar does not do this.)  Unfortunately this is an easy target for
fuzzers to create an archive that will cause ar or any other tool
processing archives to hang.  I'd implemented guards against pointing
back to the previous element, but of course that didn't last long.

So this patch implements a scheme to keep track of file offset ranges
used by elements as _bfd_read_ar_hdr is called for each element.  See
the add_range function comment.  I needed a place to stash the list,
so chose the obvious artdata.tdata backend extension to archive's
tdata, already used by xcoff.  That involved a little cleanup, because
while it would be possible to continue using different artdata.tdata
for the big and small archives, it's nicer to use a union.

If anyone is concerned this list of element ranges might grow large
and thus significantly slow down the tools, adjacent ranges are
merged.  In fact something like "ar t" will only ever have one range
on xcoff archives generated by binutils/ar.  I agree there might still
be a problem with ld random element access via the armap.

include/
	* coff/xcoff.h (SIZEOF_AR_FILE_HDR): Use sizeof.
	(SIZEOF_AR_FILE_HDR_BIG, SIZEOF_AR_HDR, SIZEOF_AR_HDR_BIG): Likewise.
	(struct ar_ranges, struct xcoff_artdata): New.
	(x_artdata): Define.
	(xcoff_big_format_p): Rewrite.
	(xcoff_ardata, xcoff_ardata_big): Delete.
bfd/
	* coff-rs6000.c: Replace uses of xcoff_ardata and
	xcoff_ardata_big throughout file.
	(_bfd_xcoff_archive_p): Adjust artdata.tdata allocation.
	(add_range): New function.
	(_bfd_xcoff_read_ar_hdr): Use it here.  Fix memory leak.
	(_bfd_xcoff_openr_next_archived_file): Remove old sanity
	checks.  Set up range for header.
	(xcoff_write_archive_contents_old): Make the temporary
	artdata.tdata used here to pass info down to
	_bfd_compute_and_write_armap a struct xcoff_artdata.
	(xcoff_write_archive_contents_big): Likewise.
	* coff64-rs6000.c: Replace uses of xcoff_ardata and
	xcoff_ardata_big throughout file.
	(xcoff64_archive_p): Adjust artdata.tdata allocation.

diff --git a/bfd/coff-rs6000.c b/bfd/coff-rs6000.c
index 5ea06fa2e09..3b451912df7 100644
--- a/bfd/coff-rs6000.c
+++ b/bfd/coff-rs6000.c
@@ -1346,7 +1346,7 @@ _bfd_xcoff_slurp_armap (bfd *abfd)
   carsym *arsym;
   bfd_byte *p;
 
-  if (xcoff_ardata (abfd) == NULL)
+  if (x_artdata (abfd) == NULL)
     {
       abfd->has_armap = false;
       return true;
@@ -1357,7 +1357,7 @@ _bfd_xcoff_slurp_armap (bfd *abfd)
       /* This is for the old format.  */
       struct xcoff_ar_hdr hdr;
 
-      GET_VALUE_IN_FIELD (off, xcoff_ardata (abfd)->symoff, 10);
+      GET_VALUE_IN_FIELD (off, x_artdata (abfd)->u.hdr.symoff, 10);
       if (off == 0)
 	{
 	  abfd->has_armap = false;
@@ -1419,7 +1419,7 @@ _bfd_xcoff_slurp_armap (bfd *abfd)
       /* This is for the new format.  */
       struct xcoff_ar_hdr_big hdr;
 
-      GET_VALUE_IN_FIELD (off, xcoff_ardata_big (abfd)->symoff, 10);
+      GET_VALUE_IN_FIELD (off, x_artdata (abfd)->u.bhdr.symoff, 10);
       if (off == 0)
 	{
 	  abfd->has_armap = false;
@@ -1548,12 +1548,12 @@ _bfd_xcoff_archive_p (bfd *abfd)
       GET_VALUE_IN_FIELD (bfd_ardata (abfd)->first_file_filepos,
 			  hdr.firstmemoff, 10);
 
-      amt = SIZEOF_AR_FILE_HDR;
+      amt = sizeof (struct xcoff_artdata);
       bfd_ardata (abfd)->tdata = bfd_zalloc (abfd, amt);
       if (bfd_ardata (abfd)->tdata == NULL)
 	goto error_ret;
 
-      memcpy (bfd_ardata (abfd)->tdata, &hdr, SIZEOF_AR_FILE_HDR);
+      memcpy (&x_artdata (abfd)->u.hdr, &hdr, SIZEOF_AR_FILE_HDR);
     }
   else
     {
@@ -1576,12 +1576,12 @@ _bfd_xcoff_archive_p (bfd *abfd)
 							    (const char **) 0,
 							    10);
 
-      amt = SIZEOF_AR_FILE_HDR_BIG;
+      amt = sizeof (struct xcoff_artdata);
       bfd_ardata (abfd)->tdata = bfd_zalloc (abfd, amt);
       if (bfd_ardata (abfd)->tdata == NULL)
 	goto error_ret;
 
-      memcpy (bfd_ardata (abfd)->tdata, &hdr, SIZEOF_AR_FILE_HDR_BIG);
+      memcpy (&x_artdata (abfd)->u.bhdr, &hdr, SIZEOF_AR_FILE_HDR_BIG);
     }
 
   if (! _bfd_xcoff_slurp_armap (abfd))
@@ -1596,6 +1596,78 @@ _bfd_xcoff_archive_p (bfd *abfd)
   return _bfd_no_cleanup;
 }
 
+/* Track file ranges occupied by elements.  Add [START,END) to the
+   list of ranges and return TRUE if there is no overlap between the
+   new and any other element or the archive file header.  Note that
+   this would seem to preclude calling _bfd_get_elt_at_filepos twice
+   for the same element, but we won't get to _bfd_xcoff_read_ar_hdr if
+   an element is read more than once.  See _bfd_get_elt_at_filepos use
+   of _bfd_look_for_bfd_in_cache.  Also, the xcoff archive code
+   doesn't call _bfd_read_ar_hdr when reading the armap, nor does it
+   need to use extended name tables.  So those other routines in
+   archive.c that call _bfd_read_ar_hdr are unused.  */
+
+static bool
+add_range (bfd *abfd, ufile_ptr start, ufile_ptr end)
+{
+  if (end <= start)
+    {
+    err:
+      bfd_set_error (bfd_error_malformed_archive);
+      return false;
+    }
+
+  /* This list is kept sorted by address.  Find the highest address
+     range on the list that ends before the new range starts.  Exit
+     the loop with that range in LO, and the mext higher range in HI.  */
+  struct ar_ranges *hi = &x_artdata (abfd)->ranges;
+  struct ar_ranges *lo = NULL;
+  while (hi && hi->end <= start)
+    {
+      lo = hi;
+      hi = hi->next;
+    }
+
+  if (lo == NULL)
+    /* Start overlaps the file header or elements adjacent to it.  */
+    goto err;
+
+  if (hi && hi->start < end)
+    /* Overlap with another element.  */
+    goto err;
+
+  unsigned min_elt = x_artdata (abfd)->min_elt;
+  if (start - lo->end < min_elt)
+    {
+      /* Merge into an existing range.  */
+      lo->end = end;
+      if (hi && hi->start - end < min_elt)
+	{
+	  /* In fact, we can merge two ranges.  */
+	  lo->end = hi->end;
+	  lo->next = hi->next;
+	  /* The list uses bfd_alloc so don't free HI.  */
+	}
+      return true;
+    }
+
+  if (hi && hi->start - end < min_elt)
+    {
+      /* Merge into an existing range.  */
+      hi->start = start;
+      return true;
+    }
+
+  struct ar_ranges *newr = bfd_alloc (abfd, sizeof (*newr));
+  if (newr == NULL)
+    return false;
+  newr->start = start;
+  newr->end = end;
+  newr->next = hi;
+  lo->next = newr;
+  return true;
+}
+
 /* Read the archive header in an XCOFF archive.  */
 
 void *
@@ -1604,6 +1676,7 @@ _bfd_xcoff_read_ar_hdr (bfd *abfd)
   bfd_size_type namlen;
   struct areltdata *ret;
   bfd_size_type amt;
+  ufile_ptr start = abfd->where;
 
   if (! xcoff_big_format_p (abfd))
     {
@@ -1669,8 +1742,12 @@ _bfd_xcoff_read_ar_hdr (bfd *abfd)
   ret->extra_size = namlen + (namlen & 1) + SXCOFFARFMAG;
 
   /* Skip over the XCOFFARFMAG at the end of the file name.  */
-  if (bfd_seek (abfd, (file_ptr) ((namlen & 1) + SXCOFFARFMAG), SEEK_CUR) != 0)
-    return NULL;
+  if (bfd_seek (abfd, (namlen & 1) + SXCOFFARFMAG, SEEK_CUR) != 0
+      || !add_range (abfd, start, abfd->where + ret->parsed_size))
+    {
+      free (ret);
+      return NULL;
+    }
 
   return ret;
 }
@@ -1681,9 +1758,8 @@ bfd *
 _bfd_xcoff_openr_next_archived_file (bfd *archive, bfd *last_file)
 {
   file_ptr filestart;
-  file_ptr laststart, lastend;
 
-  if (xcoff_ardata (archive) == NULL)
+  if (x_artdata (archive) == NULL)
     {
       bfd_set_error (bfd_error_invalid_operation);
       return NULL;
@@ -1694,32 +1770,20 @@ _bfd_xcoff_openr_next_archived_file (bfd *archive, bfd *last_file)
       if (last_file == NULL)
 	{
 	  filestart = bfd_ardata (archive)->first_file_filepos;
-	  laststart = 0;
-	  lastend = SIZEOF_AR_FILE_HDR;
+	  if (x_artdata (archive)->min_elt == 0)
+	    {
+	      x_artdata (archive)->ranges.end = SIZEOF_AR_FILE_HDR;
+	      x_artdata (archive)->min_elt = SIZEOF_AR_HDR + SXCOFFARFMAG + 2;
+	    }
 	}
       else
-	{
-	  struct areltdata *arel = arch_eltdata (last_file);
-
-	  GET_VALUE_IN_FIELD (filestart, arch_xhdr (last_file)->nextoff, 10);
-	  laststart = last_file->proxy_origin;
-	  lastend = laststart + arel->parsed_size;
-	  laststart -= SIZEOF_AR_HDR + arel->extra_size;
-	}
-
-      /* Sanity check that we aren't pointing into the previous element,
-	 or into the header.  */
-      if (filestart != 0
-	  && (filestart < SIZEOF_AR_FILE_HDR
-	      || (filestart >= laststart && filestart < lastend)))
-	{
-	  bfd_set_error (bfd_error_malformed_archive);
-	  return NULL;
-	}
+	GET_VALUE_IN_FIELD (filestart, arch_xhdr (last_file)->nextoff, 10);
 
       if (filestart == 0
-	  || EQ_VALUE_IN_FIELD (filestart, xcoff_ardata (archive)->memoff, 10)
-	  || EQ_VALUE_IN_FIELD (filestart, xcoff_ardata (archive)->symoff, 10))
+	  || EQ_VALUE_IN_FIELD (filestart,
+				x_artdata (archive)->u.hdr.memoff, 10)
+	  || EQ_VALUE_IN_FIELD (filestart,
+				x_artdata (archive)->u.hdr.symoff, 10))
 	{
 	  bfd_set_error (bfd_error_no_more_archived_files);
 	  return NULL;
@@ -1730,32 +1794,20 @@ _bfd_xcoff_openr_next_archived_file (bfd *archive, bfd *last_file)
       if (last_file == NULL)
 	{
 	  filestart = bfd_ardata (archive)->first_file_filepos;
-	  laststart = 0;
-	  lastend = SIZEOF_AR_FILE_HDR_BIG;
+	  if (x_artdata (archive)->min_elt == 0)
+	    {
+	      x_artdata (archive)->ranges.end = SIZEOF_AR_FILE_HDR_BIG;
+	      x_artdata (archive)->min_elt = SIZEOF_AR_HDR_BIG + SXCOFFARFMAG + 2;
+	    }
 	}
       else
-	{
-	  struct areltdata *arel = arch_eltdata (last_file);
-
-	  GET_VALUE_IN_FIELD (filestart, arch_xhdr_big (last_file)->nextoff, 10);
-	  laststart = last_file->proxy_origin;
-	  lastend = laststart + arel->parsed_size;
-	  laststart -= SIZEOF_AR_HDR_BIG + arel->extra_size;
-	}
-
-      /* Sanity check that we aren't pointing into the previous element
-	 or into the header.  */
-      if (filestart != 0
-	  && (filestart < SIZEOF_AR_FILE_HDR_BIG
-	      || (filestart >= laststart && filestart < lastend)))
-	{
-	  bfd_set_error (bfd_error_malformed_archive);
-	  return NULL;
-	}
+	GET_VALUE_IN_FIELD (filestart, arch_xhdr_big (last_file)->nextoff, 10);
 
       if (filestart == 0
-	  || EQ_VALUE_IN_FIELD (filestart, xcoff_ardata_big (archive)->memoff, 10)
-	  || EQ_VALUE_IN_FIELD (filestart, xcoff_ardata_big (archive)->symoff, 10))
+	  || EQ_VALUE_IN_FIELD (filestart,
+				x_artdata (archive)->u.bhdr.memoff, 10)
+	  || EQ_VALUE_IN_FIELD (filestart,
+				x_artdata (archive)->u.bhdr.symoff, 10))
 	{
 	  bfd_set_error (bfd_error_no_more_archived_files);
 	  return NULL;
@@ -1832,7 +1884,8 @@ xcoff_write_armap_old (bfd *abfd, unsigned int elength ATTRIBUTE_UNUSED,
   memset (&hdr, 0, sizeof hdr);
   sprintf (hdr.size, "%ld", (long) (4 + orl_count * 4 + stridx));
   sprintf (hdr.nextoff, "%d", 0);
-  memcpy (hdr.prevoff, xcoff_ardata (abfd)->memoff, XCOFFARMAG_ELEMENT_SIZE);
+  memcpy (hdr.prevoff, x_artdata (abfd)->u.hdr.memoff,
+	  XCOFFARMAG_ELEMENT_SIZE);
   sprintf (hdr.date, "%d", 0);
   sprintf (hdr.uid, "%d", 0);
   sprintf (hdr.gid, "%d", 0);
@@ -2004,7 +2057,7 @@ xcoff_write_armap_big (bfd *abfd, unsigned int elength ATTRIBUTE_UNUSED,
   /* Explicit cast to int for compiler.  */
   BFD_ASSERT ((int)(str_64 + str_32) == stridx);
 
-  fhdr = xcoff_ardata_big (abfd);
+  fhdr = &x_artdata (abfd)->u.bhdr;
 
   /* xcoff_write_archive_contents_big passes nextoff in symoff. */
   READ20 (fhdr->memoff, prevoff);
@@ -2217,7 +2270,8 @@ static bool
 xcoff_write_archive_contents_old (bfd *abfd)
 {
   struct archive_iterator iterator;
-  struct xcoff_ar_file_hdr fhdr;
+  struct xcoff_artdata xtdata;
+  struct xcoff_ar_file_hdr *fhdr = &xtdata.u.hdr;
   bfd_size_type count;
   bfd_size_type total_namlen;
   file_ptr *offsets;
@@ -2231,10 +2285,10 @@ xcoff_write_archive_contents_old (bfd *abfd)
   char *p;
   char decbuf[XCOFFARMAG_ELEMENT_SIZE + 1];
 
-  memset (&fhdr, 0, sizeof fhdr);
-  (void) memcpy (fhdr.magic, XCOFFARMAG, SXCOFFARMAG);
-  sprintf (fhdr.firstmemoff, "%d", SIZEOF_AR_FILE_HDR);
-  sprintf (fhdr.freeoff, "%d", 0);
+  memset (&xtdata, 0, sizeof (xtdata));
+  memcpy (fhdr->magic, XCOFFARMAG, SXCOFFARMAG);
+  sprintf (fhdr->firstmemoff, "%zu", SIZEOF_AR_FILE_HDR);
+  sprintf (fhdr->freeoff, "%d", 0);
 
   count = 0;
   total_namlen = 0;
@@ -2342,13 +2396,13 @@ xcoff_write_archive_contents_old (bfd *abfd)
       prevoff = iterator.current.offset;
     }
 
-  sprintf (fhdr.lastmemoff, "%ld", (long) prevoff);
+  sprintf (fhdr->lastmemoff, "%ld", (long) prevoff);
 
   /* Write out the member table.  */
 
   nextoff = iterator.next.offset;
   BFD_ASSERT (nextoff == bfd_tell (abfd));
-  sprintf (fhdr.memoff, "%ld", (long) nextoff);
+  sprintf (fhdr->memoff, "%ld", (long) nextoff);
 
   memset (&ahdr, 0, sizeof ahdr);
   sprintf (ahdr.size, "%ld", (long) (XCOFFARMAG_ELEMENT_SIZE
@@ -2413,26 +2467,27 @@ xcoff_write_archive_contents_old (bfd *abfd)
 
   /* Write out the armap, if appropriate.  */
   if (! makemap || ! hasobjects)
-    sprintf (fhdr.symoff, "%d", 0);
+    sprintf (fhdr->symoff, "%d", 0);
   else
     {
       BFD_ASSERT (nextoff == bfd_tell (abfd));
-      sprintf (fhdr.symoff, "%ld", (long) nextoff);
-      bfd_ardata (abfd)->tdata = &fhdr;
-      if (! _bfd_compute_and_write_armap (abfd, 0))
+      sprintf (fhdr->symoff, "%ld", (long) nextoff);
+      bfd_ardata (abfd)->tdata = &xtdata;
+      bool ret = _bfd_compute_and_write_armap (abfd, 0);
+      bfd_ardata (abfd)->tdata = NULL;
+      if (!ret)
 	return false;
     }
 
   /* Write out the archive file header.  */
 
   /* We need spaces, not null bytes, in the header.  */
-  for (p = (char *) &fhdr; p < (char *) &fhdr + SIZEOF_AR_FILE_HDR; p++)
+  for (p = (char *) fhdr; p < (char *) fhdr + SIZEOF_AR_FILE_HDR; p++)
     if (*p == '\0')
       *p = ' ';
 
-  if (bfd_seek (abfd, (file_ptr) 0, SEEK_SET) != 0
-      || (bfd_bwrite (&fhdr, (bfd_size_type) SIZEOF_AR_FILE_HDR, abfd)
-	  != SIZEOF_AR_FILE_HDR))
+  if (bfd_seek (abfd, 0, SEEK_SET) != 0
+      || (bfd_bwrite (fhdr, SIZEOF_AR_FILE_HDR, abfd) != SIZEOF_AR_FILE_HDR))
     return false;
 
   return true;
@@ -2441,7 +2496,8 @@ xcoff_write_archive_contents_old (bfd *abfd)
 static bool
 xcoff_write_archive_contents_big (bfd *abfd)
 {
-  struct xcoff_ar_file_hdr_big fhdr;
+  struct xcoff_artdata xtdata;
+  struct xcoff_ar_file_hdr_big *fhdr = &xtdata.u.bhdr;
   bfd_size_type count;
   bfd_size_type total_namlen;
   file_ptr *offsets;
@@ -2456,8 +2512,8 @@ xcoff_write_archive_contents_big (bfd *abfd)
   bfd_vma member_table_size;
   struct archive_iterator iterator;
 
-  memset (&fhdr, 0, SIZEOF_AR_FILE_HDR_BIG);
-  memcpy (fhdr.magic, XCOFFARMAGBIG, SXCOFFARMAG);
+  memset (&xtdata, 0, sizeof (xtdata));
+  memcpy (fhdr->magic, XCOFFARMAGBIG, SXCOFFARMAG);
 
   if (bfd_seek (abfd, (file_ptr) SIZEOF_AR_FILE_HDR_BIG, SEEK_SET) != 0)
     return false;
@@ -2575,8 +2631,8 @@ xcoff_write_archive_contents_big (bfd *abfd)
 
   if (count)
     {
-      PRINT20 (fhdr.firstmemoff, offsets[0]);
-      PRINT20 (fhdr.lastmemoff, prevoff);
+      PRINT20 (fhdr->firstmemoff, offsets[0]);
+      PRINT20 (fhdr->lastmemoff, prevoff);
     }
 
   /* Write out the member table.
@@ -2668,7 +2724,7 @@ xcoff_write_archive_contents_big (bfd *abfd)
 
   free (member_table);
 
-  PRINT20 (fhdr.memoff, nextoff);
+  PRINT20 (fhdr->memoff, nextoff);
 
   prevoff = nextoff;
   nextoff += member_table_size;
@@ -2676,24 +2732,26 @@ xcoff_write_archive_contents_big (bfd *abfd)
   /* Write out the armap, if appropriate.  */
 
   if (! makemap || ! hasobjects)
-    PRINT20 (fhdr.symoff, 0);
+    PRINT20 (fhdr->symoff, 0);
   else
     {
       BFD_ASSERT (nextoff == bfd_tell (abfd));
 
-      /* Save nextoff in fhdr.symoff so the armap routine can use it.  */
-      PRINT20 (fhdr.symoff, nextoff);
+      /* Save nextoff in fhdr->symoff so the armap routine can use it.  */
+      PRINT20 (fhdr->symoff, nextoff);
 
-      bfd_ardata (abfd)->tdata = &fhdr;
-      if (! _bfd_compute_and_write_armap (abfd, 0))
+      bfd_ardata (abfd)->tdata = &xtdata;
+      bool ret = _bfd_compute_and_write_armap (abfd, 0);
+      bfd_ardata (abfd)->tdata = NULL;
+      if (!ret)
 	return false;
     }
 
   /* Write out the archive file header.  */
 
-  if (bfd_seek (abfd, (file_ptr) 0, SEEK_SET) != 0
-      || (bfd_bwrite (&fhdr, (bfd_size_type) SIZEOF_AR_FILE_HDR_BIG,
-		      abfd) != SIZEOF_AR_FILE_HDR_BIG))
+  if (bfd_seek (abfd, 0, SEEK_SET) != 0
+      || bfd_bwrite (fhdr,
+		     SIZEOF_AR_FILE_HDR_BIG, abfd) != SIZEOF_AR_FILE_HDR_BIG)
     return false;
 
   return true;
diff --git a/bfd/coff64-rs6000.c b/bfd/coff64-rs6000.c
index a4f631ce0e0..71d21583ee5 100644
--- a/bfd/coff64-rs6000.c
+++ b/bfd/coff64-rs6000.c
@@ -1811,13 +1811,13 @@ xcoff64_slurp_armap (bfd *abfd)
   /* This is for the new format.  */
   struct xcoff_ar_hdr_big hdr;
 
-  if (xcoff_ardata (abfd) == NULL)
+  if (x_artdata (abfd) == NULL)
     {
       abfd->has_armap = false;
       return true;
     }
 
-  off = bfd_scan_vma (xcoff_ardata_big (abfd)->symoff64,
+  off = bfd_scan_vma (x_artdata (abfd)->u.bhdr.symoff64,
 		      (const char **) NULL, 10);
   if (off == 0)
     {
@@ -1943,12 +1943,12 @@ xcoff64_archive_p (bfd *abfd)
 							(const char **) NULL,
 							10);
 
-  amt = SIZEOF_AR_FILE_HDR_BIG;
+  amt = sizeof (struct xcoff_artdata);
   bfd_ardata (abfd)->tdata = bfd_zalloc (abfd, amt);
   if (bfd_ardata (abfd)->tdata == NULL)
     goto error_ret;
 
-  memcpy (bfd_ardata (abfd)->tdata, &hdr, SIZEOF_AR_FILE_HDR_BIG);
+  memcpy (&x_artdata (abfd)->u.bhdr, &hdr, SIZEOF_AR_FILE_HDR_BIG);
 
   if (! xcoff64_slurp_armap (abfd))
     {
@@ -1968,7 +1968,7 @@ xcoff64_archive_p (bfd *abfd)
 static bfd *
 xcoff64_openr_next_archived_file (bfd *archive, bfd *last_file)
 {
-  if ((xcoff_ardata (archive) == NULL)
+  if (x_artdata (archive) == NULL
       || ! xcoff_big_format_p (archive))
     {
       bfd_set_error (bfd_error_invalid_operation);
diff --git a/include/coff/xcoff.h b/include/coff/xcoff.h
index 049ccd863ac..08afc000bf0 100644
--- a/include/coff/xcoff.h
+++ b/include/coff/xcoff.h
@@ -526,7 +526,7 @@ struct xcoff_ar_file_hdr
   char freeoff[XCOFFARMAG_ELEMENT_SIZE];
 };
 
-#define SIZEOF_AR_FILE_HDR (SXCOFFARMAG + 5 * XCOFFARMAG_ELEMENT_SIZE)
+#define SIZEOF_AR_FILE_HDR (sizeof (struct xcoff_ar_file_hdr))
 
 /* This is the equivalent data structure for the big archive format.  */
 
@@ -557,7 +557,7 @@ struct xcoff_ar_file_hdr_big
   char freeoff[XCOFFARMAGBIG_ELEMENT_SIZE];
 };
 
-#define SIZEOF_AR_FILE_HDR_BIG (SXCOFFARMAG + 6 * XCOFFARMAGBIG_ELEMENT_SIZE)
+#define SIZEOF_AR_FILE_HDR_BIG (sizeof (struct xcoff_ar_file_hdr_big))
 
 /* Each XCOFF archive member starts with this (printable) structure.  */
 
@@ -595,7 +595,7 @@ struct xcoff_ar_hdr
      bytes is given in the size field.  */
 };
 
-#define SIZEOF_AR_HDR (3 * XCOFFARMAG_ELEMENT_SIZE + 4 * 12 + 4)
+#define SIZEOF_AR_HDR (sizeof (struct xcoff_ar_hdr))
 
 /* The equivalent for the big archive format.  */
 
@@ -633,35 +633,47 @@ struct xcoff_ar_hdr_big
      bytes is given in the size field.  */
 };
 
-#define SIZEOF_AR_HDR_BIG (3 * XCOFFARMAGBIG_ELEMENT_SIZE + 4 * 12 + 4)
+#define SIZEOF_AR_HDR_BIG (sizeof (struct xcoff_ar_hdr_big))
+
+/* Track archive file offsets used by elements and the header.  */
+struct ar_ranges
+{
+  ufile_ptr start, end;
+  struct ar_ranges *next;
+};
+
+/* An archive bfd has tdata pointing to a struct artdata.  The xcoff
+   backend has artdata.tdata pointing to the following.  */
+struct xcoff_artdata
+{
+  union
+  {
+    struct xcoff_ar_file_hdr hdr;
+    struct xcoff_ar_file_hdr_big bhdr;
+  } u;
+  struct ar_ranges ranges;
+  /* Anything less than this size can't hold an archive element.  */
+  unsigned int min_elt;
+};
+
+#define x_artdata(abfd) ((struct xcoff_artdata *) bfd_ardata (abfd)->tdata)
 
 /* We often have to distinguish between the old and big file format.
-   Make it a bit cleaner.  We can use `xcoff_ardata' here because the
-   `hdr' member has the same size and position in both formats.  
-   <bigaf> is the default format, return TRUE even when xcoff_ardata is 
-   NULL. */
+   u.hdr.magic and u.bhdr.magic have the same size and position.  */
 #ifndef SMALL_ARCHIVE
 /* Creates big archives by default */
 #define xcoff_big_format_p(abfd) \
-  ((NULL != bfd_ardata (abfd) && NULL == xcoff_ardata (abfd)) || \
-   ((NULL != bfd_ardata (abfd)) && \
-    (NULL != xcoff_ardata (abfd)) && \
-    (xcoff_ardata (abfd)->magic[1] == 'b')))
+  (bfd_ardata (abfd) != NULL				\
+   && (x_artdata (abfd) == NULL			\
+       || x_artdata (abfd)->u.hdr.magic[1] == 'b'))
 #else
 /* Creates small archives by default. */
 #define xcoff_big_format_p(abfd) \
-  (((NULL != bfd_ardata (abfd)) && \
-    (NULL != xcoff_ardata (abfd)) && \
-    (xcoff_ardata (abfd)->magic[1] == 'b')))
+  (bfd_ardata (abfd) != NULL				\
+   && x_artdata (abfd) != NULL			\
+   && x_artdata (abfd)->u.hdr.magic[1] == 'b')
 #endif
 
-/* We store a copy of the xcoff_ar_file_hdr in the tdata field of the
-   artdata structure.  Similar for the big archive.  */
-#define xcoff_ardata(abfd) \
-  ((struct xcoff_ar_file_hdr *) bfd_ardata (abfd)->tdata)
-#define xcoff_ardata_big(abfd) \
-  ((struct xcoff_ar_file_hdr_big *) bfd_ardata (abfd)->tdata)
-
 /* We store a copy of the xcoff_ar_hdr in the arelt_data field of an
    archive element.  Similar for the big archive.  */
 #define arch_eltdata(bfd) ((struct areltdata *) ((bfd)->arelt_data))

-- 
Alan Modra
Australia Development Lab, IBM

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2023-07-12  0:01 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-28  6:15 Keeping track of rs6000-coff archive element pointers Alan Modra
  -- strict thread matches above, loose matches on Subject: below --
2023-07-12  0:00 Alan Modra
2023-04-21  2:49 Alan Modra

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).