* [RFA] mi/10586 @ 2011-11-11 21:30 Keith Seitz 2011-11-14 15:45 ` Tom Tromey 0 siblings, 1 reply; 15+ messages in thread From: Keith Seitz @ 2011-11-11 21:30 UTC (permalink / raw) To: gdb-patches@sourceware.org ml [-- Attachment #1: Type: text/plain, Size: 1295 bytes --] Hi, This bug deals with anonymous structs/unions in varobj. As in: anonymous structs/unions are unaddressable by MI clients. This bug has been sitting around for quite some time. Vladimir Prus first mentioned this in 2006 (yikes!), and this bug was filed in 2009. At this time, Nick Roberts responded to the bug with a patch that fixed the problem. I don't know why Nick never submitted the patch here, so I've tweaked his original patch and written some tests for it, and I am now submitting this. Tested on x86_64-linux. Keith ChangeLog 2011-11-11 Keith Seitz <keiths@redhat.com> Based on work by Nick Roberts <nickrob@snap.net.nz>: * varobj.c (c_describe_child): Synthesize a variable name for anonymous structs and unions. (cplus_describe_child): Likewise. testsuite/ChangeLog 2011-11-11 Keith Seitz <keiths@redhat.com> * gdb.mi/mi-var-cp.cc (anonymous_structs): New function. (class A): New class. (class B): New class. (main): Call anonymous_structs. * gdb.mi/mi-var-cp.exp: Add anonymous struct tests and adjust test results. * gdb.mi/mi2-var-cp.exp: Likewise. * gdb.mi/var-cmd.c (struct _simple_struct): Add two anonymous structs. * gdb.mi/mi-var-dislay.exp: Add anonymous struct tests and adjust test results. * gdb.mi/mi2-var-display.exp: Likewise. [-- Attachment #2: 10586.patch --] [-- Type: text/x-patch, Size: 9703 bytes --] gdb/testsuite/gdb.mi/mi-var-cp.cc | 42 +++++++++++++++++++ gdb/testsuite/gdb.mi/mi-var-cp.exp | 64 ++++++++++++++++++++++++++++++ gdb/testsuite/gdb.mi/mi-var-display.exp | 14 ++++++- gdb/testsuite/gdb.mi/mi2-var-display.exp | 14 ++++++- gdb/testsuite/gdb.mi/var-cmd.c | 8 ++++ gdb/varobj.c | 58 ++++++++++++++++++--------- 6 files changed, 179 insertions(+), 21 deletions(-) diff --git a/gdb/testsuite/gdb.mi/mi-var-cp.cc b/gdb/testsuite/gdb.mi/mi-var-cp.cc index 54439e6..8cd189f 100644 --- a/gdb/testsuite/gdb.mi/mi-var-cp.cc +++ b/gdb/testsuite/gdb.mi/mi-var-cp.cc @@ -205,6 +205,47 @@ int path_expression () /*: END: path_expression :*/ } +class A +{ +public: + struct { + int a; + float b; + }; + struct { + int c; + float d; + }; +}; + +class B : public A +{ +public: + struct { + int e; + float f; + }; + struct { + int g; + float h; + }; +}; + +static void +anonymous_structs (void) +{ + B b; + b.a = 1; + b.b = 2.2; + b.c = 3; + b.d = 4.4; + b.e = 5; + b.f = 6.6; + b.g = 7; + b.h = 8.8; /* anonymous_structs breakpoint */ + return; +} + int main () { reference_update_tests (); @@ -212,5 +253,6 @@ int main () reference_to_pointer (); reference_to_struct (); path_expression (); + anonymous_structs (); return 0; } diff --git a/gdb/testsuite/gdb.mi/mi-var-cp.exp b/gdb/testsuite/gdb.mi/mi-var-cp.exp index 4ca3a68..e905f6b 100644 --- a/gdb/testsuite/gdb.mi/mi-var-cp.exp +++ b/gdb/testsuite/gdb.mi/mi-var-cp.exp @@ -46,5 +46,69 @@ mi_run_inline_test reference_to_pointer mi_run_inline_test reference_to_struct mi_run_inline_test path_expression +# Test anonymous structures +set lineno [gdb_get_line_number "anonymous_structs breakpoint"] +mi_create_breakpoint \ + "$srcfile:$lineno" {[0-9]+} keep {anonymous_structs\(\)} \ + ".*mi-var-cp.cc" $lineno $hex "break-insert anonymous_structs" +mi_execute_to "exec-continue" "breakpoint-hit" "anonymous_structs" "" \ + ".*" ".*" {"" "disp=\"keep\""} "continue to anonymous_structs breakpoint" + +mi_create_varobj "b" "b" "create varobj for b" +mi_list_varobj_children "b" { + {b.A A 1 A} + {b.public public 2} +} "list children of b" + +mi_list_varobj_children "b.A" { + {b.A.public public 2} +} "list children of b.A" + +mi_list_varobj_children "b.A.public" { + {b.A.public.anonymous0 anonymous0 1 "struct {...}"} + {b.A.public.anonymous1 anonymous1 1 "struct {...}"} +} "list children of b.A.public" + +mi_list_varobj_children "b.A.public.anonymous0" { + {b.A.public.anonymous0.public public 2} +} "list children of b.A.public.anonymous0" + +mi_list_varobj_children "b.A.public.anonymous0.public" { + {b.A.public.anonymous0.public.a a 0 int} + {b.A.public.anonymous0.public.b b 0 float} +} "list children of b.A.public.anonymous0.public" + +mi_list_varobj_children "b.A.public.anonymous1" { + {b.A.public.anonymous1.public public 2} +} "list children of b.A.public.anonymous1" + +mi_list_varobj_children "b.A.public.anonymous1.public" { + {b.A.public.anonymous1.public.c c 0 int} + {b.A.public.anonymous1.public.d d 0 float} +} "list children of b.A.public.anonymous1.public" + +mi_list_varobj_children "b.public" { + {b.public.anonymous1 anonymous1 1 "struct {...}"} + {b.public.anonymous2 anonymous2 1 "struct {...}"} +} "list children of b.public" + +mi_list_varobj_children "b.public.anonymous1" { + {b.public.anonymous1.public public 2} +} "list children of b.public.anonymous1" + +mi_list_varobj_children "b.public.anonymous1.public" { + {b.public.anonymous1.public.e e 0 int} + {b.public.anonymous1.public.f f 0 float} +} "list children of b.public.anonymous1.public" + +mi_list_varobj_children "b.public.anonymous2" { + {b.public.anonymous2.public public 2} +} "list children of b.public.anonymous2" + +mi_list_varobj_children "b.public.anonymous2.public" { + {b.public.anonymous2.public.g g 0 int} + {b.public.anonymous2.public.h h 0 float} +} "list children of b.public.anonymous2.public" + mi_gdb_exit return 0 diff --git a/gdb/testsuite/gdb.mi/mi-var-display.exp b/gdb/testsuite/gdb.mi/mi-var-display.exp index ff27407..a56b19b 100644 --- a/gdb/testsuite/gdb.mi/mi-var-display.exp +++ b/gdb/testsuite/gdb.mi/mi-var-display.exp @@ -474,7 +474,7 @@ mi_gdb_test "-var-show-attributes s" \ # Test: c_variable-7.34 # Desc: number of children of s mi_gdb_test "-var-info-num-children s" \ - "\\^done,numchild=\"6\"" \ + "\\^done,numchild=\"8\"" \ "get number of children of s" # Test: c_variable-7.35 @@ -486,9 +486,21 @@ mi_list_varobj_children s { {s.signed_character signed_character 0 "signed char"} {s.char_ptr char_ptr 1 {char \*}} {s.array_of_10 array_of_10 10 {int \[10\]}} + {s.anonymous6 anonymous6 2 "struct {...}"} + {s.anonymous7 anonymous7 2 "struct {...}"} } "get children of s" #} {integer unsigned_integer character signed_character char_ptr array_of_10} +mi_list_varobj_children s.anonymous6 { + {s.anonymous6.a a 0 int} + {s.anonymous6.b b 0 float} +} "get childern of s.anonymous6" + +mi_list_varobj_children s.anonymous7 { + {s.anonymous7.c c 0 int} + {s.anonymous7.d d 0 float} +} "get children of s.anonymous7" + # Test: c_variable-7.40 # Desc: create anons mi_create_varobj anons anons "create local variable anons" diff --git a/gdb/testsuite/gdb.mi/mi2-var-display.exp b/gdb/testsuite/gdb.mi/mi2-var-display.exp index 828614b..dde1830 100644 --- a/gdb/testsuite/gdb.mi/mi2-var-display.exp +++ b/gdb/testsuite/gdb.mi/mi2-var-display.exp @@ -473,7 +473,7 @@ mi_gdb_test "-var-show-attributes s" \ # Test: c_variable-7.34 # Desc: number of children of s mi_gdb_test "-var-info-num-children s" \ - "\\^done,numchild=\"6\"" \ + "\\^done,numchild=\"8\"" \ "get number of children of s" # Test: c_variable-7.35 @@ -485,9 +485,21 @@ mi_list_varobj_children s { {s.signed_character signed_character 0 "signed char"} {s.char_ptr char_ptr 1 {char \*}} {s.array_of_10 array_of_10 10 {int \[10\]}} + {s.anonymous6 anonymous6 2 "struct {...}"} + {s.anonymous7 anonymous7 2 "struct {...}"} } "get children of s" #} {integer unsigned_integer character signed_character char_ptr array_of_10} +mi_list_varobj_children s.anonymous6 { + {s.anonymous6.a a 0 int} + {s.anonymous6.b b 0 float} +} "get childern of s.anonymous6" + +mi_list_varobj_children s.anonymous7 { + {s.anonymous7.c c 0 int} + {s.anonymous7.d d 0 float} +} "get children of s.anonymous7" + # Test: c_variable-7.40 # Desc: create anons mi_create_varobj anons anons "create local variable anons" diff --git a/gdb/testsuite/gdb.mi/var-cmd.c b/gdb/testsuite/gdb.mi/var-cmd.c index 71c5b9d..5b6af0e 100644 --- a/gdb/testsuite/gdb.mi/var-cmd.c +++ b/gdb/testsuite/gdb.mi/var-cmd.c @@ -26,6 +26,14 @@ struct _simple_struct { signed char signed_character; char *char_ptr; int array_of_10[10]; + struct { + int a; + float b; + }; + struct { + int c; + float d; + }; }; typedef struct _simple_struct simpleton; diff --git a/gdb/varobj.c b/gdb/varobj.c index f17ff1a..17efd1d 100644 --- a/gdb/varobj.c +++ b/gdb/varobj.c @@ -3027,26 +3027,38 @@ c_describe_child (struct varobj *parent, int index, case TYPE_CODE_STRUCT: case TYPE_CODE_UNION: - if (cname) - *cname = xstrdup (TYPE_FIELD_NAME (type, index)); + { + char *type_name, *name; - if (cvalue && value) - { - /* For C, varobj index is the same as type index. */ - *cvalue = value_struct_element_index (value, index); - } + type_name = TYPE_FIELD_NAME (type, index); + if (*type_name == '\0') + name = xstrprintf ("anonymous%d", index); + else + name = type_name; - if (ctype) - *ctype = TYPE_FIELD_TYPE (type, index); + if (cname) + *cname = xstrdup (name); - if (cfull_expression) - { - char *join = was_ptr ? "->" : "."; + if (cvalue && value) + { + /* For C, varobj index is the same as type index. */ + *cvalue = value_struct_element_index (value, index); + } - *cfull_expression = xstrprintf ("(%s)%s%s", parent_expression, join, - TYPE_FIELD_NAME (type, index)); - } + if (ctype) + *ctype = TYPE_FIELD_TYPE (type, index); + + if (cfull_expression) + { + char *join = was_ptr ? "->" : "."; + + *cfull_expression = xstrprintf ("(%s)%s%s", parent_expression, join, + name); + } + if (*type_name == '\0') + xfree (name); + } break; case TYPE_CODE_PTR: @@ -3432,6 +3444,7 @@ cplus_describe_child (struct varobj *parent, int index, enum accessibility acc = public_field; int vptr_fieldno; struct type *basetype = NULL; + char *name, *type_name; vptr_fieldno = get_vptr_fieldno (type, &basetype); if (strcmp (parent->name, "private") == 0) @@ -3450,8 +3463,14 @@ cplus_describe_child (struct varobj *parent, int index, } --type_index; + type_name = TYPE_FIELD_NAME (type, type_index); + if (*type_name == '\0') + name = xstrprintf ("anonymous%d", type_index); + else + name = type_name; + if (cname) - *cname = xstrdup (TYPE_FIELD_NAME (type, type_index)); + *cname = xstrdup (name); if (cvalue && value) *cvalue = value_struct_element_index (value, type_index); @@ -3461,9 +3480,10 @@ cplus_describe_child (struct varobj *parent, int index, if (cfull_expression) *cfull_expression - = xstrprintf ("((%s)%s%s)", parent_expression, - join, - TYPE_FIELD_NAME (type, type_index)); + = xstrprintf ("((%s)%s%s)", parent_expression, join, name); + + if (*type_name == '\0') + xfree (name); } else if (index < TYPE_N_BASECLASSES (type)) { -- 1.7.6.4 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFA] mi/10586 2011-11-11 21:30 [RFA] mi/10586 Keith Seitz @ 2011-11-14 15:45 ` Tom Tromey 2011-11-14 18:04 ` Keith Seitz 0 siblings, 1 reply; 15+ messages in thread From: Tom Tromey @ 2011-11-14 15:45 UTC (permalink / raw) To: Keith Seitz; +Cc: gdb-patches@sourceware.org ml >>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes: Keith> This bug deals with anonymous structs/unions in varobj. As in: Keith> anonymous structs/unions are unaddressable by MI clients. Keith> This bug has been sitting around for quite some time. Vladimir Prus Keith> first mentioned this in 2006 (yikes!), and this bug was filed in Keith> 2009. At this time, Nick Roberts responded to the bug with a patch Keith> that fixed the problem. Keith> I don't know why Nick never submitted the patch here, so I've tweaked Keith> his original patch and written some tests for it, and I am now Keith> submitting this. Thanks. Keith> 2011-11-11 Keith Seitz <keiths@redhat.com> Keith> Based on work by Nick Roberts <nickrob@snap.net.nz>: Keith> * varobj.c (c_describe_child): Synthesize a variable name for Keith> anonymous structs and unions. Keith> (cplus_describe_child): Likewise. ChangeLog should mention the PR. Keith> + type_name = TYPE_FIELD_NAME (type, index); Keith> + if (*type_name == '\0') Keith> + name = xstrprintf ("anonymous%d", index); Keith> + else Keith> + name = type_name; What happens if there is a clash between a named field's name and the generated name for an anonymous field? Tom ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFA] mi/10586 2011-11-14 15:45 ` Tom Tromey @ 2011-11-14 18:04 ` Keith Seitz 2011-11-14 18:18 ` Tom Tromey 0 siblings, 1 reply; 15+ messages in thread From: Keith Seitz @ 2011-11-14 18:04 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches@sourceware.org ml On 11/14/2011 07:45 AM, Tom Tromey wrote: > ChangeLog should mention the PR. That would be useful, no? I don't know why I sometimes forget. I've added that. > Keith> + type_name = TYPE_FIELD_NAME (type, index); > Keith> + if (*type_name == '\0') > Keith> + name = xstrprintf ("anonymous%d", index); > Keith> + else > Keith> + name = type_name; > > What happens if there is a clash between a named field's name and the > generated name for an anonymous field? Ha. My guess: bad things (TM). Given that variables cannot start with numbers, I've changed the name string to "%d_anonymous". Other comments? Keith ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFA] mi/10586 2011-11-14 18:04 ` Keith Seitz @ 2011-11-14 18:18 ` Tom Tromey 2011-11-14 19:28 ` Keith Seitz 0 siblings, 1 reply; 15+ messages in thread From: Tom Tromey @ 2011-11-14 18:18 UTC (permalink / raw) To: Keith Seitz; +Cc: gdb-patches@sourceware.org ml >>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes: Keith> Given that variables cannot start with numbers, I've changed the name Keith> string to "%d_anonymous". Keith> Other comments? It also seems like it will do the wrong thing for -var-info-expression and -var-info-path-expression. Tom ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFA] mi/10586 2011-11-14 18:18 ` Tom Tromey @ 2011-11-14 19:28 ` Keith Seitz 2011-11-14 20:28 ` Tom Tromey 0 siblings, 1 reply; 15+ messages in thread From: Keith Seitz @ 2011-11-14 19:28 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches@sourceware.org ml On 11/14/2011 10:17 AM, Tom Tromey wrote: > It also seems like it will do the wrong thing for -var-info-expression and > -var-info-path-expression. Hmm. Yeah, those will need special handling. Consider: struct a { struct { int b; }; struct { int c; }; }; -var-create a * a -var-list-children a = "public" (if c++) -var-list-children a.public = "0_anonymous", "1_anonymous" -var-list-children a.public.0_anonymous = "b" (via "public" if c++) -var-list-children a.public.1_anonymous = "c" (via "public" if c++) So far, so good, I think. Now: -var-info-expression a = "a" -var-info-expression a.public = "public" -var-info-expression a.public.0_anonymous = "0_anonymous" -var-info-expression a.public.0_anonymous.b = "b" According to the documentation, -var-info-expression is supposed to return a name of the variable/child which is to be presented to the user. I don't think we want to present "0_anonymous". GCC uses "<anonymous struct>", and that seems like a reasonable convention to follow. -var-info-path-expression a = "a" -var-info-path-expression a.public = "" -var-info-path-expression a.public.0_anonymous = "((a).2_anonymous)" -var-info-path-expression a.public.0_anonymous.b = "((((a).0_anonymous)).b)" The documentation on this command says that it should return a valid expression that may be used, e.g., to create another varobj or to set a watchpoint. Clearly the two last elements dealing with 0_anonymous are incorrect. I believe these should be: -var-info-path-expression a.public.0_anonymous = "" -var-info-path-expression a.public.0_anonymous.b = "((a).b)" If we can agree what to do, I shall set about implementing the desired change. Keith ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFA] mi/10586 2011-11-14 19:28 ` Keith Seitz @ 2011-11-14 20:28 ` Tom Tromey 2011-11-15 17:10 ` Keith Seitz 0 siblings, 1 reply; 15+ messages in thread From: Tom Tromey @ 2011-11-14 20:28 UTC (permalink / raw) To: Keith Seitz; +Cc: gdb-patches@sourceware.org ml >>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes: Keith> According to the documentation, -var-info-expression is supposed to Keith> return a name of the variable/child which is to be presented to the Keith> user. I don't think we want to present "0_anonymous". GCC uses Keith> "<anonymous struct>", and that seems like a reasonable convention to Keith> follow. Actually, thinking about it more, it seems to me that it would be ok for these cases to just be errors. There's no really good way to refer to the anonymous field as its own entity, and I don't think we should hack up the parser and whatever else to support this. Keith> Clearly the two last elements dealing with 0_anonymous are Keith> incorrect. I believe these should be: Keith> -var-info-path-expression a.public.0_anonymous = "" This one, I think should be an error. Keith> -var-info-path-expression a.public.0_anonymous.b = "((a).b)" But I agree about this one. What do you think? Tom ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFA] mi/10586 2011-11-14 20:28 ` Tom Tromey @ 2011-11-15 17:10 ` Keith Seitz 2011-11-15 17:30 ` Tom Tromey 2011-12-02 22:28 ` Keith Seitz 0 siblings, 2 replies; 15+ messages in thread From: Keith Seitz @ 2011-11-15 17:10 UTC (permalink / raw) To: Tom Tromey; +Cc: gdb-patches@sourceware.org ml On 11/14/2011 12:28 PM, Tom Tromey wrote: > Actually, thinking about it more, it seems to me that it would be ok for > these cases to just be errors. There's no really good way to refer to > the anonymous field as its own entity, and I don't think we should hack > up the parser and whatever else to support this. An error... I'm not so sure that I like that, but to be honest, I'm not sure I like/dislike it sufficiently to argue about it. > Keith> Clearly the two last elements dealing with 0_anonymous are > Keith> incorrect. I believe these should be: > Keith> -var-info-path-expression a.public.0_anonymous = "" > > This one, I think should be an error. > Yes, that one could be an error. I was just mirroring what the "fake" children currently do. (-var-info-path-expression a.public = ""). > Keith> -var-info-path-expression a.public.0_anonymous.b = "((a).b)" > > But I agree about this one. Ok, so we're close. This is probably the "trickier" bit to get correct, so I can start on writing some more elaborate tests for this. Keith ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFA] mi/10586 2011-11-15 17:10 ` Keith Seitz @ 2011-11-15 17:30 ` Tom Tromey 2011-12-02 22:28 ` Keith Seitz 1 sibling, 0 replies; 15+ messages in thread From: Tom Tromey @ 2011-11-15 17:30 UTC (permalink / raw) To: Keith Seitz; +Cc: gdb-patches@sourceware.org ml >>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes: Keith> On 11/14/2011 12:28 PM, Tom Tromey wrote: >> Actually, thinking about it more, it seems to me that it would be ok for >> these cases to just be errors. There's no really good way to refer to >> the anonymous field as its own entity, and I don't think we should hack >> up the parser and whatever else to support this. Keith> An error... I'm not so sure that I like that, but to be honest, I'm Keith> not sure I like/dislike it sufficiently to argue about it. I guess it could be done by returning something like: *(struct whatever*) ((char *) original_expr + offset) That is only mildly horrible. Keith> Yes, that one could be an error. I was just mirroring what the "fake" Keith> children currently do. (-var-info-path-expression a.public = ""). Oh, ok. That is fine then. Tom ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFA] mi/10586 2011-11-15 17:10 ` Keith Seitz 2011-11-15 17:30 ` Tom Tromey @ 2011-12-02 22:28 ` Keith Seitz 2011-12-13 1:28 ` Keith Seitz ` (2 more replies) 1 sibling, 3 replies; 15+ messages in thread From: Keith Seitz @ 2011-12-02 22:28 UTC (permalink / raw) To: gdb-patches@sourceware.org ml [-- Attachment #1: Type: text/plain, Size: 2124 bytes --] On 11/15/2011 09:10 AM, Keith Seitz wrote: > On 11/14/2011 12:28 PM, Tom Tromey wrote: > >> Actually, thinking about it more, it seems to me that it would be ok for >> these cases to just be errors. There's no really good way to refer to >> the anonymous field as its own entity, and I don't think we should hack >> up the parser and whatever else to support this. > > An error... I'm not so sure that I like that, but to be honest, I'm not > sure I like/dislike it sufficiently to argue about it. Ok, so in the end, I've decided against making this an error. Instead, what I've implemented is analogous to how varobj/MI currently deal with the CPLUS_FAKE_CHILD cases: path expressions and the like are "". If you would like me to do otherwise, please let me know. I am attaching the latest incarnation of this patch, which covers all these cases (AFAIK) and utilizes the new varobj tree testing that I committed last week. What do you think? Keith ChangeLog 2011-12-02 Keith Seitz <keiths@redhat.com> PR mi/10586 Based on work by Nick Roberts <nickrob@snap.net.nz>: * varobj.c [ANONYMOUS_STRUCT_NAME]: New constant. [ANONYMOUS_UNION_NAME]: Likewise. (is_anonymous_child): New function. (create_child_with_value): If the child is nameless, synthesize a name for it. (c_describe_child): If the child has no field or type names for a union or sturct, use ANONYMOUS_STRUCT_NAME or ANONYMOUS_UNION_NAME. Set path expression for these fields to the empty string. (cplus_describe_child): Likewise. (is_path_expr_parent): New function. (get_path_expr_parent): New function. testsuite/ChangeLog 2011-12-02 Keith Seitz <keiths@redhat.com> PR mi/10586 * gdb.mi/mi-var-cp.cc (anonymous_structs): New function. (class A): New class. (class B): New class. (main): Call anonymous_structs. * gdb.mi/mi-var-cp.exp: Add anonymous struct tests and adjust test results. * gdb.mi/mi2-var-cp.exp: Likewise. * gdb.mi/var-cmd.c (struct _simple_struct): Add two anonymous structs. * gdb.mi/mi-var-dislay.exp: Add anonymous struct tests and adjust test results. * gdb.mi/mi2-var-display.exp: Likewise. [-- Attachment #2: 10586-2.patch --] [-- Type: text/x-patch, Size: 13916 bytes --] diff --git a/gdb/testsuite/gdb.mi/mi-var-cp.cc b/gdb/testsuite/gdb.mi/mi-var-cp.cc index 54439e6..51d30cf 100644 --- a/gdb/testsuite/gdb.mi/mi-var-cp.cc +++ b/gdb/testsuite/gdb.mi/mi-var-cp.cc @@ -205,6 +205,51 @@ int path_expression () /*: END: path_expression :*/ } +class Anonymous +{ +public: + struct { /* index: 0 */ + int b; + }; + struct { /* index: 1 */ + int c; + }; + struct { /* index: 2 */ + int d; + struct { /* index: 1 */ + int e; + struct { /* index: 0 */ + int f; + union { /* index: 0 */ + int g; + char h; + }; + }; + union { /* index: 0 */ + int i; + char j; + }; + }; + }; +}; + +/* Test anonymous structs and unions. */ +int +anonymous_structs_and_unions (void) +{ + Anonymous a; + a.b = 1; + a.c = 2; + a.d = 3; + a.e = 4; + a.f = 5; + a.g = 6; + a.h = '7'; + a.i = 8; + a.j = '8'; + return 0; /* anonymous_structs_and_unions */ +} + int main () { reference_update_tests (); @@ -212,5 +257,6 @@ int main () reference_to_pointer (); reference_to_struct (); path_expression (); + anonymous_structs_and_unions (); return 0; } diff --git a/gdb/testsuite/gdb.mi/mi-var-cp.exp b/gdb/testsuite/gdb.mi/mi-var-cp.exp index 4ca3a68..258a3af 100644 --- a/gdb/testsuite/gdb.mi/mi-var-cp.exp +++ b/gdb/testsuite/gdb.mi/mi-var-cp.exp @@ -46,5 +46,78 @@ mi_run_inline_test reference_to_pointer mi_run_inline_test reference_to_struct mi_run_inline_test path_expression +set lineno [gdb_get_line_number "/* anonymous_structs_and_unions */"] +mi_create_breakpoint \ + "$srcfile:$lineno" {[0-9]+} keep {anonymous_structs_and_unions\(\)} \ + ".*mi-var-cp.cc" $lineno $hex "break in anonymous_structs_and_unions" +mi_execute_to "exec-continue" "breakpoint-hit" \ + "anonymous_structs_and_unions" "" ".*" ".*" {"" "disp=\"keep\""} \ + "continue to anonymous_structs breakpoint" + +set tree { + Anonymous a { + {} public { + anonymous struct { + {} public { + int b {} + } + } + anonymous struct { + {} public { + int c {} + } + } + anonymous struct { + {} public { + int d {} + anonymous struct { + {} public { + int e {} + anonymous struct { + {} public { + int f {} + anonymous union { + {} public { + int g {} + char h {} + } + } + } + } + anonymous union { + {} public { + int i {} + char j {} + } + } + } + } + } + } + } + } +} + +proc verify_everything {variable_name} { + # Test -var-list-children + mi_varobj_tree_test_children_callback $variable_name + + # Bring the variable named by VARIABLE_NAME into the current scope + # in VAROBJ. + upvar #0 $variable_name varobj + + # Test -var-info-path-expression + mi_gdb_test "-var-info-path-expression $varobj(obj_name)" \ + "\\^done,path_expr=\"[string_to_regexp $varobj(path_expr)]\"" \ + "path expression for $varobj(obj_name)" + + # Test -var-info-expression + mi_gdb_test "-var-info-expression $varobj(obj_name)" \ + "\\^done,lang=\"C\\+\\+\",exp=\"[string_to_regexp $varobj(display_name)]\"" \ + "expression for $varobj(obj_name)" +} + +mi_walk_varobj_tree $tree verify_everything + mi_gdb_exit return 0 diff --git a/gdb/testsuite/gdb.mi/mi2-var-child.exp b/gdb/testsuite/gdb.mi/mi2-var-child.exp index 0f9b4d4..a940a00 100644 --- a/gdb/testsuite/gdb.mi/mi2-var-child.exp +++ b/gdb/testsuite/gdb.mi/mi2-var-child.exp @@ -1,4 +1,5 @@ -# Copyright 1999, 2000, 2001, 2002, 2003, 2004, 2009 Free Software Foundation +# Copyright 1999, 2000, 2001, 2002, 2003, 2004, 2009, 2011 +# Free Software Foundation # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -1139,7 +1140,69 @@ mi_gdb_test "-var-update *" \ "update all vars psnp->next->next->long_ptr (and 2.long_ptr) changed" +# Anonymous type tests +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" +mi_execute_to "exec-continue" "breakpoint-hit" "do_anonymous_type_tests" ""\ + ".*" ".*" {"" "disp=\"keep\""} \ + "continue to do_anonymous_type_tests breakpoint" + +# Run the varobj tree on variable "ptr". +set tree { + {struct anonymous **} ptr { + {struct anonymous *} {*ptr} { + int a {} + anonymous struct { + int b {} + {char *} c { + char {*c} {} + } + anonymous union { + int d {} + {void *} e {} + char f {} + anonymous struct { + char g {} + {const char **} h { + {const char *} {*h} { + {const char} {**h} {} + } + } + {simpleton ***} simple { + {simpleton **} {*simple} { + {simpleton *} {**simple} { + int integer {} + {unsigned int} unsigned_integer {} + char character {} + {signed char} signed_character {} + {char *} char_ptr { + char {*char_ptr} {} + } + {int [10]} array_of_10 { + int 0 {} + int 1 {} + int 2 {} + int 3 {} + int 4 {} + int 5 {} + int 6 {} + int 7 {} + int 8 {} + int 9 {} + } + } + } + } + } + } + } + } + } +} +mi_walk_varobj_tree $tree mi_gdb_exit return 0 diff --git a/gdb/testsuite/gdb.mi/var-cmd.c b/gdb/testsuite/gdb.mi/var-cmd.c index 71c5b9d..b048f45 100644 --- a/gdb/testsuite/gdb.mi/var-cmd.c +++ b/gdb/testsuite/gdb.mi/var-cmd.c @@ -92,6 +92,24 @@ struct _struct_n_pointer { struct _struct_n_pointer *next; }; +struct anonymous { + int a; + struct { + int b; + char *c; + union { + int d; + void *e; + char f; + struct { + char g; + const char **h; + simpleton ***simple; + }; + }; + }; +}; + void do_locals_tests (void); void do_block_tests (void); void subroutine1 (int, long *); @@ -503,6 +521,26 @@ void do_bitfield_tests () /*: END: bitfield :*/ } +void +do_anonymous_type_tests (void) +{ + struct anonymous *anon; + struct anonymous **ptr; + + anon = malloc (sizeof (struct anonymous)); + anon->a = 1; + anon->b = 2; + anon->c = (char *) 3; + anon->d = 4; + anon->g = '5'; + anon->h = (const char **) 6; + anon->simple = (simpleton ***) 7; + + ptr = &anon; + free (anon); + return; /* anonymous type tests breakpoint */ +} + int main (int argc, char *argv []) { @@ -513,6 +551,7 @@ main (int argc, char *argv []) do_frozen_tests (); do_at_tests (); do_bitfield_tests (); + do_anonymous_type_tests (); exit (0); } diff --git a/gdb/varobj.c b/gdb/varobj.c index 7c68a93..8da744b 100644 --- a/gdb/varobj.c +++ b/gdb/varobj.c @@ -43,6 +43,12 @@ typedef int PyObject; #endif +/* The names of varobjs representing anonymous structs or unions. + Note that is_name_anonymous_type makes assumptions about these two + constants. */ +#define ANONYMOUS_STRUCT_NAME _("<anonymous struct>") +#define ANONYMOUS_UNION_NAME _("<anonymous union>") + /* Non-zero if we want to see trace of varobj level stuff. */ int varobjdebug = 0; @@ -1289,6 +1295,39 @@ varobj_get_gdb_type (struct varobj *var) return var->type; } +/* Is VAR a path expression parent, i.e., can it be used to construct + a valid path expression? */ + +static int +is_path_expr_parent (struct varobj *var) +{ + struct type *type; + + /* "Fake" children are not path_expr parents. */ + if (CPLUS_FAKE_CHILD (var)) + return 0; + + type = 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); +} + +/* Return the path expression parent for VAR. */ + +static struct varobj * +get_path_expr_parent (struct varobj *var) +{ + struct varobj *parent = var; + + while (!is_path_expr_parent (parent)) + parent = parent->parent; + + return parent; +} + /* Return a pointer to the full rooted expression of varobj VAR. If it has not been computed yet, compute it. */ char * @@ -2170,6 +2209,19 @@ create_child (struct varobj *parent, int index, char *name) value_of_child (parent, index)); } +/* Does CHILD represent a child with no name? This happens when + the child is an anonmous struct or union and it has no field name + in its parent variable. + + This has already been determined by *_describe_child. The easiest + thing to do is see if the child's name begins with "<anonymous ". */ + +static int +is_anonymous_child (struct varobj *child) +{ + return strncmp (child->name, ANONYMOUS_STRUCT_NAME, 11) == 0; +} + static struct varobj * create_child_with_value (struct varobj *parent, int index, const char *name, struct value *value) @@ -2185,8 +2237,13 @@ create_child_with_value (struct varobj *parent, int index, const char *name, child->index = index; child->parent = parent; child->root = parent->root; - childs_name = xstrprintf ("%s.%s", parent->obj_name, name); + + if (is_anonymous_child (child)) + childs_name = xstrprintf ("%s.%d_anonymous", parent->obj_name, index); + else + childs_name = xstrprintf ("%s.%s", parent->obj_name, name); child->obj_name = childs_name; + install_variable (child); /* Compute the type of the child. Must do this before @@ -2955,7 +3012,7 @@ c_describe_child (struct varobj *parent, int index, if (cfull_expression) { *cfull_expression = NULL; - parent_expression = varobj_get_path_expr (parent); + parent_expression = varobj_get_path_expr (get_path_expr_parent (parent)); } adjust_value_for_child_access (&value, &type, &was_ptr); @@ -2990,26 +3047,49 @@ c_describe_child (struct varobj *parent, int index, case TYPE_CODE_STRUCT: case TYPE_CODE_UNION: - if (cname) - *cname = xstrdup (TYPE_FIELD_NAME (type, index)); + { + char *field_name; - if (cvalue && value) - { - /* For C, varobj index is the same as type index. */ - *cvalue = value_struct_element_index (value, index); - } + /* If the type is anonymous and the field has no name, + set an appropriate name. */ + field_name = TYPE_FIELD_NAME (type, index); + if (*field_name == '\0') + { + if (cname) + { + if (TYPE_CODE (TYPE_FIELD_TYPE (type, index)) + == TYPE_CODE_STRUCT) + *cname = xstrdup (ANONYMOUS_STRUCT_NAME); + else + *cname = xstrdup (ANONYMOUS_UNION_NAME); + } - if (ctype) - *ctype = TYPE_FIELD_TYPE (type, index); + if (cfull_expression) + *cfull_expression = xstrdup (""); + } + else + { + if (cname) + *cname = xstrdup (field_name); - if (cfull_expression) - { - char *join = was_ptr ? "->" : "."; + if (cfull_expression) + { + char *join = was_ptr ? "->" : "."; - *cfull_expression = xstrprintf ("(%s)%s%s", parent_expression, join, - TYPE_FIELD_NAME (type, index)); - } + *cfull_expression = xstrprintf ("(%s)%s%s", parent_expression, + join, field_name); + } + } + if (cvalue && value) + { + /* For C, varobj index is the same as type index. */ + *cvalue = value_struct_element_index (value, index); + } + + if (ctype) + *ctype = TYPE_FIELD_TYPE (type, index); + } break; case TYPE_CODE_PTR: @@ -3357,14 +3437,16 @@ cplus_describe_child (struct varobj *parent, int index, value = parent->parent->value; type = get_value_type (parent->parent); if (cfull_expression) - parent_expression = varobj_get_path_expr (parent->parent); + parent_expression + = varobj_get_path_expr (get_path_expr_parent (parent->parent)); } else { value = parent->value; type = get_value_type (parent); if (cfull_expression) - parent_expression = varobj_get_path_expr (parent); + parent_expression + = varobj_get_path_expr (get_path_expr_parent (parent)); } adjust_value_for_child_access (&value, &type, &was_ptr); @@ -3386,6 +3468,7 @@ cplus_describe_child (struct varobj *parent, int index, enum accessibility acc = public_field; int vptr_fieldno; struct type *basetype = NULL; + char *field_name; vptr_fieldno = get_vptr_fieldno (type, &basetype); if (strcmp (parent->name, "private") == 0) @@ -3404,20 +3487,40 @@ cplus_describe_child (struct varobj *parent, int index, } --type_index; - if (cname) - *cname = xstrdup (TYPE_FIELD_NAME (type, type_index)); + /* If the type is anonymous and the field has no name, + set an appopriate name. */ + field_name = TYPE_FIELD_NAME (type, type_index); + if (*field_name == '\0') + { + if (cname) + { + if (TYPE_CODE (TYPE_FIELD_TYPE (type, type_index)) + == TYPE_CODE_STRUCT) + *cname = xstrdup (ANONYMOUS_STRUCT_NAME); + else if (TYPE_CODE (TYPE_FIELD_TYPE (type, type_index)) + == TYPE_CODE_UNION) + *cname = xstrdup (ANONYMOUS_UNION_NAME); + } + + if (cfull_expression) + *cfull_expression = xstrdup (""); + } + else + { + if (cname) + *cname = xstrdup (TYPE_FIELD_NAME (type, type_index)); + + if (cfull_expression) + *cfull_expression + = xstrprintf ("((%s)%s%s)", parent_expression, join, + field_name); + } if (cvalue && value) *cvalue = value_struct_element_index (value, type_index); if (ctype) *ctype = TYPE_FIELD_TYPE (type, type_index); - - if (cfull_expression) - *cfull_expression - = xstrprintf ("((%s)%s%s)", parent_expression, - join, - TYPE_FIELD_NAME (type, type_index)); } else if (index < TYPE_N_BASECLASSES (type)) { ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFA] mi/10586 2011-12-02 22:28 ` Keith Seitz @ 2011-12-13 1:28 ` Keith Seitz 2011-12-13 19:58 ` Tom Tromey 2011-12-13 20:34 ` Crash regression with Eclipse [Re: [RFA] mi/10586] Jan Kratochvil 2 siblings, 0 replies; 15+ messages in thread From: Keith Seitz @ 2011-12-13 1:28 UTC (permalink / raw) To: gdb-patches@sourceware.org ml Ping On 12/02/2011 02:28 PM, Keith Seitz wrote: > On 11/15/2011 09:10 AM, Keith Seitz wrote: >> On 11/14/2011 12:28 PM, Tom Tromey wrote: >> >>> Actually, thinking about it more, it seems to me that it would be ok for >>> these cases to just be errors. There's no really good way to refer to >>> the anonymous field as its own entity, and I don't think we should hack >>> up the parser and whatever else to support this. >> >> An error... I'm not so sure that I like that, but to be honest, I'm not >> sure I like/dislike it sufficiently to argue about it. > > Ok, so in the end, I've decided against making this an error. Instead, > what I've implemented is analogous to how varobj/MI currently deal with > the CPLUS_FAKE_CHILD cases: path expressions and the like are "". > > If you would like me to do otherwise, please let me know. > > I am attaching the latest incarnation of this patch, which covers all > these cases (AFAIK) and utilizes the new varobj tree testing that I > committed last week. > > What do you think? > > Keith > > ChangeLog > 2011-12-02 Keith Seitz <keiths@redhat.com> > > PR mi/10586 > Based on work by Nick Roberts <nickrob@snap.net.nz>: > * varobj.c [ANONYMOUS_STRUCT_NAME]: New constant. > [ANONYMOUS_UNION_NAME]: Likewise. > (is_anonymous_child): New function. > (create_child_with_value): If the child is nameless, > synthesize a name for it. > (c_describe_child): If the child has no field or type names > for a union or sturct, use ANONYMOUS_STRUCT_NAME or > ANONYMOUS_UNION_NAME. > Set path expression for these fields to the empty string. > (cplus_describe_child): Likewise. > (is_path_expr_parent): New function. > (get_path_expr_parent): New function. > > testsuite/ChangeLog > 2011-12-02 Keith Seitz <keiths@redhat.com> > > PR mi/10586 > * gdb.mi/mi-var-cp.cc (anonymous_structs): New function. > (class A): New class. > (class B): New class. > (main): Call anonymous_structs. > * gdb.mi/mi-var-cp.exp: Add anonymous struct tests and > adjust test results. > * gdb.mi/mi2-var-cp.exp: Likewise. > * gdb.mi/var-cmd.c (struct _simple_struct): Add two anonymous > structs. > * gdb.mi/mi-var-dislay.exp: Add anonymous struct tests and > adjust test results. > * gdb.mi/mi2-var-display.exp: Likewise. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFA] mi/10586 2011-12-02 22:28 ` Keith Seitz 2011-12-13 1:28 ` Keith Seitz @ 2011-12-13 19:58 ` Tom Tromey 2011-12-17 1:55 ` Keith Seitz 2011-12-13 20:34 ` Crash regression with Eclipse [Re: [RFA] mi/10586] Jan Kratochvil 2 siblings, 1 reply; 15+ messages in thread From: Tom Tromey @ 2011-12-13 19:58 UTC (permalink / raw) To: Keith Seitz; +Cc: gdb-patches@sourceware.org ml >>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes: Keith> +/* The names of varobjs representing anonymous structs or unions. Keith> + Note that is_name_anonymous_type makes assumptions about these two Keith> + constants. */ I didn't see 'is_name_anonymous_type' anywhere in the tree or in the patch. I guess it needs an update. Keith> +static int Keith> +is_path_expr_parent (struct varobj *var) [...] Keith> + type = get_value_type (var); Extra space. Keith> + return strncmp (child->name, ANONYMOUS_STRUCT_NAME, 11) == 0; I think this is wrong since the macros have _() in their expansion. I think you have to use strlen. Keith> + field_name = TYPE_FIELD_NAME (type, index); Keith> + if (*field_name == '\0') Can field_name == NULL? It is not clear to me. There is some code in gdb that checks this, but I don't know whether that is defensive programming or checking a condition that is truly possible. Otherwise this looks good to me. Tom ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFA] mi/10586 2011-12-13 19:58 ` Tom Tromey @ 2011-12-17 1:55 ` Keith Seitz 2011-12-20 16:03 ` Tom Tromey 0 siblings, 1 reply; 15+ messages in thread From: Keith Seitz @ 2011-12-17 1:55 UTC (permalink / raw) To: gdb-patches@sourceware.org ml [-- Attachment #1: Type: text/plain, Size: 2930 bytes --] On 12/13/2011 11:39 AM, Tom Tromey wrote: > I didn't see 'is_name_anonymous_type' anywhere in the tree or in the patch. > I guess it needs an update. I hate it when that happens! Comment updated. > Keith> +static int > Keith> +is_path_expr_parent (struct varobj *var) > [...] > Keith> + type = get_value_type (var); > > Extra space. Fixed. > Keith> + return strncmp (child->name, ANONYMOUS_STRUCT_NAME, 11) == 0; > > I think this is wrong since the macros have _() in their expansion. > I think you have to use strlen. Yeah, very wrong indeed! I've fixed that test. > Keith> + field_name = TYPE_FIELD_NAME (type, index); > Keith> + if (*field_name == '\0') > > Can field_name == NULL? > It is not clear to me. There is some code in gdb that checks this, but > I don't know whether that is defensive programming or checking a > condition that is truly possible. Comments in gdbtypes.h seem to indicate that name could be NULL, but I've not seen that. To be defensive, I've treated name == NULL and name == "" the same. On 12/13/2011 11:58 AM, Jan Kratochvil wrote: > Program received signal SIGSEGV, Segmentation fault. > 0x00000000007ab58b in get_value_type (var=0x0) at varobj.c:2397 > 2397 if (var->value) > (gdb) bt > #0 in get_value_type (var=0x0) at varobj.c:2397 > #1 in is_path_expr_parent (var=0x0) at varobj.c:1310 > #2 in get_path_expr_parent (var=0x2cbbd80) at varobj.c:1325 > #3 in c_describe_child (parent=0x2cbbd80, index=0, cname=0x0, cvalue=0x0, ctype=0x0, cfull_expression=0x2cbf5f8) at varobj.c:3015 > #4 in c_path_expr_of_child (child=0x2cbf5f0) at varobj.c:3140 [snip] I've fixed this, too, and added a test for that. A very simple omission: get_path_expr_parent was attempting to walk the parent chain past the root variable. Whoops. Keith ChangeLog 2011-12-16 Keith Seitz <keiths@redhat.com> 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. testsuite/ChangeLog 2011-12-16 Keith Seitz <keiths@redhat.com> 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. [-- Attachment #2: 10586-3.patch --] [-- Type: text/x-patch, Size: 14992 bytes --] diff --git a/gdb/testsuite/gdb.mi/mi-var-cp.cc b/gdb/testsuite/gdb.mi/mi-var-cp.cc index 54439e6..51d30cf 100644 --- a/gdb/testsuite/gdb.mi/mi-var-cp.cc +++ b/gdb/testsuite/gdb.mi/mi-var-cp.cc @@ -205,6 +205,51 @@ int path_expression () /*: END: path_expression :*/ } +class Anonymous +{ +public: + struct { /* index: 0 */ + int b; + }; + struct { /* index: 1 */ + int c; + }; + struct { /* index: 2 */ + int d; + struct { /* index: 1 */ + int e; + struct { /* index: 0 */ + int f; + union { /* index: 0 */ + int g; + char h; + }; + }; + union { /* index: 0 */ + int i; + char j; + }; + }; + }; +}; + +/* Test anonymous structs and unions. */ +int +anonymous_structs_and_unions (void) +{ + Anonymous a; + a.b = 1; + a.c = 2; + a.d = 3; + a.e = 4; + a.f = 5; + a.g = 6; + a.h = '7'; + a.i = 8; + a.j = '8'; + return 0; /* anonymous_structs_and_unions */ +} + int main () { reference_update_tests (); @@ -212,5 +257,6 @@ int main () reference_to_pointer (); reference_to_struct (); path_expression (); + anonymous_structs_and_unions (); return 0; } diff --git a/gdb/testsuite/gdb.mi/mi-var-cp.exp b/gdb/testsuite/gdb.mi/mi-var-cp.exp index 4ca3a68..4fea332 100644 --- a/gdb/testsuite/gdb.mi/mi-var-cp.exp +++ b/gdb/testsuite/gdb.mi/mi-var-cp.exp @@ -46,5 +46,78 @@ mi_run_inline_test reference_to_pointer mi_run_inline_test reference_to_struct mi_run_inline_test path_expression +set lineno [gdb_get_line_number "/* anonymous_structs_and_unions */"] +mi_create_breakpoint \ + "$srcfile:$lineno" {[0-9]+} keep {anonymous_structs_and_unions\(\)} \ + ".*mi-var-cp.cc" $lineno $hex "break in anonymous_structs_and_unions" +mi_execute_to "exec-continue" "breakpoint-hit" \ + "anonymous_structs_and_unions" "" ".*" ".*" {"" "disp=\"keep\""} \ + "continue to anonymous_structs breakpoint" + +set tree { + Anonymous a { + {} public { + anonymous struct { + {} public { + int b {} + } + } + anonymous struct { + {} public { + int c {} + } + } + anonymous struct { + {} public { + int d {} + anonymous struct { + {} public { + int e {} + anonymous struct { + {} public { + int f {} + anonymous union { + {} public { + int g {} + char h {} + } + } + } + } + anonymous union { + {} public { + int i {} + char j {} + } + } + } + } + } + } + } + } +} + +proc verify_everything {variable_name} { + # Test -var-list-children + mi_varobj_tree_test_children_callback $variable_name + + # Bring the variable named by VARIABLE_NAME into the current scope + # in VAROBJ. + upvar #0 $variable_name varobj + + # Test -var-info-path-expression + mi_gdb_test "-var-info-path-expression $varobj(obj_name)" \ + "\\^done,path_expr=\"[string_to_regexp $varobj(path_expr)]\"" \ + "path expression for $varobj(obj_name)" + + # Test -var-info-expression + mi_gdb_test "-var-info-expression $varobj(obj_name)" \ + "\\^done,lang=\"C\\+\\+\",exp=\"[string_to_regexp $varobj(display_name)]\"" \ + "expression for $varobj(obj_name)" +} + +mi_walk_varobj_tree c++ $tree verify_everything + mi_gdb_exit return 0 diff --git a/gdb/testsuite/gdb.mi/mi2-var-child.exp b/gdb/testsuite/gdb.mi/mi2-var-child.exp index 0f9b4d4..740a79f 100644 --- a/gdb/testsuite/gdb.mi/mi2-var-child.exp +++ b/gdb/testsuite/gdb.mi/mi2-var-child.exp @@ -1,4 +1,5 @@ -# Copyright 1999, 2000, 2001, 2002, 2003, 2004, 2009 Free Software Foundation +# Copyright 1999, 2000, 2001, 2002, 2003, 2004, 2009, 2011 +# Free Software Foundation # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License as published by @@ -1139,7 +1140,102 @@ mi_gdb_test "-var-update *" \ "update all vars psnp->next->next->long_ptr (and 2.long_ptr) changed" +# Anonymous type tests +proc verify_everything {variable_name} { + # Test -var-list-children + mi_varobj_tree_test_children_callback $variable_name + # Bring the variable named by VARIABLE_NAME into the current scope + # in VAROBJ. + upvar #0 $variable_name varobj + + # Test -var-info-path-expression + mi_gdb_test "-var-info-path-expression $varobj(obj_name)" \ + "\\^done,path_expr=\"[string_to_regexp $varobj(path_expr)]\"" \ + "path expression for $varobj(obj_name)" + + # Test -var-info-expression + mi_gdb_test "-var-info-expression $varobj(obj_name)" \ + "\\^done,lang=\"C\",exp=\"[string_to_regexp $varobj(display_name)]\"" \ + "expression for $varobj(obj_name)" +} + +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" +mi_execute_to "exec-continue" "breakpoint-hit" "do_anonymous_type_tests" ""\ + ".*" ".*" {"" "disp=\"keep\""} \ + "continue to do_anonymous_type_tests breakpoint" + +# Run the varobj tree on variable "ptr". +set tree { + {struct anonymous **} ptr { + {struct anonymous *} {*ptr} { + int a {} + anonymous struct { + int b {} + {char *} c { + char {*c} {} + } + anonymous union { + int d {} + {void *} e {} + char f {} + anonymous struct { + char g {} + {const char **} h { + {const char *} {*h} { + {const char} {**h} {} + } + } + {simpleton ***} simple { + {simpleton **} {*simple} { + {simpleton *} {**simple} { + int integer {} + {unsigned int} unsigned_integer {} + char character {} + {signed char} signed_character {} + {char *} char_ptr { + char {*char_ptr} {} + } + {int [10]} array_of_10 { + int 0 {} + int 1 {} + int 2 {} + int 3 {} + int 4 {} + int 5 {} + int 6 {} + int 7 {} + int 8 {} + int 9 {} + } + } + } + } + } + } + } + } + } +} + +mi_walk_varobj_tree c $tree verify_everything + +set tree { + {struct {...}} v { + int x {} + anonymous struct { + int a {} + } + anonymous struct { + 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 71c5b9d..f724eb0 100644 --- a/gdb/testsuite/gdb.mi/var-cmd.c +++ b/gdb/testsuite/gdb.mi/var-cmd.c @@ -92,6 +92,24 @@ struct _struct_n_pointer { struct _struct_n_pointer *next; }; +struct anonymous { + int a; + struct { + int b; + char *c; + union { + int d; + void *e; + char f; + struct { + char g; + const char **h; + simpleton ***simple; + }; + }; + }; +}; + void do_locals_tests (void); void do_block_tests (void); void subroutine1 (int, long *); @@ -503,6 +521,38 @@ void do_bitfield_tests () /*: END: bitfield :*/ } +void +do_anonymous_type_tests (void) +{ + struct anonymous *anon; + struct anonymous **ptr; + struct + { + int x; + struct + { + int a; + }; + struct + { + int b; + }; + } v = {1, {2}, {3}}; + + anon = malloc (sizeof (struct anonymous)); + anon->a = 1; + anon->b = 2; + anon->c = (char *) 3; + anon->d = 4; + anon->g = '5'; + anon->h = (const char **) 6; + anon->simple = (simpleton ***) 7; + + ptr = &anon; + free (anon); + return; /* anonymous type tests breakpoint */ +} + int main (int argc, char *argv []) { @@ -513,6 +563,7 @@ main (int argc, char *argv []) do_frozen_tests (); do_at_tests (); do_bitfield_tests (); + do_anonymous_type_tests (); exit (0); } diff --git a/gdb/varobj.c b/gdb/varobj.c index 7c68a93..ab9dd4a 100644 --- a/gdb/varobj.c +++ b/gdb/varobj.c @@ -43,6 +43,10 @@ typedef int PyObject; #endif +/* The names of varobjs representing anonymous structs or unions. */ +#define ANONYMOUS_STRUCT_NAME _("<anonymous struct>") +#define ANONYMOUS_UNION_NAME _("<anonymous union>") + /* Non-zero if we want to see trace of varobj level stuff. */ int varobjdebug = 0; @@ -1289,6 +1293,39 @@ varobj_get_gdb_type (struct varobj *var) return var->type; } +/* Is VAR a path expression parent, i.e., can it be used to construct + a valid path expression? */ + +static int +is_path_expr_parent (struct varobj *var) +{ + struct type *type; + + /* "Fake" children are not path_expr parents. */ + if (CPLUS_FAKE_CHILD (var)) + return 0; + + type = 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); +} + +/* Return the path expression parent for VAR. */ + +static struct varobj * +get_path_expr_parent (struct varobj *var) +{ + struct varobj *parent = var; + + while (!is_root_p (parent) && !is_path_expr_parent (parent)) + parent = parent->parent; + + return parent; +} + /* Return a pointer to the full rooted expression of varobj VAR. If it has not been computed yet, compute it. */ char * @@ -2170,6 +2207,20 @@ create_child (struct varobj *parent, int index, char *name) value_of_child (parent, index)); } +/* Does CHILD represent a child with no name? This happens when + the child is an anonmous struct or union and it has no field name + in its parent variable. + + This has already been determined by *_describe_child. The easiest + thing to do is to compare the child's name with ANONYMOUS_*_NAME. */ + +static int +is_anonymous_child (struct varobj *child) +{ + return (strcmp (child->name, ANONYMOUS_STRUCT_NAME) == 0 + || strcmp (child->name, ANONYMOUS_UNION_NAME) == 0); +} + static struct varobj * create_child_with_value (struct varobj *parent, int index, const char *name, struct value *value) @@ -2185,8 +2236,13 @@ create_child_with_value (struct varobj *parent, int index, const char *name, child->index = index; child->parent = parent; child->root = parent->root; - childs_name = xstrprintf ("%s.%s", parent->obj_name, name); + + if (is_anonymous_child (child)) + childs_name = xstrprintf ("%s.%d_anonymous", parent->obj_name, index); + else + childs_name = xstrprintf ("%s.%s", parent->obj_name, name); child->obj_name = childs_name; + install_variable (child); /* Compute the type of the child. Must do this before @@ -2955,7 +3011,7 @@ c_describe_child (struct varobj *parent, int index, if (cfull_expression) { *cfull_expression = NULL; - parent_expression = varobj_get_path_expr (parent); + parent_expression = varobj_get_path_expr (get_path_expr_parent (parent)); } adjust_value_for_child_access (&value, &type, &was_ptr); @@ -2990,26 +3046,49 @@ c_describe_child (struct varobj *parent, int index, case TYPE_CODE_STRUCT: case TYPE_CODE_UNION: - if (cname) - *cname = xstrdup (TYPE_FIELD_NAME (type, index)); + { + char *field_name; - if (cvalue && value) - { - /* For C, varobj index is the same as type index. */ - *cvalue = value_struct_element_index (value, index); - } + /* If the type is anonymous and the field has no name, + set an appropriate name. */ + field_name = TYPE_FIELD_NAME (type, index); + if (field_name == NULL || *field_name == '\0') + { + if (cname) + { + if (TYPE_CODE (TYPE_FIELD_TYPE (type, index)) + == TYPE_CODE_STRUCT) + *cname = xstrdup (ANONYMOUS_STRUCT_NAME); + else + *cname = xstrdup (ANONYMOUS_UNION_NAME); + } - if (ctype) - *ctype = TYPE_FIELD_TYPE (type, index); + if (cfull_expression) + *cfull_expression = xstrdup (""); + } + else + { + if (cname) + *cname = xstrdup (field_name); - if (cfull_expression) - { - char *join = was_ptr ? "->" : "."; + if (cfull_expression) + { + char *join = was_ptr ? "->" : "."; - *cfull_expression = xstrprintf ("(%s)%s%s", parent_expression, join, - TYPE_FIELD_NAME (type, index)); - } + *cfull_expression = xstrprintf ("(%s)%s%s", parent_expression, + join, field_name); + } + } + if (cvalue && value) + { + /* For C, varobj index is the same as type index. */ + *cvalue = value_struct_element_index (value, index); + } + + if (ctype) + *ctype = TYPE_FIELD_TYPE (type, index); + } break; case TYPE_CODE_PTR: @@ -3357,14 +3436,16 @@ cplus_describe_child (struct varobj *parent, int index, value = parent->parent->value; type = get_value_type (parent->parent); if (cfull_expression) - parent_expression = varobj_get_path_expr (parent->parent); + parent_expression + = varobj_get_path_expr (get_path_expr_parent (parent->parent)); } else { value = parent->value; type = get_value_type (parent); if (cfull_expression) - parent_expression = varobj_get_path_expr (parent); + parent_expression + = varobj_get_path_expr (get_path_expr_parent (parent)); } adjust_value_for_child_access (&value, &type, &was_ptr); @@ -3386,6 +3467,7 @@ cplus_describe_child (struct varobj *parent, int index, enum accessibility acc = public_field; int vptr_fieldno; struct type *basetype = NULL; + char *field_name; vptr_fieldno = get_vptr_fieldno (type, &basetype); if (strcmp (parent->name, "private") == 0) @@ -3404,20 +3486,40 @@ cplus_describe_child (struct varobj *parent, int index, } --type_index; - if (cname) - *cname = xstrdup (TYPE_FIELD_NAME (type, type_index)); + /* If the type is anonymous and the field has no name, + set an appopriate name. */ + field_name = TYPE_FIELD_NAME (type, type_index); + if (field_name == NULL || *field_name == '\0') + { + if (cname) + { + if (TYPE_CODE (TYPE_FIELD_TYPE (type, type_index)) + == TYPE_CODE_STRUCT) + *cname = xstrdup (ANONYMOUS_STRUCT_NAME); + else if (TYPE_CODE (TYPE_FIELD_TYPE (type, type_index)) + == TYPE_CODE_UNION) + *cname = xstrdup (ANONYMOUS_UNION_NAME); + } + + if (cfull_expression) + *cfull_expression = xstrdup (""); + } + else + { + if (cname) + *cname = xstrdup (TYPE_FIELD_NAME (type, type_index)); + + if (cfull_expression) + *cfull_expression + = xstrprintf ("((%s)%s%s)", parent_expression, join, + field_name); + } if (cvalue && value) *cvalue = value_struct_element_index (value, type_index); if (ctype) *ctype = TYPE_FIELD_TYPE (type, type_index); - - if (cfull_expression) - *cfull_expression - = xstrprintf ("((%s)%s%s)", parent_expression, - join, - TYPE_FIELD_NAME (type, type_index)); } else if (index < TYPE_N_BASECLASSES (type)) { ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFA] mi/10586 2011-12-17 1:55 ` Keith Seitz @ 2011-12-20 16:03 ` Tom Tromey 2012-01-12 23:05 ` Keith Seitz 0 siblings, 1 reply; 15+ messages in thread From: Tom Tromey @ 2011-12-20 16:03 UTC (permalink / raw) To: Keith Seitz; +Cc: gdb-patches@sourceware.org ml >>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes: Keith> ChangeLog Keith> 2011-12-16 Keith Seitz <keiths@redhat.com> Keith> PR mi/10586 Keith> * varobj.c (ANONYMOUS_STRUCT_NAME): Define. Keith> (ANONYMOUS_UNION_NAME): Define. Keith> (is_path_expr_parent): New function. Keith> (get_path_expr_parent): New function. Keith> (is_anonymous_child): New function. Keith> (create_child_with_value): If the child is anonymous and without Keith> a name, assign an object name to it. Keith> (c_describe_child): Use get_path_expr_parent to determine Keith> the parent expression. Keith> If there field represents an anonymous struct or union and Keith> has no name, set an appropriate display name and expression. Keith> (cplus_describe_child): Likewise. Keith> testsuite/ChangeLog Keith> 2011-12-16 Keith Seitz <keiths@redhat.com> Keith> PR mi/10586 Keith> * gdb.mi/var-cmd.c (struct anonymous): New structure. Keith> (do_anonymous_type_tests): New function. Keith> (main): Call do_anonymous_type_tests. Keith> * gdb.mi/mi2-var-child.exp: Add anonymous type tests. Keith> (verify_everything): New procedure. Keith> * gdb.mi/mi-var-cp.cc (class A): New class. Keith> (anonymous_structs_and_unions): New function. Keith> (main): Call anonymous_structs_and_unions. Keith> * gdb.mi/mi-var-cp.exp: Add anonymous type tests. Keith> (verify_everything): New procedure. Ok. Thanks. Tom ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [RFA] mi/10586 2011-12-20 16:03 ` Tom Tromey @ 2012-01-12 23:05 ` Keith Seitz 0 siblings, 0 replies; 15+ messages in thread From: Keith Seitz @ 2012-01-12 23:05 UTC (permalink / raw) To: gdb-patches@sourceware.org ml On 12/20/2011 08:00 AM, Tom Tromey wrote: >>>>>> "Keith" == Keith Seitz<keiths@redhat.com> writes: > > Keith> ChangeLog > Keith> 2011-12-16 Keith Seitz<keiths@redhat.com> > > Keith> PR mi/10586 > Keith> * varobj.c (ANONYMOUS_STRUCT_NAME): Define. > Keith> (ANONYMOUS_UNION_NAME): Define. > Keith> (is_path_expr_parent): New function. > Keith> (get_path_expr_parent): New function. > Keith> (is_anonymous_child): New function. > Keith> (create_child_with_value): If the child is anonymous and without > Keith> a name, assign an object name to it. > Keith> (c_describe_child): Use get_path_expr_parent to determine > Keith> the parent expression. > Keith> If there field represents an anonymous struct or union and > Keith> has no name, set an appropriate display name and expression. > Keith> (cplus_describe_child): Likewise. > > Keith> testsuite/ChangeLog > Keith> 2011-12-16 Keith Seitz<keiths@redhat.com> > > Keith> PR mi/10586 > Keith> * gdb.mi/var-cmd.c (struct anonymous): New structure. > Keith> (do_anonymous_type_tests): New function. > Keith> (main): Call do_anonymous_type_tests. > Keith> * gdb.mi/mi2-var-child.exp: Add anonymous type tests. > Keith> (verify_everything): New procedure. > Keith> * gdb.mi/mi-var-cp.cc (class A): New class. > Keith> (anonymous_structs_and_unions): New function. > Keith> (main): Call anonymous_structs_and_unions. > Keith> * gdb.mi/mi-var-cp.exp: Add anonymous type tests. > Keith> (verify_everything): New procedure. > > Ok. > Thanks. Now that the testsuite updates are checked in, I've double-checked this patch and committed it. Keith ^ permalink raw reply [flat|nested] 15+ messages in thread
* Crash regression with Eclipse [Re: [RFA] mi/10586] 2011-12-02 22:28 ` Keith Seitz 2011-12-13 1:28 ` Keith Seitz 2011-12-13 19:58 ` Tom Tromey @ 2011-12-13 20:34 ` Jan Kratochvil 2 siblings, 0 replies; 15+ messages in thread From: Jan Kratochvil @ 2011-12-13 20:34 UTC (permalink / raw) To: Keith Seitz; +Cc: gdb-patches@sourceware.org ml On Fri, 02 Dec 2011 23:28:19 +0100, Keith Seitz wrote: > I am attaching the latest incarnation of this patch, which covers > all these cases (AFAIK) and utilizes the new varobj tree testing > that I committed last week. Is it tested with Eclipse? Eclipse from Fedora 16 will crash GDB. It does not crash with FSF GDB HEAD, it just does not expand "v" in Variables window. This is a regression. int main (void) { struct { int x; struct { int a; }; struct { int b; }; } v = {1, {2}, {3}}; struct s { int x, y; } n = {10, 20}; return 0; /* Step in Eclipse here, then expand "v" in Variables. */ } Program received signal SIGSEGV, Segmentation fault. 0x00000000007ab58b in get_value_type (var=0x0) at varobj.c:2397 2397 if (var->value) (gdb) bt #0 in get_value_type (var=0x0) at varobj.c:2397 #1 in is_path_expr_parent (var=0x0) at varobj.c:1310 #2 in get_path_expr_parent (var=0x2cbbd80) at varobj.c:1325 #3 in c_describe_child (parent=0x2cbbd80, index=0, cname=0x0, cvalue=0x0, ctype=0x0, cfull_expression=0x2cbf5f8) at varobj.c:3015 #4 in c_path_expr_of_child (child=0x2cbf5f0) at varobj.c:3140 #5 in varobj_get_path_expr (var=0x2cbf5f0) at varobj.c:1344 #6 in mi_cmd_var_info_path_expression (command=0x256a200 "var-info-path-expression", argv=0x2cdd570, argc=1) at ./mi/mi-cmd-var.c:501 #7 in mi_cmd_execute (parse=0x2d0bee0) at ./mi/mi-main.c:2110 #8 in captured_mi_execute_command (uiout=0x28aca70, context=0x2d0bee0) at ./mi/mi-main.c:1854 #9 in mi_execute_command (cmd=0x2cbf6d0 "41-var-info-path-expression var1.x", from_tty=1) at ./mi/mi-main.c:1976 #10 in mi_execute_command_wrapper (cmd=0x2cbf6d0 "41-var-info-path-expression var1.x") at ./mi/mi-interp.c:291 #11 in gdb_readline2 (client_data=0x0) at event-top.c:717 #12 in stdin_event_handler (error=0, client_data=0x0) at event-top.c:375 #13 in handle_file_event (data=...) at event-loop.c:828 #14 in process_event () at event-loop.c:402 #15 in gdb_do_one_event () at event-loop.c:466 #16 in start_event_loop () at event-loop.c:491 #17 in mi_command_loop (mi_version=2) at ./mi/mi-interp.c:321 #18 in mi2_command_loop () at ./mi/mi-interp.c:303 #19 in current_interp_command_loop () at interps.c:304 #20 in captured_command_loop (data=0x0) at ./main.c:234 #21 in catch_errors (func=0x487987 <captured_command_loop>, func_args=0x0, errstring=0xe75107 "", mask=6) at exceptions.c:504 #22 in captured_main (data=0x7fff42a37b90) at ./main.c:944 #23 in catch_errors (func=0x4879d6 <captured_main>, func_args=0x7fff42a37b90, errstring=0xe75107 "", mask=6) at exceptions.c:504 #24 in gdb_main (args=0x7fff42a37b90) at ./main.c:953 #25 in main (argc=4, argv=0x7fff42a37c98) at gdb.c:35 Thanks, Jan ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2012-01-12 22:52 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2011-11-11 21:30 [RFA] mi/10586 Keith Seitz 2011-11-14 15:45 ` Tom Tromey 2011-11-14 18:04 ` Keith Seitz 2011-11-14 18:18 ` Tom Tromey 2011-11-14 19:28 ` Keith Seitz 2011-11-14 20:28 ` Tom Tromey 2011-11-15 17:10 ` Keith Seitz 2011-11-15 17:30 ` Tom Tromey 2011-12-02 22:28 ` Keith Seitz 2011-12-13 1:28 ` Keith Seitz 2011-12-13 19:58 ` Tom Tromey 2011-12-17 1:55 ` Keith Seitz 2011-12-20 16:03 ` Tom Tromey 2012-01-12 23:05 ` Keith Seitz 2011-12-13 20:34 ` Crash regression with Eclipse [Re: [RFA] mi/10586] Jan Kratochvil
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).