Bug 10586

Summary: Anonymous unions/structs not handled correctly under MI
Product: gdb Reporter: Colin Fowler <elethiomel>
Component: miAssignee: Keith Seitz <keiths>
Status: RESOLVED FIXED    
Severity: normal CC: gdb-prs, keiths
Priority: P2    
Version: 6.8   
Target Milestone: 7.5   
Host: x86_64-linux-gnu Target: x86_64-linux-gnu
Build: x86_64-linux-gnu Last reconfirmed:

Description Colin Fowler 2009-09-01 20:07:51 UTC
When attempting to var-list-children on an anonymous union I get "Duplicate
variable object name" if there are multiple anonymous unions or structs. Below
is a code sample and GDB/MI session

Version is 6.8.50.20090901 (The latest weekly cvs snapshot)

This bug was apparently noticed in 2006 by Apple's Jim Ingham and fixed in their
tree. http://sourceware.org/ml/gdb/2006-11/msg00104.html . Their source is
available at http://www.opensource.apple.com/release/mac-os-x-106/ and indeed,
there is mention of the problem in the code and a relevant fix. I lack the
expertise to integrate their changes back into gdb though.

#include <iostream>

struct test
{
        struct{
                int a;
                float b;
        };

        struct{
                int c;
                float d;
        };
};

int main()
{
    test bar;

    std::cout << bar.a << std::endl;
    std::cout << bar.b << std::endl;
}



663-stack-list-locals 0
663^done,locals=[name="bar"]
(gdb)
664 whatis bar
&"whatis bar\n"
~"type = test\n"
664^done
(gdb)
665 ptype test
&"ptype test\n"
~"type = struct test {\n"
~"    test::<anonymous struct>;\n"
~"    test::<anonymous struct>;\n"
~"}\n"
665^done
(gdb)
666-var-create - * bar
666^done,name="var1",numchild="1",value="{...}",type="test",thread-id="1"
(gdb)
667-var-evaluate-expression var1
667^done,value="{...}"
(gdb)
668-var-list-children var1
668^done,numchild="1",children=[child={name="var1.public",exp="public",numchild="2",thread-id="1"}]
(gdb)
669-var-info-expression var1
669^done,lang="C++",exp="bar"
(gdb)
670-var-list-children var1.public
670^error,msg="Duplicate variable object name"
(gdb)
671-var-list-children var1
671^done,numchild="1",children=[child={name="var1.public",exp="public",numchild="2",thread-id="1"}]
(gdb)
672-var-list-children var1.public
672^error,msg="Duplicate variable object name"
(gdb)
Comment 1 Tom Tromey 2009-09-01 20:11:44 UTC
This came from Eclipse:
https://bugs.eclipse.org/bugs/show_bug.cgi?id=286996
More info is there.
Comment 2 Colin Fowler 2009-09-01 21:14:51 UTC
A mail to Jim Ingham who implemented the fix in Apple's fork got the following
speedy reply with his procedure for fixing this behavior.

"... the approach I suggested worked fine.  It was also straight forward to
implement, you have to cons up a name for the anonymous union, which is easy. 
Then you have to go hunt down all the places where the varobj child gets looked
up by name and convert them to look up by index, since the name is not unique. 
It's probably far easier in this case to implement the idea from scratch in the
FSF sources than to try to look at the changes in the Apple sources get them to
apply to the current TOT.  varobj.c is one of the files we've heavily modified."
Comment 3 Colin Fowler 2009-10-08 16:57:38 UTC
I'm changing to status to this as confirmed as it is known longstanding problem
(see Apple fixes) and it was confirmed by the Eclipse CDT devs.
Comment 4 Nick Roberts 2009-10-10 00:42:38 UTC
Subject:  Anonymous unions/structs not handled correctly under MI

 > I'm changing to status to this as confirmed as it is known longstanding
 > problem (see Apple fixes) and it was confirmed by the Eclipse CDT devs.

The patch below seems to fix this.  I may have missed something because the
thread suggests that any fix isn't that simple.  If that's so, the MI
testsuite overlooks it too because there are no unknown failures.

-- 
Nick                                           http://users.snap.net.nz/~nickrob


2009-10-10  Nick Roberts  <nickrob@snap.net.nz>

	* varobj.c (c_describe_child): Use index for the name
        for anonymous structs/unions.


*** varobj.c	20 Sep 2009 11:44:22 +1200	1.149
--- varobj.c	10 Oct 2009 12:37:14 +1300	
*************** c_describe_child (struct varobj *parent,
*** 2816,2823 ****
      case TYPE_CODE_STRUCT:
      case TYPE_CODE_UNION:
        if (cname)
! 	*cname = xstrdup (TYPE_FIELD_NAME (type, index));
! 
        if (cvalue && value)
  	{
  	  /* For C, varobj index is the same as type index.  */
--- 2816,2830 ----
      case TYPE_CODE_STRUCT:
      case TYPE_CODE_UNION:
        if (cname)
! 	{
! 	  *cname = xstrdup (TYPE_FIELD_NAME (type, index));
! 	  /* Anonymous case.  */
! 	  if (strlen (*cname) == 0)
! 	    {
! 	      *cname = (char*) malloc (24);
! 	      sprintf (*cname, "%d", index);
! 	    }
! 	}
        if (cvalue && value)
  	{
  	  /* For C, varobj index is the same as type index.  */
Comment 5 Colin Fowler 2009-10-10 02:23:31 UTC
Thanks for looking at this. I have applied the patch to the latest cvs snapshot
of GDB (7.0.50.20091009) and still get "Duplicate variable object name" using
the posted testcase. Here's the debugger session.

(gdb) 
343-stack-list-arguments 0 0 0
343^done,stack-args=[frame={level="0",args=[]}]
(gdb) 
344-stack-list-locals 0
344^done,locals=[name="bar"]
(gdb) 
345 whatis bar
&"whatis bar\n"
~"type = test\n"
345^done
(gdb) 
346 ptype test
&"ptype test\n"
~"type = struct test {\n"
~"    test::<anonymous struct>;\n"
~"    test::<anonymous struct>;\n"
~"}\n"
346^done
(gdb) 
347-var-create - * bar
347^done,name="var1",numchild="1",value="{...}",type="test",thread-id="1",has_more="0"
(gdb) 
348-var-evaluate-expression var1
348^done,value="{...}"
(gdb) 
349-var-show-attributes var1
349^done,attr="noneditable"
(gdb) 
350-data-evaluate-expression bar
350^done,value="{{a = 1209049076, b = 3.9881805e-34}, {c = 134514617, d =
148095.812}}"
(gdb) 
351-var-list-children var1
351^done,numchild="1",children=[child={name="var1.public",exp="public",numchild="2",thread-id="1"}],has_more="0"
(gdb) 
352-var-info-expression var1
352^done,lang="C++",exp="bar"
(gdb) 
353-var-list-children var1.public
353^error,msg="Duplicate variable object name"
(gdb) 
354-var-list-children var1
354^done,numchild="1",children=[child={name="var1.public",exp="public",numchild="2",thread-id="1"}],has_more="0"
(gdb) 
355-var-list-children var1.public
355^error,msg="Duplicate variable object name"
(gdb) 
356-data-evaluate-expression bar
356^done,value="{{a = 1209049076, b = 3.9881805e-34}, {c = 134514617, d =
148095.812}}"
(gdb) 
Comment 6 Nick Roberts 2009-10-10 03:10:58 UTC
Subject:  Anonymous unions/structs not handled correctly under MI

 > Thanks for looking at this. I have applied the patch to the latest cvs snapshot
 > of GDB (7.0.50.20091009) and still get "Duplicate variable object name" using
 > the posted testcase. Here's the debugger session.

Ah, yes.  I got sidetracked and considered the equivalent C case below.  I'll
have a look at the C++ case that you posted,

-- 
Nick                                           http://users.snap.net.nz/~nickrob


#include <stdio.h>

typedef struct
{
        struct{
                int a;
                float b;
        };

        struct{
                int c;
                float d;
        };
} test;

int main()
{
  test bar = {{1, 2.2}, {3, 4.4}};

    bar.a = 5;
    bar.b = 6.6;
    printf ("%d\n", bar.a);
    printf ("%f\n", bar.b);
}

previously:

-var-create - * bar
^done,name="var1",numchild="2",value="{...}",type="test",thread-id="1",has_more="0"
(gdb) 
-var-list-children var1
^error,msg="Duplicate variable object name"
(gdb) 

with patch:

-var-list-children var1
^done,numchild="2",children=[child={name="var1.0",exp="0",numchild="2",type="struct {...}",thread-id="1"},child={name="var1.1",exp="1",numchild="2",type="struct {...}",thread-id="1"}],has_more="0"
(gdb) 
-var-list-children var1.0 
^done,numchild="2",children=[child={name="var1.0.a",exp="a",numchild="0",type="int",thread-id="1"},child={name="var1.0.b",exp="b",numchild="0",type="float",thread-id="1"}],has_more="0"
(gdb) 
-var-list-children var1.1
^done,numchild="2",children=[child={name="var1.1.c",exp="c",numchild="0",type="int",thread-id="1"},child={name="var1.1.d",exp="d",numchild="0",type="float",thread-id="1"}],has_more="0"
(gdb) 
=
Comment 7 Nick Roberts 2009-10-10 03:33:41 UTC
Subject:  Anonymous unions/structs not handled correctly under MI

 > Thanks for looking at this. I have applied the patch to the latest cvs snapshot
 > of GDB (7.0.50.20091009) and still get "Duplicate variable object name" using
 > the posted testcase. Here's the debugger session.

I think the C++ case can be handled the same way like below.

-- 
Nick                                           http://users.snap.net.nz/~nickrob


2009-10-10  Nick Roberts  <nickrob@snap.net.nz>

	* varobj.c (c_describe_child, cplus_describe_child): Use the index
        for the name for anonymous structs/unions.


*** varobj.c	20 Sep 2009 11:44:22 +1200	1.149
--- varobj.c	10 Oct 2009 16:25:01 +1300	
*************** c_describe_child (struct varobj *parent,
*** 2816,2823 ****
      case TYPE_CODE_STRUCT:
      case TYPE_CODE_UNION:
        if (cname)
! 	*cname = xstrdup (TYPE_FIELD_NAME (type, index));
! 
        if (cvalue && value)
  	{
  	  /* For C, varobj index is the same as type index.  */
--- 2816,2830 ----
      case TYPE_CODE_STRUCT:
      case TYPE_CODE_UNION:
        if (cname)
! 	{
! 	  *cname = xstrdup (TYPE_FIELD_NAME (type, index));
! 	  /* Anonymous case.  */
! 	  if (strlen (*cname) == 0)
! 	    {
! 	      *cname = (char*) malloc (24);
! 	      sprintf (*cname, "%d", index);
! 	    }
! 	}
        if (cvalue && value)
  	{
  	  /* For C, varobj index is the same as type index.  */
*************** cplus_describe_child (struct varobj *par
*** 3215,3223 ****
  	    }
  	  --type_index;
  
! 	  if (cname)
  	    *cname = xstrdup (TYPE_FIELD_NAME (type, type_index));
! 
  	  if (cvalue && value)
  	    *cvalue = value_struct_element_index (value, type_index);
  
--- 3222,3236 ----
  	    }
  	  --type_index;
  
! 	  if (cname) {
  	    *cname = xstrdup (TYPE_FIELD_NAME (type, type_index));
! 	    /* Anonymous case.  */
! 	    if (strlen (*cname) == 0)
! 	      {
! 		*cname = (char*) malloc (24);
! 		sprintf (*cname, "%d", type_index);
! 	      }
! 	  }
  	  if (cvalue && value)
  	    *cvalue = value_struct_element_index (value, type_index);
  
Comment 8 Colin Fowler 2009-10-10 13:27:09 UTC
Thanks again :) The situation is definitely improved. I can now expand the
anonymous unions. However, there are still some errors. Specifically from 522
onwards.

(gdb) 
512-stack-list-arguments 0 0 0
512^done,stack-args=[frame={level="0",args=[]}]
(gdb) 
513-stack-list-locals 0
513^done,locals=[name="bar"]
(gdb) 
514 whatis bar
&"whatis bar\n"
~"type = test\n"
514^done
(gdb) 
515 ptype test
&"ptype test\n"
~"type = struct test {\n"
~"    test::<anonymous struct>;\n"
~"    test::<anonymous struct>;\n"
~"}\n"
515^done
(gdb) 
516-var-create - * bar
516^done,name="var1",numchild="1",value="{...}",type="test",thread-id="1",has_more="0"
(gdb) 
517-var-evaluate-expression var1
517^done,value="{...}"
(gdb) 
518-var-list-children var1
518^done,numchild="1",children=[child={name="var1.public",exp="public",numchild="2",thread-id="1"}],has_more="0"
(gdb) 
519-var-info-expression var1
519^done,lang="C++",exp="bar"
(gdb) 
520-var-list-children var1.public
520^done,numchild="2",children=[child={name="var1.public.0",exp="0",numchild="1",type="test::<anonymous
struct>",thread-id="1"},child={name="var1.public.1",exp="1",numchild="1",type="test::<anonymous
struct>",thread-id="1"}],has_more="0"
(gdb) 
521-var-info-expression var1.public
521^done,lang="C++",exp="public"
(gdb) 
522 ptype test::<anonymous struct>
&"ptype test::<anonymous struct>\n"
&"A syntax error in expression, near `<anonymous struct>'.\n"
522^error,msg="A syntax error in expression, near `<anonymous struct>'."
(gdb) 
523 ptype (bar).0
&"ptype (bar).0\n"
&"A syntax error in expression, near `.0'.\n"
523^error,msg="A syntax error in expression, near `.0'."
(gdb) 
524-var-evaluate-expression var1.public.0
524^done,value="{...}"
(gdb) 
525-var-evaluate-expression var1.public.1
525^done,value="{...}"
(gdb) 
526-var-list-children var1.public.0
526^done,numchild="1",children=[child={name="var1.public.0.public",exp="public",numchild="2",thread-id="1"}],has_more="0"
(gdb) 
527-var-info-expression var1.public.0
527^done,lang="C++",exp="0"
(gdb) 
528-var-list-children var1.public.0.public
528^done,numchild="2",children=[child={name="var1.public.0.public.a",exp="a",numchild="0",type="int",thread-id="1"},child={name="var1.public.0.public.b",exp="b",numchild="0",type="float",thread-id="1"}],has_more="0"
(gdb) 
529-var-info-expression var1.public.0.public
529^done,lang="C++",exp="public"
(gdb) 
530-var-evaluate-expression var1.public.0.public.a
530^done,value="1209049076"
(gdb) 
531-var-evaluate-expression var1.public.0.public.b
531^done,value="3.9881805e-34"
(gdb) 
532-var-evaluate-expression var1.public.0.public.b
532^done,value="3.9881805e-34"
(gdb) 
533-var-show-attributes var1.public.0.public.a
533^done,attr="editable"
(gdb) 
534-data-evaluate-expression ((bar).0).a
534^error,msg="A syntax error in expression, near `.0).a'."
(gdb) 
Comment 9 Nick Roberts 2009-10-10 22:35:57 UTC
Subject:  Anonymous unions/structs not handled correctly under MI

 > Thanks again :) The situation is definitely improved. I can now expand the
 > anonymous unions. However, there are still some errors. Specifically from 522
 > onwards.
 > 

 >...
 > (gdb) 
 > 522 ptype test::<anonymous struct>
 > &"ptype test::<anonymous struct>\n"
 > &"A syntax error in expression, near `<anonymous struct>'.\n"
 > 522^error,msg="A syntax error in expression, near `<anonymous struct>'."

This isn't a valid expression or data type, so I wouldn't expect it to work

 > (gdb) 
 > 523 ptype (bar).0
 > &"ptype (bar).0\n"
 > &"A syntax error in expression, near `.0'.\n"
 > 523^error,msg="A syntax error in expression, near `.0'."
 > (gdb) 

I don't think that GDB can currently refer to anonymous structs/unions.  It
might be useful to do so but that would require a syntax to be defined and
documented.  I think this is additional to the original bug report about
variable objects and that the patch is sufficient for front ends that use
MI (or at least Emacs).


 >...
 > (gdb) 
 > 534-data-evaluate-expression ((bar).0).a
 > 534^error,msg="A syntax error in expression, near `.0).a'."
 > (gdb) 

When you you need to issue such a command?

If there is a need then you could file a further bug report once/if the first
patch gets approved.


Comment 10 Colin Fowler 2009-10-10 22:56:39 UTC
The above session is via Eclipse/CDT, not manually controlled by me. It's the
output from the CDT set to verbose. If CDT is in fact issuing senseless
commands, I'll definitely file a new bug with them .

The version of Eclipse/CDT I use is Galileo-SR1. It's the version you'll get if
you choose "Eclipse IDE for C/C++ Developers" from http://www.eclipse.org/downloads/

The patch definitely works with the simple tests I posted. When I get into work
tomorrow I'll throw my main code at it (It uses anonymous structs/unions heavily).

As a matter of style, can I suggest names such as anon_union_1 or anon_struct_5
rather than just numbers?

Thanks for all your work on this. This fixes a major problem for me.
Comment 11 Nick Roberts 2009-10-11 00:36:07 UTC
Subject:  Anonymous unions/structs not handled correctly under MI

 > The above session is via Eclipse/CDT, not manually controlled by me. It's the
 > output from the CDT set to verbose. If CDT is in fact issuing senseless
 > commands, I'll definitely file a new bug with them .

OK, that's interesting.  I would presume that there is a purpose.  In that case:

 > > (gdb) 
 > > 522 ptype test::<anonymous struct>
 > > &"ptype test::<anonymous struct>\n"
 > > &"A syntax error in expression, near `<anonymous struct>'.\n"
 > > 522^error,msg="A syntax error in expression, near `<anonymous struct>'."

I think this is a bug with CDT as it shouldn't be using ptype and, if it does,
it needs to ensure it has a valid expression or data type as an argument.

 > > (gdb) 
 > > 534-data-evaluate-expression ((bar).0).a
 > > 534^error,msg="A syntax error in expression, near `.0).a'."

This would be a bug in GDB (if my patch is installed) ...

 > The version of Eclipse/CDT I use is Galileo-SR1. It's the version you'll get if
 > you choose "Eclipse IDE for C/C++ Developers" from http://www.eclipse.org/downloads/
 > 
 > The patch definitely works with the simple tests I posted. When I get into work
 > tomorrow I'll throw my main code at it (It uses anonymous structs/unions heavily).

... however, if behaviour is improved then, although not perfect, it's probably
worth installing this change.

 > As a matter of style, can I suggest names such as anon_union_1 or anon_struct_5
 > rather than just numbers?

I suggest proposing such changes on the gdb mailing list where it will receive
wider attention.

Comment 12 Keith Seitz 2011-11-11 21:31:22 UTC
There has been no movement on this for over two years. Let's get this sorted out and officially upstream. Patch based on Nick's comments posted to gdb-patches.
Comment 13 Sourceware Commits 2012-01-12 22:50:54 UTC
CVSROOT:	/cvs/src
Module name:	src
Changes by:	kseitz@sourceware.org	2012-01-12 22:50:49

Modified files:
	gdb/testsuite  : ChangeLog 
	gdb/testsuite/gdb.mi: var-cmd.c mi2-var-child.exp mi-var-cp.cc 
	                      mi-var-cp.exp 

Log message:
	PR mi/10586
	* gdb.mi/var-cmd.c (struct anonymous): New structure.
	(do_anonymous_type_tests): New function.
	(main): Call do_anonymous_type_tests.
	* gdb.mi/mi2-var-child.exp: Add anonymous type tests.
	(verify_everything): New procedure.
	* gdb.mi/mi-var-cp.cc (class A): New class.
	(anonymous_structs_and_unions): New function.
	(main): Call anonymous_structs_and_unions.
	* gdb.mi/mi-var-cp.exp: Add anonymous type tests.
	(verify_everything): New procedure.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/testsuite/ChangeLog.diff?cvsroot=src&r1=1.3018&r2=1.3019
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/testsuite/gdb.mi/var-cmd.c.diff?cvsroot=src&r1=1.25&r2=1.26
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/testsuite/gdb.mi/mi2-var-child.exp.diff?cvsroot=src&r1=1.17&r2=1.18
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/testsuite/gdb.mi/mi-var-cp.cc.diff?cvsroot=src&r1=1.15&r2=1.16
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/testsuite/gdb.mi/mi-var-cp.exp.diff?cvsroot=src&r1=1.13&r2=1.14
Comment 14 Sourceware Commits 2012-01-12 22:51:16 UTC
CVSROOT:	/cvs/src
Module name:	src
Changes by:	kseitz@sourceware.org	2012-01-12 22:51:11

Modified files:
	gdb            : ChangeLog varobj.c 

Log message:
	PR mi/10586
	* varobj.c (ANONYMOUS_STRUCT_NAME): Define.
	(ANONYMOUS_UNION_NAME): Define.
	(is_path_expr_parent): New function.
	(get_path_expr_parent): New function.
	(is_anonymous_child): New function.
	(create_child_with_value): If the child is anonymous and without
	a name, assign an object name to it.
	(c_describe_child): Use get_path_expr_parent to determine
	the parent expression.
	If there field represents an anonymous struct or union and
	has no name, set an appropriate display name and expression.
	(cplus_describe_child): Likewise.

Patches:
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/ChangeLog.diff?cvsroot=src&r1=1.13733&r2=1.13734
http://sourceware.org/cgi-bin/cvsweb.cgi/src/gdb/varobj.c.diff?cvsroot=src&r1=1.187&r2=1.188
Comment 15 Keith Seitz 2012-01-12 22:55:01 UTC
This should now be fixed (at long last). If there are any problems, please do not hesitate to report them.