diff mbox

Improve MI -var-info-path-expression for nested struct/union case.

Message ID 1397736351-20306-1-git-send-email-aburgess@broadcom.com
State Superseded
Headers show

Commit Message

Andrew Burgess April 17, 2014, 12:05 p.m. UTC
The MI command -var-info-path-expression currently does not handle some
cases of nested structures / unions correctly, consider the following
source file:

int
main ( void )
{
  struct a_t
  {
    int var;
  };

  struct s_t
  {
    struct a_t a;
  };

  struct s_t s;

  return 0; /* Break Here (line 16) */
}

And this gdb session:

(gdb) break 16
Breakpoint 1 at 0x400550: file nested.c, line 16.
(gdb) run
Starting program: /projects/firepath_work/aburgess/tmp/nested.x 
Can't read symbols from system-supplied DSO at 0x2aaaaaaab000: File truncated

Breakpoint 1, main () at nested.c:16
16        return 0; /* Break Here (line 16) */
(gdb) interpreter-exec mi "-var-create var1 @ s"
^done,name="var1",numchild="1",value="{...}",type="struct s_t",thread-id="1",has_more="0"
(gdb) interpreter-exec mi "-var-list-children var1"
^done,numchild="1",children=[child={name="var1.a",exp="a",numchild="1",type="struct a_t",thread-id="1"}],has_more="0"
(gdb) interpreter-exec mi "-var-list-children var1.a"
^done,numchild="1",children=[child={name="var1.a.var",exp="var",numchild="0",type="int",thread-id="1"}],has_more="0"
(gdb) interpreter-exec mi "-var-info-path-expression var1.a.var"
^done,path_expr="(s).var"
(gdb) interpreter-exec mi "-data-evaluate-expression (s).var"
^error,msg="There is no member named var."

Notice that the result of the -var-info-path-expression is wrong, and, not
surprisingly causes and error when passed into -data-evaluate-expression.

With this patch the result is instead this:

(gdb)  interpreter-exec mi "-var-info-path-expression var1.a.var"
^done,path_expr="((s).a).var"
(gdb) interpreter-exec mi "-data-evaluate-expression ((s).a).var"
^done,value="-8320"

The changes to varobj.c feel like it might be possible to simplify them,
but I can't figure out how, any change I make seems to regress something,
so for now I'm proposing this patch.  The change is very contained anyway,
so if someone can improve it later it's not going to cause any disruption.

OK to apply?

Thanks,
Andrew


gdb/ChangeLog:

	* varobj.c (is_path_expr_parent): Improve test for anonymous
	structures and unions.

gdb/testsuite/ChangeLog:

	* gdb.mi/var-cmd.c (do_nested_struct_union_tests): New function
	setting up test structures.
	(main): Call new test function.
	* gdb.mi/mi2-var-child.exp: Create additional breakpoint in new
	test function, continue into test function and walk test
	structures.
---
 gdb/testsuite/gdb.mi/mi2-var-child.exp | 61 +++++++++++++++++++++++++++++++
 gdb/testsuite/gdb.mi/var-cmd.c         | 65 ++++++++++++++++++++++++++++++++++
 gdb/varobj.c                           | 30 +++++++++++++---
 3 files changed, 152 insertions(+), 4 deletions(-)

Comments

Andrew Burgess April 29, 2014, 2:17 p.m. UTC | #1
Ping.

On 17/04/2014 1:05 PM, Andrew Burgess wrote:
> The MI command -var-info-path-expression currently does not handle some
> cases of nested structures / unions correctly, consider the following
> source file:
> 
> int
> main ( void )
> {
>   struct a_t
>   {
>     int var;
>   };
> 
>   struct s_t
>   {
>     struct a_t a;
>   };
> 
>   struct s_t s;
> 
>   return 0; /* Break Here (line 16) */
> }
> 
> And this gdb session:
> 
> (gdb) break 16
> Breakpoint 1 at 0x400550: file nested.c, line 16.
> (gdb) run
> Starting program: /projects/firepath_work/aburgess/tmp/nested.x 
> Can't read symbols from system-supplied DSO at 0x2aaaaaaab000: File truncated
> 
> Breakpoint 1, main () at nested.c:16
> 16        return 0; /* Break Here (line 16) */
> (gdb) interpreter-exec mi "-var-create var1 @ s"
> ^done,name="var1",numchild="1",value="{...}",type="struct s_t",thread-id="1",has_more="0"
> (gdb) interpreter-exec mi "-var-list-children var1"
> ^done,numchild="1",children=[child={name="var1.a",exp="a",numchild="1",type="struct a_t",thread-id="1"}],has_more="0"
> (gdb) interpreter-exec mi "-var-list-children var1.a"
> ^done,numchild="1",children=[child={name="var1.a.var",exp="var",numchild="0",type="int",thread-id="1"}],has_more="0"
> (gdb) interpreter-exec mi "-var-info-path-expression var1.a.var"
> ^done,path_expr="(s).var"
> (gdb) interpreter-exec mi "-data-evaluate-expression (s).var"
> ^error,msg="There is no member named var."
> 
> Notice that the result of the -var-info-path-expression is wrong, and, not
> surprisingly causes and error when passed into -data-evaluate-expression.
> 
> With this patch the result is instead this:
> 
> (gdb)  interpreter-exec mi "-var-info-path-expression var1.a.var"
> ^done,path_expr="((s).a).var"
> (gdb) interpreter-exec mi "-data-evaluate-expression ((s).a).var"
> ^done,value="-8320"
> 
> The changes to varobj.c feel like it might be possible to simplify them,
> but I can't figure out how, any change I make seems to regress something,
> so for now I'm proposing this patch.  The change is very contained anyway,
> so if someone can improve it later it's not going to cause any disruption.
> 
> OK to apply?
> 
> Thanks,
> Andrew
> 
> 
> gdb/ChangeLog:
> 
> 	* varobj.c (is_path_expr_parent): Improve test for anonymous
> 	structures and unions.
> 
> gdb/testsuite/ChangeLog:
> 
> 	* gdb.mi/var-cmd.c (do_nested_struct_union_tests): New function
> 	setting up test structures.
> 	(main): Call new test function.
> 	* gdb.mi/mi2-var-child.exp: Create additional breakpoint in new
> 	test function, continue into test function and walk test
> 	structures.
> ---
>  gdb/testsuite/gdb.mi/mi2-var-child.exp | 61 +++++++++++++++++++++++++++++++
>  gdb/testsuite/gdb.mi/var-cmd.c         | 65 ++++++++++++++++++++++++++++++++++
>  gdb/varobj.c                           | 30 +++++++++++++---
>  3 files changed, 152 insertions(+), 4 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.mi/mi2-var-child.exp b/gdb/testsuite/gdb.mi/mi2-var-child.exp
> index d2f65c5..d9f5991 100644
> --- a/gdb/testsuite/gdb.mi/mi2-var-child.exp
> +++ b/gdb/testsuite/gdb.mi/mi2-var-child.exp
> @@ -1162,6 +1162,10 @@ set lineno [gdb_get_line_number "anonymous type tests breakpoint"]
>  mi_create_breakpoint \
>      "$srcfile:$lineno" {[0-9]+} keep {do_anonymous_type_tests} \
>      ".*var-cmd.c" $lineno $hex "break in do_anonymous_type_tests"
> +set lineno [gdb_get_line_number "nested struct union tests breakpoint"]
> +mi_create_breakpoint \
> +    "$srcfile:$lineno" {[0-9]+} keep {do_nested_struct_union_tests} \
> +    ".*var-cmd.c" $lineno $hex "break in do_nested_struct_union_tests"
>  mi_execute_to "exec-continue" "breakpoint-hit" "do_anonymous_type_tests" ""\
>      ".*" ".*" {"" "disp=\"keep\""} \
>      "continue to do_anonymous_type_tests breakpoint"
> @@ -1235,5 +1239,62 @@ set tree {
>  
>  mi_walk_varobj_tree c $tree verify_everything
>  
> +mi_send_resuming_command "exec-continue" \
> +    "continuing execution to enter do_nested_struct_union_tests"
> +mi_expect_stop "breakpoint-hit" "do_nested_struct_union_tests" ".*" ".*" ".*" \
> +    {.* disp="keep"} "Run till MI stops in do_nested_struct_union_tests"
> +
> +set tree {
> +  {struct ss} var {
> +    {struct s_a} a1 {
> +      int a {}
> +    }
> +    {struct s_b} b1 {
> +      int b {}
> +    }
> +    {union u_ab} u1 {
> +      {struct s_a} a {
> +        int a {}
> +      }
> +      {struct s_b} b {
> +        int b {}
> +      }
> +    }
> +    anonymous union {
> +      {struct s_a} a2 {
> +        int a {}
> +      }
> +      {struct s_b} b2 {
> +        int b {}
> +      }
> +    }
> +    {union {...}} u2 {
> +      {struct s_a} a3 {
> +        int a {}
> +      }
> +      {struct s_b} b3 {
> +        int b {}
> +      }
> +    }
> +  }
> +}
> +
> +mi_walk_varobj_tree c $tree verify_everything
> +
> +set tree {
> +  {struct {...}} var2 {
> +    {td_u_ab} ab {
> +      {td_s_a} a {
> +	int a {}
> +      }
> +      {td_s_b} b {
> +	int b {}
> +      }
> +    }
> +  }
> +}
> +
> +mi_walk_varobj_tree c $tree verify_everything
> +
>  mi_gdb_exit
>  return 0
> diff --git a/gdb/testsuite/gdb.mi/var-cmd.c b/gdb/testsuite/gdb.mi/var-cmd.c
> index 698b294..c519aa1 100644
> --- a/gdb/testsuite/gdb.mi/var-cmd.c
> +++ b/gdb/testsuite/gdb.mi/var-cmd.c
> @@ -552,6 +552,70 @@ do_anonymous_type_tests (void)
>    return; /* anonymous type tests breakpoint */
>  }
>  
> +void
> +do_nested_struct_union_tests (void)
> +{
> +  struct s_a
> +  {
> +    int a;
> +  };
> +  struct s_b
> +  {
> +    int b;
> +  };
> +  union u_ab
> +  {
> +    struct s_a a;
> +    struct s_b b;
> +  };
> +  struct ss
> +  {
> +    struct s_a a1;
> +    struct s_b b1;
> +    union u_ab u1;
> +
> +    /* Anonymous union.  */
> +    union
> +    {
> +      struct s_a a2;
> +      struct s_b b2;
> +    };
> +
> +    union
> +    {
> +      struct s_a a3;
> +      struct s_b b3;
> +    } u2;
> +  };
> +
> +  typedef struct
> +  {
> +    int a;
> +  } td_s_a;
> +
> +  typedef struct
> +  {
> +    int b;
> +  } td_s_b;
> +
> +  typedef union
> +  {
> +    td_s_a a;
> +    td_s_b b;
> +  } td_u_ab;
> +
> +  struct ss var;
> +  struct
> +  {
> +    td_u_ab ab;
> +  } var2;
> +
> +  memset (&var, 0, sizeof (var));
> +  memset (&var2, 0, sizeof (var2));
> +
> +  return; /* nested struct union tests breakpoint */
> +}
> +
>  int
>  main (int argc, char *argv [])
>  {
> @@ -563,6 +627,7 @@ main (int argc, char *argv [])
>    do_at_tests ();
>    do_bitfield_tests ();
>    do_anonymous_type_tests ();
> +  do_nested_struct_union_tests ();
>    exit (0);
>  }
>  
> diff --git a/gdb/varobj.c b/gdb/varobj.c
> index 10ef5b7..33083b2 100644
> --- a/gdb/varobj.c
> +++ b/gdb/varobj.c
> @@ -1075,12 +1075,34 @@ is_path_expr_parent (struct varobj *var)
>    if (CPLUS_FAKE_CHILD (var))
>      return 0;
>  
> -  type = varobj_get_value_type (var);
> +  type = varobj_get_gdb_type (var);
>  
>    /* Anonymous unions and structs are also not path_expr parents.  */
> -  return !((TYPE_CODE (type) == TYPE_CODE_STRUCT
> -	    || TYPE_CODE (type) == TYPE_CODE_UNION)
> -	   && TYPE_NAME (type) == NULL);
> +  if ((TYPE_CODE (type) == TYPE_CODE_STRUCT
> +       || TYPE_CODE (type) == TYPE_CODE_UNION)
> +      && TYPE_NAME (type) == NULL
> +      && TYPE_TAG_NAME (type) == NULL)
> +    {
> +      if (var->parent != NULL)
> +	{
> +	  const struct type *parent_type;
> +
> +	  parent_type = varobj_get_gdb_type (var->parent);
> +	  if (TYPE_CODE (parent_type) == TYPE_CODE_STRUCT
> +	      || TYPE_CODE (parent_type) == TYPE_CODE_UNION)
> +	    {
> +	      const char *field_name;
> +
> +	      gdb_assert (var->index < TYPE_NFIELDS (parent_type));
> +	      field_name = TYPE_FIELD_NAME (parent_type, var->index);
> +	      return !(field_name == NULL || *field_name == '\0');
> +	    }
> +	}
> +
> +      return 0;
> +    }
> +
> +  return 1;
>  }
>  
>  /* Return the path expression parent for VAR.  */
>
Vladimir Prus May 8, 2014, 11:02 a.m. UTC | #2
Andrew,

it happens to be a bunch of public holidays here, but I'll take a look at your patch next Monday if nobody
else beats me to it.

Thanks,

On 04/29/2014 06:17 PM, Andrew Burgess wrote:
> Ping.
>
> On 17/04/2014 1:05 PM, Andrew Burgess wrote:
>> The MI command -var-info-path-expression currently does not handle some
>> cases of nested structures / unions correctly, consider the following
>> source file:
>>
>> int
>> main ( void )
>> {
>>    struct a_t
>>    {
>>      int var;
>>    };
>>
>>    struct s_t
>>    {
>>      struct a_t a;
>>    };
>>
>>    struct s_t s;
>>
>>    return 0; /* Break Here (line 16) */
>> }
>>
>> And this gdb session:
>>
>> (gdb) break 16
>> Breakpoint 1 at 0x400550: file nested.c, line 16.
>> (gdb) run
>> Starting program: /projects/firepath_work/aburgess/tmp/nested.x
>> Can't read symbols from system-supplied DSO at 0x2aaaaaaab000: File truncated
>>
>> Breakpoint 1, main () at nested.c:16
>> 16        return 0; /* Break Here (line 16) */
>> (gdb) interpreter-exec mi "-var-create var1 @ s"
>> ^done,name="var1",numchild="1",value="{...}",type="struct s_t",thread-id="1",has_more="0"
>> (gdb) interpreter-exec mi "-var-list-children var1"
>> ^done,numchild="1",children=[child={name="var1.a",exp="a",numchild="1",type="struct a_t",thread-id="1"}],has_more="0"
>> (gdb) interpreter-exec mi "-var-list-children var1.a"
>> ^done,numchild="1",children=[child={name="var1.a.var",exp="var",numchild="0",type="int",thread-id="1"}],has_more="0"
>> (gdb) interpreter-exec mi "-var-info-path-expression var1.a.var"
>> ^done,path_expr="(s).var"
>> (gdb) interpreter-exec mi "-data-evaluate-expression (s).var"
>> ^error,msg="There is no member named var."
>>
>> Notice that the result of the -var-info-path-expression is wrong, and, not
>> surprisingly causes and error when passed into -data-evaluate-expression.
>>
>> With this patch the result is instead this:
>>
>> (gdb)  interpreter-exec mi "-var-info-path-expression var1.a.var"
>> ^done,path_expr="((s).a).var"
>> (gdb) interpreter-exec mi "-data-evaluate-expression ((s).a).var"
>> ^done,value="-8320"
>>
>> The changes to varobj.c feel like it might be possible to simplify them,
>> but I can't figure out how, any change I make seems to regress something,
>> so for now I'm proposing this patch.  The change is very contained anyway,
>> so if someone can improve it later it's not going to cause any disruption.
>>
>> OK to apply?
>>
>> Thanks,
>> Andrew
>>
>>
>> gdb/ChangeLog:
>>
>> 	* varobj.c (is_path_expr_parent): Improve test for anonymous
>> 	structures and unions.
>>
>> gdb/testsuite/ChangeLog:
>>
>> 	* gdb.mi/var-cmd.c (do_nested_struct_union_tests): New function
>> 	setting up test structures.
>> 	(main): Call new test function.
>> 	* gdb.mi/mi2-var-child.exp: Create additional breakpoint in new
>> 	test function, continue into test function and walk test
>> 	structures.
>> ---
>>   gdb/testsuite/gdb.mi/mi2-var-child.exp | 61 +++++++++++++++++++++++++++++++
>>   gdb/testsuite/gdb.mi/var-cmd.c         | 65 ++++++++++++++++++++++++++++++++++
>>   gdb/varobj.c                           | 30 +++++++++++++---
>>   3 files changed, 152 insertions(+), 4 deletions(-)
>>
>> diff --git a/gdb/testsuite/gdb.mi/mi2-var-child.exp b/gdb/testsuite/gdb.mi/mi2-var-child.exp
>> index d2f65c5..d9f5991 100644
>> --- a/gdb/testsuite/gdb.mi/mi2-var-child.exp
>> +++ b/gdb/testsuite/gdb.mi/mi2-var-child.exp
>> @@ -1162,6 +1162,10 @@ set lineno [gdb_get_line_number "anonymous type tests breakpoint"]
>>   mi_create_breakpoint \
>>       "$srcfile:$lineno" {[0-9]+} keep {do_anonymous_type_tests} \
>>       ".*var-cmd.c" $lineno $hex "break in do_anonymous_type_tests"
>> +set lineno [gdb_get_line_number "nested struct union tests breakpoint"]
>> +mi_create_breakpoint \
>> +    "$srcfile:$lineno" {[0-9]+} keep {do_nested_struct_union_tests} \
>> +    ".*var-cmd.c" $lineno $hex "break in do_nested_struct_union_tests"
>>   mi_execute_to "exec-continue" "breakpoint-hit" "do_anonymous_type_tests" ""\
>>       ".*" ".*" {"" "disp=\"keep\""} \
>>       "continue to do_anonymous_type_tests breakpoint"
>> @@ -1235,5 +1239,62 @@ set tree {
>>
>>   mi_walk_varobj_tree c $tree verify_everything
>>
>> +mi_send_resuming_command "exec-continue" \
>> +    "continuing execution to enter do_nested_struct_union_tests"
>> +mi_expect_stop "breakpoint-hit" "do_nested_struct_union_tests" ".*" ".*" ".*" \
>> +    {.* disp="keep"} "Run till MI stops in do_nested_struct_union_tests"
>> +
>> +set tree {
>> +  {struct ss} var {
>> +    {struct s_a} a1 {
>> +      int a {}
>> +    }
>> +    {struct s_b} b1 {
>> +      int b {}
>> +    }
>> +    {union u_ab} u1 {
>> +      {struct s_a} a {
>> +        int a {}
>> +      }
>> +      {struct s_b} b {
>> +        int b {}
>> +      }
>> +    }
>> +    anonymous union {
>> +      {struct s_a} a2 {
>> +        int a {}
>> +      }
>> +      {struct s_b} b2 {
>> +        int b {}
>> +      }
>> +    }
>> +    {union {...}} u2 {
>> +      {struct s_a} a3 {
>> +        int a {}
>> +      }
>> +      {struct s_b} b3 {
>> +        int b {}
>> +      }
>> +    }
>> +  }
>> +}
>> +
>> +mi_walk_varobj_tree c $tree verify_everything
>> +
>> +set tree {
>> +  {struct {...}} var2 {
>> +    {td_u_ab} ab {
>> +      {td_s_a} a {
>> +	int a {}
>> +      }
>> +      {td_s_b} b {
>> +	int b {}
>> +      }
>> +    }
>> +  }
>> +}
>> +
>> +mi_walk_varobj_tree c $tree verify_everything
>> +
>>   mi_gdb_exit
>>   return 0
>> diff --git a/gdb/testsuite/gdb.mi/var-cmd.c b/gdb/testsuite/gdb.mi/var-cmd.c
>> index 698b294..c519aa1 100644
>> --- a/gdb/testsuite/gdb.mi/var-cmd.c
>> +++ b/gdb/testsuite/gdb.mi/var-cmd.c
>> @@ -552,6 +552,70 @@ do_anonymous_type_tests (void)
>>     return; /* anonymous type tests breakpoint */
>>   }
>>
>> +void
>> +do_nested_struct_union_tests (void)
>> +{
>> +  struct s_a
>> +  {
>> +    int a;
>> +  };
>> +  struct s_b
>> +  {
>> +    int b;
>> +  };
>> +  union u_ab
>> +  {
>> +    struct s_a a;
>> +    struct s_b b;
>> +  };
>> +  struct ss
>> +  {
>> +    struct s_a a1;
>> +    struct s_b b1;
>> +    union u_ab u1;
>> +
>> +    /* Anonymous union.  */
>> +    union
>> +    {
>> +      struct s_a a2;
>> +      struct s_b b2;
>> +    };
>> +
>> +    union
>> +    {
>> +      struct s_a a3;
>> +      struct s_b b3;
>> +    } u2;
>> +  };
>> +
>> +  typedef struct
>> +  {
>> +    int a;
>> +  } td_s_a;
>> +
>> +  typedef struct
>> +  {
>> +    int b;
>> +  } td_s_b;
>> +
>> +  typedef union
>> +  {
>> +    td_s_a a;
>> +    td_s_b b;
>> +  } td_u_ab;
>> +
>> +  struct ss var;
>> +  struct
>> +  {
>> +    td_u_ab ab;
>> +  } var2;
>> +
>> +  memset (&var, 0, sizeof (var));
>> +  memset (&var2, 0, sizeof (var2));
>> +
>> +  return; /* nested struct union tests breakpoint */
>> +}
>> +
>>   int
>>   main (int argc, char *argv [])
>>   {
>> @@ -563,6 +627,7 @@ main (int argc, char *argv [])
>>     do_at_tests ();
>>     do_bitfield_tests ();
>>     do_anonymous_type_tests ();
>> +  do_nested_struct_union_tests ();
>>     exit (0);
>>   }
>>
>> diff --git a/gdb/varobj.c b/gdb/varobj.c
>> index 10ef5b7..33083b2 100644
>> --- a/gdb/varobj.c
>> +++ b/gdb/varobj.c
>> @@ -1075,12 +1075,34 @@ is_path_expr_parent (struct varobj *var)
>>     if (CPLUS_FAKE_CHILD (var))
>>       return 0;
>>
>> -  type = varobj_get_value_type (var);
>> +  type = varobj_get_gdb_type (var);
>>
>>     /* Anonymous unions and structs are also not path_expr parents.  */
>> -  return !((TYPE_CODE (type) == TYPE_CODE_STRUCT
>> -	    || TYPE_CODE (type) == TYPE_CODE_UNION)
>> -	   && TYPE_NAME (type) == NULL);
>> +  if ((TYPE_CODE (type) == TYPE_CODE_STRUCT
>> +       || TYPE_CODE (type) == TYPE_CODE_UNION)
>> +      && TYPE_NAME (type) == NULL
>> +      && TYPE_TAG_NAME (type) == NULL)
>> +    {
>> +      if (var->parent != NULL)
>> +	{
>> +	  const struct type *parent_type;
>> +
>> +	  parent_type = varobj_get_gdb_type (var->parent);
>> +	  if (TYPE_CODE (parent_type) == TYPE_CODE_STRUCT
>> +	      || TYPE_CODE (parent_type) == TYPE_CODE_UNION)
>> +	    {
>> +	      const char *field_name;
>> +
>> +	      gdb_assert (var->index < TYPE_NFIELDS (parent_type));
>> +	      field_name = TYPE_FIELD_NAME (parent_type, var->index);
>> +	      return !(field_name == NULL || *field_name == '\0');
>> +	    }
>> +	}
>> +
>> +      return 0;
>> +    }
>> +
>> +  return 1;
>>   }
>>
>>   /* Return the path expression parent for VAR.  */
>>
>
>
Vladimir Prus May 13, 2014, 11:27 a.m. UTC | #3
On 04/17/2014 04:05 PM, Andrew Burgess wrote:
> The MI command -var-info-path-expression currently does not handle some
> cases of nested structures / unions correctly, consider the following
> source file:
>
> int
> main ( void )
> {
>    struct a_t
>    {
>      int var;
>    };
>
>    struct s_t
>    {
>      struct a_t a;
>    };
>
>    struct s_t s;
>
>    return 0; /* Break Here (line 16) */
> }

> The changes to varobj.c feel like it might be possible to simplify them,
> but I can't figure out how, any change I make seems to regress something,
> so for now I'm proposing this patch.  The change is very contained anyway,
> so if someone can improve it later it's not going to cause any disruption.
>
> OK to apply?

Andrew,

thanks for the patch - this is quite a complicated area. It seems like I found a bug
though. I've modified your test function to end with:

   struct ss *ptr;

   memset (&var, 0, sizeof (var));
   memset (&var2, 0, sizeof (var2));
   ptr = &var;

and I see this interaction:

	-var-create var * ptr
	^done,name="var",numchild="5",value="0x7fffffffd860",type="struct ss *",thread-id="1",has_more="0"
	(gdb)
	-var-list-children var
	^done,numchild="5",children=[child={name="var.a1",exp="a1",numchild="1",type="struct s_a",thread-id="1"},child=	
	{name="var.b1",exp="b1",numchild="1",type="struct s_b",thread-id="1"},child={name="var.u1",exp="u1",numchild="2",type="union
	u_ab",thread-	id="1"},child={name="var.3_anonymous",exp="<anonymous union>",numchild="2",type="union {...}",thread-id="1"},child=
	{name="var.u2",exp="u2",numchild="2",type="union {...}",thread-id="1"}],has_more="0"
	(gdb)
	-var-list-children var.u2
	^done,numchild="2",children=[child={name="var.u2.a3",exp="a3",numchild="1",type="struct s_a",thread-id="1"},child=		
	{name="var.u2.b3",exp="b3",numchild="1",type="struct s_b",thread-id="1"}],has_more="0"
	(gdb)
	-var-info-path-expression var.u2.a3
	^done,path_expr="(ptr).a3"
	(gdb)

ss.u2 is a named field, so I would expect the above to produce (ptr)->u2.a3. Am I wrong, or this is a bug?

For reference, here are my notes.

Basically anonymous union or struct is this:

	struct S {
		union { int a; int b; };
	};

So the inner declaration both has no tag name for union nor declarator. Existing GDB code just
checks for tagless union, which fails for this case:

	struct S {
		union { int a; int b; } u;
	};

With your patch, we first check for tagless union, and then separately check when it has a field name,
catching the second case. I though that maybe we could directly check for empty field name, using the
code below like this.

	diff --git a/gdb/varobj.c b/gdb/varobj.c
	index 8016368..ea0bb5b 100644
	--- a/gdb/varobj.c
	+++ b/gdb/varobj.c
	@@ -1069,18 +1069,46 @@ varobj_get_gdb_type (struct varobj *var)
	 static int
	 is_path_expr_parent (struct varobj *var)
	 {
	-  struct type *type;
	+  struct type *type, *parent_type;
	+  int r1, r2;
	
	   /* "Fake" children are not path_expr parents.  */
	   if (CPLUS_FAKE_CHILD (var))
	     return 0;
	
	-  type = varobj_get_value_type (var);
	-
	-  /* Anonymous unions and structs are also not path_expr parents.  */
	-  return !((TYPE_CODE (type) == TYPE_CODE_STRUCT
	-           || TYPE_CODE (type) == TYPE_CODE_UNION)
	-          && TYPE_NAME (type) == NULL);
	+  // See whether this is anonymous structure or union, like so:
	+  // struct S {
	+  //   union { int a; int b; };
	+  // };
	+  //
	+  // In which case each member is union in injected in eclosing scope (here, S)
	+  // we need to use varobj corresponding to S to obtain path expression.
	+  //
	+  // This situation is only possible where a varobj has structure type and its
	+  // parent has structure type, and varobj does not have a field name within
	+  // its parent.
	+  if (var->parent != NULL)
	+    {
	+      int var_is_structural, parent_is_structural;
	+      type = varobj_get_value_type (var);
	+      var_is_structural = (TYPE_CODE (type) == TYPE_CODE_STRUCT
	+                          || TYPE_CODE (type) == TYPE_CODE_UNION);
	+
	+      parent_type = varobj_get_value_type (var->parent);
	+      parent_type = get_target_type (parent_type);
	+      parent_is_structural = (TYPE_CODE (parent_type) == TYPE_CODE_STRUCT
	+                             || TYPE_CODE (parent_type) == TYPE_CODE_UNION);
	+
	+      if (var_is_structural && parent_is_structural) {
	+       const char *field_name;
	+
	+       gdb_assert (var->index < TYPE_NFIELDS (parent_type));
	+       field_name = TYPE_FIELD_NAME (parent_type, var->index);
	+       if (field_name == NULL || *field_name == '\0')
	+         return 0;
	+      }
	+    }
	+  return 1;
	 }


This code failed testing pretty quickly. When we have a pointer to structure containing anonymous unions,
the parent varobj has PTR as type code. It's only inside c-varobj that we know that for listing children
of such varobj, we need to look at the structure. Therefore, my code below fails in that case. And looks
like yours code fails in a similar way.

It actually seems that the code above deals with C and C++ specific logic, but it's in varobj.c, not in
c-varobj.c, which is rather inconvenient. Inside c-varobj.c, we could have used
adjust_value_for_child_access.


Thanks,
Volodya
diff mbox

Patch

diff --git a/gdb/testsuite/gdb.mi/mi2-var-child.exp b/gdb/testsuite/gdb.mi/mi2-var-child.exp
index d2f65c5..d9f5991 100644
--- a/gdb/testsuite/gdb.mi/mi2-var-child.exp
+++ b/gdb/testsuite/gdb.mi/mi2-var-child.exp
@@ -1162,6 +1162,10 @@  set lineno [gdb_get_line_number "anonymous type tests breakpoint"]
 mi_create_breakpoint \
     "$srcfile:$lineno" {[0-9]+} keep {do_anonymous_type_tests} \
     ".*var-cmd.c" $lineno $hex "break in do_anonymous_type_tests"
+set lineno [gdb_get_line_number "nested struct union tests breakpoint"]
+mi_create_breakpoint \
+    "$srcfile:$lineno" {[0-9]+} keep {do_nested_struct_union_tests} \
+    ".*var-cmd.c" $lineno $hex "break in do_nested_struct_union_tests"
 mi_execute_to "exec-continue" "breakpoint-hit" "do_anonymous_type_tests" ""\
     ".*" ".*" {"" "disp=\"keep\""} \
     "continue to do_anonymous_type_tests breakpoint"
@@ -1235,5 +1239,62 @@  set tree {
 
 mi_walk_varobj_tree c $tree verify_everything
 
+mi_send_resuming_command "exec-continue" \
+    "continuing execution to enter do_nested_struct_union_tests"
+mi_expect_stop "breakpoint-hit" "do_nested_struct_union_tests" ".*" ".*" ".*" \
+    {.* disp="keep"} "Run till MI stops in do_nested_struct_union_tests"
+
+set tree {
+  {struct ss} var {
+    {struct s_a} a1 {
+      int a {}
+    }
+    {struct s_b} b1 {
+      int b {}
+    }
+    {union u_ab} u1 {
+      {struct s_a} a {
+        int a {}
+      }
+      {struct s_b} b {
+        int b {}
+      }
+    }
+    anonymous union {
+      {struct s_a} a2 {
+        int a {}
+      }
+      {struct s_b} b2 {
+        int b {}
+      }
+    }
+    {union {...}} u2 {
+      {struct s_a} a3 {
+        int a {}
+      }
+      {struct s_b} b3 {
+        int b {}
+      }
+    }
+  }
+}
+
+mi_walk_varobj_tree c $tree verify_everything
+
+set tree {
+  {struct {...}} var2 {
+    {td_u_ab} ab {
+      {td_s_a} a {
+	int a {}
+      }
+      {td_s_b} b {
+	int b {}
+      }
+    }
+  }
+}
+
+mi_walk_varobj_tree c $tree verify_everything
+
 mi_gdb_exit
 return 0
diff --git a/gdb/testsuite/gdb.mi/var-cmd.c b/gdb/testsuite/gdb.mi/var-cmd.c
index 698b294..c519aa1 100644
--- a/gdb/testsuite/gdb.mi/var-cmd.c
+++ b/gdb/testsuite/gdb.mi/var-cmd.c
@@ -552,6 +552,70 @@  do_anonymous_type_tests (void)
   return; /* anonymous type tests breakpoint */
 }
 
+void
+do_nested_struct_union_tests (void)
+{
+  struct s_a
+  {
+    int a;
+  };
+  struct s_b
+  {
+    int b;
+  };
+  union u_ab
+  {
+    struct s_a a;
+    struct s_b b;
+  };
+  struct ss
+  {
+    struct s_a a1;
+    struct s_b b1;
+    union u_ab u1;
+
+    /* Anonymous union.  */
+    union
+    {
+      struct s_a a2;
+      struct s_b b2;
+    };
+
+    union
+    {
+      struct s_a a3;
+      struct s_b b3;
+    } u2;
+  };
+
+  typedef struct
+  {
+    int a;
+  } td_s_a;
+
+  typedef struct
+  {
+    int b;
+  } td_s_b;
+
+  typedef union
+  {
+    td_s_a a;
+    td_s_b b;
+  } td_u_ab;
+
+  struct ss var;
+  struct
+  {
+    td_u_ab ab;
+  } var2;
+
+  memset (&var, 0, sizeof (var));
+  memset (&var2, 0, sizeof (var2));
+
+  return; /* nested struct union tests breakpoint */
+}
+
 int
 main (int argc, char *argv [])
 {
@@ -563,6 +627,7 @@  main (int argc, char *argv [])
   do_at_tests ();
   do_bitfield_tests ();
   do_anonymous_type_tests ();
+  do_nested_struct_union_tests ();
   exit (0);
 }
 
diff --git a/gdb/varobj.c b/gdb/varobj.c
index 10ef5b7..33083b2 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -1075,12 +1075,34 @@  is_path_expr_parent (struct varobj *var)
   if (CPLUS_FAKE_CHILD (var))
     return 0;
 
-  type = varobj_get_value_type (var);
+  type = varobj_get_gdb_type (var);
 
   /* Anonymous unions and structs are also not path_expr parents.  */
-  return !((TYPE_CODE (type) == TYPE_CODE_STRUCT
-	    || TYPE_CODE (type) == TYPE_CODE_UNION)
-	   && TYPE_NAME (type) == NULL);
+  if ((TYPE_CODE (type) == TYPE_CODE_STRUCT
+       || TYPE_CODE (type) == TYPE_CODE_UNION)
+      && TYPE_NAME (type) == NULL
+      && TYPE_TAG_NAME (type) == NULL)
+    {
+      if (var->parent != NULL)
+	{
+	  const struct type *parent_type;
+
+	  parent_type = varobj_get_gdb_type (var->parent);
+	  if (TYPE_CODE (parent_type) == TYPE_CODE_STRUCT
+	      || TYPE_CODE (parent_type) == TYPE_CODE_UNION)
+	    {
+	      const char *field_name;
+
+	      gdb_assert (var->index < TYPE_NFIELDS (parent_type));
+	      field_name = TYPE_FIELD_NAME (parent_type, var->index);
+	      return !(field_name == NULL || *field_name == '\0');
+	    }
+	}
+
+      return 0;
+    }
+
+  return 1;
 }
 
 /* Return the path expression parent for VAR.  */