[PATCH v4 13/15] gdb/testsuite: fix gdb.base/info-types-c++ with clang
Bruno Larsen
blarsen@redhat.com
Wed Sep 14 11:31:49 GMT 2022
On 12/09/2022 16:35, Andrew Burgess wrote:
> Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:
>
>> When g++ compiles nameles structs defined in a typedef, it adds a
>> DW_AT_linkage_name with the name defined in the typedef. So when
>> running gdb.base/info-types-c++.exp by default, we get the following
>> output
>>
>> All defined types:
>>
>> File ../../../common/git-repos/binutils-gdb/gdb/testsuite/gdb.base/info-types.c:
>> 98: CL;
>> 42: anon_struct_t;
>> 65: anon_union_t;
>> 21: baz_t;
>> 33: enum_t;
>> 56: union_t;
>> 52: typedef enum {...} anon_enum_t;
>> 45: typedef anon_struct_t anon_struct_t;
>> 68: typedef anon_union_t anon_union_t;
>>
>> Clang[++] does not add DW_AT_linkage_name, and so it's output looks like
>> this:
>>
>> All defined types:
>>
>> File ../../../common/git-repos/binutils-gdb/gdb/testsuite/gdb.base/info-types.c:
>> 98: CL;
>> 21: baz_t;
>> 33: enum_t;
>> 56: union_t;
>> 52: typedef enum {...} anon_enum_t;
>> 45: typedef struct {...} anon_struct_t;
>> 68: typedef union {...} anon_union_t;
>>
>> Which is still correct output for GDB, but shows up as a failure when
>> running the test. This commit changes the test to allow for this output
>> when the compiler is clang.
>> ---
>> gdb/testsuite/gdb.base/info-types.exp.tcl | 109 +++++++++++++++-------
>> 1 file changed, 73 insertions(+), 36 deletions(-)
>>
>> diff --git a/gdb/testsuite/gdb.base/info-types.exp.tcl b/gdb/testsuite/gdb.base/info-types.exp.tcl
>> index 2dd9b9e5489..b6a03276f69 100644
>> --- a/gdb/testsuite/gdb.base/info-types.exp.tcl
>> +++ b/gdb/testsuite/gdb.base/info-types.exp.tcl
>> @@ -41,42 +41,79 @@ proc run_test { lang } {
>> set file_re "File .*[string_to_regexp $srcfile]:"
>>
>> if { $lang == "c++" } {
>> - set output_lines \
>> - [list \
>> - "^All defined types:" \
>> - ".*" \
>> - $file_re \
>> - "98:\[\t \]+CL;" \
>> - "42:\[\t \]+anon_struct_t;" \
>> - "65:\[\t \]+anon_union_t;" \
>> - "21:\[\t \]+baz_t;" \
>> - "33:\[\t \]+enum_t;" \
>> - "56:\[\t \]+union_t;" \
>> - "52:\[\t \]+typedef enum {\\.\\.\\.} anon_enum_t;" \
>> - "45:\[\t \]+typedef anon_struct_t anon_struct_t;" \
>> - "68:\[\t \]+typedef anon_union_t anon_union_t;" \
>> - "28:\[\t \]+typedef baz_t baz;" \
>> - "31:\[\t \]+typedef baz_t \\* baz_ptr;" \
>> - "27:\[\t \]+typedef baz_t baz_t;" \
>> - "\[\t \]+double" \
>> - "\[\t \]+float" \
>> - "\[\t \]+int" \
>> - "103:\[\t \]+typedef CL my_cl;" \
>> - "38:\[\t \]+typedef enum_t my_enum_t;" \
>> - "17:\[\t \]+typedef float my_float_t;" \
>> - "16:\[\t \]+typedef int my_int_t;" \
>> - "104:\[\t \]+typedef CL \\* my_ptr;" \
>> - "54:\[\t \]+typedef enum {\\.\\.\\.} nested_anon_enum_t;" \
>> - "47:\[\t \]+typedef anon_struct_t nested_anon_struct_t;" \
>> - "70:\[\t \]+typedef anon_union_t nested_anon_union_t;" \
>> - "30:\[\t \]+typedef baz_t nested_baz;" \
>> - "29:\[\t \]+typedef baz_t nested_baz_t;" \
>> - "39:\[\t \]+typedef enum_t nested_enum_t;" \
>> - "19:\[\t \]+typedef float nested_float_t;" \
>> - "18:\[\t \]+typedef int nested_int_t;" \
>> - "62:\[\t \]+typedef union_t nested_union_t;(" \
>> - "\[\t \]+unsigned int)?" \
>> - "($|\r\n.*)"]
>> + if { [test_compiler_info "clang-*"] } {
>> + set output_lines \
>> + [list \
>> + "^All defined types:" \
>> + ".*" \
>> + $file_re \
>> + "98:\[\t \]+CL;" \
>> + "21:\[\t \]+baz_t;" \
>> + "33:\[\t \]+enum_t;" \
>> + "56:\[\t \]+union_t;" \
>> + "52:\[\t \]+typedef enum {\\.\\.\\.} anon_enum_t;" \
>> + "45:\[\t \]+typedef struct {\\.\\.\\.} anon_struct_t;" \
>> + "68:\[\t \]+typedef union {\\.\\.\\.} anon_union_t;" \
>> + "28:\[\t \]+typedef baz_t baz;" \
>> + "31:\[\t \]+typedef baz_t \\* baz_ptr;" \
>> + "27:\[\t \]+typedef baz_t baz_t;" \
>> + "\[\t \]+double" \
>> + "\[\t \]+float" \
>> + "\[\t \]+int" \
>> + "103:\[\t \]+typedef CL my_cl;" \
>> + "38:\[\t \]+typedef enum_t my_enum_t;" \
>> + "17:\[\t \]+typedef float my_float_t;" \
>> + "16:\[\t \]+typedef int my_int_t;" \
>> + "104:\[\t \]+typedef CL \\* my_ptr;" \
>> + "54:\[\t \]+typedef enum {\\.\\.\\.} nested_anon_enum_t;" \
>> + "47:\[\t \]+typedef struct {\\.\\.\\.} nested_anon_struct_t;" \
>> + "70:\[\t \]+typedef union {\\.\\.\\.} nested_anon_union_t;" \
>> + "30:\[\t \]+typedef baz_t nested_baz;" \
>> + "29:\[\t \]+typedef baz_t nested_baz_t;" \
>> + "39:\[\t \]+typedef enum_t nested_enum_t;" \
>> + "19:\[\t \]+typedef float nested_float_t;" \
>> + "18:\[\t \]+typedef int nested_int_t;" \
>> + "62:\[\t \]+typedef union_t nested_union_t;(" \
>> + "\[\t \]+unsigned int)?" \
>> + "($|\r\n.*)"]
>> + } else {
>> + set output_lines \
>> + [list \
>> + "^All defined types:" \
>> + ".*" \
>> + $file_re \
>> + "98:\[\t \]+CL;" \
>> + "42:\[\t \]+anon_struct_t;" \
>> + "65:\[\t \]+anon_union_t;" \
>> + "21:\[\t \]+baz_t;" \
>> + "33:\[\t \]+enum_t;" \
>> + "56:\[\t \]+union_t;" \
>> + "52:\[\t \]+typedef enum {\\.\\.\\.} anon_enum_t;" \
>> + "45:\[\t \]+typedef anon_struct_t anon_struct_t;" \
>> + "68:\[\t \]+typedef anon_union_t anon_union_t;" \
>> + "28:\[\t \]+typedef baz_t baz;" \
>> + "31:\[\t \]+typedef baz_t \\* baz_ptr;" \
>> + "27:\[\t \]+typedef baz_t baz_t;" \
>> + "\[\t \]+double" \
>> + "\[\t \]+float" \
>> + "\[\t \]+int" \
>> + "103:\[\t \]+typedef CL my_cl;" \
>> + "38:\[\t \]+typedef enum_t my_enum_t;" \
>> + "17:\[\t \]+typedef float my_float_t;" \
>> + "16:\[\t \]+typedef int my_int_t;" \
>> + "104:\[\t \]+typedef CL \\* my_ptr;" \
>> + "54:\[\t \]+typedef enum {\\.\\.\\.} nested_anon_enum_t;" \
>> + "47:\[\t \]+typedef anon_struct_t nested_anon_struct_t;" \
>> + "70:\[\t \]+typedef anon_union_t nested_anon_union_t;" \
>> + "30:\[\t \]+typedef baz_t nested_baz;" \
>> + "29:\[\t \]+typedef baz_t nested_baz_t;" \
>> + "39:\[\t \]+typedef enum_t nested_enum_t;" \
>> + "19:\[\t \]+typedef float nested_float_t;" \
>> + "18:\[\t \]+typedef int nested_int_t;" \
>> + "62:\[\t \]+typedef union_t nested_union_t;(" \
>> + "\[\t \]+unsigned int)?" \
>> + "($|\r\n.*)"]
>> + }
>> } else {
>> set output_lines \
>> [list \
>> --
>> 2.31.1
>
> Rather than duplicating the whole pattern block like this, I wondered if
> we could rewrite the test to, I hope, make things a little clearer.
>
> Below is my attempt to do this. This patch applies instead of your
> patch above, and I believe results in the test passing with GCC and
> Clang.
>
> I wonder what you think of this as an alternative approach?
I think this approach is definitely better. It is much more readable in
the end, and re-uses as much code as possible, like you said.
I'll remove my patch for this test from my series, and I encourage you
to either approve your own patch or post it as a standalone, if you want
more eyes on it.
Cheers,
Bruno
>
> Thanks,
> Andrew
>
> ---
>
> commit 313bcb258d6a1e00dd8e93fbbbaa2df2d4a49e61
> Author: Andrew Burgess <aburgess@redhat.com>
> Date: Mon Sep 12 15:24:14 2022 +0100
>
> gdb: rewrite gdb.base/info-types.exp.tcl
>
> This patch was inspired by this proposed change:
>
> https://sourceware.org/pipermail/gdb-patches/2022-July/190926.html
>
> the goal of that change was to make the info-types tests pass when
> using Clang as a compiler. The difference between g++ and Clang, is
> that g++ will give a (made up) name to an anonymous struct or union
> that is used in a typedef, Clang doesn't. The solution proposed in
> the patch above is to create a third test pattern to be used for
> C++/Clang, at this point the test script seemed to be getting a little
> bloated.
>
> In this commit I rewrite the test. Instead of running the 'info
> types' command and expecting a specific set of lines in a particular
> order, we now read all the lines from the command output, and place
> the lines into a dictionary. We can then query the dictionary to
> check the all the expected lines are present.
>
> Checking for each output line independently has a couple of
> advantages:
>
> 1. We no longer care about the output order. I don't think the
> ordering is particularly important for this test, so long as all the
> expected lines are present,
>
> 2. For lines that are present in both C and C++ we can now share the
> code to check these patterns, I think this makes it clearer what is
> going on, and
>
> 3. It is easy to place a few lines within an `if` block, and so, only
> check for those lines when using a specific compiler.
>
> I've check gdb.base/info-types-c.exp and gdb.base/info-types-c++.exp
> using both GCC and Clang.
>
> diff --git a/gdb/testsuite/gdb.base/info-types.exp.tcl b/gdb/testsuite/gdb.base/info-types.exp.tcl
> index 2dd9b9e5489..8aa006d0179 100644
> --- a/gdb/testsuite/gdb.base/info-types.exp.tcl
> +++ b/gdb/testsuite/gdb.base/info-types.exp.tcl
> @@ -16,6 +16,113 @@
> # Check that 'info types' produces the expected output for an inferior
> # containing a number of different types.
>
> +
> +# Run the 'info types' command, and place the results into RESULT_VAR,
> +# which should be the name of a variable in the callers scope.
> +#
> +# RESULT_VAR will be cleared, and set to an associative array, the
> +# keys of this associative array will be the line numbers, and the
> +# value for each line number will be a list of the types GDB
> +# understands to be declared on that line.
> +#
> +# Some types don't have a line number for their declartion
> +# (e.g. builtin types), these are placed into a list for the special
> +# line number 'NONE'.
> +#
> +# Finally, only types from the reported in SRCFILE are collected, any
> +# types reported from other files (e.g. libraries linked into the
> +# test) should be ignored.
> +proc collect_output { result_var } {
> + upvar $result_var result_obj
> +
> + array set result_obj {}
> +
> + set collect_lines false
> + gdb_test_multiple "info types" "" {
> + -re "^info types\r\n" {
> + exp_continue
> + }
> +
> + -re "^\r\n" {
> + exp_continue
> + }
> +
> + -re "^All defined types:\r\n" {
> + exp_continue
> + }
> +
> + -re "^File (\[^\r\n\]+)\r\n" {
> + set filename $expect_out(1,string)
> + set collect_lines [regexp [string_to_regexp $::srcfile] $filename]
> + exp_continue
> + }
> +
> + -re "^($::decimal):\\s+(\[^\r\n\]+);\r\n" {
> + set lineno $expect_out(1,string)
> + set text $expect_out(2,string)
> + if { $collect_lines } {
> + if { ! [info exists result_obj($lineno)] } {
> + set result_obj($lineno) [list]
> + }
> + lappend result_obj($lineno) $text
> + }
> + exp_continue
> + }
> +
> + -re "^\\s+(\[^\r\n;\]+)\r\n" {
> + set text $expect_out(1,string)
> + if { $collect_lines } {
> + if { ![info exists result_obj(NONE)] } {
> + set result_obj(NONE) [list]
> + }
> + lappend result_obj(NONE) $text
> + }
> + exp_continue
> + }
> +
> + -re "^$::gdb_prompt $" {
> + }
> + }
> +}
> +
> +# RESULT_VAR is the name of a variable in the parent scope that
> +# contains an associative array of results as returned from the
> +# collect_output proc.
> +#
> +# LINE is a line number, or the empty string, and TEXT is the
> +# declaration of a type that GDB should have seen on that line.
> +#
> +# This proc checks in RESULT_VAR for a matching entry to LINE and
> +# TEXT, and, if a matching entry is found, calls pass, otherwise, fail
> +# is called. The testname used for the pass/fail call is based on
> +# LINE and TEXT.
> +#
> +# If LINE is the empty string then this proc looks for a result
> +# associated with the special line number 'NONE', see collect_output
> +# for more details.
> +proc require_line { result_var line text } {
> + upvar $result_var result_obj
> +
> + set testname "check for $text"
> + if { $line != "" } {
> + set testname "$testname on line $line"
> + } else {
> + set line "NONE"
> + }
> +
> + if { ![info exists result_obj($line)] } {
> + fail $testname
> + return
> + }
> +
> + if { [lsearch -exact $result_obj($line) $text] < 0 } {
> + fail $testname
> + return
> + }
> +
> + pass $testname
> +}
> +
> # Run 'info types' test, compiling the test file for language LANG,
> # which should be either 'c' or 'c++'.
> proc run_test { lang } {
> @@ -38,79 +145,71 @@ proc run_test { lang } {
> return 0
> }
>
> - set file_re "File .*[string_to_regexp $srcfile]:"
> + # Run 'info types' and place the results in RESULT_OBJ.
> + collect_output result_obj
> +
> + # Some results are common to C and C++. These are the builtin
> + # types which are not declared on any specific line.
> + require_line result_obj "" "double"
> + require_line result_obj "" "float"
> + require_line result_obj "" "int"
> +
> + # These typedefs are common to C and C++, but GDB should see a
> + # specific line number for these type declarations.
> + require_line result_obj "16" "typedef int my_int_t"
> + require_line result_obj "17" "typedef float my_float_t"
> + require_line result_obj "18" "typedef int nested_int_t"
> + require_line result_obj "19" "typedef float nested_float_t"
>
> if { $lang == "c++" } {
> - set output_lines \
> - [list \
> - "^All defined types:" \
> - ".*" \
> - $file_re \
> - "98:\[\t \]+CL;" \
> - "42:\[\t \]+anon_struct_t;" \
> - "65:\[\t \]+anon_union_t;" \
> - "21:\[\t \]+baz_t;" \
> - "33:\[\t \]+enum_t;" \
> - "56:\[\t \]+union_t;" \
> - "52:\[\t \]+typedef enum {\\.\\.\\.} anon_enum_t;" \
> - "45:\[\t \]+typedef anon_struct_t anon_struct_t;" \
> - "68:\[\t \]+typedef anon_union_t anon_union_t;" \
> - "28:\[\t \]+typedef baz_t baz;" \
> - "31:\[\t \]+typedef baz_t \\* baz_ptr;" \
> - "27:\[\t \]+typedef baz_t baz_t;" \
> - "\[\t \]+double" \
> - "\[\t \]+float" \
> - "\[\t \]+int" \
> - "103:\[\t \]+typedef CL my_cl;" \
> - "38:\[\t \]+typedef enum_t my_enum_t;" \
> - "17:\[\t \]+typedef float my_float_t;" \
> - "16:\[\t \]+typedef int my_int_t;" \
> - "104:\[\t \]+typedef CL \\* my_ptr;" \
> - "54:\[\t \]+typedef enum {\\.\\.\\.} nested_anon_enum_t;" \
> - "47:\[\t \]+typedef anon_struct_t nested_anon_struct_t;" \
> - "70:\[\t \]+typedef anon_union_t nested_anon_union_t;" \
> - "30:\[\t \]+typedef baz_t nested_baz;" \
> - "29:\[\t \]+typedef baz_t nested_baz_t;" \
> - "39:\[\t \]+typedef enum_t nested_enum_t;" \
> - "19:\[\t \]+typedef float nested_float_t;" \
> - "18:\[\t \]+typedef int nested_int_t;" \
> - "62:\[\t \]+typedef union_t nested_union_t;(" \
> - "\[\t \]+unsigned int)?" \
> - "($|\r\n.*)"]
> + # These results are specific to 'C++'.
> + require_line result_obj "21" "baz_t"
> + require_line result_obj "27" "typedef baz_t baz_t"
> + require_line result_obj "28" "typedef baz_t baz"
> + require_line result_obj "29" "typedef baz_t nested_baz_t"
> + require_line result_obj "30" "typedef baz_t nested_baz"
> + require_line result_obj "31" "typedef baz_t * baz_ptr"
> + require_line result_obj "33" "enum_t"
> + require_line result_obj "38" "typedef enum_t my_enum_t"
> + require_line result_obj "39" "typedef enum_t nested_enum_t"
> + require_line result_obj "52" "typedef enum {...} anon_enum_t"
> + require_line result_obj "54" "typedef enum {...} nested_anon_enum_t"
> + require_line result_obj "56" "union_t"
> + require_line result_obj "62" "typedef union_t nested_union_t"
> + require_line result_obj "98" "CL"
> + require_line result_obj "103" "typedef CL my_cl"
> + require_line result_obj "104" "typedef CL * my_ptr"
> +
> + if { [test_compiler_info "gcc-*"] } {
> + # GCC give a name to anonymous structs and unions. Not
> + # all compilers do this, e.g. Clang does not.
> + require_line result_obj "42" "anon_struct_t"
> + require_line result_obj "45" "typedef anon_struct_t anon_struct_t"
> + require_line result_obj "47" "typedef anon_struct_t nested_anon_struct_t"
> + require_line result_obj "65" "anon_union_t"
> + require_line result_obj "68" "typedef anon_union_t anon_union_t"
> + require_line result_obj "70" "typedef anon_union_t nested_anon_union_t"
> + }
> +
> } else {
> - set output_lines \
> - [list \
> - "^All defined types:" \
> - ".*" \
> - $file_re \
> - "52:\[\t \]+typedef enum {\\.\\.\\.} anon_enum_t;" \
> - "45:\[\t \]+typedef struct {\\.\\.\\.} anon_struct_t;" \
> - "68:\[\t \]+typedef union {\\.\\.\\.} anon_union_t;" \
> - "28:\[\t \]+typedef struct baz_t baz;" \
> - "31:\[\t \]+typedef struct baz_t \\* baz_ptr;" \
> - "21:\[\t \]+struct baz_t;" \
> - "\[\t \]+double" \
> - "33:\[\t \]+enum enum_t;" \
> - "\[\t \]+float" \
> - "\[\t \]+int" \
> - "38:\[\t \]+typedef enum enum_t my_enum_t;" \
> - "17:\[\t \]+typedef float my_float_t;" \
> - "16:\[\t \]+typedef int my_int_t;" \
> - "54:\[\t \]+typedef enum {\\.\\.\\.} nested_anon_enum_t;" \
> - "47:\[\t \]+typedef struct {\\.\\.\\.} nested_anon_struct_t;" \
> - "70:\[\t \]+typedef union {\\.\\.\\.} nested_anon_union_t;" \
> - "30:\[\t \]+typedef struct baz_t nested_baz;" \
> - "29:\[\t \]+typedef struct baz_t nested_baz_t;" \
> - "39:\[\t \]+typedef enum enum_t nested_enum_t;" \
> - "19:\[\t \]+typedef float nested_float_t;" \
> - "18:\[\t \]+typedef int nested_int_t;" \
> - "62:\[\t \]+typedef union union_t nested_union_t;" \
> - "56:\[\t \]+union union_t;(" \
> - "\[\t \]+unsigned int)?" \
> - "($|\r\n.*)"]
> + # These results are specific to 'C'.
> + require_line result_obj "21" "struct baz_t"
> + require_line result_obj "28" "typedef struct baz_t baz"
> + require_line result_obj "29" "typedef struct baz_t nested_baz_t"
> + require_line result_obj "30" "typedef struct baz_t nested_baz"
> + require_line result_obj "31" "typedef struct baz_t * baz_ptr"
> + require_line result_obj "33" "enum enum_t"
> + require_line result_obj "38" "typedef enum enum_t my_enum_t"
> + require_line result_obj "39" "typedef enum enum_t nested_enum_t"
> + require_line result_obj "45" "typedef struct {...} anon_struct_t"
> + require_line result_obj "47" "typedef struct {...} nested_anon_struct_t"
> + require_line result_obj "52" "typedef enum {...} anon_enum_t"
> + require_line result_obj "54" "typedef enum {...} nested_anon_enum_t"
> + require_line result_obj "56" "union union_t"
> + require_line result_obj "62" "typedef union union_t nested_union_t"
> + require_line result_obj "68" "typedef union {...} anon_union_t"
> + require_line result_obj "70" "typedef union {...} nested_anon_union_t"
> }
> -
> - gdb_test_lines "info types" "" [multi_line {*}$output_lines]
> }
>
> run_test $lang
>
More information about the Gdb-patches
mailing list