Re-evaluate floating varobj as part of varobj_invalidate.

Andrew Burgess
2012-07-25T16:59:50Z

Hi,

I have a bug & patch, but no great way to reproduce the issue as it's a use of pointer after free issue, so it depends on having the memory reused after the free. It was originally spotted on an older version of gdb (7.0) but it still applies, it just isn't so easy to trigger.

Using x86/linux/glibc I can hook the free() call to scribble over all free'd memory, in this case I can cause the issue to be triggered on latest head. I'm happy to post a patch for this if anyone wants to reproduce the bug this way.

So, the bug...

Using this test program (var.c):

int global_var = 3;

int
main ( void )
{
return 0;
}

Compiled up as var.x (gcc -g -o var.x var.c), I start gdb as:

gdb -i mi2 ./var.x

Then I enter the commands:

1-var-create - @ global_var
2-file-symbol-file var.x
3-var-set-format var1 hexadecimal

That's it, gdb can segfault while running command 3-var-set-format.

The reason for the failure is this:

* In 1-var-create we evaluate the expression "global_var" this returns a "struct value" that has a type, this type and the value are cached on the "struct varobj".
* During 2-file-symbol-file all the types from the objfile are free'd, this will include the type referenced by the value on the varobj, and the type cached directly on the varobj. As part of the symbol file loading we do call varobj_invalidate which calls varobj_invalidate_iter, however, our varobj is left untouched as it is a "floating" varobj.
* During 3-var-set-format we change the format of the varobj then call varobj_value_is_changeable_p, which calls (usually) default_value_is_changeable_p, which in turn calls get_value_type, which finally, calls check_typedef on the type which was free'd.

The patch below simply removes the special case for floating varobj objects from varobj_invalidate_iter, and causes them to be re-evaluated or marked as invalid.

Is this OK to apply?

Thanks

Andrew

gdb/Change

2012-07-25 Andrew Burgess <aburgess@broadcom.com>

* varobj.c (varobj_invalidate_iter): All varobj must be marked as
invalid or reevaluated to prevent prevent references to possibly
delete'd type objects being left in the varobj.

diff --git a/gdb/varobj.c b/gdb/varobj.c
index 99b158e..a75a40d 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -4186,18 +4186,17 @@ _initialize_varobj (void)
}

/* Invalidate varobj VAR if it is tied to locals and re-create it if it is
- defined on globals. It is a helper for varobj_invalidate. */
+ defined on globals. It is a helper for varobj_invalidate.
+
+ This function is called after changing the symbol file, in this case the
+ pointers to "struct type" stored by the varobj are no longer valid. All
+ varobj must be either re-evaluated, or marked as invalid here. */

static void
varobj_invalidate_iter (struct varobj *var, void *unused)
{
- /* Floating varobjs are reparsed on each stop, so we don't care if the
- presently parsed expression refers to something that's gone. */
- if (var->root->floating)
- return;
-
- /* global var must be re-evaluated. */
- if (var->root->valid_block == NULL)
+ /* global and floating var must be re-evaluated. */
+ if (var->root->floating || var->root->valid_block == NULL)
{
struct varobj *tmp_var;