public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Lancelot SIX <lancelot.six@amd.com>
To: <gdb-patches@sourceware.org>
Cc: <lsix@lancelotsix.com>, Lancelot SIX <lancelot.six@amd.com>
Subject: [PATCH 3/3] gdb/varobj: Fix varobj_invalidate_iter
Date: Fri, 17 Jun 2022 11:10:24 +0100	[thread overview]
Message-ID: <20220617101024.2830260-4-lancelot.six@amd.com> (raw)
In-Reply-To: <20220617101024.2830260-1-lancelot.six@amd.com>

The varobj_invalidate function is used meant to be called when
restarting a process, and check at this point if some of the previously
existing varobj can be recreated in the context of the new process.

Two kind of varobj are subject to re-creation:  global varobj (i.e.
varobj which reference a global variable), and floating varobj (i.e.
varobj which are always re-evaluated in the context of whatever is
the currently selected frame at the time of evaluation).

However, in the re-creation process, the varobj_invalidate_iter
recreates floating varobj as non-floating, due to an invalid parameter.
This patches fixes this and adds an assertion to check that if a varobj
is indeed recreated, it matches the original varobj "floating" property.

Another issue is that if at this recreation time the expression watched
by the floating varobj is not in scope, then the varobj is marked as
invalid.  If later the user selects a frame where the expression becomes
valid, the varobj remains invalid and this is wrong.  This patch also
make sure that floating varobj are not invalidated if they cannot be
evaluated.

The last important thing to note is that due to the previous patch, when
varobj_invalidate is executed (in the context of a new process), any
global var have already been invalidated (this has been done when the
objfile it referred to got invalidated).  As a consequence,
varobj_invalidate tries to recreate vars which are already marked as
invalid.  This does not entirely feels right, but I keep this behavior
for backward compatibility.

Tested on x86_64-linux
---
 .../gdb.mi/mi-var-invalidate-shlib.exp        | 11 ++++++++++
 gdb/varobj.c                                  | 20 ++++++++++++-------
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/gdb/testsuite/gdb.mi/mi-var-invalidate-shlib.exp b/gdb/testsuite/gdb.mi/mi-var-invalidate-shlib.exp
index 5219946d845..3ae76755309 100644
--- a/gdb/testsuite/gdb.mi/mi-var-invalidate-shlib.exp
+++ b/gdb/testsuite/gdb.mi/mi-var-invalidate-shlib.exp
@@ -77,4 +77,15 @@ with_test_prefix "in new process" {
     mi_gdb_test "-var-update floating_local" \
 	"\\^done,changelist=\\\[{name=\"floating_local\",in_scope=\"false\",type_changed=\"false\",has_more=\"0\"}\\\]" \
 	"floating_local still valid but not in scope"
+
+    # Continue inside foo
+    mi_gdb_test "299-break-insert -f -t foo" \
+	"&\"Function \\\\\"foo\\\\\" not defined.\\\\n\"\r\n299\\^done,[mi_make_breakpoint_pending -type breakpoint -disp del -pending foo]"
+    mi_send_resuming_command "exec-continue" "continue to foo"
+    mi_expect_stop "breakpoint-hit" foo ".*" ".*" "\[0-9\]+" { "" "disp=\"del\"" } "arrived at foo"
+
+    # Floating varobj is still valid, and now in scope.
+    mi_gdb_test "-var-update floating_local" \
+	"\\^done,changelist=\\\[{name=\"floating_local\",in_scope=\"true\",type_changed=\"false\",has_more=\"0\"}\\\]" \
+	"floating_local still valid and in scope"
 }
diff --git a/gdb/varobj.c b/gdb/varobj.c
index caaffe4ea70..a316f90c9e2 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -2360,22 +2360,28 @@ static void
 varobj_invalidate_iter (struct varobj *var)
 {
   /* global and floating var must be re-evaluated.  */
-  if (var->root->floating || var->root->valid_block == NULL)
+  if (var->root->floating || var->root->valid_block == nullptr)
     {
       struct varobj *tmp_var;
 
       /* Try to create a varobj with same expression.  If we succeed
 	 replace the old varobj, otherwise invalidate it.  */
-      tmp_var = varobj_create (NULL, var->name.c_str (), (CORE_ADDR) 0,
-			       USE_CURRENT_FRAME);
-      if (tmp_var != NULL) 
-	{ 
+      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)
+	{
+	  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
-	var->root->is_valid = false;
+      else if (!var->root->floating)
+	{
+	  /* Only invalidate globals as floating vars might still be valid in
+	     some other frame.  */
+	  var->root->is_valid = false;
+	}
     }
   else /* locals must be invalidated.  */
     var->root->is_valid = false;
-- 
2.25.1


      parent reply	other threads:[~2022-06-17 10:10 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-17 10:10 [PATCH 0/3] Fix some use-after-free errors in varobj code Lancelot SIX
2022-06-17 10:10 ` [PATCH 1/3] MI: mi_runto -pending Lancelot SIX
2022-06-17 10:10 ` [PATCH 2/3] gdb/varobj: Fix use after free in varobj Lancelot SIX
2022-06-17 16:09   ` Andrew Burgess
2022-06-17 16:38     ` Lancelot SIX
2022-06-20 15:52       ` Lancelot SIX
2022-06-30 18:43     ` Formatting/indentation of lambdas (Re: [PATCH 2/3] gdb/varobj: Fix use after free in varobj) Pedro Alves
2022-07-05 13:33       ` Lancelot SIX
2022-06-17 10:10 ` Lancelot SIX [this message]

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=20220617101024.2830260-4-lancelot.six@amd.com \
    --to=lancelot.six@amd.com \
    --cc=gdb-patches@sourceware.org \
    --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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).