Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Lancelot SIX via Gdb-patches <gdb-patches@sourceware.org>
To: gdb-patches@sourceware.org
Cc: lsix@lancelotsix.com, Lancelot SIX <lancelot.six@amd.com>
Subject: [PATCH 1/3] gdb/varobj: Do not invalidate locals in varobj_invalidate_iter
Date: Thu,  4 Aug 2022 14:02:29 +0100	[thread overview]
Message-ID: <20220804130231.2126565-2-lancelot.six@amd.com> (raw)
In-Reply-To: <20220804130231.2126565-1-lancelot.six@amd.com>

The varobj_invalidate_iter function has logic to invalidate any local
varobj it can find.  However since bc20e562ec0 "gdb/varobj: Fix use
after free in varobj" all varobj containing references to an objfile are
cleared when the objfile goes out of scope.  This means that at this
point any local varobj seen by varobj_invalidate_iter either has
already been invalidated by varobj_invalidate_if_uses_objfile or only
contains valid references and there is no reason to invalidate it.

This patch proposes to remove this un-necessary invalidation and adds a
testcase which exercises a scenario where a local varobj can legitimately
survive a call to varobj_invalidate_iter.

At this point the varobj_invalidate and varobj_invalidate seem misnamed
since they deal with re-creating invalid objects and do not do
invalidation, but this will be fixed in a following patch.

Tested on x86_64-linux.
---
 .../gdb.mi/mi-var-invalidate-shlib.exp        | 40 +++++++++++++++++--
 gdb/varobj.c                                  |  2 -
 2 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/gdb/testsuite/gdb.mi/mi-var-invalidate-shlib.exp b/gdb/testsuite/gdb.mi/mi-var-invalidate-shlib.exp
index 87d1d4a6a9d..9ba5af0c028 100644
--- a/gdb/testsuite/gdb.mi/mi-var-invalidate-shlib.exp
+++ b/gdb/testsuite/gdb.mi/mi-var-invalidate-shlib.exp
@@ -40,10 +40,6 @@ if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable $opt
 }
 
 proc do_test { separate_debuginfo } {
-    if { $separate_debuginfo } {
-	gdb_gnu_strip_debug $::shlib_path
-    }
-
     if { [mi_clean_restart] } {
 	unsupported "failed to start GDB"
 	return
@@ -132,6 +128,42 @@ proc do_test { separate_debuginfo } {
     }
 }
 
+proc_with_prefix local_not_invalidated { separate_debuginfo } {
+    if { [mi_clean_restart] } {
+	unsupported "failed to start GDB"
+	return
+    }
+
+    # Start the process once and create varobjs referencing the loaded objfiles.
+    with_test_prefix "setup" {
+	mi_load_shlibs $::shlib_path
+	if { $separate_debuginfo } {
+	    mi_load_shlibs ${::shlib_path}.debug
+	}
+
+	mi_gdb_reinitialize_dir $::srcdir/$::subdir
+	mi_gdb_load $::binfile
+
+	mi_runto foo -pending
+	mi_next "next"
+	mi_create_varobj local_var local_var "create local varobj"
+    }
+
+    # A this point we are stoped in the shared library.  If we reload symbols
+    # for the main binary, symbols for the shared library remain valid.
+    # A varobj tracking variables in the scope of the shared library only
+    # should not be invalidated.
+    mi_gdb_load ${::binfile}
+    mi_gdb_test "-var-update local_var" \
+	"\\^done,changelist=\\\[\\\]" \
+	"local_var preserved"
+}
+
 foreach_with_prefix separate_debuginfo {0 1} {
+    if { $separate_debuginfo } {
+	gdb_gnu_strip_debug $::shlib_path
+    }
+
     do_test $separate_debuginfo
+    local_not_invalidated $separate_debuginfo
 }
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 0683af1991e..a142bb02e03 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -2387,8 +2387,6 @@ varobj_invalidate_iter (struct varobj *var)
 	  var->root->is_valid = false;
 	}
     }
-  else /* locals must be invalidated.  */
-    var->root->is_valid = false;
 }
 
 /* Invalidate the varobjs that are tied to locals and re-create the ones that
-- 
2.34.1


  reply	other threads:[~2022-08-04 13:03 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-04 13:02 [PATCH 0/3] Varobj re_set clanup Lancelot SIX via Gdb-patches
2022-08-04 13:02 ` Lancelot SIX via Gdb-patches [this message]
2022-08-05 15:03   ` [PATCH 1/3] gdb/varobj: Do not invalidate locals in varobj_invalidate_iter Tom de Vries via Gdb-patches
2022-08-04 13:02 ` [PATCH 2/3] gdb/varobj: Reset varobj after relocations have been computed Lancelot SIX via Gdb-patches
2022-08-09 10:55   ` Tom de Vries via Gdb-patches
2022-08-04 13:02 ` [PATCH 3/3] gdb/varobj: Only re-evaluate invalid globals during re_set Lancelot SIX via Gdb-patches
2022-08-09 10:55   ` Tom de Vries via Gdb-patches
2022-08-11 14:07     ` Lancelot SIX via Gdb-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220804130231.2126565-2-lancelot.six@amd.com \
    --to=gdb-patches@sourceware.org \
    --cc=lancelot.six@amd.com \
    --cc=lsix@lancelotsix.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox