Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: "Andrew Burgess" <aburgess@broadcom.com>
To: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
Subject: Re-evaluate floating varobj as part of varobj_invalidate.
Date: Wed, 25 Jul 2012 16:59:00 -0000	[thread overview]
Message-ID: <501025E0.30708@broadcom.com> (raw)

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;


             reply	other threads:[~2012-07-25 16:59 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-25 16:59 Andrew Burgess [this message]
2012-07-25 17:52 ` Tom Tromey
2012-07-26  8:35   ` Jan Kratochvil
2012-07-27  8:21     ` Andrew Burgess
2012-07-27 14:06       ` Jan Kratochvil
2012-07-30 12:27         ` Andrew Burgess

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=501025E0.30708@broadcom.com \
    --to=aburgess@broadcom.com \
    --cc=gdb-patches@sourceware.org \
    /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