[patch] Fix for varobj.c assertions.

[patch] Fix for varobj.c assertions.

[Aleksandar Ristovski]

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

When listing children of a variable, if variable was not initialized we will
ignore return value from gdb_value_ind.

This patch fixes this situation. Fixes at least one problem from PR 2309.


---
Aleksandar Ristovski
QNX Software Systems



[-- Attachment #2: ChangeLog.txt --]
[-- Type: text/plain, Size: 233 bytes --]

2008-01-10  Aleksandar Ristovski  <aristovski@qnx.com>

	* varobj (adjust_value_for_child_access): Added checking for 
	returned value from gdb_value_ind.
	(c_describe_child): Likewise.
	(cplus_describe_child): Fixed a typo.


[-- Attachment #3: varobj.c.patch --]
[-- Type: application/octet-stream, Size: 1397 bytes --]

Index: gdb/varobj.c
===================================================================
RCS file: /cvs/src/src/gdb/varobj.c,v
retrieving revision 1.99
diff -u -p -r1.99 varobj.c
--- gdb/varobj.c	1 Jan 2008 22:53:13 -0000	1.99
+++ gdb/varobj.c	10 Jan 2008 21:59:04 -0000
@@ -1920,7 +1920,11 @@ adjust_value_for_child_access (struct va
 	  || TYPE_CODE (target_type) == TYPE_CODE_UNION)
 	{
 	  if (value && *value)
-	    gdb_value_ind (*value, value);	  
+	    {
+	      int success = gdb_value_ind (*value, value);	  
+	      if (!success)
+		*value = NULL;
+	    }
 	  *type = target_type;
 	  if (was_ptr)
 	    *was_ptr = 1;
@@ -2114,7 +2118,11 @@ c_describe_child (struct varobj *parent,
 	*cname = xstrprintf ("*%s", parent->name);
 
       if (cvalue && value)
-	gdb_value_ind (value, cvalue);
+	{
+	  int success = gdb_value_ind (value, cvalue);
+	  if (!success)
+	    *cvalue = NULL;
+	}
 
       /* Don't use get_target_type because it calls
 	 check_typedef and here, we want to show the true
@@ -2415,7 +2423,7 @@ cplus_describe_child (struct varobj *par
   adjust_value_for_child_access (&value, &type, &was_ptr);
 
   if (TYPE_CODE (type) == TYPE_CODE_STRUCT
-      || TYPE_CODE (type) == TYPE_CODE_STRUCT)
+      || TYPE_CODE (type) == TYPE_CODE_UNION)
     {
       char *join = was_ptr ? "->" : ".";
       if (CPLUS_FAKE_CHILD (parent))

Re: [patch] Fix for varobj.c assertions.

[Nick Roberts]

 > When listing children of a variable, if variable was not initialized we will
 > ignore return value from gdb_value_ind.
 > 
 > This patch fixes this situation. Fixes at least one problem from PR 2309.

I've looked at your new testcase in GNATS (I've only just seen how to get
cc'ed).  It doesn't fail for me, even with the script file, which surprised
me because I didn't think MI commands would work in a script.

I use:

gcc version 4.1.2 (Ubuntu 4.1.2-0ubuntu4)
Ubuntu 7.04 (2.6.20-15-generic)

 > ---
 > Aleksandar Ristovski
 > QNX Software Systems
 > 
 > 
 > 2008-01-10  Aleksandar Ristovski  <aristovski@qnx.com>
 > 
 > 	* varobj (adjust_value_for_child_access): Added checking for 
 > 	returned value from gdb_value_ind.
 > 	(c_describe_child): Likewise.
 > 	(cplus_describe_child): Fixed a typo.
 > 

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: [patch] Fix for varobj.c assertions.

[Aleksandar Ristovski]

> 
> I use:
> 
> gcc version 4.1.2 (Ubuntu 4.1.2-0ubuntu4)
> Ubuntu 7.04 (2.6.20-15-generic)
> 

What is your gdb version?

Re: [patch] Fix for varobj.c assertions.

[Nick Roberts]

Aleksandar Ristovski writes:
 > > 
 > > I use:
 > > 
 > > gcc version 4.1.2 (Ubuntu 4.1.2-0ubuntu4)
 > > Ubuntu 7.04 (2.6.20-15-generic)
 > > 
 > 
 > What is your gdb version?

Perhaps I should have said.  Current CVS: GNU gdb 6.7.50.20080109-cvs.

Can anybody else duplicate this problem? (see pr2309)

-- 
Nick                                           http://www.inet.net.nz/~nickrob

RE: [patch] Fix for varobj.c assertions.

[Aleksandar Ristovski]

> 
> Aleksandar Ristovski writes:
>  > >
>  > > I use:
>  > >
>  > > gcc version 4.1.2 (Ubuntu 4.1.2-0ubuntu4)
>  > > Ubuntu 7.04 (2.6.20-15-generic)
>  > >
>  >
>  > What is your gdb version?
> 
> Perhaps I should have said.  Current CVS: GNU gdb 6.7.50.20080109-cvs.
> 
> Can anybody else duplicate this problem? (see pr2309)
> 

I did some more testing, it turns out it depends on the toolchain version.

Toolchain that produced the problem:
gcc 4.1.1
binutils 2.17

However, with
gcc 4.1.2
binutils 2.18
the problem does not reproduce.


The patch fixes the problem in the case of problematic toolchain version.

RE: [patch] Fix for varobj.c assertions.

[Nick Roberts]

 > > Can anybody else duplicate this problem? (see pr2309)
 > > 
 > 
 > I did some more testing, it turns out it depends on the toolchain version.
 > 
 > Toolchain that produced the problem:
 > gcc 4.1.1
 > binutils 2.17
 > 
 > However, with
 > gcc 4.1.2
 > binutils 2.18
 > the problem does not reproduce.
 > 
 > 
 > The patch fixes the problem in the case of problematic toolchain version.

Maybe it cures the symptom and not the cause.  We really need to know what is
happening here.  In your oringinal report the failed assertion occurred in
my_value_equal but that function has gone now.  Presumably it now occurs in
install_new_value, but at which line?

When there is no problem, presumably *value (in adjust_value_for_child_access)
or value (in c_describe_child) are null.  How do they become non null for
inaccessible memory in the failing case?

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: [patch] Fix for varobj.c assertions.

[Daniel Jacobowitz]

On Wed, Jan 16, 2008 at 11:19:26AM +1300, Nick Roberts wrote:
> Maybe it cures the symptom and not the cause.  We really need to know what is
> happening here.  In your oringinal report the failed assertion occurred in
> my_value_equal but that function has gone now.  Presumably it now occurs in
> install_new_value, but at which line?
> 
> When there is no problem, presumably *value (in adjust_value_for_child_access)
> or value (in c_describe_child) are null.  How do they become non null for
> inaccessible memory in the failing case?

I can reproduce this too.  With GDB 6.7.1:

~"/tmp/buildd/gdb-6.7.1/gdb/varobj.c:2011: internal-error:
value_struct_element_index: Assertion `TYPE_CODE (type) ==
TYPE_CODE_STRUCT || TYPE_CODE (type) == TYPE_CODE_UNION' failed.\n"
~"A problem internal to GDB has been detected,\n"
~"further debugging may prove unreliable.\n"
~"Quit this debugging session? (y or n) "

With HEAD it's on line 2008, but otherwise the same.  type is a
pointer type, "B *".  *value was a B * value; indirecting it failed
because the pointer hasn't been initialized yet at the start of main.
When gdb_value_ind returns zero, it has not changed *result.  So we
go ahead as if *result was set.

Aleksandar's change to c_describe_child is safe but not necessary,
because *cvalue will already be NULL if gdb_value_ind fails.  The
change to adjust_value_for_child_access is right, and fixes the bug.
And the change to cplus_describe_child is definitely right (*whoops*).

So I've checked in the patch.

-- 
Daniel Jacobowitz
CodeSourcery