From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTPS id AD99F3955CAF for ; Tue, 31 May 2022 14:27:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org AD99F3955CAF Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-193-8XV66m8tNrmX8AQDAw52Uw-1; Tue, 31 May 2022 10:27:40 -0400 X-MC-Unique: 8XV66m8tNrmX8AQDAw52Uw-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.rdu2.redhat.com [10.11.54.1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 08EC22809CB0; Tue, 31 May 2022 14:27:40 +0000 (UTC) Received: from [10.97.116.24] (ovpn-116-24.gru2.redhat.com [10.97.116.24]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 45E3940CF8ED; Tue, 31 May 2022 14:27:39 +0000 (UTC) Message-ID: <3afac104-40e9-cfca-81a7-c594ecab3f87@redhat.com> Date: Tue, 31 May 2022 11:27:37 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.0 Subject: Re: [PATCH] Use unique_ptr for objfiles To: Tom Tromey , gdb-patches@sourceware.org References: <20220530191421.397009-1-tom@tromey.com> From: Bruno Larsen In-Reply-To: <20220530191421.397009-1-tom@tromey.com> X-Scanned-By: MIMEDefang 2.84 on 10.11.54.1 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-14.2 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_NONE, 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 X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 31 May 2022 14:27:46 -0000 On 5/30/22 16:14, Tom Tromey wrote: > A while back, I changed objfiles to be held via a shared_ptr. The > idea at the time was that this was a step toward writing to the index > cache in the background, and this would let gdb keep a reference alive > to do so. However, since then we've rewritten the DWARF reader, and > the new index can do this without requiring a shared pointer -- in > fact there are patches pending to implement this. > > This patch switches objfile management to unique_ptr, which makes more > sense now. > This patch seems obvious enough. It seems fine for me, I'd suggest you approve your patch. > Regression tested on x86-64 Fedora 34. Also tested on x86 fedora 35 and using clang. No regressions here either. Cheers! Bruno Larsen > --- > gdb/objfiles.c | 4 +--- > gdb/objfiles.h | 2 +- > gdb/progspace.c | 6 +++--- > gdb/progspace.h | 8 ++++---- > 4 files changed, 9 insertions(+), 11 deletions(-) > > diff --git a/gdb/objfiles.c b/gdb/objfiles.c > index 80f68fda1c1..60d8aa5cb78 100644 > --- a/gdb/objfiles.c > +++ b/gdb/objfiles.c > @@ -468,9 +468,7 @@ objfile::make (bfd *bfd_, const char *name_, objfile_flags flags_, > if (parent != nullptr) > add_separate_debug_objfile (result, parent); > > - /* Using std::make_shared might be a bit nicer here, but that would > - require making the constructor public. */ > - current_program_space->add_objfile (std::shared_ptr (result), > + current_program_space->add_objfile (std::unique_ptr (result), > parent); > > /* Rebuild section map next time we need it. */ > diff --git a/gdb/objfiles.h b/gdb/objfiles.h > index 9da12ff12e0..a7098b46279 100644 > --- a/gdb/objfiles.h > +++ b/gdb/objfiles.h > @@ -409,7 +409,7 @@ struct objfile > remove it from the program space's list. In some cases, you may > need to hold a reference to an objfile that is independent of its > existence on the program space's list; for this case, the > - destructor must be public so that shared_ptr can reference > + destructor must be public so that unique_ptr can reference > it. */ > ~objfile (); > > diff --git a/gdb/progspace.c b/gdb/progspace.c > index 1ee4fe3f940..8735343fabc 100644 > --- a/gdb/progspace.c > +++ b/gdb/progspace.c > @@ -174,7 +174,7 @@ program_space::free_all_objfiles () > /* See progspace.h. */ > > void > -program_space::add_objfile (std::shared_ptr &&objfile, > +program_space::add_objfile (std::unique_ptr &&objfile, > struct objfile *before) > { > if (before == nullptr) > @@ -182,7 +182,7 @@ program_space::add_objfile (std::shared_ptr &&objfile, > else > { > auto iter = std::find_if (objfiles_list.begin (), objfiles_list.end (), > - [=] (const std::shared_ptr<::objfile> &objf) > + [=] (const std::unique_ptr<::objfile> &objf) > { > return objf.get () == before; > }); > @@ -203,7 +203,7 @@ program_space::remove_objfile (struct objfile *objfile) > reinit_frame_cache (); > > auto iter = std::find_if (objfiles_list.begin (), objfiles_list.end (), > - [=] (const std::shared_ptr<::objfile> &objf) > + [=] (const std::unique_ptr<::objfile> &objf) > { > return objf.get () == objfile; > }); > diff --git a/gdb/progspace.h b/gdb/progspace.h > index 73beb7a4710..662e569488e 100644 > --- a/gdb/progspace.h > +++ b/gdb/progspace.h > @@ -41,9 +41,9 @@ struct program_space_data; > struct address_space_data; > struct so_list; > > -typedef std::list> objfile_list; > +typedef std::list> objfile_list; > > -/* An iterator that wraps an iterator over std::shared_ptr, > +/* An iterator that wraps an iterator over std::unique_ptr, > and dereferences the returned object. This is useful for iterating > over a list of shared pointers and returning raw pointers -- which > helped avoid touching a lot of code when changing how objfiles are > @@ -234,7 +234,7 @@ struct program_space > /* Add OBJFILE to the list of objfiles, putting it just before > BEFORE. If BEFORE is nullptr, it will go at the end of the > list. */ > - void add_objfile (std::shared_ptr &&objfile, > + void add_objfile (std::unique_ptr &&objfile, > struct objfile *before); > > /* Remove OBJFILE from the list of objfiles. */ > @@ -354,7 +354,7 @@ struct program_space > struct objfile *symfile_object_file = NULL; > > /* All known objfiles are kept in a linked list. */ > - std::list> objfiles_list; > + std::list> objfiles_list; > > /* List of shared objects mapped into this space. Managed by > solib.c. */