Re: [PATCH 2/2] gdb/testsuite: Accept PIE/noPIE programs in gdb.mi/mi-var-invalidate-shlib.exp

[Tom de Vries via Gdb-patches <gdb-patches@sourceware.org>]

[-- Attachment #1: Type: text/plain, Size: 1752 bytes --]

On 8/3/22 11:35, Six, Lancelot wrote:
> [AMD Official Use Only - General]
> 
>> Hi,
>>
>> thanks for the analysis.
>>
>> After looking into this a bit, I wonder if the root cause is really
>> looking at the values in the old process.
>>
>> Maybe we should either:
>> - not try to sample the values when recreating, or
>> - move the recreation to a later point when we have access to the memory
>>     of the new process.
>>
>> At least we would have consistency between PIE and no-PIE with either
>> solution.
>>
>> Thanks,
>> - Tom
> 
> Hi,
> 
> I do agree that it is strange to look for a value for the new objfile in the old process.  I wonder if there are legitimate usecases where one would want to still use "file ..." to load new symbols but keep the same process running while keeping a global varobj around.  If we delay the recreation of the varobj, we would break such usecase.
> 

You're right,  apparently that's a valid use case.

> One way could be to be just invalidate the varobj when the objfile it relies on goes away and give the user a "-var-reinflate" command (or "-var-update -reinflate") so he can try to revive an invalidated varobj at his convenience.  This would break existing behavior but could avoid.

I've now reached the conclusion that the bug is merely that for 
recreating the varobjs we don't wait until relocation has happened, as 
we do for breakpoints.

This WIP patch delays recreation of the varobj to the point that the 
file command has relocated the PIE exec.

This makes behaviour equal between PIE and no-PIE (meaning we see the 
same two FAILs now for gdb.mi/mi-var-invalidate-shlib.exp).

I'm not sure about the split between invalidate and re_set, I'm not 
familiar with this code.

WDYT?

Thanks,
- Tom

[-- Attachment #2: tmp.patch --]
[-- Type: text/x-patch, Size: 2226 bytes --]

diff --git a/gdb/symfile.c b/gdb/symfile.c
index aea8c762ee6..d0a32d13231 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1656,6 +1656,9 @@ symbol_file_command (const char *args, int from_tty)
 
       /* Now it's safe to re-add the breakpoints.  */
       breakpoint_re_set ();
+
+      /* Also, it's safe to re-add varobjs.  */
+      varobj_re_set ();
     }
 }
 
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 0683af1991e..ecc53f2643d 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -2362,6 +2362,26 @@ all_root_varobjs (gdb::function_view<void (struct varobj *var)> func)
 
 static void
 varobj_invalidate_iter (struct varobj *var)
+{
+  /* global and floating var must be re-evaluated.  */
+  if (var->root->floating || var->root->global)
+    ;
+  else /* locals must be invalidated.  */
+    var->root->is_valid = false;
+}
+
+/* Invalidate the varobjs that are tied to locals and re-create the ones that
+   are defined on globals.
+   Invalidated varobjs will be always printed in_scope="invalid".  */
+
+void
+varobj_invalidate (void)
+{
+  all_root_varobjs (varobj_invalidate_iter);
+}
+
+static void
+varobj_re_set_iter (struct varobj *var)
 {
   /* global and floating var must be re-evaluated.  */
   if (var->root->floating || var->root->global)
@@ -2387,18 +2407,12 @@ 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
-   are defined on globals.
-   Invalidated varobjs will be always printed in_scope="invalid".  */
-
 void 
-varobj_invalidate (void)
+varobj_re_set (void)
 {
-  all_root_varobjs (varobj_invalidate_iter);
+  all_root_varobjs (varobj_re_set_iter);
 }
 
 /* Ensure that no varobj keep references to OBJFILE.  */
diff --git a/gdb/varobj.h b/gdb/varobj.h
index 073da60d772..f3cce8ebfbd 100644
--- a/gdb/varobj.h
+++ b/gdb/varobj.h
@@ -316,6 +316,8 @@ extern std::vector<varobj_update_result>
 
 extern void varobj_invalidate (void);
 
+extern void varobj_re_set (void);
+
 extern bool varobj_editable_p (const struct varobj *var);
 
 extern bool varobj_floating_p (const struct varobj *var);