[binutils-gdb] gdb/varobj: Fix varobj_invalidate_iter

[binutils-gdb] gdb/varobj: Fix varobj_invalidate_iter

[Lancelot SIX]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=6c96b937df01064526470694da9b6d24761e57fc

commit 6c96b937df01064526470694da9b6d24761e57fc
Author: Lancelot SIX <lancelot.six@amd.com>
Date:   Fri Jul 8 11:37:19 2022 +0100

    gdb/varobj: Fix varobj_invalidate_iter
    
    The varobj_invalidate function is 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

Diff:
---
 gdb/testsuite/gdb.mi/mi-var-invalidate-shlib.c   |  2 ++
 gdb/testsuite/gdb.mi/mi-var-invalidate-shlib.exp | 44 ++++++++++++++++++++++++
 gdb/varobj.c                                     | 20 +++++++----
 3 files changed, 59 insertions(+), 7 deletions(-)

diff --git a/gdb/testsuite/gdb.mi/mi-var-invalidate-shlib.c b/gdb/testsuite/gdb.mi/mi-var-invalidate-shlib.c
index f9237c3b853..2cb70104d80 100644
--- a/gdb/testsuite/gdb.mi/mi-var-invalidate-shlib.c
+++ b/gdb/testsuite/gdb.mi/mi-var-invalidate-shlib.c
@@ -17,6 +17,8 @@
 
 #include <dlfcn.h>
 
+int global_var = 5;
+
 static void
 no_varobj_in_scope (void)
 {
diff --git a/gdb/testsuite/gdb.mi/mi-var-invalidate-shlib.exp b/gdb/testsuite/gdb.mi/mi-var-invalidate-shlib.exp
index b405c832f29..87d1d4a6a9d 100644
--- a/gdb/testsuite/gdb.mi/mi-var-invalidate-shlib.exp
+++ b/gdb/testsuite/gdb.mi/mi-var-invalidate-shlib.exp
@@ -61,6 +61,7 @@ proc do_test { separate_debuginfo } {
 
 	mi_runto foo -pending
 
+	mi_create_varobj global_var global_var "create global global_var"
 	mi_create_varobj global_shlib_var global_shlib_var "create global gloal_shlib_var"
 	mi_create_floating_varobj floating_local local_var "create floating local_var"
 
@@ -79,6 +80,10 @@ proc do_test { separate_debuginfo } {
 	    "\\^done,changelist=\\\[{name=\"floating_local\",in_scope=\"false\",type_changed=\"false\",has_more=\"0\"}\\\]" \
 	    "floating_local still valid but not in scope"
 
+	mi_gdb_test "-var-update global_var" \
+	    "\\^done,changelist=\\\[\\\]" \
+	    "global_var still valid"
+
 	# The varobj can be re-evaluated if the expression is valid in the current
 	# frame.
 	mi_continue_to "floating_varobj_in_scope"
@@ -86,6 +91,45 @@ proc do_test { separate_debuginfo } {
 	    "\\^done,changelist=\\\[{name=\"floating_local\",in_scope=\"true\",type_changed=\"true\",new_type=\"int\",new_num_children=\"0\",has_more=\"0\"}\\\]" \
 	    "floating_local in scope with new type and value"
     }
+
+    # Reload the entire process
+    with_test_prefix "restart process" {
+	mi_delete_breakpoints
+	mi_gdb_load ${::binfile}
+	mi_runto_main
+    }
+
+    with_test_prefix "in new process" {
+	# When reloading the symbol file, only the var for the global in the main
+	# executable is re-created.
+	mi_gdb_test "-var-update global_var" \
+	    "\\^done,changelist=\\\[{name=\"global_var\",in_scope=\"true\",type_changed=\"false\",has_more=\"0\"}\\\]" \
+	    "global_var recreated"
+	mi_gdb_test "-var-update global_shlib_var" \
+	    "\\^done,changelist=\\\[{name=\"global_shlib_var\",in_scope=\"invalid\",has_more=\"0\"}\\\]" \
+	    "global_shlib_var invalid"
+
+	# Floating varobj should still be valid, but out of scope at the moment.
+	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=\"true\",new_type=\"struct bar\",new_num_children=\"2\",has_more=\"0\"}\\\]" \
+	    "floating_local still valid and in scope"
+
+	# The var for the global in the shlib stays invalid even after reloading the shlib.
+	mi_gdb_test "-var-update global_shlib_var" \
+	    "\\^done,changelist=\\\[{name=\"global_shlib_var\",in_scope=\"invalid\",has_more=\"0\"}\\\]" \
+	    "global_shlib_var invalid after shlib loaded"
+    }
 }
 
 foreach_with_prefix separate_debuginfo {0 1} {
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 2b6dfff6888..e558794617a 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -2359,22 +2359,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;