From 9a9a76082919371f4ceb571f6c9892325b80a2e0 Mon Sep 17 00:00:00 2001 From: Andrew Burgess Date: Mon, 7 Jul 2014 19:22:36 +0100 Subject: [PATCH] Improve MI -var-info-path-expression for nested struct/union case. https://sourceware.org/ml/gdb-patches/2014-05/msg00383.html The MI command -var-info-path-expression currently does not handle non-anonymous structs / unions nested within other structs / unions, it will skip parts of the expression. Consider this example: ## START EXAMPLE ## $ cat ex.c #include int main () { struct s1 { int a; }; struct ss { struct s1 x; }; struct ss an_ss; memset (&an_ss, 0, sizeof (an_ss)); return 0; } $ gcc -g -o ex.x ex.c $ gdb ex.x (gdb) break 18 Breakpoint 1 at 0x80483ba: file ex.c, line 18. (gdb) run Starting program: /home/user/ex.x Breakpoint 1, main () at ex.c:18 18 return 0; (gdb) interpreter-exec mi "-var-create an_ss * an_ss" (gdb) interpreter-exec mi "-var-list-children an_ss" ^done,numchild="1",children=[child={name="an_ss.x",exp="x",numchild="1",type="struct s1",thread-id="1"}],has_more="0" (gdb) interpreter-exec mi "-var-list-children an_ss.x" ^done,numchild="1",children=[child={name="an_ss.x.a",exp="a",numchild="0",type="int",thread-id="1"}],has_more="0" (gdb) interpreter-exec mi "-var-list-children an_ss.x.a" ^done,numchild="0",has_more="0" (gdb) interpreter-exec mi "-var-info-path-expression an_ss.x.a" ^done,path_expr="(an_ss).a" (gdb) print (an_ss).a There is no member named a. ## END EXAMPLE ## Notice that the path expression returned is wrong, and as a result the print command fails. This patch adds a new method to the varobj_ops structure called is_path_expr_parent, to allow language specific control over finding the parent varobj, the current logic becomes the C/C++ version and is extended to handle the nested cases. No other language currently uses this code, so all other languages just get a default method. With this patch, the above example now finishes like this: ## START EXAMPLE ## $ gdb ex.x (gdb) break 18 Breakpoint 1 at 0x80483ba: file ex.c, line 18. (gdb) run Starting program: /home/user/ex.x Breakpoint 1, main () at ex.c:18 18 return 0; (gdb) interpreter-exec mi "-var-list-children an_ss" ^done,numchild="1",children=[child={name="an_ss.x",exp="x",numchild="1",type="struct s1",thread-id="1"}],has_more="0" (gdb) interpreter-exec mi "-var-list-children an_ss.x" ^done,numchild="1",children=[child={name="an_ss.x.a",exp="a",numchild="0",type="int",thread-id="1"}],has_more="0" (gdb) interpreter-exec mi "-var-list-children an_ss.x.a" ^done,numchild="0",has_more="0" (gdb) interpreter-exec mi "-var-info-path-expression an_ss.x.a" ^done,path_expr="((an_ss).x).a" (gdb) print ((an_ss).x).a $1 = 0 ## END EXAMPLE ## Notice that the path expression is now correct, and the print is a success. gdb/ChangeLog: * ada-varobj.c (ada_varobj_ops): Fill in is_path_expr_parent field. * c-varobj.c (c_is_path_expr_parent): New function, moved core from varobj.c, with additional checks. (c_varobj_ops): Fill in is_path_expr_parent field. (cplus_varobj_ops): Fill in is_path_expr_parent field. * jv-varobj.c (java_varobj_ops): Fill in is_path_expr_parent field. * varobj.c (is_path_expr_parent): Call is_path_expr_parent varobj ops method. (varobj_default_is_path_expr_parent): New function. * varobj.h (lang_varobj_ops): Add is_path_expr_parent field. (varobj_default_is_path_expr_parent): Declare new function. 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/ChangeLog | 16 +++++++ gdb/ada-varobj.c | 3 +- gdb/c-varobj.c | 56 ++++++++++++++++++++++++- gdb/jv-varobj.c | 3 +- gdb/testsuite/ChangeLog | 9 ++++ gdb/testsuite/gdb.mi/mi2-var-child.exp | 76 ++++++++++++++++++++++++++++++++++ gdb/testsuite/gdb.mi/var-cmd.c | 68 ++++++++++++++++++++++++++++++ gdb/varobj.c | 20 ++++----- gdb/varobj.h | 9 ++++ 9 files changed, 246 insertions(+), 14 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index f267098..b25f8f5 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,19 @@ +2014-07-09 Andrew Burgess + + * ada-varobj.c (ada_varobj_ops): Fill in is_path_expr_parent + field. + * c-varobj.c (c_is_path_expr_parent): New function, moved core + from varobj.c, with additional checks. + (c_varobj_ops): Fill in is_path_expr_parent field. + (cplus_varobj_ops): Fill in is_path_expr_parent field. + * jv-varobj.c (java_varobj_ops): Fill in is_path_expr_parent + field. + * varobj.c (is_path_expr_parent): Call is_path_expr_parent varobj + ops method. + (varobj_default_is_path_expr_parent): New function. + * varobj.h (lang_varobj_ops): Add is_path_expr_parent field. + (varobj_default_is_path_expr_parent): Declare new function. + 2014-07-08 Markus Metzger * infcmd.c (finish_backward): Turn internal error into normal error. diff --git a/gdb/ada-varobj.c b/gdb/ada-varobj.c index b9f83be..3d56526 100644 --- a/gdb/ada-varobj.c +++ b/gdb/ada-varobj.c @@ -1026,5 +1026,6 @@ const struct lang_varobj_ops ada_varobj_ops = ada_type_of_child, ada_value_of_variable, ada_value_is_changeable_p, - ada_value_has_mutated + ada_value_has_mutated, + varobj_default_is_path_expr_parent }; diff --git a/gdb/c-varobj.c b/gdb/c-varobj.c index 9c2860d..f7bc52b 100644 --- a/gdb/c-varobj.c +++ b/gdb/c-varobj.c @@ -126,6 +126,56 @@ adjust_value_for_child_access (struct value **value, } } +/* Is VAR a path expression parent, i.e., can it be used to construct + a valid path expression? */ + +static int +c_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 = varobj_get_gdb_type (var); + + /* Anonymous unions and structs are also not path_expr parents. */ + if ((TYPE_CODE (type) == TYPE_CODE_STRUCT + || TYPE_CODE (type) == TYPE_CODE_UNION) + && TYPE_NAME (type) == NULL + && TYPE_TAG_NAME (type) == NULL) + { + struct varobj *parent = var->parent; + + while (parent != NULL && CPLUS_FAKE_CHILD (parent)) + parent = parent->parent; + + if (parent != NULL) + { + struct type *parent_type; + int was_ptr; + + parent_type = varobj_get_value_type (parent); + adjust_value_for_child_access (NULL, &parent_type, &was_ptr, 0); + + 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; +} + /* C */ static int @@ -493,7 +543,8 @@ const struct lang_varobj_ops c_varobj_ops = c_type_of_child, c_value_of_variable, varobj_default_value_is_changeable_p, - NULL /* value_has_mutated */ + NULL, /* value_has_mutated */ + c_is_path_expr_parent /* is_path_expr_parent */ }; /* A little convenience enum for dealing with C++/Java. */ @@ -904,7 +955,8 @@ const struct lang_varobj_ops cplus_varobj_ops = cplus_type_of_child, cplus_value_of_variable, varobj_default_value_is_changeable_p, - NULL /* value_has_mutated */ + NULL, /* value_has_mutated */ + c_is_path_expr_parent /* is_path_expr_parent */ }; diff --git a/gdb/jv-varobj.c b/gdb/jv-varobj.c index 0bb8e64..fb4d417 100644 --- a/gdb/jv-varobj.c +++ b/gdb/jv-varobj.c @@ -101,5 +101,6 @@ const struct lang_varobj_ops java_varobj_ops = java_type_of_child, java_value_of_variable, varobj_default_value_is_changeable_p, - NULL /* value_has_mutated */ + NULL, /* value_has_mutated */ + varobj_default_is_path_expr_parent }; diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 575e0d8..b926bf6 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,12 @@ +2014-07-09 Andrew Burgess + + * 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. + 2014-07-02 Yao Qi * gdb.trace/entry-values.c: Define labels 'foo_start' and diff --git a/gdb/testsuite/gdb.mi/mi2-var-child.exp b/gdb/testsuite/gdb.mi/mi2-var-child.exp index f992a63..fc02066 100644 --- a/gdb/testsuite/gdb.mi/mi2-var-child.exp +++ b/gdb/testsuite/gdb.mi/mi2-var-child.exp @@ -1163,6 +1163,13 @@ mi_create_breakpoint \ "$srcfile:$lineno" "break in do_anonymous_type_tests" \ -disp keep -func do_anonymous_type_tests \ -file ".*var-cmd.c" -line $lineno + +set lineno [gdb_get_line_number "nested struct union tests breakpoint"] +mi_create_breakpoint \ + "$srcfile:$lineno" "break in do_nested_struct_union_tests" \ + -disp keep -func do_nested_struct_union_tests \ + -file ".*var-cmd.c" -line $lineno + mi_execute_to "exec-continue" "breakpoint-hit" "do_anonymous_type_tests" ""\ ".*" ".*" {"" "disp=\"keep\""} \ "continue to do_anonymous_type_tests breakpoint" @@ -1236,5 +1243,74 @@ 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 struct_ss_tree { + {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 {} + } + } + } + +set tree " + {struct ss} var { + $struct_ss_tree + } +" + +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 + +set tree " + {struct ss *} ss_ptr { + $struct_ss_tree + } +" + +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..4bb2746 100644 --- a/gdb/testsuite/gdb.mi/var-cmd.c +++ b/gdb/testsuite/gdb.mi/var-cmd.c @@ -552,6 +552,73 @@ 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; + + struct ss *ss_ptr; + + memset (&var, 0, sizeof (var)); + memset (&var2, 0, sizeof (var2)); + ss_ptr = &var; + + return; /* nested struct union tests breakpoint */ +} + int main (int argc, char *argv []) { @@ -563,6 +630,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 7446f10..cf1b8a8 100644 --- a/gdb/varobj.c +++ b/gdb/varobj.c @@ -1004,18 +1004,18 @@ varobj_get_gdb_type (struct varobj *var) 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; + gdb_assert (var->root->lang_ops->is_path_expr_parent != NULL); + return var->root->lang_ops->is_path_expr_parent (var); +} - type = varobj_get_value_type (var); +/* Is VAR a path expression parent, i.e., can it be used to construct + a valid path expression? By default we assume any VAR can be a path + parent. */ - /* 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); +int +varobj_default_is_path_expr_parent (struct varobj *var) +{ + return 1; } /* Return the path expression parent for VAR. */ diff --git a/gdb/varobj.h b/gdb/varobj.h index 74d41cf..d5ab0f9 100644 --- a/gdb/varobj.h +++ b/gdb/varobj.h @@ -213,6 +213,12 @@ struct lang_varobj_ops Languages where types do not mutate can set this to NULL. */ int (*value_has_mutated) (struct varobj *var, struct value *new_value, struct type *new_type); + + /* Return nonzero if VAR is a suitable path expression parent. + + For C like languages with anonymous structures and unions an anonymous + structure or union is not a suitable parent. */ + int (*is_path_expr_parent) (struct varobj *var); }; extern const struct lang_varobj_ops c_varobj_ops; @@ -328,4 +334,7 @@ extern void varobj_formatted_print_options (struct value_print_options *opts, extern void varobj_restrict_range (VEC (varobj_p) *children, int *from, int *to); + +extern int varobj_default_is_path_expr_parent (struct varobj *var); + #endif /* VAROBJ_H */ -- 2.7.4