From: Alan Modra <amodra@gmail.com>
To: binutils@sourceware.org
Cc: gdb-patches@sourceware.org
Subject: Re: Keeping track of rs6000-coff archive element pointers
Date: Wed, 12 Jul 2023 09:30:55 +0930 [thread overview]
Message-ID: <ZK3tN+1IaJAmnuLS@squeak.grove.modra.org> (raw)
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
next reply other threads:[~2023-07-12 0:01 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-07-12 0:00 Alan Modra [this message]
-- strict thread matches above, loose matches on Subject: below --
2023-04-28 6:15 Alan Modra
2023-04-21 2:49 Alan Modra
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZK3tN+1IaJAmnuLS@squeak.grove.modra.org \
--to=amodra@gmail.com \
--cc=binutils@sourceware.org \
--cc=gdb-patches@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).