From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id SstTIt4O7WiNni4AWB0awg (envelope-from ) for ; Mon, 13 Oct 2025 10:38:22 -0400 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=RJpB8GMU; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 7EC0B1E0B6; Mon, 13 Oct 2025 10:38:22 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-3.4 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_VALIDITY_CERTIFIED_BLOCKED, RCVD_IN_VALIDITY_RPBL_BLOCKED,RCVD_IN_VALIDITY_SAFE_BLOCKED autolearn=ham autolearn_force=no version=4.0.1 Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange x25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 3A1961E047 for ; Mon, 13 Oct 2025 10:38:21 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 7C87B3858C2D for ; Mon, 13 Oct 2025 14:38:20 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 7C87B3858C2D Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=RJpB8GMU Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id 15C913858D26 for ; Mon, 13 Oct 2025 14:37:39 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 15C913858D26 Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 15C913858D26 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1760366260; cv=none; b=GCma9tqaawrUsQDIYPutc72Cw8cGAlO21z6f9VpqPgv/RzZ/mZkpR1X6kUPZzx4Jtkdl0r2EhxO85LSrypyFzdRZbFkijpz8yYjRP+xYkeSY9JzH3+ieNb8Nkmjg8vuEHhkK9PqIsgW2DI5HJvKqMQ5Rm6hfGTUGVuNwu7jxSAE= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1760366260; c=relaxed/simple; bh=iWa7QqW4Yvxrkqlb4FtRJpx7/Nl6lnUSRyeO44i/hJw=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=hIR8ztW9WLhG7nRm1kAc0ckHW4GUnPyycDKtZVITLp3RKTmwoeEy/0angc49p2J5rRtmKEP3FdoP/J3yYjg4xgu/A6EISukyVzT6P32u6hX91JZ6Nb0sfPIgp3l6bFN4qJAu6vJcgPpSM8Er2wPJcYYFN8PNzEP7xjHpnoAJl3I= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 15C913858D26 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1760366259; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Fu4p860dmO8tY/MNBOemEMJ7L2N2UNpiyhgW+MxdYu8=; b=RJpB8GMUmlh7jq5G/e37t8tuuPnEQ/5ErqihRMzMwWN5DqZ4r2hEjFU8Ul3ZO0K5J6ls6J K9Jp7UxwcW4MSeuyolWdWOh4x2pgi3MYJhChsXJqz+ClzKpu6VQBYIKuT/kHsZEmk5J1X2 8aseGW5+DC+a2tev1OP3OSpZ/6XoN3A= Received: from mail-wr1-f71.google.com (mail-wr1-f71.google.com [209.85.221.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-84-iryzwGF7MJeVXwvUOURiGA-1; Mon, 13 Oct 2025 10:37:37 -0400 X-MC-Unique: iryzwGF7MJeVXwvUOURiGA-1 X-Mimecast-MFC-AGG-ID: iryzwGF7MJeVXwvUOURiGA_1760366257 Received: by mail-wr1-f71.google.com with SMTP id ffacd0b85a97d-3f3c118cbb3so4284402f8f.3 for ; Mon, 13 Oct 2025 07:37:37 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1760366256; x=1760971056; h=content-transfer-encoding:mime-version:message-id:date:references :in-reply-to:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=q9tR7eBfOjyZgAXiwFugmfRuUS8/hMYmnidV6n5XypY=; b=gx94xBZf7Zpuxa4R5pNklYQ1AKJxRj9nr4r0hj7kjPqKaWxrwGdE1sM1mCRkIngQk2 bR2/RDUKL9AUBF9pg3RVn+GeXiWaRF+QrEN5dBVJkhbrR+1ddMdyb7A34dVHp9BRkHHs aUBfeEdKmdV7YPO37UJCippuo5ttjR9EGVHcYx269pJUf7ndVve10KqJHmnRlOQwzzH1 U4EEr04eszMGD3X26Pr8iAmp6BAPcKFZnMWMldxIOtLXVCbKt3IXNsy/+KEQ/tb1VRzj r/9PRwz6NViXIPGruBrXVjhBoUtoiVBIo0icBnivn0jmrrKK9T+4VsmL1BoTDPSO1EEE Faww== X-Gm-Message-State: AOJu0Yy+vMJAxUarz2gfSOWy4JKeD4gPogrzTD2GbHZW0aC3UOApoFRI M1mkdW23OL7DGhWYf9TmyiyYeMAEnmP4AJhW32OOHx4F6zuhyb+LLWaBnsIPngTrSR/gITBLXZe ECl9QJ7dh1UVfTl0T6JeRkMQcwDe8hh9gfN73KOdEAbH2N792zOJyY2HZcCpyjZk= X-Gm-Gg: ASbGnctGOBK7OR2yoAZ8uKwzKoXnZj9bbqyXQzN56fJBmLPnjL6JnSbgMR5lnIjQJYL K8/OzgZ6qa0EoVmT8xwJoZdcZrN79Yshfttmew5ZvsSiCP00cxBTWbqZG4DI62SqgvTM9wX9DK1 nahp9PReHQgC+0GnmjjccAOE+vdnBjDjznavMfTo18b2pg5oJvPHcHoF6mUw9RvkAcs3HcZuBiI sx8ahVo1YYI6Y/wpc7kR5n3TXQm2FciocjWulc7U5SIiU7d5pffH10/OCQaqoirqNzPSzgHLXhq PZeLELq0EIqvTOpMZXbZta7o20u1cN9f4lk= X-Received: by 2002:a05:6000:4716:b0:426:da92:d390 with SMTP id ffacd0b85a97d-426da92d4cfmr4212173f8f.10.1760366256379; Mon, 13 Oct 2025 07:37:36 -0700 (PDT) X-Google-Smtp-Source: AGHT+IG4+TPhqdS2+SpBo/T4RwtG0DlC0Cxg8W1le4a+m/KnT2GQKVUxAT2si5JraMNIRvVGmUPD/g== X-Received: by 2002:a05:6000:4716:b0:426:da92:d390 with SMTP id ffacd0b85a97d-426da92d4cfmr4212149f8f.10.1760366255800; Mon, 13 Oct 2025 07:37:35 -0700 (PDT) Received: from localhost ([31.111.84.207]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-46fab3d92c0sm129471615e9.3.2025.10.13.07.37.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 13 Oct 2025 07:37:35 -0700 (PDT) From: Andrew Burgess To: Lancelot SIX Cc: gdb-patches@sourceware.org, Tom Tromey Subject: Re: [PATCHv2 2/3] gdb: make structured core file mappings processing global In-Reply-To: References: Date: Mon, 13 Oct 2025 15:37:34 +0100 Message-ID: <87jz0yc1g1.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: lFoNoOCUhe8uYXF48wuXo3H5VooxVaHkii-rqnb4okI_1760366257 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org Lancelot SIX writes: > Hi Andrew, > > I think there is an issue with this patch. When running with this and > address sanitizer, I=E2=80=AFend-up with use-after-free errors. > > See below for what I think is the issue. > > On Tue, Sep 23, 2025 at 02:44:07PM +0100, Andrew Burgess wrote: >> In corelow.c, within core_target::build_file_mappings, we have code >> that wraps around a call to gdbarch_read_core_file_mappings and >> provides more structure to the results. >>=20 >> Specifically, gdbarch_read_core_file_mappings calls a callback once >> for every region of every mapped file. The wrapper code groups all of >> the mappings for one file into an instance of 'struct mapped_file', >> this allows all of the mapped regions to be associated with the >> build-id and filename of a file. >>=20 >> In the next commit I plan to make this information available via the >> Python API, and so I need to allow access to this structured wrapping >> outside of corelow.c. >>=20 >> This commit renames 'struct mapped_file' to 'struct core_mapped_file' >> and moves the struct into gdbcore.h. Then a new global function >> gdb_read_core_file_mappings is created into which I move the code to >> build the structured data. >>=20 >> Then corelow.c is updated to call gdb_read_core_file_mappings. >>=20 >> This commit does not extend the Python API, that is for the next >> commit. >>=20 >> There should be no user visible changes after this commit. >>=20 >> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=3D32844 >>=20 >> Approved-By: Tom Tromey >> --- >> gdb/corelow.c | 218 +++++++++++++++++++++++++++----------------------- >> gdb/gdbcore.h | 43 ++++++++++ >> 2 files changed, 160 insertions(+), 101 deletions(-) >>=20 >> diff --git a/gdb/corelow.c b/gdb/corelow.c >> index 2f202dc1fbf..ee97e29556e 100644 >> --- a/gdb/corelow.c >> +++ b/gdb/corelow.c >> @@ -366,108 +366,27 @@ core_target::core_target () >> void >> core_target::build_file_mappings () >> { >> - /* Type holding information about a single file mapped into the infer= ior >> - at the point when the core file was created. Associates a build-i= d >> - with the list of regions the file is mapped into. */ >> - struct mapped_file >> - { >> - /* Type for a region of a file that was mapped into the inferior wh= en >> - the core file was generated. */ >> - struct region >> - { >> - /* Constructor. See member variables for argument descriptions.= */ >> - region (CORE_ADDR start_, CORE_ADDR end_, CORE_ADDR file_ofs_) >> -=09: start (start_), >> -=09 end (end_), >> -=09 file_ofs (file_ofs_) >> - { /* Nothing. */ } >> - >> - /* The inferior address for the start of the mapped region. */ >> - CORE_ADDR start; >> - >> - /* The inferior address immediately after the mapped region. */ >> - CORE_ADDR end; >> - >> - /* The offset within the mapped file for this content. */ >> - CORE_ADDR file_ofs; >> - }; >> - >> - /* If not nullptr, then this is the build-id associated with this >> - file. */ >> - const bfd_build_id *build_id =3D nullptr; >> - >> - /* If true then we have seen multiple different build-ids associate= d >> - with the same filename. The build_id field will have been set b= ack >> - to nullptr, and we should not set build_id in future. */ >> - bool ignore_build_id_p =3D false; >> - >> - /* All the mapped regions of this file. */ >> - std::vector regions; >> - }; >> - >> gdb::unordered_map bfd_map; >> gdb::unordered_set unavailable_paths; >> =20 >> /* All files mapped into the core file. The key is the filename. */ >> - gdb::unordered_map mapped_files; >> + std::vector mapped_files >> + =3D gdb_read_core_file_mappings (m_core_gdbarch, >> +=09=09=09=09 current_program_space->core_bfd ()); >> =20 >> - /* See linux_read_core_file_mappings() in linux-tdep.c for an example >> - read_core_file_mappings method. */ >> - gdbarch_read_core_file_mappings (m_core_gdbarch, >> -=09=09=09=09 current_program_space->core_bfd (), >> - >> - /* After determining the number of mappings, read_core_file_mapping= s >> - will invoke this lambda. */ >> - [&] (ULONGEST) >> - { >> - }, >> - >> - /* read_core_file_mappings will invoke this lambda for each mapping >> - that it finds. */ >> - [&] (int num, ULONGEST start, ULONGEST end, ULONGEST file_ofs, >> -=09 const char *filename, const bfd_build_id *build_id) >> - { >> -=09/* Architecture-specific read_core_mapping methods are expected to >> -=09 weed out non-file-backed mappings. */ >> -=09gdb_assert (filename !=3D nullptr); >> - >> -=09/* Add this mapped region to the data for FILENAME. */ >> -=09mapped_file &file_data =3D mapped_files[filename]; >> -=09file_data.regions.emplace_back (start, end, file_ofs); >> -=09if (build_id !=3D nullptr && !file_data.ignore_build_id_p) >> -=09 { >> -=09 if (file_data.build_id =3D=3D nullptr) >> -=09 file_data.build_id =3D build_id; >> -=09 else if (!build_id_equal (build_id, file_data.build_id)) >> -=09 { >> -=09=09warning (_("Multiple build-ids found for %ps"), >> -=09=09=09 styled_string (file_name_style.style (), filename)); >> -=09=09file_data.build_id =3D nullptr; >> -=09=09file_data.ignore_build_id_p =3D true; >> -=09 } >> -=09 } >> - }); >> - >> - /* Get the build-id of the core file. */ >> - const bfd_build_id *core_build_id >> - =3D build_id_bfd_get (current_program_space->core_bfd ()); >> - >> - for (const auto &[filename, file_data] : mapped_files) >> + for (const core_mapped_file &file_data : mapped_files) >> { >> - /* If this mapped file has the same build-id as was discovered fo= r >> -=09 the core-file itself, then we assume this is the main >> -=09 executable. Record the filename as we can use this later. */ >> - if (file_data.build_id !=3D nullptr >> -=09 && m_expected_exec_filename.empty () >> -=09 && build_id_equal (file_data.build_id, core_build_id)) >> -=09m_expected_exec_filename =3D filename; >> + /* If this mapped file is marked as the main executable then reco= rd >> +=09 the filename as we can use this later. */ >> + if (file_data.is_main_exec && m_expected_exec_filename.empty ()) >> +=09m_expected_exec_filename =3D file_data.filename; >> =20 >> /* Use exec_file_find() to do sysroot expansion. It'll >> =09 also strip the potential sysroot "target:" prefix. If >> =09 there is no sysroot, an equivalent (possibly more >> =09 canonical) pathname will be provided. */ >> gdb::unique_xmalloc_ptr expanded_fname >> -=09=3D exec_file_find (filename.c_str (), nullptr); >> +=09=3D exec_file_find (file_data.filename.c_str (), nullptr); >> =20 >> bool build_id_mismatch =3D false; >> if (expanded_fname !=3D nullptr && file_data.build_id !=3D nullpt= r) >> @@ -509,7 +428,7 @@ core_target::build_file_mappings () >> =09{ >> =09 abfd =3D find_objfile_by_build_id (current_program_space, >> =09=09=09=09=09 file_data.build_id, >> -=09=09=09=09=09 filename.c_str ()); >> +=09=09=09=09=09 file_data.filename.c_str ()); >> =20 >> =09 if (abfd !=3D nullptr) >> =09 { >> @@ -527,7 +446,7 @@ core_target::build_file_mappings () >> =09} >> =20 >> std::vector ranges; >> - for (const mapped_file::region ®ion : file_data.regions) >> + for (const core_mapped_file::region ®ion : file_data.regions) >> =09ranges.emplace_back (region.start, region.end - region.start); >> =20 >> if (expanded_fname =3D=3D nullptr >> @@ -545,7 +464,7 @@ core_target::build_file_mappings () >> =09 bool content_is_in_core_file_p =3D true; >> =20 >> =09 /* Record all regions for this file as unavailable. */ >> -=09 for (const mapped_file::region ®ion : file_data.regions) >> +=09 for (const core_mapped_file::region ®ion : file_data.regions) >> =09 { >> =09 /* Check to see if the region is available within the core >> =09=09 file. */ >> @@ -577,33 +496,33 @@ core_target::build_file_mappings () >> =09 if (build_id_mismatch) >> =09 { >> =09 if (expanded_fname =3D=3D nullptr >> -=09=09 || filename =3D=3D expanded_fname.get ()) >> +=09=09 || file_data.filename =3D=3D expanded_fname.get ()) >> =09=09warning (_("File %ps doesn't match build-id from core-file " >> =09=09=09 "during file-backed mapping processing"), >> =09=09=09 styled_string (file_name_style.style (), >> -=09=09=09=09=09filename.c_str ())); >> +=09=09=09=09=09file_data.filename.c_str ())); >> =09 else >> =09=09warning (_("File %ps which was expanded to %ps, doesn't match " >> =09=09=09 "build-id from core-file during file-backed " >> =09=09=09 "mapping processing"), >> =09=09=09 styled_string (file_name_style.style (), >> -=09=09=09=09=09filename.c_str ()), >> +=09=09=09=09=09file_data.filename.c_str ()), >> =09=09=09 styled_string (file_name_style.style (), >> =09=09=09=09=09expanded_fname.get ())); >> =09 } >> =09 else if (!content_is_in_core_file_p) >> =09 { >> =09 if (expanded_fname =3D=3D nullptr >> -=09=09 || filename =3D=3D expanded_fname.get ()) >> +=09=09 || file_data.filename =3D=3D expanded_fname.get ()) >> =09=09warning (_("Can't open file %ps during file-backed mapping " >> =09=09=09 "note processing"), >> =09=09=09 styled_string (file_name_style.style (), >> -=09=09=09=09=09filename.c_str ())); >> +=09=09=09=09=09file_data.filename.c_str ())); >> =09 else >> =09=09warning (_("Can't open file %ps which was expanded to %ps " >> =09=09=09 "during file-backed mapping note processing"), >> =09=09=09 styled_string (file_name_style.style (), >> -=09=09=09=09=09filename.c_str ()), >> +=09=09=09=09=09file_data.filename.c_str ()), >> =09=09=09 styled_string (file_name_style.style (), >> =09=09=09=09=09expanded_fname.get ())); >> =09 } >> @@ -617,7 +536,7 @@ core_target::build_file_mappings () >> =09=09=09=09 abfd.get ()); >> =20 >> =09 /* Create sections for each mapped region. */ >> -=09 for (const mapped_file::region ®ion : file_data.regions) >> +=09 for (const core_mapped_file::region ®ion : file_data.regions) >> =09 { >> =09 /* Make new BFD section. All sections have the same name, >> =09=09 which is permitted by bfd_make_section_anyway(). */ >> @@ -653,7 +572,7 @@ core_target::build_file_mappings () >> =09 soname =3D gdb_bfd_read_elf_soname (actual_filename); >> =09 } >> =20 >> -=09 m_mapped_file_info.add (soname.get (), filename.c_str (), >> +=09 m_mapped_file_info.add (soname.get (), file_data.filename.c_str ()= , >> =09=09=09=09 actual_filename, std::move (ranges), >> =09=09=09=09 file_data.build_id); >> =09} >> @@ -2163,6 +2082,103 @@ mapped_file_info::lookup (const char *filename, >> =20 >> /* See gdbcore.h. */ >> =20 >> +std::vector >> +gdb_read_core_file_mappings (struct gdbarch *gdbarch, struct bfd *cbfd) >> +{ >> + std::vector results; >> + >> + /* A map entry used while building RESULTS. */ >> + struct map_entry >> + { >> + explicit map_entry (core_mapped_file *ptr) >> + : file_data (ptr) >> + { /* Nothing. */ } >> + >> + /* Points to an entry in RESULTS, this allows entries to be quickly >> + looked up and updated as new mappings are read. */ >> + core_mapped_file *file_data =3D nullptr; >> + >> + /* If true then we have seen multiple different build-ids associate= d >> + with the filename of FILE_DATA. The FILE_DATA->build_id field w= ill >> + have been set to nullptr, and we should not set FILE_DATA->build= _id >> + in future. */ >> + bool ignore_build_id_p =3D false; >> + }; >> + >> + /* All files mapped into the core file. The key is the filename. */ >> + gdb::unordered_map mapped_files; >> + >> + /* Get the build-id of the core file. At least on Linux, this will b= e >> + the build-id for the main executable. If other targets add the >> + gdbarch_read_core_file_mappings method, then it might turn out tha= t >> + this logic is no longer true, in which case this might need to mov= e >> + into the gdbarch_read_core_file_mappings method. */ >> + const bfd_build_id *core_build_id =3D build_id_bfd_get (cbfd); >> + >> + /* See linux_read_core_file_mappings() in linux-tdep.c for an example >> + read_core_file_mappings method. */ >> + gdbarch_read_core_file_mappings (gdbarch, cbfd, >> + /* After determining the number of mappings, read_core_file_mapping= s >> + will invoke this lambda. */ >> + [&] (ULONGEST) >> + { >> + }, >> + >> + /* read_core_file_mappings will invoke this lambda for each mapping >> + that it finds. */ >> + [&] (int num, ULONGEST start, ULONGEST end, ULONGEST file_ofs, >> +=09 const char *filename, const bfd_build_id *build_id) >> + { >> +=09/* Architecture-specific read_core_mapping methods are expected to >> +=09 weed out non-file-backed mappings. */ >> +=09gdb_assert (filename !=3D nullptr); >> + >> +=09/* Add this mapped region to the data for FILENAME. */ >> +=09auto iter =3D mapped_files.find (filename); >> +=09if (iter =3D=3D mapped_files.end ()) >> +=09 { >> +=09 /* Create entry in results list. */ >> +=09 results.emplace_back (); >> + >> +=09 /* The entry to be added to the lookup map. */ >> +=09 map_entry entry (&results.back ()); > > This bit is (I think) invalid. You are taking the address of an element > in the vector to place it in the map_entry cache (mapped_files). > However, taking the address of the element of a vector in this case is > wrong. As you grow the vector (results.emplace_back), the backing > storage might be grown, and as a consequence values moved and existing > pointers/references/iterators are invalidated. > > Using a list could be a drop-in replacement, or storing the index in the > vector rather than the element address would fix the issue. > > I can submit a patch doing either of those, depending on what you > prefer. > Thanks for tracking this down. If you have a patch or want to create the patch, then feel free to go with whatever approach you feel is best. But I don't want to dump my mess on you. I'm happy to fix this up. Just let me know. Thanks, Andrew