From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id EOnNEELRUWgibBIAWB0awg (envelope-from ) for ; Tue, 17 Jun 2025 16:34:10 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1750192450; bh=UFCxuvmQ4GJqxjnPOzmZ2a1py0b8UBjBNNZhOW2ZH4A=; h=Date:Subject:To:References:From:In-Reply-To:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=v/OeUOhpiuYSCYlN96KdX0gVk4WWKB4JViIbClE7OwuyMNwEguVNXNY57rOc927WP ojDo5Imuxxvi27iPpn0S+pqcPfXZBW4PR8mWctgjiriQXEPmI0/U1o56hqSj7U1csp tceG/1TGs7tsrYNDumSWFAcbLFC9JE/GsSdeZTos= Received: by simark.ca (Postfix, from userid 112) id 40C2D1E11C; Tue, 17 Jun 2025 16:34:10 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-9.1 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_VALIDITY_CERTIFIED,RCVD_IN_VALIDITY_RPBL, RCVD_IN_VALIDITY_SAFE autolearn=unavailable autolearn_force=no version=4.0.1 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=simark.ca header.i=@simark.ca header.a=rsa-sha256 header.s=mail header.b=le0RkrUo; dkim=pass (1024-bit key) header.d=simark.ca header.i=@simark.ca header.a=rsa-sha256 header.s=mail header.b=K26gJs85; dkim-atps=neutral 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 CDD0A1E0C2 for ; Tue, 17 Jun 2025 16:34:09 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 7A20B38693FA for ; Tue, 17 Jun 2025 20:34:09 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 7A20B38693FA Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=simark.ca header.i=@simark.ca header.a=rsa-sha256 header.s=mail header.b=le0RkrUo; dkim=pass (1024-bit key) header.d=simark.ca header.i=@simark.ca header.a=rsa-sha256 header.s=mail header.b=K26gJs85 Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id 076DC3868D08 for ; Tue, 17 Jun 2025 20:33:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 076DC3868D08 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark.ca ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 076DC3868D08 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=158.69.221.121 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1750192411; cv=none; b=n3QtBsf2CjJQVfkyKy6eCem2RKWYQ3EamXTihMKnQhurbO8H8RFMI44jCeP7NczIXkE31aC1AqSKDXMLcoZYjG52vs0H4NwjgSab3pzYiKLLbSL3maEr/K8ugur7ILdwAitEtjbk3IHpiTXtBZK7kdNE9iIz4HE0PBm0Hd4ardI= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1750192411; c=relaxed/simple; bh=UFCxuvmQ4GJqxjnPOzmZ2a1py0b8UBjBNNZhOW2ZH4A=; h=DKIM-Signature:DKIM-Signature:Message-ID:Date:MIME-Version: Subject:To:From; b=XAzxR8tha7wQAc92oaANLthLBg5sjCbgd6aryvzKsswDlu9nfA+Eg+vTenthA8OsGmnpoxbe1LyjFpGuoBBdfVXcnGMmksqoOj1GvpJ1wsO2bpm1noCUhP1nuGmWuWO2IVneg9unX+5NuR7G0qwAVPJpjkJ6StytKDz+h152UZo= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 076DC3868D08 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1750192410; bh=UFCxuvmQ4GJqxjnPOzmZ2a1py0b8UBjBNNZhOW2ZH4A=; h=Date:Subject:To:References:From:In-Reply-To:From; b=le0RkrUoPiIQE6F0bAugAG+r/zeLGFCgsWmh9SbUhRYby5Y7CMPQzFyb5BJlCuKYX Y8cthmMHKs2jkGcEbV3LzTCPiALFi/H1MIINWyI+lhQinhHXZaaF2RskRlEhpaNrwX l6IIUZic4ThAp0yW9P+hv+6QPFHjhvAB3hMFd1t0= Received: by simark.ca (Postfix, from userid 112) id A62E71E11F; Tue, 17 Jun 2025 16:33:30 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1750192408; bh=UFCxuvmQ4GJqxjnPOzmZ2a1py0b8UBjBNNZhOW2ZH4A=; h=Date:Subject:To:References:From:In-Reply-To:From; b=K26gJs85us03UzGBVAlq+Nmqs2QZQCsYlVmMVmOHcBFWpk4ssT0U3pZvERBoxAHdn ICv7gWM2q8jE+ky+6UCzU6dOMVZsVcJ5zHZDXxS+Zql0wwFceHdnKDiSFAk462d0dI ZXzvpngTfxFbNXIgvkVssyWi4BmkaBYFGI19P5HE= Received: from [172.16.0.192] (192-222-132-26.qc.cable.ebox.net [192.222.132.26]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 6463D1E0C2; Tue, 17 Jun 2025 16:33:28 -0400 (EDT) Message-ID: <1085bd17-956a-449a-bb9d-eb89a90ac7fc@simark.ca> Date: Tue, 17 Jun 2025 16:33:27 -0400 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3 5/6] gdb/progspace: add solib_ops pointer in program_space To: Guinevere Larsen , Simon Marchi , gdb-patches@sourceware.org References: <20250616193443.16703-1-simon.marchi@efficios.com> <20250616193443.16703-5-simon.marchi@efficios.com> Content-Language: fr From: Simon Marchi In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit 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 On 6/17/25 3:45 PM, Guinevere Larsen wrote: > On 6/16/25 4:33 PM, Simon Marchi wrote: >> New in v3: >> >> - add some ops nullptr checks in print_solib_list_table and >> info_linker_namespace_command >> >> The subsequent C++ification patch in this series will allocate one >> instance of solib_ops per program space. That instance will be held in >> struct program_space. As a small step towards this, add an `solib_ops >> *` field to `struct program_space`. This field represents the solib_ops >> currently used to manage the solibs in that program space. Initialize >> it with the result of `gdbarch_so_ops` in `post_create_inferior`, and >> use it whenever we need to do some solib stuff, rather than using >> `gdbarch_so_ops` directly. >> >> The difficulty here is knowing when exactly to set and unset the solib >> ops. What I have here passes the testsuite on Linux, but with more >> testing we will probably discover more spots where it's needed. >> >> The C++ification patch will turn this field into a unique pointer. >> >> With this patch, the message we get when running "info >> linker-namespaces" becomes always the same, so update the test in >> gdb.base/dlmopen-ns-ids.exp. >> >> Change-Id: Ide8ddc57328895720fcd645d46dc34491f84c656 > > Thanks for working on this series! I have one very minor nit in this patch, but with that fixes, you can feel free to add my review tag > > Reviewed-By: Guinevere Larsen > > Feel free to add it to patches 1-4 as well, as I looked over them and they all LGTM. I still don't feel qualified to offer it for patch 6, though. Thank you! Given your review, I plan to push this series probably tomorrow. You can start rebasing and adjusting your series on top, and then I can hopefully review it this week (note that I'm off this Friday, so it would have to be Thursday max). >> @@ -1021,16 +1024,21 @@ print_solib_list_table (std::vector solib_list, >> gdbarch *gdbarch = current_inferior ()->arch (); >> /* "0x", a little whitespace, and two hex digits per byte of pointers. */ >> int addr_width = 4 + (gdbarch_ptr_bit (gdbarch) / 4); >> - const solib_ops *ops = gdbarch_so_ops (gdbarch); >> + const solib_ops *ops = current_program_space->solib_ops (); >> struct ui_out *uiout = current_uiout; >> bool so_missing_debug_info = false; >> + if (ops == nullptr) >> + return; >> + >> /* There are 3 conditions for this command to print solib namespaces, >> first PRINT_NAMESPACE has to be true, second the solib_ops has to >> support multiple namespaces, and third there must be more than one >> active namespace. Fold all these into the PRINT_NAMESPACE condition. */ >> - print_namespace = print_namespace && ops->num_active_namespaces != nullptr >> - && ops->num_active_namespaces () > 1; >> + print_namespace = (print_namespace >> + && ops != nullptr > > This check is redundant with the previous early exit. The only reason > I bring this up is because the comment brings up "3 conditions" but > there are 4 checks in this expression. The comment isn't wrong, but it > does the code confusing until you figure out that checking if > solib_ops is present isn't one of the conditions. You're right, it's redundant, I'll fix it. Note to self: returning early means that the table will not be output at all. Check if returning early is a problem for e.g. MI, when the inferior is not running. Simon