Re: [PATCH v4 13/15] gdb/testsuite: fix gdb.base/info-types-c++ with clang

[Andrew Burgess <aburgess@redhat.com>]

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?

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