From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by sourceware.org (Postfix) with ESMTPS id 4D127385AC22 for ; Tue, 9 Aug 2022 10:55:57 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 4D127385AC22 Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 824CA34CC8; Tue, 9 Aug 2022 10:55:56 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 64F3913AA1; Tue, 9 Aug 2022 10:55:56 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id +Mf+Fjw98mIFYQAAMHmgww (envelope-from ); Tue, 09 Aug 2022 10:55:56 +0000 Message-ID: Date: Tue, 9 Aug 2022 12:55:56 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 Subject: Re: [PATCH 3/3] gdb/varobj: Only re-evaluate invalid globals during re_set Content-Language: en-US To: Lancelot SIX , gdb-patches@sourceware.org Cc: lsix@lancelotsix.com References: <20220804130231.2126565-1-lancelot.six@amd.com> <20220804130231.2126565-4-lancelot.six@amd.com> From: Tom de Vries In-Reply-To: <20220804130231.2126565-4-lancelot.six@amd.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, 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 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, 09 Aug 2022 10:55:58 -0000 On 8/4/22 15:02, Lancelot SIX wrote: > When doing varobj_re_set, we currently try to recreate floating varobj. > This was introduced by 4e969b4f0128 "Re-evaluate floating varobj as part > of varobj_invalidate" to deal with use a after free issue. However > since bc20e562ec0 "Fix use after free in varobj" we now ensure that we > never have dangling pointers so this all recreation is not strictly > needed anymore for floating varobjs. > > This commit proposes to remove this recreation process for floating > varobjs. > > Tested on x86_64-linux. LGTM. Thanks, - Tom > --- > gdb/testsuite/gdb.mi/mi-var-invalidate.exp | 4 ++-- > gdb/varobj.c | 18 +++++------------- > 2 files changed, 7 insertions(+), 15 deletions(-) > > diff --git a/gdb/testsuite/gdb.mi/mi-var-invalidate.exp b/gdb/testsuite/gdb.mi/mi-var-invalidate.exp > index 1b2c68df18e..348515671c1 100644 > --- a/gdb/testsuite/gdb.mi/mi-var-invalidate.exp > +++ b/gdb/testsuite/gdb.mi/mi-var-invalidate.exp > @@ -75,8 +75,8 @@ mi_runto_main > # Change format of floating variable immediately after reload reveals a > # bug where gdb still uses a free'd pointer. > mi_gdb_test "-var-set-format float_simple hexadecimal" \ > - "\\^done,format=\"hexadecimal\",value=\"\\\[-1\\\]\"" \ > - "set format variable float_simple" > + "\\^done,format=\"hexadecimal\",value=\"\\\[3\\\]\"" \ > + "set format variable float_simple" > > # Check local variable is "invalid". > mi_gdb_test "-var-update linteger" \ > diff --git a/gdb/varobj.c b/gdb/varobj.c > index 55a7bd97f43..d3df608c55f 100644 > --- a/gdb/varobj.c > +++ b/gdb/varobj.c > @@ -2359,29 +2359,21 @@ all_root_varobjs (gdb::function_view func) > static void > varobj_re_set_iter (struct varobj *var) > { > - /* Invalidated globals and floating var must be re-evaluated. */ > - if (var->root->global || var->root->floating) > + /* Invalidated global varobjs must be re-evaluated. */ > + if (!var->root->is_valid && var->root->global) > { > struct varobj *tmp_var; > > /* Try to create a varobj with same expression. If we succeed > - replace the old varobj, otherwise invalidate it. */ > + and have a global replace the old varobj. */ > tmp_var = varobj_create (nullptr, var->name.c_str (), (CORE_ADDR) 0, > - var->root->floating > - ? USE_SELECTED_FRAME : USE_CURRENT_FRAME); > - if (tmp_var != nullptr) > + USE_CURRENT_FRAME); > + if (tmp_var != nullptr && tmp_var->root->global) > { > - gdb_assert (var->root->floating == tmp_var->root->floating); > tmp_var->obj_name = var->obj_name; > varobj_delete (var, 0); > install_variable (tmp_var); > } > - else if (var->root->global) > - { > - /* Only invalidate globals as floating vars might still be valid in > - some other frame. */ > - var->root->is_valid = false; > - } > } > } >