From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: by sourceware.org (Postfix, from userid 1062) id 7CE633858404; Wed, 12 Jul 2023 01:16:14 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 7CE633858404 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable From: Alan Modra To: bfd-cvs@sourceware.org Subject: [binutils-gdb/binutils-2_41-branch] Re: Keeping track of rs6000-coff archive element pointers X-Act-Checkin: binutils-gdb X-Git-Author: Alan Modra X-Git-Refname: refs/heads/binutils-2_41-branch X-Git-Oldrev: e729325e2ba4507e3891e57aadc39e67b67377e2 X-Git-Newrev: aedbd857394be10fed19c9763502bb1fac887651 Message-Id: <20230712011614.7CE633858404@sourceware.org> Date: Wed, 12 Jul 2023 01:16:14 +0000 (GMT) X-BeenThere: binutils-cvs@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils-cvs mailing list List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 12 Jul 2023 01:16:14 -0000 https://sourceware.org/git/gitweb.cgi?p=3Dbinutils-gdb.git;h=3Daedbd857394b= e10fed19c9763502bb1fac887651 commit aedbd857394be10fed19c9763502bb1fac887651 Author: Alan Modra Date: Tue Jul 11 08:19:20 2023 +0930 Re: Keeping track of rs6000-coff archive element pointers =20 bfd/ * coff-rs6000.c (add_range): Revise comment, noting possible fa= il. (_bfd_xcoff_openr_next_archived_file): Start with clean ranges. binutils/ * bfdtest1.c: Enhance to catch errors on second scan. =20 (cherry picked from commit 1052fb3ecb1ae46bcf22634c48739c12e585196a) Diff: --- bfd/coff-rs6000.c | 55 +++++++++++++++++++++++++++++++++++--------------= ---- binutils/bfdtest1.c | 8 +++++++- 2 files changed, 44 insertions(+), 19 deletions(-) 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) =20 /* 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 =3D bfd_openr_next_archived_file (archive, NULL); + . last; + . last =3D next) + . { + . do_something_with (last); + . next =3D 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.) */ =20 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 =3D=3D 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 =3D 0; + x_artdata (archive)->ranges.end =3D SIZEOF_AR_FILE_HDR; + x_artdata (archive)->ranges.next =3D NULL; + x_artdata (archive)->ar_hdr_size =3D SIZEOF_AR_HDR; filestart =3D bfd_ardata (archive)->first_file_filepos; - if (x_artdata (archive)->ar_hdr_size =3D=3D 0) - { - x_artdata (archive)->ranges.end =3D SIZEOF_AR_FILE_HDR; - x_artdata (archive)->ar_hdr_size =3D 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 =3D=3D NULL) { + x_artdata (archive)->ranges.start =3D 0; + x_artdata (archive)->ranges.end =3D SIZEOF_AR_FILE_HDR_BIG; + x_artdata (archive)->ranges.next =3D NULL; + x_artdata (archive)->ar_hdr_size =3D SIZEOF_AR_HDR_BIG; filestart =3D bfd_ardata (archive)->first_file_filepos; - if (x_artdata (archive)->ar_hdr_size =3D=3D 0) - { - x_artdata (archive)->ranges.end =3D SIZEOF_AR_FILE_HDR_BIG; - x_artdata (archive)->ar_hdr_size =3D 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; =20 if (argc !=3D 2) die ("usage: bfdtest1 "); @@ -47,12 +48,13 @@ main (int argc, char **argv) die ("bfd_check_format"); } =20 - for (last =3D bfd_openr_next_archived_file (archive, NULL); + for (count =3D 0, last =3D bfd_openr_next_archived_file (archive, NULL); last; last =3D next) { next =3D bfd_openr_next_archived_file (archive, last); bfd_close (last); + count++; } =20 for (last =3D bfd_openr_next_archived_file (archive, NULL); @@ -61,8 +63,12 @@ main (int argc, char **argv) { next =3D bfd_openr_next_archived_file (archive, last); bfd_close (last); + count--; } =20 + if (count !=3D 0) + die ("element count differs in second scan"); + if (!bfd_close (archive)) die ("bfd_close");