From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf1-x434.google.com (mail-pf1-x434.google.com [IPv6:2607:f8b0:4864:20::434]) by sourceware.org (Postfix) with ESMTPS id 3D53E3858D1E; Wed, 12 Jul 2023 00:01:01 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 3D53E3858D1E Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-pf1-x434.google.com with SMTP id d2e1a72fcca58-6686c74183cso5596753b3a.1; Tue, 11 Jul 2023 17:01:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1689120059; x=1691712059; h=content-disposition:mime-version:message-id:subject:cc:to:from:date :from:to:cc:subject:date:message-id:reply-to; bh=rsuB1nyX0+VICKenqHYGObbKUwl7u4oKg4HnWMD1Ics=; b=V0QLzG2/AYRx7/FpOf6dpns5SazL4yKp1czIUFENU9TXHejZDQKA3AynXhxPfHw4f/ hn+4PcU+tbJOmIcIULJ60jE/7pv62tKDjo0ctwGyQ3E/xOUDVK8YGM4IequWH1TZZv4j kXkGV6So/6t4eVq5KAIOQ4LjpozNYHGgg5OE7ebizqAgO1A+LuIm5aV/ZWttEz8Wjau7 N9gM4cHYRFmXS+BCYF8nPT6fbn8VkbMwFYZ50BPW6sj7QdTFXwXsDQziITCgbjA7xWns wmhJKeE96Xl2CG7vPqTCQ+cNkbSiwJxH9oqUB9OdRnBuQt0/tugaBcqu3WIYVx9sp/1w CVlQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1689120059; x=1691712059; h=content-disposition:mime-version:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=rsuB1nyX0+VICKenqHYGObbKUwl7u4oKg4HnWMD1Ics=; b=hVJO1myX+Fu0jKfSFz9qVaFP4/XHWQs5GGu+K4Bdec9OfovB/wKPAU2HRSRKqz+n7+ rRE4u4MSI0YhqpF4VykcpBhtPOxPmHS8L+uWSPyyOVdtLics3ZHxNllXyv0/as1GMHLR QbwkhJGYS5bU1rPezhxieF9ke7Dx46sRrUA/WPNnQNj3dViGkmvRzn+A01Fu26EUmX5l Z0UsYng7CIxufkBhosMX6yCTzmTXG4khnbuAb1EqJJiMGHbgknXoMZwCsPtWnD7Lm22D +DIuCNqUH+3NbUq0uX4jPPpphnXE7XETh9njPQPNv2hgCGdaP4WNK1hTL0vMGt0LAzzA TWKw== X-Gm-Message-State: ABy/qLZzMmOj51eZWA/Unbq91OaYoCF3EfpsIvHO4Wv4TMbjGr6oB25X 2Oh/kDCwOTMqQCvdo3D4KcqYdYpoVks= X-Google-Smtp-Source: APBJJlEx5M3o9ZEkS0flNpvrK3la2HgT8UFLsyuNZOj1pQcuiaxX3HXIUHV8EXzS1WQdyAHq+pY3ug== X-Received: by 2002:a05:6a20:948a:b0:12f:8fdc:2f94 with SMTP id hs10-20020a056a20948a00b0012f8fdc2f94mr9209047pzb.22.1689120059396; Tue, 11 Jul 2023 17:00:59 -0700 (PDT) Received: from squeak.grove.modra.org (158.106.96.58.static.exetel.com.au. [58.96.106.158]) by smtp.gmail.com with ESMTPSA id d17-20020aa78691000000b0066a4e561beesm2321624pfo.173.2023.07.11.17.00.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 11 Jul 2023 17:00:57 -0700 (PDT) Received: by squeak.grove.modra.org (Postfix, from userid 1000) id 6316B1142A10; Wed, 12 Jul 2023 09:30:55 +0930 (ACST) Date: Wed, 12 Jul 2023 09:30:55 +0930 From: Alan Modra To: binutils@sourceware.org Cc: gdb-patches@sourceware.org Subject: Re: Keeping track of rs6000-coff archive element pointers Message-ID: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Spam-Status: No, score=-3034.5 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 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 "); @@ -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