[1/2] Fix varobj updation after symbol removal
Commit Message
This problem was observed while loading and unloading symbols using
add-symbol-file and remove-symbol-file. When remove-symbol-file
command is invoked, it calls clear_symtab_users that calls varobj_invalidate
to invalidate variable objects. This function invalidates the varobjs
that are tied to locals and re-create the ones that are defined on
globals. During this re-creation of globals, variable objects are
re-evaluated that can result in new value. But this change is not recorded
and because of this, -var-update for such modified variable objects
gives empty change list.
Proposed Fix:
Comments
Ping !
Thanks,
Saqlain
On 10/17/19 3:03 PM, Raza, Saqlain wrote:
> This problem was observed while loading and unloading symbols using
> add-symbol-file and remove-symbol-file. When remove-symbol-file
> command is invoked, it calls clear_symtab_users that calls varobj_invalidate
> to invalidate variable objects. This function invalidates the varobjs
> that are tied to locals and re-create the ones that are defined on
> globals. During this re-creation of globals, variable objects are
> re-evaluated that can result in new value. But this change is not recorded
> and because of this, -var-update for such modified variable objects
> gives empty change list.
>
> Proposed Fix:
> =============
> GDB has mechanism of marking varobj's updated if they are set via
> varobj_set_value operation. This allows any next -var-update to report
> this change. Same approach should be used during varobj invalidation.
> If value of newly created varobj is different from previous one, mark it
> updated so that -var-update can get this change.
>
> Variable object invalidation code is cleaned up to avoid using pointers
> whose target has been already freed.
>
> Fix Testing:
> ===========
> This fix has been regression tested on both simulator and real boards
> for x86_64 and arm-none-linux-gnueabi targets.
>
> 2019-10-17 Saqlain Raza <saqlain_raza@mentor.com>
> Taimoor Mirza <taimoor_mirza@mentor.com>
> Maciej W. Rozycki <macro@codesourcery.com>
>
> gdb/
> * objfiles.c: Include varobj.h.
> (invalidate_objfile_varobj_type_iter): New function.
> (free_objfile): Call it.
> * varobj.c (varobj_is_valid_p, varobj_set_invalid): New functions.
> (varobj_invalidate_iter): Mark re-created global object updated
> if its value is different from previous value.
> * varobj.h (varobj_is_valid_p, varobj_set_invalid): New
> prototypes.
>
> Signed-off-by: Raza, Saqlain <Saqlain_Raza@mentor.com>
> ---
> gdb/ChangeLog | 13 +++++++++++++
> gdb/objfiles.c | 19 +++++++++++++++++++
> gdb/varobj.c | 35 +++++++++++++++++++++++++++++++++++
> gdb/varobj.h | 4 ++++
> 4 files changed, 71 insertions(+)
>
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index ced508f..b92da1b 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,16 @@
> +2019-10-17 Saqlain Raza <saqlain_raza@mentor.com>
> + Taimoor Mirza <taimoor_mirza@mentor.com>
> + Maciej W. Rozycki <macro@codesourcery.com>
> +
> + * objfiles.c: Include varobj.h.
> + (invalidate_objfile_varobj_type_iter): New function.
> + (free_objfile): Call it.
> + * varobj.c (varobj_is_valid_p, varobj_set_invalid): New functions.
> + (varobj_invalidate_iter): Mark re-created global object updated
> + if its value is different from previous value.
> + * varobj.h (varobj_is_valid_p, varobj_set_invalid): New
> + prototypes.
> +
> 2019-10-16 Tom Tromey <tom@tromey.com>
>
> * objfiles.h (struct objfile) <original_name>: Now const.
> diff --git a/gdb/objfiles.c b/gdb/objfiles.c
> index f9e7d20..8704814 100644
> --- a/gdb/objfiles.c
> +++ b/gdb/objfiles.c
> @@ -32,6 +32,7 @@
> #include "bcache.h"
> #include "expression.h"
> #include "parser-defs.h"
> +#include "varobj.h"
>
> #include <sys/types.h>
> #include <sys/stat.h>
> @@ -568,6 +569,21 @@ free_objfile_separate_debug (struct objfile *objfile)
> }
> }
>
> +/* Mark the variable object VAR invalid if built upon a type coming from
> + the objfile requested, passed as DATA. Also clear the type reference. */
> +
> +static void
> +invalidate_objfile_varobj_type_iter (struct varobj *var, void *data)
> +{
> + struct objfile *objfile = (struct objfile*)data;
> +
> + if (varobj_is_valid_p (var) && TYPE_OBJFILE (var->type) == objfile)
> + {
> + varobj_set_invalid (var);
> + var->type = NULL;
> + }
> +}
> +
> /* Destroy an objfile and all the symtabs and psymtabs under it. */
>
> objfile::~objfile ()
> @@ -613,6 +629,9 @@ objfile::~objfile ()
> lists. */
> preserve_values (this);
>
> + /* Varobj may refer to types stored in objfile's obstack. */
> + all_root_varobjs (invalidate_objfile_varobj_type_iter, this);
> +
> /* It still may reference data modules have associated with the objfile and
> the symbol file data. */
> forget_cached_source_info_for_objfile (this);
> diff --git a/gdb/varobj.c b/gdb/varobj.c
> index 37a522b..f7696e7 100644
> --- a/gdb/varobj.c
> +++ b/gdb/varobj.c
> @@ -2431,6 +2431,22 @@ varobj_floating_p (const struct varobj *var)
> return var->root->floating;
> }
>
> +/* Get the valid flag of varobj VAR. */
> +
> +int
> +varobj_is_valid_p (struct varobj *var)
> +{
> + return var->root->is_valid;
> +}
> +
> +/* Clear the valid flag on varobj VAR. */
> +
> +void
> +varobj_set_invalid (struct varobj *var)
> +{
> + var->root->is_valid = 0;
> +}
> +
> /* Implement the "value_is_changeable_p" varobj callback for most
> languages. */
>
> @@ -2492,6 +2508,7 @@ varobj_invalidate_iter (struct varobj *var, void *unused)
> if (var->root->floating || var->root->valid_block == NULL)
> {
> struct varobj *tmp_var;
> + std::string tmp_var_value, var_value;
>
> /* Try to create a varobj with same expression. If we succeed
> replace the old varobj, otherwise invalidate it. */
> @@ -2500,6 +2517,24 @@ varobj_invalidate_iter (struct varobj *var, void *unused)
> if (tmp_var != NULL)
> {
> tmp_var->obj_name = var->obj_name;
> + tmp_var_value = varobj_get_value (tmp_var);
> + var_value = varobj_get_value (var);
> +
> + /* Since varobjs are re-evaluated during creation, there is a
> + chance the new value is different from old one. Compare
> + old varobj and the newly created varobj and mark varobj
> + updated ff new value is different. */
> + if (var_value.empty() && tmp_var_value.empty())
> + ; /* Equal. */
> + else if (var_value.empty() || tmp_var_value.empty())
> + tmp_var->updated = 1;
> + else
> + {
> + /* Mark tmp_var updated if the new value is different. */
> + if (tmp_var_value != var_value)
> + tmp_var->updated = 1;
> + }
> +
> varobj_delete (var, 0);
> install_variable (tmp_var);
> }
> diff --git a/gdb/varobj.h b/gdb/varobj.h
> index 66db780..0885f70 100644
> --- a/gdb/varobj.h
> +++ b/gdb/varobj.h
> @@ -323,6 +323,10 @@ extern bool varobj_editable_p (const struct varobj *var);
>
> extern bool varobj_floating_p (const struct varobj *var);
>
> +extern int varobj_is_valid_p (struct varobj *var);
> +
> +extern void varobj_set_invalid (struct varobj *var);
> +
> extern void varobj_set_visualizer (struct varobj *var,
> const char *visualizer);
>
=============
GDB has mechanism of marking varobj's updated if they are set via
varobj_set_value operation. This allows any next -var-update to report
this change. Same approach should be used during varobj invalidation.
If value of newly created varobj is different from previous one, mark it
updated so that -var-update can get this change.
Variable object invalidation code is cleaned up to avoid using pointers
whose target has been already freed.
Fix Testing:
===========
This fix has been regression tested on both simulator and real boards
for x86_64 and arm-none-linux-gnueabi targets.
2019-10-17 Saqlain Raza <saqlain_raza@mentor.com>
Taimoor Mirza <taimoor_mirza@mentor.com>
Maciej W. Rozycki <macro@codesourcery.com>
gdb/
* objfiles.c: Include varobj.h.
(invalidate_objfile_varobj_type_iter): New function.
(free_objfile): Call it.
* varobj.c (varobj_is_valid_p, varobj_set_invalid): New functions.
(varobj_invalidate_iter): Mark re-created global object updated
if its value is different from previous value.
* varobj.h (varobj_is_valid_p, varobj_set_invalid): New
prototypes.
Signed-off-by: Raza, Saqlain <Saqlain_Raza@mentor.com>
---
gdb/ChangeLog | 13 +++++++++++++
gdb/objfiles.c | 19 +++++++++++++++++++
gdb/varobj.c | 35 +++++++++++++++++++++++++++++++++++
gdb/varobj.h | 4 ++++
4 files changed, 71 insertions(+)
@@ -1,3 +1,16 @@
+2019-10-17 Saqlain Raza <saqlain_raza@mentor.com>
+ Taimoor Mirza <taimoor_mirza@mentor.com>
+ Maciej W. Rozycki <macro@codesourcery.com>
+
+ * objfiles.c: Include varobj.h.
+ (invalidate_objfile_varobj_type_iter): New function.
+ (free_objfile): Call it.
+ * varobj.c (varobj_is_valid_p, varobj_set_invalid): New functions.
+ (varobj_invalidate_iter): Mark re-created global object updated
+ if its value is different from previous value.
+ * varobj.h (varobj_is_valid_p, varobj_set_invalid): New
+ prototypes.
+
2019-10-16 Tom Tromey <tom@tromey.com>
* objfiles.h (struct objfile) <original_name>: Now const.
@@ -32,6 +32,7 @@
#include "bcache.h"
#include "expression.h"
#include "parser-defs.h"
+#include "varobj.h"
#include <sys/types.h>
#include <sys/stat.h>
@@ -568,6 +569,21 @@ free_objfile_separate_debug (struct objfile *objfile)
}
}
+/* Mark the variable object VAR invalid if built upon a type coming from
+ the objfile requested, passed as DATA. Also clear the type reference. */
+
+static void
+invalidate_objfile_varobj_type_iter (struct varobj *var, void *data)
+{
+ struct objfile *objfile = (struct objfile*)data;
+
+ if (varobj_is_valid_p (var) && TYPE_OBJFILE (var->type) == objfile)
+ {
+ varobj_set_invalid (var);
+ var->type = NULL;
+ }
+}
+
/* Destroy an objfile and all the symtabs and psymtabs under it. */
objfile::~objfile ()
@@ -613,6 +629,9 @@ objfile::~objfile ()
lists. */
preserve_values (this);
+ /* Varobj may refer to types stored in objfile's obstack. */
+ all_root_varobjs (invalidate_objfile_varobj_type_iter, this);
+
/* It still may reference data modules have associated with the objfile and
the symbol file data. */
forget_cached_source_info_for_objfile (this);
@@ -2431,6 +2431,22 @@ varobj_floating_p (const struct varobj *var)
return var->root->floating;
}
+/* Get the valid flag of varobj VAR. */
+
+int
+varobj_is_valid_p (struct varobj *var)
+{
+ return var->root->is_valid;
+}
+
+/* Clear the valid flag on varobj VAR. */
+
+void
+varobj_set_invalid (struct varobj *var)
+{
+ var->root->is_valid = 0;
+}
+
/* Implement the "value_is_changeable_p" varobj callback for most
languages. */
@@ -2492,6 +2508,7 @@ varobj_invalidate_iter (struct varobj *var, void *unused)
if (var->root->floating || var->root->valid_block == NULL)
{
struct varobj *tmp_var;
+ std::string tmp_var_value, var_value;
/* Try to create a varobj with same expression. If we succeed
replace the old varobj, otherwise invalidate it. */
@@ -2500,6 +2517,24 @@ varobj_invalidate_iter (struct varobj *var, void *unused)
if (tmp_var != NULL)
{
tmp_var->obj_name = var->obj_name;
+ tmp_var_value = varobj_get_value (tmp_var);
+ var_value = varobj_get_value (var);
+
+ /* Since varobjs are re-evaluated during creation, there is a
+ chance the new value is different from old one. Compare
+ old varobj and the newly created varobj and mark varobj
+ updated ff new value is different. */
+ if (var_value.empty() && tmp_var_value.empty())
+ ; /* Equal. */
+ else if (var_value.empty() || tmp_var_value.empty())
+ tmp_var->updated = 1;
+ else
+ {
+ /* Mark tmp_var updated if the new value is different. */
+ if (tmp_var_value != var_value)
+ tmp_var->updated = 1;
+ }
+
varobj_delete (var, 0);
install_variable (tmp_var);
}
@@ -323,6 +323,10 @@ extern bool varobj_editable_p (const struct varobj *var);
extern bool varobj_floating_p (const struct varobj *var);
+extern int varobj_is_valid_p (struct varobj *var);
+
+extern void varobj_set_invalid (struct varobj *var);
+
extern void varobj_set_visualizer (struct varobj *var,
const char *visualizer);