[PATCH 00/11] gdb/testsuite: Cleanup gdb.base for clang testing

[PATCH 00/11] gdb/testsuite: Cleanup gdb.base for clang testing

[Bruno Larsen via Gdb-patches]

When testing GDB with clang, gdb.base had over 50 more failures than when
testing with gcc.  Examining the failed tests led to a few clang bugs, a
few GDB bugs, and many testsuite assumptions that could be changed.

After this patch series, nothing should be changed for testing with gcc,
and testing with clang should only show non-trivial failures for
maint.exp and macscp.exp, along with GCC failures.

Bruno Larsen (11):
  change gdb.base/skip.exp to use finish instead of step
  change gdb.base/symbol-alias to xfail with clang
  Change gdb.base/skip-solib.exp deal with lack of epilogue information
  change gdb.base/nodebug.c to not fail with clang
  update gdb.base/info-program.exp to not fail with clang
  fix gdb.base/access-mem-running.exp for clang testing
  fix gdb.base/call-ar-st to work with clang
  add xfails to gdb.base/complex-parts.exp when testing with clang
  gdb/testsuite: don't test gdb.base/msym-bp-shl with clang
  make use of finish to leave function in gdb.base/skip-inline.exp
  explicitly test for stderr in gdb.base/dprintf.exp

 gdb/testsuite/gdb.base/access-mem-running.c |  2 +-
 gdb/testsuite/gdb.base/call-ar-st.exp       | 13 +++--
 gdb/testsuite/gdb.base/complex-parts.exp    |  5 ++
 gdb/testsuite/gdb.base/dprintf.exp          | 10 ++++
 gdb/testsuite/gdb.base/info-program.exp     |  2 +-
 gdb/testsuite/gdb.base/msym-bp-shl.exp      |  7 +++
 gdb/testsuite/gdb.base/nodebug.c            |  2 +-
 gdb/testsuite/gdb.base/nodebug.exp          |  2 +-
 gdb/testsuite/gdb.base/skip-inline.exp      | 18 ++++---
 gdb/testsuite/gdb.base/skip-solib.exp       | 11 +++-
 gdb/testsuite/gdb.base/skip.exp             | 56 +++++++++------------
 gdb/testsuite/gdb.base/symbol-alias.exp     |  9 +++-
 12 files changed, 86 insertions(+), 51 deletions(-)

-- 
2.31.1

[PATCH 01/11] change gdb.base/skip.exp to use finish instead of step

[Bruno Larsen via Gdb-patches]

skip.exp was making use of a fixed amount of step commands to exit
some functions.  This caused some problems when testing GDB with clang,
as it doesn't add epilogue information for functions.  Since the step
commands weren't testing important features, they were changed to finish
commands.
---
 gdb/testsuite/gdb.base/skip.exp | 56 ++++++++++++++-------------------
 1 file changed, 23 insertions(+), 33 deletions(-)

diff --git a/gdb/testsuite/gdb.base/skip.exp b/gdb/testsuite/gdb.base/skip.exp
index 7c71bb07a84..a961fbbdf41 100644
--- a/gdb/testsuite/gdb.base/skip.exp
+++ b/gdb/testsuite/gdb.base/skip.exp
@@ -117,9 +117,8 @@ with_test_prefix "step after deleting 1" {
 	return
     }
 
-    gdb_test "step" "foo \\(\\) at.*" "step 1"
-    gdb_test "step" ".*" "step 2" ; # Return from foo()
-    gdb_test "step" "main \\(\\) at.*" "step 3"
+    gdb_test "step" "foo \\(\\) at.*" "step"
+    gdb_test "finish" ".*" "finish" ; # Return from foo()
 }
 
 # Now disable the skiplist entry for  skip1.c.  We should now
@@ -137,13 +136,11 @@ with_test_prefix "step after disabling 3" {
     }
 
     gdb_test "step" "bar \\(\\) at.*" "step 1"
-    gdb_test "step" ".*" "step 2"; # Return from bar()
-    # With gcc 9.2.0 we jump once back to main before entering foo here.
-    # If that happens try to step a second time.
-    gdb_test "step" "foo \\(\\) at.*" "step 3" \
-	"main \\(\\) at .*\r\n$gdb_prompt " "step"
-    gdb_test "step" ".*" "step 4"; # Return from foo()
-    gdb_test "step" "main \\(\\) at.*" "step 5"
+    # guarantee that we jump once back to main before entering foo here.
+    gdb_test "finish" ".*" "finish 1"; # Return to main()
+    gdb_test "step" "foo \\(\\) at.*" "step 2" \
+	"main \\(\\) at .*\r\n$gdb_prompt " "step" # enter foo()
+    gdb_test "finish" ".*" "finish 2"; # Return from foo()
 }
 
 # Enable skiplist entry 3 and make sure we step over it like before.
@@ -159,9 +156,8 @@ with_test_prefix "step after enable 3" {
 	return
     }
 
-    gdb_test "step" "foo \\(\\) at.*" "step 1"
-    gdb_test "step" ".*" "step 2"; # Return from foo()
-    gdb_test "step" "main \\(\\) at.*" "step 3"
+    gdb_test "step" "foo \\(\\) at.*" "step"
+    gdb_test "finish" ".*" "finish"; # Return from foo()
 }
 
 # Admin tests (disable,enable,delete).
@@ -230,9 +226,8 @@ with_test_prefix "step using -fi" {
 
     gdb_test_no_output "skip disable"
     gdb_test_no_output "skip enable 5"
-    gdb_test "step" "foo \\(\\) at.*" "step 1"
-    gdb_test "step" ".*" "step 2"; # Return from foo()
-    gdb_test "step" "main \\(\\) at.*" "step 3"
+    gdb_test "step" "foo \\(\\) at.*" "step"
+    gdb_test "finish" ".*" "finish"; # Return from foo()
 }
 
 with_test_prefix "step using -gfi" {
@@ -242,9 +237,8 @@ with_test_prefix "step using -gfi" {
 
     gdb_test_no_output "skip disable"
     gdb_test_no_output "skip enable 6"
-    gdb_test "step" "foo \\(\\) at.*" "step 1"
-    gdb_test "step" ".*" "step 2"; # Return from foo()
-    gdb_test "step" "main \\(\\) at.*" "step 3"
+    gdb_test "step" "foo \\(\\) at.*" "step"
+    gdb_test "finish" ".*" "finish"; # Return from foo()
 }
 
 with_test_prefix "step using -fu for baz" {
@@ -255,13 +249,11 @@ with_test_prefix "step using -fu for baz" {
     gdb_test_no_output "skip disable"
     gdb_test_no_output "skip enable 7"
     gdb_test "step" "bar \\(\\) at.*" "step 1"
-    gdb_test "step" ".*" "step 2"; # Return from bar()
-    # With gcc 9.2.0 we jump once back to main before entering foo here.
-    # If that happens try to step a second time.
-    gdb_test "step" "foo \\(\\) at.*" "step 3" \
-	"main \\(\\) at .*\r\n$gdb_prompt " "step"
-    gdb_test "step" ".*" "step 4"; # Return from foo()
-    gdb_test "step" "main \\(\\) at.*" "step 5"
+    # guarantee that we jump once back to main before entering foo here.
+    gdb_test "finish" ".*" "finish 1"; # Return to main()
+    gdb_test "step" "foo \\(\\) at.*" "step 2" \
+	"main \\(\\) at .*\r\n$gdb_prompt " "step" # enter foo()
+    gdb_test "finish" ".*" "finish 2"; # Return from foo()
 }
 
 with_test_prefix "step using -rfu for baz" {
@@ -272,13 +264,11 @@ with_test_prefix "step using -rfu for baz" {
     gdb_test_no_output "skip disable"
     gdb_test_no_output "skip enable 8"
     gdb_test "step" "bar \\(\\) at.*" "step 1"
-    gdb_test "step" ".*" "step 2"; # Return from bar()
-    # With gcc 9.2.0 we jump once back to main before entering foo here.
-    # If that happens try to step a second time.
-    gdb_test "step" "foo \\(\\) at.*" "step 3" \
-	"main \\(\\) at .*\r\n$gdb_prompt " "step"
-    gdb_test "step" ".*" "step 4"; # Return from foo()
-    gdb_test "step" "main \\(\\) at.*" "step 5"
+    # guarantee that we jump once back to main before entering foo here.
+    gdb_test "finish" ".*" "finish 1"; # Return to main()
+    gdb_test "step" "foo \\(\\) at.*" "step 2" \
+	"main \\(\\) at .*\r\n$gdb_prompt " "step" # enter foo()
+    gdb_test "finish" ".*" "finish 2"; # Return from foo()
 }
 
 # Test -fi + -fu.
-- 
2.31.1

[PATCH 02/11] change gdb.base/symbol-alias to xfail with clang

[Bruno Larsen via Gdb-patches]

Clang does not issue alias information, even when using -gfull.  This
commit adds an xfail to that test in case we are using clang to reduce
noise when testing.
---
 gdb/testsuite/gdb.base/symbol-alias.exp | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/gdb.base/symbol-alias.exp b/gdb/testsuite/gdb.base/symbol-alias.exp
index 2b53cc31053..00cf388e967 100644
--- a/gdb/testsuite/gdb.base/symbol-alias.exp
+++ b/gdb/testsuite/gdb.base/symbol-alias.exp
@@ -31,6 +31,11 @@ foreach f {"func" "func_alias"} {
 }
 
 # Variables.
-foreach v {"g_var_s" "g_var_s_alias"} {
-    gdb_test "p $v" "= {field1 = 1, field2 = 2}"
+gdb_test "p g_var_s" "= {field1 = 1, field2 = 2}"
+
+# clang doesn't include variable alias information in the dwarf:
+# https://github.com/llvm/llvm-project/issues/52664
+if [test_compiler_info {clang*}] {
+    setup_xfail "clang/52664" *-*-*
 }
+gdb_test "p g_var_s_alias" "= {field1 = 1, field2 = 2}"
-- 
2.31.1

[PATCH 03/11] Change gdb.base/skip-solib.exp deal with lack of epilogue information

[Bruno Larsen via Gdb-patches]

When testing with clang, this feature worked, but the test failed
because it checked specifically to see if we were at the main function.
Without epilogue information, the inferior would be at libc_start_main
(or equivalent) instead. Having this test changed would allows us to
identify if the test stops working for real at some point.
---
 gdb/testsuite/gdb.base/skip-solib.exp | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/gdb.base/skip-solib.exp b/gdb/testsuite/gdb.base/skip-solib.exp
index ce2b080229e..99833533b6a 100644
--- a/gdb/testsuite/gdb.base/skip-solib.exp
+++ b/gdb/testsuite/gdb.base/skip-solib.exp
@@ -82,7 +82,7 @@ with_test_prefix "ignoring solib file" {
     # We shouldn't step into square(), since we skipped skip-solib-lib.c.
     #
     gdb_test "step" ""
-    gdb_test "bt" "#0\\s+main.*"
+    gdb_test "bt 1" "#0.*main.*"
 }
 
 #
@@ -114,5 +114,12 @@ with_test_prefix "ignoring solib function" {
     # the last line of square.
     #
     gdb_test "step" ""
-    gdb_test "bt" "#0\\s+square.*"
+    gdb_test_multiple "bt 1" "skipped multiply" {
+	-re "#0\\s+square.*" {
+	    pass "skipped multiply"
+	}
+	-re "#0.*main.*" {
+	    pass "skipped multiply"
+	}
+    }
 }
-- 
2.31.1

[PATCH 04/11] change gdb.base/nodebug.c to not fail with clang

[Bruno Larsen via Gdb-patches]

Clang organizes the variables differently to gcc in the original version
of this code, leading to the following differences when testing
p (int*) &dataglobal + 1

gcc:
$16 = (int *) 0x404034 <datalocal>

clang:
$16 = (int *) 0x404034 <dataglobal8>

The code change to nodebug.c makes it so gcc and clang will place the
same variable under dataglobal8, generating the same output.
---
 gdb/testsuite/gdb.base/nodebug.c   | 2 +-
 gdb/testsuite/gdb.base/nodebug.exp | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/gdb.base/nodebug.c b/gdb/testsuite/gdb.base/nodebug.c
index c7bc93991b8..572255697bb 100644
--- a/gdb/testsuite/gdb.base/nodebug.c
+++ b/gdb/testsuite/gdb.base/nodebug.c
@@ -3,8 +3,8 @@
 
 /* Test that things still (sort of) work when compiled without -g.  */
 
-int dataglobal = 3;			/* Should go in global data */
 static int datalocal = 4;		/* Should go in local data */
+int dataglobal = 3;			/* Should go in global data */
 int bssglobal;				/* Should go in global bss */
 static int bsslocal;			/* Should go in local bss */
 
diff --git a/gdb/testsuite/gdb.base/nodebug.exp b/gdb/testsuite/gdb.base/nodebug.exp
index 0dbd0ad153e..fbff6ecb395 100644
--- a/gdb/testsuite/gdb.base/nodebug.exp
+++ b/gdb/testsuite/gdb.base/nodebug.exp
@@ -187,7 +187,7 @@ if [nodebug_runto inner] then {
 	{"dataglobal + 1"		""   $dataglobal_unk_re					$dataglobal_unk_re}
 	{"&dataglobal"			""   "\\($data_var_type \\*\\) $hex <dataglobal>"	" = $data_var_type \\*"}
 	{"&dataglobal + 1"		""   $ptr_math_re					$ptr_math_re}
-	{"(int *) &dataglobal + 1"	""   " = \\(int \\*\\) $hex <datalocal>"		"int \\*"}
+	{"(int *) &dataglobal + 1"	""   " = \\(int \\*\\) $hex <dataglobal8>"		"int \\*"}
 	{"&(int) dataglobal + 1"	""   $not_mem_re					$not_mem_re}
 	{"&dataglobal, &dataglobal"	""   "\\($data_var_type \\*\\) $hex <dataglobal>"	" = $data_var_type \\*"}
 	{"*dataglobal"			""   $dataglobal_unk_re					$dataglobal_unk_re}
-- 
2.31.1

[PATCH 05/11] update gdb.base/info-program.exp to not fail with clang

[Bruno Larsen via Gdb-patches]

The updated test specifically mentions that it doesn't care where the
program stops, however it was still testing for something.  With this
correction, the test works even if the compiler doesn't add epilogue
information to functions.
---
 gdb/testsuite/gdb.base/info-program.exp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.base/info-program.exp b/gdb/testsuite/gdb.base/info-program.exp
index facc13efa2f..f652cfbf426 100644
--- a/gdb/testsuite/gdb.base/info-program.exp
+++ b/gdb/testsuite/gdb.base/info-program.exp
@@ -28,7 +28,7 @@ gdb_test "info program" "Program stopped at $hex\.\r\nIt stopped at breakpoint $
 
 # We don't really care where this step lands, so long as it gets
 # the inferior pushed off the breakpoint it's currently on...
-gdb_test "next" "$decimal\t.*" "step before info program"
+gdb_test "next" ".*" "step before info program"
 
 gdb_test "info program" "Program stopped at $hex\.\r\nIt stopped after being stepped\.\r\nType \"info stack\" or \"info registers\" for more information\." \
     "info program after next"
-- 
2.31.1

[PATCH 06/11] fix gdb.base/access-mem-running.exp for clang testing

[Bruno Larsen via Gdb-patches]

Clang was optimizing global_var away because it was not being used
anywhere. this commit fixes that by adding the attribute used it.
---
 gdb/testsuite/gdb.base/access-mem-running.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.base/access-mem-running.c b/gdb/testsuite/gdb.base/access-mem-running.c
index 6335f1bf199..cff6f0da820 100644
--- a/gdb/testsuite/gdb.base/access-mem-running.c
+++ b/gdb/testsuite/gdb.base/access-mem-running.c
@@ -19,7 +19,7 @@
 
 static unsigned int global_counter = 1;
 
-static volatile unsigned int global_var = 123;
+static volatile unsigned int __attribute__((used)) global_var = 123;
 
 static void
 maybe_stop_here ()
-- 
2.31.1

[PATCH 07/11] fix gdb.base/call-ar-st to work with clang

[Bruno Larsen via Gdb-patches]

When running this test with clang-compiled code, this test was failing
by producing an output like
array_i=<main.integer_array>, ...

instead of

array_i = <integer_array>, ...

This commit changes the testcase to accept both outputs, as they are
functionally identical.
---
 gdb/testsuite/gdb.base/call-ar-st.exp | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/gdb/testsuite/gdb.base/call-ar-st.exp b/gdb/testsuite/gdb.base/call-ar-st.exp
index d1fb3ac9ec1..e447d3e231d 100644
--- a/gdb/testsuite/gdb.base/call-ar-st.exp
+++ b/gdb/testsuite/gdb.base/call-ar-st.exp
@@ -153,9 +153,16 @@ if {!$skip_float_test && \
 
 #step
 set stop_line [gdb_get_line_number "-step1-"]
-gdb_test "step" \
-    "print_all_arrays \\(array_i=<integer_array.*>, array_c=<char_array.*> .ZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZa., array_f=<float_array.*>, array_d=<double_array.*>\\) at .*$srcfile:$stop_line\[ \t\r\n\]+$stop_line.*print_int_array\\(array_i\\);.*" \
-    "step inside print_all_arrays"
+
+gdb_test_multiple "step" "" {
+
+    -re "print_all_arrays \\(array_i=<integer_array.*>, array_c=<char_array.*> .ZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZa., array_f=<float_array.*>, array_d=<double_array.*>\\) at .*$srcfile:$stop_line\[ \t\r\n\]+$stop_line.*print_int_array\\(array_i\\);.*" {
+	pass "step inside print_all_arrays"
+    }
+    -re "print_all_arrays \\(array_i=<main.integer_array>, array_c=<main.char_array> .ZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZa., array_f=<main.float_array>, array_d=<main.double_array>\\) at .*$srcfile:$stop_line\[ \t\r\n\]+$stop_line.*print_int_array\\(array_i\\);.*" {
+	pass "step inside print_all_arrays (clang)"
+    }
+}
 
 #step -over
 if ![gdb_skip_stdio_test "next over print_int_array in print_all_arrays"] {
-- 
2.31.1

[PATCH 08/11] add xfails to gdb.base/complex-parts.exp when testing with clang

[Bruno Larsen via Gdb-patches]

clang doesn't add encoding to the name of complex variables, only says
that the type name is complex, making the relevant tests fail.
This patch adds the xfails to the tests that expect the variable name to
include it.
---
 gdb/testsuite/gdb.base/complex-parts.exp | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/gdb/testsuite/gdb.base/complex-parts.exp b/gdb/testsuite/gdb.base/complex-parts.exp
index e67fd482268..b4bb542f218 100644
--- a/gdb/testsuite/gdb.base/complex-parts.exp
+++ b/gdb/testsuite/gdb.base/complex-parts.exp
@@ -30,8 +30,13 @@ gdb_test "p z1" " = 1.5 \\+ 4.5i"
 gdb_test "p z2" " = 2.5 \\+ -5.5i"
 gdb_test "p z3" " = 3.5 \\+ 6.5i"
 
+# The following 3 tests are boken for clang.
+# More info on https://github.com/llvm/llvm-project/issues/52996
+if {[test_compiler_info clang-*-*]} { setup_xfail *-*-* }
 gdb_test "ptype z1" " = complex double"
+if {[test_compiler_info clang-*-*]} { setup_xfail *-*-* }
 gdb_test "ptype z2" " = complex float"
+if {[test_compiler_info clang-*-*]} { setup_xfail *-*-* }
 gdb_test "ptype z3" " = complex long double"
 
 with_test_prefix "double imaginary" {
-- 
2.31.1

[PATCH 09/11] gdb/testsuite: don't test gdb.base/msym-bp-shl with clang

[Bruno Larsen via Gdb-patches]

Clang will optimize away the static function in one of the files, and
the test is here to specifically test GDB's behavior when that function
is present, so it makes no sense to have this test run with that
compiler.
---
 gdb/testsuite/gdb.base/msym-bp-shl.exp | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/gdb/testsuite/gdb.base/msym-bp-shl.exp b/gdb/testsuite/gdb.base/msym-bp-shl.exp
index 42adcb191dd..95d5c393505 100644
--- a/gdb/testsuite/gdb.base/msym-bp-shl.exp
+++ b/gdb/testsuite/gdb.base/msym-bp-shl.exp
@@ -22,6 +22,13 @@ if {[skip_shlib_tests]} {
     return 0
 }
 
+# clang will optimize away the static foo, making a single breakpoint
+# so there is no point testing it here.
+if {[test_compiler_info {clang-*-*}]} {
+    untested "clang only compiles one foo"
+    return
+}
+
 standard_testfile msym-bp-shl-main.c msym-bp-shl-main-2.c msym-bp-shl-lib.c
 set srcfile ${srcdir}/${subdir}/${srcfile}
 set srcfile2 ${srcdir}/${subdir}/${srcfile2}
-- 
2.31.1

[PATCH 10/11] make use of finish to leave function in gdb.base/skip-inline.exp

[Bruno Larsen via Gdb-patches]

This test, once again, uses a set number of steps to exit a function
when it could just use finish.  Once we are inside baz, we dont need
to use "step" to confirm where we are, so those tests were changed
to using finish instead. This also makes the first part of the test
run corectly when compiled using clang.

And because the second and third part of the test require a fixed amount
of steps, the test exits early if it detects that the compiler used was
clang.
---
 gdb/testsuite/gdb.base/skip-inline.exp | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/gdb/testsuite/gdb.base/skip-inline.exp b/gdb/testsuite/gdb.base/skip-inline.exp
index f6e9926b66c..612210127cc 100644
--- a/gdb/testsuite/gdb.base/skip-inline.exp
+++ b/gdb/testsuite/gdb.base/skip-inline.exp
@@ -35,16 +35,20 @@ gdb_test "skip function foo" "Function foo will be skipped when stepping\."
 gdb_test "bt" "\\s*\\#0\\s+main.*" "in the main"
 gdb_test "step" ".*" "step into baz, since foo will be skipped"
 gdb_test "bt" "\\s*\\#0\\s+baz.*" "in the baz, since foo was skipped"
-gdb_test "step" ".*" "step in the baz"
-gdb_test "bt" "\\s*\\#0\\s+baz.*" "still in the baz"
-gdb_test "step" ".*" "step back to main"
+gdb_test "finish" ".*" "finish baz"
 gdb_test "bt" "\\s*\\#0\\s+main.*" "again in the main"
 gdb_test "step" ".*" "step again into baz, since foo will be skipped"
 gdb_test "bt" "\\s*\\#0\\s+baz.*" "again in the baz"
-gdb_test "step" ".*" "step in the baz, again"
-gdb_test "bt" "\\s*\\#0\\s+baz.*" "still in the baz, again"
-gdb_test "step" ".*" "step back to main, again"
-gdb_test "bt" "\\s*\\#0\\s+main.*" "again back to main"
+gdb_test "finish" ".*" "finish baz, again"
+gdb_test "bt" "\\s*\\#0.*main.*" "again back to main"
+
+# because clang doesn't add epilogue information, having a set number of
+# steps puts clang more and more out of sync with gcc.  It is unlikely that
+# the effort of keeping both outputs will be useful.
+if {[test_compiler_info "clang-*"]} {
+    untested "Multiple steps are not supported with clang"
+    return
+}
 
 if ![runto_main] {
     return
-- 
2.31.1

[PATCH 11/11] explicitly test for stderr in gdb.base/dprintf.exp

[Bruno Larsen via Gdb-patches]

Not all compilers explicitly add stderr information when compiling a
program, so to avoid an unrelated failure, we explicitly test to see if
the compiler has added information about it at all. This was done
thinking specifically about clang, since it doesn't add stderr
information and developers treat it as a feature.
---
 gdb/testsuite/gdb.base/dprintf.exp | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/gdb/testsuite/gdb.base/dprintf.exp b/gdb/testsuite/gdb.base/dprintf.exp
index 0b209c02a62..e214531f6dc 100644
--- a/gdb/testsuite/gdb.base/dprintf.exp
+++ b/gdb/testsuite/gdb.base/dprintf.exp
@@ -111,6 +111,16 @@ proc test_call {} {
 	test_dprintf "At foo entry.*arg=1235, g=2222\r\n" "2nd dprintf"
     }
 
+    gdb_test_multiple "print stderr" "stderr symbol check" {
+	-re "\\'stderr\\' has unknown type.*" {
+	    untested "No information available for stderr, exiting early"
+	    return
+	}
+	-re "\\\$1.*" {
+	    pass "stderr is available"
+	}
+    }
+
     with_test_prefix "fprintf" {
 	restart
 
-- 
2.31.1

[PING][PATCH 00/11] gdb/testsuite: Cleanup gdb.base for clang testing

[Bruno Larsen via Gdb-patches]

Ping for the series

On 1/26/22 16:50, Bruno Larsen wrote:
> When testing GDB with clang, gdb.base had over 50 more failures than when
> testing with gcc.  Examining the failed tests led to a few clang bugs, a
> few GDB bugs, and many testsuite assumptions that could be changed.
> 
> After this patch series, nothing should be changed for testing with gcc,
> and testing with clang should only show non-trivial failures for
> maint.exp and macscp.exp, along with GCC failures.
> 
> Bruno Larsen (11):
>    change gdb.base/skip.exp to use finish instead of step
>    change gdb.base/symbol-alias to xfail with clang
>    Change gdb.base/skip-solib.exp deal with lack of epilogue information
>    change gdb.base/nodebug.c to not fail with clang
>    update gdb.base/info-program.exp to not fail with clang
>    fix gdb.base/access-mem-running.exp for clang testing
>    fix gdb.base/call-ar-st to work with clang
>    add xfails to gdb.base/complex-parts.exp when testing with clang
>    gdb/testsuite: don't test gdb.base/msym-bp-shl with clang
>    make use of finish to leave function in gdb.base/skip-inline.exp
>    explicitly test for stderr in gdb.base/dprintf.exp
> 
>   gdb/testsuite/gdb.base/access-mem-running.c |  2 +-
>   gdb/testsuite/gdb.base/call-ar-st.exp       | 13 +++--
>   gdb/testsuite/gdb.base/complex-parts.exp    |  5 ++
>   gdb/testsuite/gdb.base/dprintf.exp          | 10 ++++
>   gdb/testsuite/gdb.base/info-program.exp     |  2 +-
>   gdb/testsuite/gdb.base/msym-bp-shl.exp      |  7 +++
>   gdb/testsuite/gdb.base/nodebug.c            |  2 +-
>   gdb/testsuite/gdb.base/nodebug.exp          |  2 +-
>   gdb/testsuite/gdb.base/skip-inline.exp      | 18 ++++---
>   gdb/testsuite/gdb.base/skip-solib.exp       | 11 +++-
>   gdb/testsuite/gdb.base/skip.exp             | 56 +++++++++------------
>   gdb/testsuite/gdb.base/symbol-alias.exp     |  9 +++-
>   12 files changed, 86 insertions(+), 51 deletions(-)
> 


-- 
Cheers!
Bruno Larsen

[PINGv2][PATCH 00/11] gdb/testsuite: Cleanup gdb.base for clang testing

[Bruno Larsen via Gdb-patches]

Pingv2 for the series

On 2/9/22 09:03, Bruno Larsen wrote:
> Ping for the series
> 
> On 1/26/22 16:50, Bruno Larsen wrote:
>> When testing GDB with clang, gdb.base had over 50 more failures than when
>> testing with gcc.  Examining the failed tests led to a few clang bugs, a
>> few GDB bugs, and many testsuite assumptions that could be changed.
>>
>> After this patch series, nothing should be changed for testing with gcc,
>> and testing with clang should only show non-trivial failures for
>> maint.exp and macscp.exp, along with GCC failures.
>>
>> Bruno Larsen (11):
>>    change gdb.base/skip.exp to use finish instead of step
>>    change gdb.base/symbol-alias to xfail with clang
>>    Change gdb.base/skip-solib.exp deal with lack of epilogue information
>>    change gdb.base/nodebug.c to not fail with clang
>>    update gdb.base/info-program.exp to not fail with clang
>>    fix gdb.base/access-mem-running.exp for clang testing
>>    fix gdb.base/call-ar-st to work with clang
>>    add xfails to gdb.base/complex-parts.exp when testing with clang
>>    gdb/testsuite: don't test gdb.base/msym-bp-shl with clang
>>    make use of finish to leave function in gdb.base/skip-inline.exp
>>    explicitly test for stderr in gdb.base/dprintf.exp
>>
>>   gdb/testsuite/gdb.base/access-mem-running.c |  2 +-
>>   gdb/testsuite/gdb.base/call-ar-st.exp       | 13 +++--
>>   gdb/testsuite/gdb.base/complex-parts.exp    |  5 ++
>>   gdb/testsuite/gdb.base/dprintf.exp          | 10 ++++
>>   gdb/testsuite/gdb.base/info-program.exp     |  2 +-
>>   gdb/testsuite/gdb.base/msym-bp-shl.exp      |  7 +++
>>   gdb/testsuite/gdb.base/nodebug.c            |  2 +-
>>   gdb/testsuite/gdb.base/nodebug.exp          |  2 +-
>>   gdb/testsuite/gdb.base/skip-inline.exp      | 18 ++++---
>>   gdb/testsuite/gdb.base/skip-solib.exp       | 11 +++-
>>   gdb/testsuite/gdb.base/skip.exp             | 56 +++++++++------------
>>   gdb/testsuite/gdb.base/symbol-alias.exp     |  9 +++-
>>   12 files changed, 86 insertions(+), 51 deletions(-)
>>
> 
> 


-- 
Cheers!
Bruno Larsen

Re: [PATCH 01/11] change gdb.base/skip.exp to use finish instead of step

[Andrew Burgess via Gdb-patches]

Thanks for looking at improving clang compatibility.

Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:

> skip.exp was making use of a fixed amount of step commands to exit
> some functions.  This caused some problems when testing GDB with clang,
> as it doesn't add epilogue information for functions.  Since the step
> commands weren't testing important features, they were changed to finish
> commands.
> ---
>  gdb/testsuite/gdb.base/skip.exp | 56 ++++++++++++++-------------------
>  1 file changed, 23 insertions(+), 33 deletions(-)
>
> diff --git a/gdb/testsuite/gdb.base/skip.exp b/gdb/testsuite/gdb.base/skip.exp
> index 7c71bb07a84..a961fbbdf41 100644
> --- a/gdb/testsuite/gdb.base/skip.exp
> +++ b/gdb/testsuite/gdb.base/skip.exp
> @@ -117,9 +117,8 @@ with_test_prefix "step after deleting 1" {
>  	return
>      }
>  
> -    gdb_test "step" "foo \\(\\) at.*" "step 1"
> -    gdb_test "step" ".*" "step 2" ; # Return from foo()
> -    gdb_test "step" "main \\(\\) at.*" "step 3"
> +    gdb_test "step" "foo \\(\\) at.*" "step"
> +    gdb_test "finish" ".*" "finish" ; # Return from foo()

The comment before this "with_test_prefix" block specifically talks
about two skips, which is no longer the case.  Can you ensure that is
updated please.

Additionally, the original third step did check that we ended up back in
main, which your finish doesn't.  I'd ideally like to see the same check
in the finish so that the test isn't getting easier.

Finally, there's really no reason to give this test a separate name if
you're just going to use the command again, so

  gdb_test "finish" "main \\(\\) at .*"	# Return from foo()

should hopefully do the job (completely untested).

I think this feedback applies throughout this patch, so I've not
repeated myself below.

>  }
>  
>  # Now disable the skiplist entry for  skip1.c.  We should now
> @@ -137,13 +136,11 @@ with_test_prefix "step after disabling 3" {
>      }
>  
>      gdb_test "step" "bar \\(\\) at.*" "step 1"
> -    gdb_test "step" ".*" "step 2"; # Return from bar()
> -    # With gcc 9.2.0 we jump once back to main before entering foo here.
> -    # If that happens try to step a second time.
> -    gdb_test "step" "foo \\(\\) at.*" "step 3" \
> -	"main \\(\\) at .*\r\n$gdb_prompt " "step"
> -    gdb_test "step" ".*" "step 4"; # Return from foo()
> -    gdb_test "step" "main \\(\\) at.*" "step 5"
> +    # guarantee that we jump once back to main before entering foo here.

Start comments with a capital letter.

> +    gdb_test "finish" ".*" "finish 1"; # Return to main()
> +    gdb_test "step" "foo \\(\\) at.*" "step 2" \
> +	"main \\(\\) at .*\r\n$gdb_prompt " "step" # enter foo()

This line with the possibility of sending an extra 'step' used to have a
comment attached, which has been removed.

I do wonder if the problem was actually with the preceeding 'step',
which you've now switched for a finish command?

Handily, the old comment had a GCC version number atteched, so it might
be possible to try and recreate the old problem.

I think that if the extra 'step' is still needed then we should be
keeping the comment about GCC 9.2.0 in some form, or, ideally, we'd show
that switching to finish removes the need for that extra 'step', and
remove it.

> +    gdb_test "finish" ".*" "finish 2"; # Return from foo()

Again, would be nicer to have a better pattern than ".*" here.

I think for the rest of the patch it's just the same feedback repeated.

Thank,
Andrew


>  }
>  
>  # Enable skiplist entry 3 and make sure we step over it like before.
> @@ -159,9 +156,8 @@ with_test_prefix "step after enable 3" {
>  	return
>      }
>  
> -    gdb_test "step" "foo \\(\\) at.*" "step 1"
> -    gdb_test "step" ".*" "step 2"; # Return from foo()
> -    gdb_test "step" "main \\(\\) at.*" "step 3"
> +    gdb_test "step" "foo \\(\\) at.*" "step"
> +    gdb_test "finish" ".*" "finish"; # Return from foo()
>  }
>  
>  # Admin tests (disable,enable,delete).
> @@ -230,9 +226,8 @@ with_test_prefix "step using -fi" {
>  
>      gdb_test_no_output "skip disable"
>      gdb_test_no_output "skip enable 5"
> -    gdb_test "step" "foo \\(\\) at.*" "step 1"
> -    gdb_test "step" ".*" "step 2"; # Return from foo()
> -    gdb_test "step" "main \\(\\) at.*" "step 3"
> +    gdb_test "step" "foo \\(\\) at.*" "step"
> +    gdb_test "finish" ".*" "finish"; # Return from foo()
>  }
>  
>  with_test_prefix "step using -gfi" {
> @@ -242,9 +237,8 @@ with_test_prefix "step using -gfi" {
>  
>      gdb_test_no_output "skip disable"
>      gdb_test_no_output "skip enable 6"
> -    gdb_test "step" "foo \\(\\) at.*" "step 1"
> -    gdb_test "step" ".*" "step 2"; # Return from foo()
> -    gdb_test "step" "main \\(\\) at.*" "step 3"
> +    gdb_test "step" "foo \\(\\) at.*" "step"
> +    gdb_test "finish" ".*" "finish"; # Return from foo()
>  }
>  
>  with_test_prefix "step using -fu for baz" {
> @@ -255,13 +249,11 @@ with_test_prefix "step using -fu for baz" {
>      gdb_test_no_output "skip disable"
>      gdb_test_no_output "skip enable 7"
>      gdb_test "step" "bar \\(\\) at.*" "step 1"
> -    gdb_test "step" ".*" "step 2"; # Return from bar()
> -    # With gcc 9.2.0 we jump once back to main before entering foo here.
> -    # If that happens try to step a second time.
> -    gdb_test "step" "foo \\(\\) at.*" "step 3" \
> -	"main \\(\\) at .*\r\n$gdb_prompt " "step"
> -    gdb_test "step" ".*" "step 4"; # Return from foo()
> -    gdb_test "step" "main \\(\\) at.*" "step 5"
> +    # guarantee that we jump once back to main before entering foo here.
> +    gdb_test "finish" ".*" "finish 1"; # Return to main()
> +    gdb_test "step" "foo \\(\\) at.*" "step 2" \
> +	"main \\(\\) at .*\r\n$gdb_prompt " "step" # enter foo()
> +    gdb_test "finish" ".*" "finish 2"; # Return from foo()
>  }
>  
>  with_test_prefix "step using -rfu for baz" {
> @@ -272,13 +264,11 @@ with_test_prefix "step using -rfu for baz" {
>      gdb_test_no_output "skip disable"
>      gdb_test_no_output "skip enable 8"
>      gdb_test "step" "bar \\(\\) at.*" "step 1"
> -    gdb_test "step" ".*" "step 2"; # Return from bar()
> -    # With gcc 9.2.0 we jump once back to main before entering foo here.
> -    # If that happens try to step a second time.
> -    gdb_test "step" "foo \\(\\) at.*" "step 3" \
> -	"main \\(\\) at .*\r\n$gdb_prompt " "step"
> -    gdb_test "step" ".*" "step 4"; # Return from foo()
> -    gdb_test "step" "main \\(\\) at.*" "step 5"
> +    # guarantee that we jump once back to main before entering foo here.
> +    gdb_test "finish" ".*" "finish 1"; # Return to main()
> +    gdb_test "step" "foo \\(\\) at.*" "step 2" \
> +	"main \\(\\) at .*\r\n$gdb_prompt " "step" # enter foo()
> +    gdb_test "finish" ".*" "finish 2"; # Return from foo()
>  }
>  
>  # Test -fi + -fu.
> -- 
> 2.31.1

Re: [PATCH 01/11] change gdb.base/skip.exp to use finish instead of step

[Pedro Alves]

On 2022-01-26 19:50, Bruno Larsen via Gdb-patches wrote:
> skip.exp was making use of a fixed amount of step commands to exit
> some functions.  This caused some problems when testing GDB with clang,
> as it doesn't add epilogue information for functions.  Since the step
> commands weren't testing important features, they were changed to finish
> commands.

I'm not a fan of _completely_ avoiding stepping out of functions, due to some
compilers misbehaving.  I mean, if we're going to do this throughout
the testsuite, then we should at least have one testcase that is explicitly
about stepping out of a function with step, one that exhibits the problem,
and we can concentrate the xfails there (and we should press the compiler
folks to fix it).  I'm not sure we have one.  If we do, you should mention it
explicitly in the commit log.  Afterall, users will be doing this too.

Re: [PATCH 03/11] Change gdb.base/skip-solib.exp deal with lack of epilogue information

[Pedro Alves]

On 2022-01-26 19:50, Bruno Larsen via Gdb-patches wrote:
> When testing with clang, this feature worked, but the test failed
> because it checked specifically to see if we were at the main function.
> Without epilogue information, the inferior would be at libc_start_main
> (or equivalent) instead. Having this test changed would allows us to
> identify if the test stops working for real at some point.

I'm having trouble understanding what you're saying.  I wonder whether
pasting a snippet of the failing case would help?

> ---
>  gdb/testsuite/gdb.base/skip-solib.exp | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.base/skip-solib.exp b/gdb/testsuite/gdb.base/skip-solib.exp
> index ce2b080229e..99833533b6a 100644
> --- a/gdb/testsuite/gdb.base/skip-solib.exp
> +++ b/gdb/testsuite/gdb.base/skip-solib.exp
> @@ -82,7 +82,7 @@ with_test_prefix "ignoring solib file" {
>      # We shouldn't step into square(), since we skipped skip-solib-lib.c.
>      #
>      gdb_test "step" ""
> -    gdb_test "bt" "#0\\s+main.*"
> +    gdb_test "bt 1" "#0.*main.*"

Is the tweak here to change the regexp to match both "main" and "libc_start_main"?

>  }
>  
>  #
> @@ -114,5 +114,12 @@ with_test_prefix "ignoring solib function" {
>      # the last line of square.
>      #
>      gdb_test "step" ""
> -    gdb_test "bt" "#0\\s+square.*"
> +    gdb_test_multiple "bt 1" "skipped multiply" {
> +	-re "#0\\s+square.*" {
> +	    pass "skipped multiply"
> +	}
> +	-re "#0.*main.*" {
> +	    pass "skipped multiply"
> +	}
> +    }
>  }

Re: [PATCH 01/11] change gdb.base/skip.exp to use finish instead of step

[Andrew Burgess via Gdb-patches]

Pedro Alves <pedro@palves.net> writes:

> On 2022-01-26 19:50, Bruno Larsen via Gdb-patches wrote:
>> skip.exp was making use of a fixed amount of step commands to exit
>> some functions.  This caused some problems when testing GDB with clang,
>> as it doesn't add epilogue information for functions.  Since the step
>> commands weren't testing important features, they were changed to finish
>> commands.
>
> I'm not a fan of _completely_ avoiding stepping out of functions, due to some
> compilers misbehaving.  I mean, if we're going to do this throughout
> the testsuite, then we should at least have one testcase that is explicitly
> about stepping out of a function with step, one that exhibits the problem,
> and we can concentrate the xfails there (and we should press the compiler
> folks to fix it).  I'm not sure we have one.  If we do, you should mention it
> explicitly in the commit log.  Afterall, users will be doing this too.

Agreed, and there's probably scope for a proc in lib/gdb.exp that does
something like "step until we reach a source line matching REGEXP",
which could be used instead of finish in some cases.

Thanks,
Andrew

Re: [PATCH 11/11] explicitly test for stderr in gdb.base/dprintf.exp

[Pedro Alves]

On 2022-01-26 19:50, Bruno Larsen via Gdb-patches wrote:
> Not all compilers explicitly add stderr information when compiling a

It helps if you say "debug information" instead of just "information".

> program, so to avoid an unrelated failure, we explicitly test to see if
> the compiler has added information about it at all. This was done
> thinking specifically about clang, since it doesn't add stderr
> information and developers treat it as a feature.

Please include a snippet of the failure in the commit log.

I ran the test locally against clang, and I do see a failure here too:

 (gdb) PASS: gdb.base/dprintf.exp: call: fprintf: set dprintf style to call
 continue
 Continuing.
 kickoff 1234
 also to stderr 1234
 'stderr' has unknown type; cast it to its declared type
 (gdb) FAIL: gdb.base/dprintf.exp: call: fprintf: 1st dprintf (timeout)

However, when I debug the program manually, I see that gdb is able to print stderr,
if you run to main first:

 (gdb) p stderr
 'stderr' has unknown type; cast it to its declared type
 (gdb) start
 Temporary breakpoint 1 at 0x4011f6: file ../../../src/gdb/testsuite/gdb.base/dprintf.c, line 35.
 Starting program: /home/pedro/rocm/gdb/build/gdb/testsuite/outputs/gdb.base/dprintf/dprintf 

 Temporary breakpoint 1, main (argc=1, argv=0x7fffffffdc18) at ../../../src/gdb/testsuite/gdb.base/dprintf.c:35
 35        int loc = 1234;
 (gdb) p stderr
 $1 = (FILE *) 0x7ffff7e4e5c0 <_IO_2_1_stderr_>

So... it doesn't seem like the premise of the patch is correct, for the testcase
runs to main -- see the "restart" call in the context of your patch, just below.
Seems like something else may be going on?  Why is GDB not finding the debug info for
stderr in the dprintf call, while manually it is able to find it?

Pedro Alves

> ---
>  gdb/testsuite/gdb.base/dprintf.exp | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/gdb/testsuite/gdb.base/dprintf.exp b/gdb/testsuite/gdb.base/dprintf.exp
> index 0b209c02a62..e214531f6dc 100644
> --- a/gdb/testsuite/gdb.base/dprintf.exp
> +++ b/gdb/testsuite/gdb.base/dprintf.exp
> @@ -111,6 +111,16 @@ proc test_call {} {
>  	test_dprintf "At foo entry.*arg=1235, g=2222\r\n" "2nd dprintf"
>      }
>  
> +    gdb_test_multiple "print stderr" "stderr symbol check" {
> +	-re "\\'stderr\\' has unknown type.*" {
> +	    untested "No information available for stderr, exiting early"
> +	    return
> +	}
> +	-re "\\\$1.*" {
> +	    pass "stderr is available"
> +	}
> +    }
> +
>      with_test_prefix "fprintf" {
>  	restart
>  

Re: [PATCH 07/11] fix gdb.base/call-ar-st to work with clang

[Pedro Alves]

On 2022-01-26 19:50, Bruno Larsen via Gdb-patches wrote:
> When running this test with clang-compiled code, this test was failing
> by producing an output like
> array_i=<main.integer_array>, ...
> 
> instead of
> 
> array_i = <integer_array>, ...
> 
> This commit changes the testcase to accept both outputs, as they are
> functionally identical.

It would be useful to explain that the symbols in question are local static variables.
"main" is the name of the function they are defined in, and Clang puts that in the elf/linkage name.
GCC instead appends a sequence number to the linkage name:

 $ nm -A call-ar-st.gcc | grep integer_
 call-ar-st/call-ar-st:00000000000061a0 b integer_array.3968

 $ nm -A call-ar-st.clang | grep integer_
 call-ar-st:00000000004061a0 b main.integer_array

Note that for GCC, even though the ELF symbol is called "integer_array.3968", GDB still
prints "integer_array", without the suffix.  I wonder how that happens?  Anyone knows offhand?
It kind of looks like the testcase was expecting that the ".3968" suffix could appear in
the output, given that it expects "array_i=<integer_array.*>" instead of "array_i=<integer_array>" ?

> ---
>  gdb/testsuite/gdb.base/call-ar-st.exp | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.base/call-ar-st.exp b/gdb/testsuite/gdb.base/call-ar-st.exp
> index d1fb3ac9ec1..e447d3e231d 100644
> --- a/gdb/testsuite/gdb.base/call-ar-st.exp
> +++ b/gdb/testsuite/gdb.base/call-ar-st.exp
> @@ -153,9 +153,16 @@ if {!$skip_float_test && \
>  
>  #step
>  set stop_line [gdb_get_line_number "-step1-"]
> -gdb_test "step" \
> -    "print_all_arrays \\(array_i=<integer_array.*>, array_c=<char_array.*> .ZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZa., array_f=<float_array.*>, array_d=<double_array.*>\\) at .*$srcfile:$stop_line\[ \t\r\n\]+$stop_line.*print_int_array\\(array_i\\);.*" \
> -    "step inside print_all_arrays"
> +
> +gdb_test_multiple "step" "" {
> +
> +    -re "print_all_arrays \\(array_i=<integer_array.*>, array_c=<char_array.*> .ZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZa., array_f=<float_array.*>, array_d=<double_array.*>\\) at .*$srcfile:$stop_line\[ \t\r\n\]+$stop_line.*print_int_array\\(array_i\\);.*" {
> +	pass "step inside print_all_arrays"
> +    }
> +    -re "print_all_arrays \\(array_i=<main.integer_array>, array_c=<main.char_array> .ZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZa., array_f=<main.float_array>, array_d=<main.double_array>\\) at .*$srcfile:$stop_line\[ \t\r\n\]+$stop_line.*print_int_array\\(array_i\\);.*" {
> +	pass "step inside print_all_arrays (clang)"
> +    }
> +}
>  

If we end up taking the approach of accepting the current GDB output, then it would seem better
to me to continue having a single expression.  Something like this:

From d024950e743df1712f9adda759023e5ba07ee6e1 Mon Sep 17 00:00:00 2001
From: Bruno Larsen <blarsen@redhat.com>
Date: Wed, 2 Mar 2022 18:09:31 +0000
Subject: [PATCH] Fix gdb.base/call-ar-st to work with Clang

When running gdb.base/call-ar-st.exp against Clang, we see one FAIL,
like so:

 print_all_arrays (array_i=<main.integer_array>, array_c=<main.char_array> "ZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZa
 ZaZaZaZaZaZaZaZaZaZaZaZa", array_f=<main.float_array>, array_d=<main.double_array>) at ../../../src/gdb/testsuite/gdb.base/call-ar-st.c:274
 274       print_int_array(array_i);     /* -step1- */
 (gdb) FAIL: gdb.base/call-ar-st.exp: step inside print_all_arrays

With GCC we instead see:

 print_all_arrays (array_i=<integer_array>, array_c=<char_array> "ZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZa", array_f=<float_array>, array_d=<double_array>) at /home/pedro/gdb/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/call-ar-st.c:274
 274       print_int_array(array_i);     /* -step1- */
 (gdb) PASS: gdb.base/call-ar-st.exp: step inside print_all_arrays

The difference is that with Clang we get:

 array_i=<main.integer_array>, ...

instead of

 array_i = <integer_array>, ...

These symbols are local static variables, and "main" is the name of
the function they are defined in.  GCC instead appends a sequence
number to the linkage name:

 $ nm -A call-ar-st.gcc | grep integer_
 call-ar-st/call-ar-st:00000000000061a0 b integer_array.3968

 $ nm -A call-ar-st.clang | grep integer_
 call-ar-st:00000000004061a0 b main.integer_array

This commit changes the testcase to accept both outputs, as they are
functionally identical.

Co-Authored-By: Pedro Alves <pedro@palves.net>
Change-Id: Iaf2ccdb9d5996e0268ed12f595a6e04b368bfcb4
---
 gdb/testsuite/gdb.base/call-ar-st.exp | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.base/call-ar-st.exp b/gdb/testsuite/gdb.base/call-ar-st.exp
index d1fb3ac9ec1..43fd6ff9f40 100644
--- a/gdb/testsuite/gdb.base/call-ar-st.exp
+++ b/gdb/testsuite/gdb.base/call-ar-st.exp
@@ -151,10 +151,21 @@ if {!$skip_float_test && \
     gdb_test "continue" ".*" ""
 }
 
+# Return a regexp that matches the linkage name of SYM, assuming SYM
+# is a local static variable inside the main function.
+proc main_static_local_re {sym} {
+    # Clang prepends the function name + '.'.
+    return "(main\\.)?${sym}"
+}
+
 #step
 set stop_line [gdb_get_line_number "-step1-"]
 gdb_test "step" \
-    "print_all_arrays \\(array_i=<integer_array.*>, array_c=<char_array.*> .ZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZa., array_f=<float_array.*>, array_d=<double_array.*>\\) at .*$srcfile:$stop_line\[ \t\r\n\]+$stop_line.*print_int_array\\(array_i\\);.*" \
+    "print_all_arrays \\(array_i=<[main_static_local_re integer_array]>,\
+			 array_c=<[main_static_local_re char_array]> .ZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZa.,\
+			 array_f=<[main_static_local_re float_array]>,\
+			 array_d=<[main_static_local_re double_array]>\\)\
+			 at .*$srcfile:$stop_line\[ \t\r\n\]+$stop_line.*print_int_array\\(array_i\\);.*" \
     "step inside print_all_arrays"
 
 #step -over

-- 
2.26.2

Re: [PATCH 08/11] add xfails to gdb.base/complex-parts.exp when testing with clang

[Pedro Alves]

On 2022-01-26 19:50, Bruno Larsen via Gdb-patches wrote:
> clang doesn't add encoding to the name of complex variables, only says
> that the type name is complex, making the relevant tests fail.
> This patch adds the xfails to the tests that expect the variable name to
> include it.
> ---
>  gdb/testsuite/gdb.base/complex-parts.exp | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/gdb/testsuite/gdb.base/complex-parts.exp b/gdb/testsuite/gdb.base/complex-parts.exp
> index e67fd482268..b4bb542f218 100644
> --- a/gdb/testsuite/gdb.base/complex-parts.exp
> +++ b/gdb/testsuite/gdb.base/complex-parts.exp
> @@ -30,8 +30,13 @@ gdb_test "p z1" " = 1.5 \\+ 4.5i"
>  gdb_test "p z2" " = 2.5 \\+ -5.5i"
>  gdb_test "p z3" " = 3.5 \\+ 6.5i"
>  
> +# The following 3 tests are boken for clang.

boken -> broken

clang -> Clang (uppercase)

> +# More info on https://github.com/llvm/llvm-project/issues/52996

on -> at

Period at end of sentence.

OK with these tweaks.

Thanks,
Pedro Alves

> +if {[test_compiler_info clang-*-*]} { setup_xfail *-*-* }
>  gdb_test "ptype z1" " = complex double"
> +if {[test_compiler_info clang-*-*]} { setup_xfail *-*-* }
>  gdb_test "ptype z2" " = complex float"
> +if {[test_compiler_info clang-*-*]} { setup_xfail *-*-* }
>  gdb_test "ptype z3" " = complex long double"
>  
>  with_test_prefix "double imaginary" {

Re: [PATCH 09/11] gdb/testsuite: don't test gdb.base/msym-bp-shl with clang

[Pedro Alves]

On 2022-01-26 19:50, Bruno Larsen via Gdb-patches wrote:
> Clang will optimize away the static function in one of the files, and
> the test is here to specifically test GDB's behavior when that function
> is present, so it makes no sense to have this test run with that
> compiler.

Please expand this info to include a snippet of the failing test.
Also, it's best if the the body of the commit log, doesn't assume the subject is read
as part of it.  You will notice that if you read the body in isolation, without the
subject line, and also without the diff context, the commit log is vague as is.

> ---
>  gdb/testsuite/gdb.base/msym-bp-shl.exp | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/gdb/testsuite/gdb.base/msym-bp-shl.exp b/gdb/testsuite/gdb.base/msym-bp-shl.exp
> index 42adcb191dd..95d5c393505 100644
> --- a/gdb/testsuite/gdb.base/msym-bp-shl.exp
> +++ b/gdb/testsuite/gdb.base/msym-bp-shl.exp
> @@ -22,6 +22,13 @@ if {[skip_shlib_tests]} {
>      return 0
>  }
>  
> +# clang will optimize away the static foo, making a single breakpoint
> +# so there is no point testing it here.

Uppercase Clang.

The patch doesn't give enough info to understand why this is the best approach.
I could imagine using atttribute used to make sure the function isn't optimized
away, as alternative approach, for example.  Please make the case for the approach
taken.

> +if {[test_compiler_info {clang-*-*}]} {
> +    untested "clang only compiles one foo"
> +    return
> +}
> +
>  standard_testfile msym-bp-shl-main.c msym-bp-shl-main-2.c msym-bp-shl-lib.c
>  set srcfile ${srcdir}/${subdir}/${srcfile}
>  set srcfile2 ${srcdir}/${subdir}/${srcfile2}

Re: [PATCH 07/11] fix gdb.base/call-ar-st to work with clang

[Tom Tromey]

Pedro> Note that for GCC, even though the ELF symbol is called
Pedro> "integer_array.3968", GDB still prints "integer_array", without
Pedro> the suffix.  I wonder how that happens?  Anyone knows offhand?

I know some suffixes are stripped in bfd_demangle, and some in
elfread.c, but I couldn't immediately find where this one would be
stripped.

Tom

Re: [PATCH 03/11] Change gdb.base/skip-solib.exp deal with lack of epilogue information

[Bruno Larsen via Gdb-patches]

On 3/2/22 13:17, Pedro Alves wrote:
> On 2022-01-26 19:50, Bruno Larsen via Gdb-patches wrote:
>> When testing with clang, this feature worked, but the test failed
>> because it checked specifically to see if we were at the main function.
>> Without epilogue information, the inferior would be at libc_start_main
>> (or equivalent) instead. Having this test changed would allows us to
>> identify if the test stops working for real at some point.
> 
> I'm having trouble understanding what you're saying.  I wonder whether
> pasting a snippet of the failing case would help?
> 
>> ---
>>   gdb/testsuite/gdb.base/skip-solib.exp | 11 +++++++++--
>>   1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/gdb/testsuite/gdb.base/skip-solib.exp b/gdb/testsuite/gdb.base/skip-solib.exp
>> index ce2b080229e..99833533b6a 100644
>> --- a/gdb/testsuite/gdb.base/skip-solib.exp
>> +++ b/gdb/testsuite/gdb.base/skip-solib.exp
>> @@ -82,7 +82,7 @@ with_test_prefix "ignoring solib file" {
>>       # We shouldn't step into square(), since we skipped skip-solib-lib.c.
>>       #
>>       gdb_test "step" ""
>> -    gdb_test "bt" "#0\\s+main.*"
>> +    gdb_test "bt 1" "#0.*main.*"
> 
> Is the tweak here to change the regexp to match both "main" and "libc_start_main"?

Sorry about the confusing commit message. Yes, this was the plan. Essentially, the test was failing because we dropped to libc_start_main and it expected to still be in main.

I have changed my mind on this approach, however. V2 should be coming shortly.

> 
>>   }
>>   
>>   #
>> @@ -114,5 +114,12 @@ with_test_prefix "ignoring solib function" {
>>       # the last line of square.
>>       #
>>       gdb_test "step" ""
>> -    gdb_test "bt" "#0\\s+square.*"
>> +    gdb_test_multiple "bt 1" "skipped multiply" {
>> +	-re "#0\\s+square.*" {
>> +	    pass "skipped multiply"
>> +	}
>> +	-re "#0.*main.*" {
>> +	    pass "skipped multiply"
>> +	}
>> +    }
>>   }
> 


-- 
Cheers!
Bruno Larsen

Re: [PATCH 01/11] change gdb.base/skip.exp to use finish instead of step

[Bruno Larsen via Gdb-patches]

On 3/2/22 13:39, Andrew Burgess wrote:
> Pedro Alves <pedro@palves.net> writes:
> 
>> On 2022-01-26 19:50, Bruno Larsen via Gdb-patches wrote:
>>> skip.exp was making use of a fixed amount of step commands to exit
>>> some functions.  This caused some problems when testing GDB with clang,
>>> as it doesn't add epilogue information for functions.  Since the step
>>> commands weren't testing important features, they were changed to finish
>>> commands.
>>
>> I'm not a fan of _completely_ avoiding stepping out of functions, due to some
>> compilers misbehaving.  I mean, if we're going to do this throughout
>> the testsuite, then we should at least have one testcase that is explicitly
>> about stepping out of a function with step, one that exhibits the problem,
>> and we can concentrate the xfails there (and we should press the compiler
>> folks to fix it).  I'm not sure we have one.  If we do, you should mention it
>> explicitly in the commit log.  Afterall, users will be doing this too.
> 
> Agreed, and there's probably scope for a proc in lib/gdb.exp that does
> something like "step until we reach a source line matching REGEXP",
> which could be used instead of finish in some cases.
> 
> Thanks,
> Andrew
> 

Interesting suggestion. I'm not sure "step until we reach a source matching REGEXP" is the best approach, since anything off in that regexp means that we might be single stepping until the program exits - a bit too time-consuming for my liking. I think that a proc that single steps until it exits the current frame. Does this sound reasonable?

-- 
Cheers!
Bruno Larsen

Re: [PATCH 07/11] fix gdb.base/call-ar-st to work with clang

[Bruno Larsen via Gdb-patches]

On 3/2/22 15:59, Pedro Alves wrote:
> On 2022-01-26 19:50, Bruno Larsen via Gdb-patches wrote:
>> When running this test with clang-compiled code, this test was failing
>> by producing an output like
>> array_i=<main.integer_array>, ...
>>
>> instead of
>>
>> array_i = <integer_array>, ...
>>
>> This commit changes the testcase to accept both outputs, as they are
>> functionally identical.
> 
> It would be useful to explain that the symbols in question are local static variables.
> "main" is the name of the function they are defined in, and Clang puts that in the elf/linkage name.
> GCC instead appends a sequence number to the linkage name:
> 
>   $ nm -A call-ar-st.gcc | grep integer_
>   call-ar-st/call-ar-st:00000000000061a0 b integer_array.3968
> 
>   $ nm -A call-ar-st.clang | grep integer_
>   call-ar-st:00000000004061a0 b main.integer_array
> 
> Note that for GCC, even though the ELF symbol is called "integer_array.3968", GDB still
> prints "integer_array", without the suffix.  I wonder how that happens?  Anyone knows offhand?
> It kind of looks like the testcase was expecting that the ".3968" suffix could appear in
> the output, given that it expects "array_i=<integer_array.*>" instead of "array_i=<integer_array>" ?

I tried to dig around for a bit, but couldn't find any obvious location. I can look some more if we ultimately decide it is a good idea to remove it, but personally, I think that knowing where the variable was declared could be helpful for debugging. Meanwhile, your patch looks good, I can add it to my tree for pushing at the end.

> 
>> ---
>>   gdb/testsuite/gdb.base/call-ar-st.exp | 13 ++++++++++---
>>   1 file changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/gdb/testsuite/gdb.base/call-ar-st.exp b/gdb/testsuite/gdb.base/call-ar-st.exp
>> index d1fb3ac9ec1..e447d3e231d 100644
>> --- a/gdb/testsuite/gdb.base/call-ar-st.exp
>> +++ b/gdb/testsuite/gdb.base/call-ar-st.exp
>> @@ -153,9 +153,16 @@ if {!$skip_float_test && \
>>   
>>   #step
>>   set stop_line [gdb_get_line_number "-step1-"]
>> -gdb_test "step" \
>> -    "print_all_arrays \\(array_i=<integer_array.*>, array_c=<char_array.*> .ZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZa., array_f=<float_array.*>, array_d=<double_array.*>\\) at .*$srcfile:$stop_line\[ \t\r\n\]+$stop_line.*print_int_array\\(array_i\\);.*" \
>> -    "step inside print_all_arrays"
>> +
>> +gdb_test_multiple "step" "" {
>> +
>> +    -re "print_all_arrays \\(array_i=<integer_array.*>, array_c=<char_array.*> .ZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZa., array_f=<float_array.*>, array_d=<double_array.*>\\) at .*$srcfile:$stop_line\[ \t\r\n\]+$stop_line.*print_int_array\\(array_i\\);.*" {
>> +	pass "step inside print_all_arrays"
>> +    }
>> +    -re "print_all_arrays \\(array_i=<main.integer_array>, array_c=<main.char_array> .ZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZa., array_f=<main.float_array>, array_d=<main.double_array>\\) at .*$srcfile:$stop_line\[ \t\r\n\]+$stop_line.*print_int_array\\(array_i\\);.*" {
>> +	pass "step inside print_all_arrays (clang)"
>> +    }
>> +}
>>   
> 
> If we end up taking the approach of accepting the current GDB output, then it would seem better
> to me to continue having a single expression.  Something like this:
> 
>  From d024950e743df1712f9adda759023e5ba07ee6e1 Mon Sep 17 00:00:00 2001
> From: Bruno Larsen <blarsen@redhat.com>
> Date: Wed, 2 Mar 2022 18:09:31 +0000
> Subject: [PATCH] Fix gdb.base/call-ar-st to work with Clang
> 
> When running gdb.base/call-ar-st.exp against Clang, we see one FAIL,
> like so:
> 
>   print_all_arrays (array_i=<main.integer_array>, array_c=<main.char_array> "ZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZa
>   ZaZaZaZaZaZaZaZaZaZaZaZa", array_f=<main.float_array>, array_d=<main.double_array>) at ../../../src/gdb/testsuite/gdb.base/call-ar-st.c:274
>   274       print_int_array(array_i);     /* -step1- */
>   (gdb) FAIL: gdb.base/call-ar-st.exp: step inside print_all_arrays
> 
> With GCC we instead see:
> 
>   print_all_arrays (array_i=<integer_array>, array_c=<char_array> "ZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZa", array_f=<float_array>, array_d=<double_array>) at /home/pedro/gdb/build/gdb/testsuite/../../../src/gdb/testsuite/gdb.base/call-ar-st.c:274
>   274       print_int_array(array_i);     /* -step1- */
>   (gdb) PASS: gdb.base/call-ar-st.exp: step inside print_all_arrays
> 
> The difference is that with Clang we get:
> 
>   array_i=<main.integer_array>, ...
> 
> instead of
> 
>   array_i = <integer_array>, ...
> 
> These symbols are local static variables, and "main" is the name of
> the function they are defined in.  GCC instead appends a sequence
> number to the linkage name:
> 
>   $ nm -A call-ar-st.gcc | grep integer_
>   call-ar-st/call-ar-st:00000000000061a0 b integer_array.3968
> 
>   $ nm -A call-ar-st.clang | grep integer_
>   call-ar-st:00000000004061a0 b main.integer_array
> 
> This commit changes the testcase to accept both outputs, as they are
> functionally identical.
> 
> Co-Authored-By: Pedro Alves <pedro@palves.net>
> Change-Id: Iaf2ccdb9d5996e0268ed12f595a6e04b368bfcb4
> ---
>   gdb/testsuite/gdb.base/call-ar-st.exp | 13 ++++++++++++-
>   1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/testsuite/gdb.base/call-ar-st.exp b/gdb/testsuite/gdb.base/call-ar-st.exp
> index d1fb3ac9ec1..43fd6ff9f40 100644
> --- a/gdb/testsuite/gdb.base/call-ar-st.exp
> +++ b/gdb/testsuite/gdb.base/call-ar-st.exp
> @@ -151,10 +151,21 @@ if {!$skip_float_test && \
>       gdb_test "continue" ".*" ""
>   }
>   
> +# Return a regexp that matches the linkage name of SYM, assuming SYM
> +# is a local static variable inside the main function.
> +proc main_static_local_re {sym} {
> +    # Clang prepends the function name + '.'.
> +    return "(main\\.)?${sym}"
> +}
> +
>   #step
>   set stop_line [gdb_get_line_number "-step1-"]
>   gdb_test "step" \
> -    "print_all_arrays \\(array_i=<integer_array.*>, array_c=<char_array.*> .ZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZa., array_f=<float_array.*>, array_d=<double_array.*>\\) at .*$srcfile:$stop_line\[ \t\r\n\]+$stop_line.*print_int_array\\(array_i\\);.*" \
> +    "print_all_arrays \\(array_i=<[main_static_local_re integer_array]>,\
> +			 array_c=<[main_static_local_re char_array]> .ZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZaZa.,\
> +			 array_f=<[main_static_local_re float_array]>,\
> +			 array_d=<[main_static_local_re double_array]>\\)\
> +			 at .*$srcfile:$stop_line\[ \t\r\n\]+$stop_line.*print_int_array\\(array_i\\);.*" \
>       "step inside print_all_arrays"
>   
>   #step -over
> 


-- 
Cheers!
Bruno Larsen

Re: [PATCH 09/11] gdb/testsuite: don't test gdb.base/msym-bp-shl with clang

[Bruno Larsen via Gdb-patches]

On 3/2/22 16:33, Pedro Alves wrote:
> On 2022-01-26 19:50, Bruno Larsen via Gdb-patches wrote:
>> Clang will optimize away the static function in one of the files, and
>> the test is here to specifically test GDB's behavior when that function
>> is present, so it makes no sense to have this test run with that
>> compiler.
> 
> Please expand this info to include a snippet of the failing test.
> Also, it's best if the the body of the commit log, doesn't assume the subject is read
> as part of it.  You will notice that if you read the body in isolation, without the
> subject line, and also without the diff context, the commit log is vague as is.

Oh, ok. Will do!

> 
>> ---
>>   gdb/testsuite/gdb.base/msym-bp-shl.exp | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/gdb/testsuite/gdb.base/msym-bp-shl.exp b/gdb/testsuite/gdb.base/msym-bp-shl.exp
>> index 42adcb191dd..95d5c393505 100644
>> --- a/gdb/testsuite/gdb.base/msym-bp-shl.exp
>> +++ b/gdb/testsuite/gdb.base/msym-bp-shl.exp
>> @@ -22,6 +22,13 @@ if {[skip_shlib_tests]} {
>>       return 0
>>   }
>>   
>> +# clang will optimize away the static foo, making a single breakpoint
>> +# so there is no point testing it here.
> 
> Uppercase Clang.
> 
> The patch doesn't give enough info to understand why this is the best approach.
> I could imagine using atttribute used to make sure the function isn't optimized
> away, as alternative approach, for example.  Please make the case for the approach
> taken.

Doh! I forgot that attribute((used)) worked for functions as well, I'll do that instead, since it also works.

> 
>> +if {[test_compiler_info {clang-*-*}]} {
>> +    untested "clang only compiles one foo"
>> +    return
>> +}
>> +
>>   standard_testfile msym-bp-shl-main.c msym-bp-shl-main-2.c msym-bp-shl-lib.c
>>   set srcfile ${srcdir}/${subdir}/${srcfile}
>>   set srcfile2 ${srcdir}/${subdir}/${srcfile2}
> 


-- 
Cheers!
Bruno Larsen

Re: [PATCH 09/11] gdb/testsuite: don't test gdb.base/msym-bp-shl with clang

[Bruno Larsen via Gdb-patches]

On 3/2/22 16:33, Pedro Alves wrote:
> On 2022-01-26 19:50, Bruno Larsen via Gdb-patches wrote:
>> Clang will optimize away the static function in one of the files, and
>> the test is here to specifically test GDB's behavior when that function
>> is present, so it makes no sense to have this test run with that
>> compiler.
> 
> Please expand this info to include a snippet of the failing test.
> Also, it's best if the the body of the commit log, doesn't assume the subject is read
> as part of it.  You will notice that if you read the body in isolation, without the
> subject line, and also without the diff context, the commit log is vague as is.

Will do! Sorry for the delay, got sidetracked with another issue.

> 
>> ---
>>   gdb/testsuite/gdb.base/msym-bp-shl.exp | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/gdb/testsuite/gdb.base/msym-bp-shl.exp b/gdb/testsuite/gdb.base/msym-bp-shl.exp
>> index 42adcb191dd..95d5c393505 100644
>> --- a/gdb/testsuite/gdb.base/msym-bp-shl.exp
>> +++ b/gdb/testsuite/gdb.base/msym-bp-shl.exp
>> @@ -22,6 +22,13 @@ if {[skip_shlib_tests]} {
>>       return 0
>>   }
>>   
>> +# clang will optimize away the static foo, making a single breakpoint
>> +# so there is no point testing it here.
> 
> Uppercase Clang.
> 
> The patch doesn't give enough info to understand why this is the best approach.
> I could imagine using atttribute used to make sure the function isn't optimized
> away, as alternative approach, for example.  Please make the case for the approach
> taken.

I tried using the attribute approach (after you mentioned it here), but clang still optimized the static foo away. I'll document it on the commit message and the comment here for v2.

> 
>> +if {[test_compiler_info {clang-*-*}]} {
>> +    untested "clang only compiles one foo"
>> +    return
>> +}
>> +
>>   standard_testfile msym-bp-shl-main.c msym-bp-shl-main-2.c msym-bp-shl-lib.c
>>   set srcfile ${srcdir}/${subdir}/${srcfile}
>>   set srcfile2 ${srcdir}/${subdir}/${srcfile2}
> 


-- 
Cheers!
Bruno Larsen

Re: [PATCH 11/11] explicitly test for stderr in gdb.base/dprintf.exp

[Bruno Larsen via Gdb-patches]

On 3/2/22 13:50, Pedro Alves wrote:
> On 2022-01-26 19:50, Bruno Larsen via Gdb-patches wrote:
>> Not all compilers explicitly add stderr information when compiling a
> 
> It helps if you say "debug information" instead of just "information".

Ok, I'll try to be more explicit.

> 
>> program, so to avoid an unrelated failure, we explicitly test to see if
>> the compiler has added information about it at all. This was done
>> thinking specifically about clang, since it doesn't add stderr
>> information and developers treat it as a feature.
> 
> Please include a snippet of the failure in the commit log.
> 
> I ran the test locally against clang, and I do see a failure here too:
> 
>   (gdb) PASS: gdb.base/dprintf.exp: call: fprintf: set dprintf style to call
>   continue
>   Continuing.
>   kickoff 1234
>   also to stderr 1234
>   'stderr' has unknown type; cast it to its declared type
>   (gdb) FAIL: gdb.base/dprintf.exp: call: fprintf: 1st dprintf (timeout)
> 
> However, when I debug the program manually, I see that gdb is able to print stderr,
> if you run to main first:
> 
>   (gdb) p stderr
>   'stderr' has unknown type; cast it to its declared type
>   (gdb) start
>   Temporary breakpoint 1 at 0x4011f6: file ../../../src/gdb/testsuite/gdb.base/dprintf.c, line 35.
>   Starting program: /home/pedro/rocm/gdb/build/gdb/testsuite/outputs/gdb.base/dprintf/dprintf
> 
>   Temporary breakpoint 1, main (argc=1, argv=0x7fffffffdc18) at ../../../src/gdb/testsuite/gdb.base/dprintf.c:35
>   35        int loc = 1234;
>   (gdb) p stderr
>   $1 = (FILE *) 0x7ffff7e4e5c0 <_IO_2_1_stderr_>
> 
> So... it doesn't seem like the premise of the patch is correct, for the testcase
> runs to main -- see the "restart" call in the context of your patch, just below.
> Seems like something else may be going on?  Why is GDB not finding the debug info for
> stderr in the dprintf call, while manually it is able to find it?

I think it might be because of external debug information. if I run the test with debuginfod, I get the same output as you:

(gdb) set debuginfod enabled on
(gdb) start
Temporary breakpoint 1 at 0x4011f6: file ../../../common/git-repos/binutils-gdb/gdb/testsuite/gdb.base/dprintf.c, line 35.
Starting program: /home/blarsen/Documents/gdb-build/gdb/testsuite/outputs/gdb.base/dprintf/dprintf
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".

Temporary breakpoint 1, main (argc=1, argv=0x7fffffffe208) at ../../../common/git-repos/binutils-gdb/gdb/testsuite/gdb.base/dprintf.c:35
35        int loc = 1234;
(gdb) p stderr
$1 = (FILE *) 0x7ffff7ec24c0 <_IO_2_1_stderr_>

But if I disable, I get the same as the testsuite:

(gdb) set debuginfod enabled off
(gdb) start
Temporary breakpoint 1 at 0x4011f6: file ../../../common/git-repos/binutils-gdb/gdb/testsuite/gdb.base/dprintf.c, line 35.
Starting program: /home/blarsen/Documents/gdb-build/gdb/testsuite/outputs/gdb.base/dprintf/dprintf
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".

Temporary breakpoint 1, main (argc=1, argv=0x7fffffffe208) at ../../../common/git-repos/binutils-gdb/gdb/testsuite/gdb.base/dprintf.c:35
35        int loc = 1234;
(gdb) p stderr
'stderr' has unknown type; cast it to its declared type


 From talking to clang devs on https://github.com/llvm/llvm-project/issues/53258, I see that this is a feature from their Point of view, as they don't need to know what variables it is worth writing at compile time (even if stderr is used in the program, the backend is different from GCC) so it is best to leave this extra information to an external debug package.

> 
> Pedro Alves
> 
>> ---
>>   gdb/testsuite/gdb.base/dprintf.exp | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>
>> diff --git a/gdb/testsuite/gdb.base/dprintf.exp b/gdb/testsuite/gdb.base/dprintf.exp
>> index 0b209c02a62..e214531f6dc 100644
>> --- a/gdb/testsuite/gdb.base/dprintf.exp
>> +++ b/gdb/testsuite/gdb.base/dprintf.exp
>> @@ -111,6 +111,16 @@ proc test_call {} {
>>   	test_dprintf "At foo entry.*arg=1235, g=2222\r\n" "2nd dprintf"
>>       }
>>   
>> +    gdb_test_multiple "print stderr" "stderr symbol check" {
>> +	-re "\\'stderr\\' has unknown type.*" {
>> +	    untested "No information available for stderr, exiting early"
>> +	    return
>> +	}
>> +	-re "\\\$1.*" {
>> +	    pass "stderr is available"
>> +	}
>> +    }
>> +
>>       with_test_prefix "fprintf" {
>>   	restart
>>   
> 


-- 
Cheers!
Bruno Larsen

Re: [PATCH 11/11] explicitly test for stderr in gdb.base/dprintf.exp

[Pedro Alves]

On 2022-03-31 14:44, Bruno Larsen wrote:
> On 3/2/22 13:50, Pedro Alves wrote:
>> On 2022-01-26 19:50, Bruno Larsen via Gdb-patches wrote:
>>> Not all compilers explicitly add stderr information when compiling a
>>
>> It helps if you say "debug information" instead of just "information".
> 
> Ok, I'll try to be more explicit.
> 
>>
>>> program, so to avoid an unrelated failure, we explicitly test to see if
>>> the compiler has added information about it at all. This was done
>>> thinking specifically about clang, since it doesn't add stderr
>>> information and developers treat it as a feature.
>>
>> Please include a snippet of the failure in the commit log.
>>
>> I ran the test locally against clang, and I do see a failure here too:
>>
>>   (gdb) PASS: gdb.base/dprintf.exp: call: fprintf: set dprintf style to call
>>   continue
>>   Continuing.
>>   kickoff 1234
>>   also to stderr 1234
>>   'stderr' has unknown type; cast it to its declared type
>>   (gdb) FAIL: gdb.base/dprintf.exp: call: fprintf: 1st dprintf (timeout)
>>
>> However, when I debug the program manually, I see that gdb is able to print stderr,
>> if you run to main first:
>>
>>   (gdb) p stderr
>>   'stderr' has unknown type; cast it to its declared type
>>   (gdb) start
>>   Temporary breakpoint 1 at 0x4011f6: file ../../../src/gdb/testsuite/gdb.base/dprintf.c, line 35.
>>   Starting program: /home/pedro/rocm/gdb/build/gdb/testsuite/outputs/gdb.base/dprintf/dprintf
>>
>>   Temporary breakpoint 1, main (argc=1, argv=0x7fffffffdc18) at ../../../src/gdb/testsuite/gdb.base/dprintf.c:35
>>   35        int loc = 1234;
>>   (gdb) p stderr
>>   $1 = (FILE *) 0x7ffff7e4e5c0 <_IO_2_1_stderr_>
>>
>> So... it doesn't seem like the premise of the patch is correct, for the testcase
>> runs to main -- see the "restart" call in the context of your patch, just below.
>> Seems like something else may be going on?  Why is GDB not finding the debug info for
>> stderr in the dprintf call, while manually it is able to find it?
> 
> I think it might be because of external debug information. if I run the test with debuginfod, I get the same output as you:


I don't have debuginfo enabled, in fact I don't even have it compiled into gdb:

 (gdb) set debuginfod enabled off
 Support for debuginfod is not compiled into GDB.

However, that's a hint -- it's really something similar.  I had:

 set debug-file-directory /usr/lib/debug

in my ~/.gdbinit, and the GDB I was using wasn't automatically picking that debug dir.  So
when I run the test via dejagnu, GDB is started with -nx, and I saw the failure, because
then I had no debug info for libc.  When I run GDB manually (without -nx), I have debug info
due to the debug-file-directory setting, and I can print stderr.

Doh!

Re: [PATCH 09/11] gdb/testsuite: don't test gdb.base/msym-bp-shl with clang

[Pedro Alves]

On 2022-03-30 13:19, Bruno Larsen wrote:
> On 3/2/22 16:33, Pedro Alves wrote:
>> On 2022-01-26 19:50, Bruno Larsen via Gdb-patches wrote:
>>> Clang will optimize away the static function in one of the files, and
>>> the test is here to specifically test GDB's behavior when that function
>>> is present, so it makes no sense to have this test run with that
>>> compiler.
>>
>> Please expand this info to include a snippet of the failing test.
>> Also, it's best if the the body of the commit log, doesn't assume the subject is read
>> as part of it.  You will notice that if you read the body in isolation, without the
>> subject line, and also without the diff context, the commit log is vague as is.
> 
> Will do! Sorry for the delay, got sidetracked with another issue.
> 
>>
>>> ---
>>>   gdb/testsuite/gdb.base/msym-bp-shl.exp | 7 +++++++
>>>   1 file changed, 7 insertions(+)
>>>
>>> diff --git a/gdb/testsuite/gdb.base/msym-bp-shl.exp b/gdb/testsuite/gdb.base/msym-bp-shl.exp
>>> index 42adcb191dd..95d5c393505 100644
>>> --- a/gdb/testsuite/gdb.base/msym-bp-shl.exp
>>> +++ b/gdb/testsuite/gdb.base/msym-bp-shl.exp
>>> @@ -22,6 +22,13 @@ if {[skip_shlib_tests]} {
>>>       return 0
>>>   }
>>>   +# clang will optimize away the static foo, making a single breakpoint
>>> +# so there is no point testing it here.
>>
>> Uppercase Clang.
>>
>> The patch doesn't give enough info to understand why this is the best approach.
>> I could imagine using atttribute used to make sure the function isn't optimized
>> away, as alternative approach, for example.  Please make the case for the approach
>> taken.
> 
> I tried using the attribute approach (after you mentioned it here), but clang still optimized the static foo away. I'll document it on the commit message and the comment here for v2.

Odd, this makes it pass for me, with both Clang 10 and Clang 14:

~~~
diff --git c/gdb/testsuite/gdb.base/msym-bp-shl-main-2.c w/gdb/testsuite/gdb.base/msym-bp-shl-main-2.c
index e047ac1145a..009656fd9ea 100644
--- c/gdb/testsuite/gdb.base/msym-bp-shl-main-2.c
+++ w/gdb/testsuite/gdb.base/msym-bp-shl-main-2.c
@@ -15,7 +15,7 @@
    You should have received a copy of the GNU General Public License
    along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
 
-static void
+static void __attribute__ ((used))
 foo (void)
 {
 }
~~~

Can you double check?

Re: [PATCH 09/11] gdb/testsuite: don't test gdb.base/msym-bp-shl with clang

[Bruno Larsen via Gdb-patches]

On 3/31/22 15:49, Pedro Alves wrote:
> On 2022-03-30 13:19, Bruno Larsen wrote:
>> On 3/2/22 16:33, Pedro Alves wrote:
>>> On 2022-01-26 19:50, Bruno Larsen via Gdb-patches wrote:
>>>> Clang will optimize away the static function in one of the files, and
>>>> the test is here to specifically test GDB's behavior when that function
>>>> is present, so it makes no sense to have this test run with that
>>>> compiler.
>>>
>>> Please expand this info to include a snippet of the failing test.
>>> Also, it's best if the the body of the commit log, doesn't assume the subject is read
>>> as part of it.  You will notice that if you read the body in isolation, without the
>>> subject line, and also without the diff context, the commit log is vague as is.
>>
>> Will do! Sorry for the delay, got sidetracked with another issue.
>>
>>>
>>>> ---
>>>>    gdb/testsuite/gdb.base/msym-bp-shl.exp | 7 +++++++
>>>>    1 file changed, 7 insertions(+)
>>>>
>>>> diff --git a/gdb/testsuite/gdb.base/msym-bp-shl.exp b/gdb/testsuite/gdb.base/msym-bp-shl.exp
>>>> index 42adcb191dd..95d5c393505 100644
>>>> --- a/gdb/testsuite/gdb.base/msym-bp-shl.exp
>>>> +++ b/gdb/testsuite/gdb.base/msym-bp-shl.exp
>>>> @@ -22,6 +22,13 @@ if {[skip_shlib_tests]} {
>>>>        return 0
>>>>    }
>>>>    +# clang will optimize away the static foo, making a single breakpoint
>>>> +# so there is no point testing it here.
>>>
>>> Uppercase Clang.
>>>
>>> The patch doesn't give enough info to understand why this is the best approach.
>>> I could imagine using atttribute used to make sure the function isn't optimized
>>> away, as alternative approach, for example.  Please make the case for the approach
>>> taken.
>>
>> I tried using the attribute approach (after you mentioned it here), but clang still optimized the static foo away. I'll document it on the commit message and the comment here for v2.
> 
> Odd, this makes it pass for me, with both Clang 10 and Clang 14:
> 
> ~~~
> diff --git c/gdb/testsuite/gdb.base/msym-bp-shl-main-2.c w/gdb/testsuite/gdb.base/msym-bp-shl-main-2.c
> index e047ac1145a..009656fd9ea 100644
> --- c/gdb/testsuite/gdb.base/msym-bp-shl-main-2.c
> +++ w/gdb/testsuite/gdb.base/msym-bp-shl-main-2.c
> @@ -15,7 +15,7 @@
>      You should have received a copy of the GNU General Public License
>      along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>   
> -static void
> +static void __attribute__ ((used))
>   foo (void)
>   {
>   }
> ~~~
> 
> Can you double check?
> 

My turn to Doh! I am not sure what I did wrong, but this time it worked. I'll use the attribute instead.

-- 
Cheers!
Bruno Larsen