public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH v5 0/7] Clean gdb.base when testing with clang
@ 2022-09-14 13:14 Bruno Larsen
  2022-09-14 13:14 ` [PATCH v5 1/7] gdb/testsuite: Add a proc to test where compiler links the epilogue Bruno Larsen
                   ` (7 more replies)
  0 siblings, 8 replies; 10+ messages in thread
From: Bruno Larsen @ 2022-09-14 13:14 UTC (permalink / raw)
  To: 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 the same GCC failures.

Changes in v5:
    * Removed patches previously numbered 1,4-11 given Andrew's approval
    * Split previous patch 3 in two, to make it more focused
    * Reworded some commits
    * Dropped patch 13, in favor of Andrew's suggestion (not included).

Changes in v4:
    * Added new test to step through a function's epoligue
    * renamed gdb_step_until_regexp -> gdb_step_until
    * small style and wording changes in patches 1 and 2
    * Dropped patch 3 - no longer necessary
    * patch 4 was reworked based on Andrew's suggestion
small note, Andrew has OK'd patched 5 and 6 (currently 4 and 5) in v3;

Changes in v3:
    * Fixed some issues that only showed up on CXX_FOR_TARGET=clang
    * rebased on current master

Changes in v2:
    * Introduced gdb_step_until_regexp, based on Pedro's and Andrew's suggestions
    * reworked fixes for: skip.exp, skip-solib.exp and msym-bp-shl.exp
    * Used Pedro's suggestion for call-ar-st
    * reordered patches slightly


Bruno Larsen (7):
  gdb/testsuite: Add a proc to test where compiler links the epilogue
  Change gdb.base/skip-solib.exp deal with lack of epilogue information
  gdb/testsuite: fix testing gdb.base/skip-inline.exp with clang
  gdb/testsuite: fix gdb.base/msym-bp-shl when running with Clang
  fix gdb.base/jit-elf.exp when testing with clang
  gdb.base/skip.exp: Use finish to exit functions
  gdb/testsuite: Add test to step through function epilogue

 gdb/testsuite/gdb.base/jit-elf.exp            |   2 +-
 gdb/testsuite/gdb.base/msym-bp-shl-main-2.c   |   2 +-
 gdb/testsuite/gdb.base/skip-inline.exp        | 107 ++++++++++--------
 gdb/testsuite/gdb.base/skip-solib-lib.c       |   3 +-
 gdb/testsuite/gdb.base/skip-solib-main.c      |   3 +-
 gdb/testsuite/gdb.base/skip.exp               |  34 +++---
 .../gdb.base/step-through-epilogue.c          |  38 +++++++
 .../gdb.base/step-through-epilogue.exp        |  98 ++++++++++++++++
 gdb/testsuite/lib/gdb.exp                     |  32 ++++++
 9 files changed, 253 insertions(+), 66 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/step-through-epilogue.c
 create mode 100644 gdb/testsuite/gdb.base/step-through-epilogue.exp

-- 
2.37.3


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v5 1/7] gdb/testsuite: Add a proc to test where compiler links the epilogue
  2022-09-14 13:14 [PATCH v5 0/7] Clean gdb.base when testing with clang Bruno Larsen
@ 2022-09-14 13:14 ` Bruno Larsen
  2022-09-14 13:14 ` [PATCH v5 2/7] Change gdb.base/skip-solib.exp deal with lack of epilogue information Bruno Larsen
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Bruno Larsen @ 2022-09-14 13:14 UTC (permalink / raw)
  To: gdb-patches

Different compilers link the epilogue of functions to different lines.
As an example, gcc links it to the closing brace of the function,
whereas clang links it to the last statement of the function.  This
difference is important for the testsuite, since the where GDB will land
after a step can be wildly different.  Where possible, this dependency
should be side-stepped in the testsuite, but it isn't always possible,
so this commit adds a gdb_caching_proc that is able to detect where the
epilogue is linked, so tests can react accordingly.
---

Andrew OK'd this patch previously, but there was no patches that
required it, so I held off on pushing it

 gdb/testsuite/lib/gdb.exp | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/gdb/testsuite/lib/gdb.exp b/gdb/testsuite/lib/gdb.exp
index 0f6bb20b49c..68536b00f14 100644
--- a/gdb/testsuite/lib/gdb.exp
+++ b/gdb/testsuite/lib/gdb.exp
@@ -8872,5 +8872,37 @@ proc gdb_step_until { regexp {test_name ""} {max_steps 10} } {
     }
 }
 
+# Check if the compiler emits epilogue information associated
+# with the closing brace or with the last statement line.
+#
+# This proc restarts GDB
+#
+# Returns True if it is associated with the closing brace,
+# False if it is the last statement
+gdb_caching_proc have_epilogue_line_info {
+
+    set main {
+	int
+	main ()
+	{
+	    return 0;
+	}
+    }
+    if {![gdb_simple_compile "simple_program" $main]} {
+	 return False
+    }
+
+    clean_restart $obj
+
+    gdb_test_multiple "info line 6" "epilogue test" {
+	-re -wrap ".*starts at address.*and ends at.*" {
+	    return True
+	}
+	-re -wrap ".*" {
+	    return False
+	}
+    }
+}
+
 # Always load compatibility stuff.
 load_lib future.exp
-- 
2.37.3


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v5 2/7] Change gdb.base/skip-solib.exp deal with lack of epilogue information
  2022-09-14 13:14 [PATCH v5 0/7] Clean gdb.base when testing with clang Bruno Larsen
  2022-09-14 13:14 ` [PATCH v5 1/7] gdb/testsuite: Add a proc to test where compiler links the epilogue Bruno Larsen
@ 2022-09-14 13:14 ` Bruno Larsen
  2022-09-14 13:14 ` [PATCH v5 3/7] gdb/testsuite: fix testing gdb.base/skip-inline.exp with clang Bruno Larsen
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Bruno Larsen @ 2022-09-14 13:14 UTC (permalink / raw)
  To: gdb-patches

When running gdb.base/skip-solib.exp, the backtrace tests could fail with
compilers that associated epilogue instructions with the last statement
line of the function, instead of associating it with the closing brace,
despite the feature being fully functional.  As an example, when testing
skipping the function square, the testsuite would show

Breakpoint 1, main () at (...)/binutils-gdb/gdb/testsuite/gdb.base/skip-solib-main.c:5
5         return square(0);
(gdb) step
0x00007ffff7cef560 in __libc_start_call_main () from /lib64/libc.so.6
(gdb) PASS: gdb.base/skip-solib.exp: ignoring solib file: step
bt
 #0  0x00007ffff7cef560 in __libc_start_call_main () from /lib64/libc.so.6
 #1  0x00007ffff7cef60c in __libc_start_main_impl () from /lib64/libc.so.6
 #2  0x0000000000401065 in _start ()
(gdb) FAIL: gdb.base/skip-solib.exp: ignoring solib file: bt

Which means that the feature is working, the testsuite is just
mis-identifying it.  To avoid this problem, the skipped function calls
have been sent to a line before `return`, so epilogues won't factor in.
---
 gdb/testsuite/gdb.base/skip-solib-lib.c  | 3 ++-
 gdb/testsuite/gdb.base/skip-solib-main.c | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/gdb.base/skip-solib-lib.c b/gdb/testsuite/gdb.base/skip-solib-lib.c
index b2c4d86d703..341f1440a3b 100644
--- a/gdb/testsuite/gdb.base/skip-solib-lib.c
+++ b/gdb/testsuite/gdb.base/skip-solib-lib.c
@@ -7,5 +7,6 @@ int multiply(int a, int b)
 
 int square(int num)
 {
-  return multiply(num, num);
+  int res = multiply(num, num);
+  return res;
 }
diff --git a/gdb/testsuite/gdb.base/skip-solib-main.c b/gdb/testsuite/gdb.base/skip-solib-main.c
index 746bb5f36bb..a3b6d417935 100644
--- a/gdb/testsuite/gdb.base/skip-solib-main.c
+++ b/gdb/testsuite/gdb.base/skip-solib-main.c
@@ -2,5 +2,6 @@ int square(int num);
 
 int main()
 {
-  return square(0);
+  int s = square(0);
+  return s;
 }
-- 
2.37.3


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v5 3/7] gdb/testsuite: fix testing gdb.base/skip-inline.exp with clang
  2022-09-14 13:14 [PATCH v5 0/7] Clean gdb.base when testing with clang Bruno Larsen
  2022-09-14 13:14 ` [PATCH v5 1/7] gdb/testsuite: Add a proc to test where compiler links the epilogue Bruno Larsen
  2022-09-14 13:14 ` [PATCH v5 2/7] Change gdb.base/skip-solib.exp deal with lack of epilogue information Bruno Larsen
@ 2022-09-14 13:14 ` Bruno Larsen
  2022-09-14 13:14 ` [PATCH v5 4/7] gdb/testsuite: fix gdb.base/msym-bp-shl when running with Clang Bruno Larsen
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Bruno Larsen @ 2022-09-14 13:14 UTC (permalink / raw)
  To: gdb-patches

When testing gdb.base/skip-inline.exp using clang, we get failures
when trying to step out of functions, since clang requires one fewer
step when compared to gcc.  The inferior gets increasingly out of sync
as the test continues because of this difference, which generates those
failures.

This commit fixes this by switching those hardcoded steps to
gdb_step_until, to guarantee that the inferior is always synced to what
the test expects.  This approach does not work for the parts that use
step 2 or step 3, so when we identify that clang is being used, those
tests are skipped.
---
 gdb/testsuite/gdb.base/skip-inline.exp | 107 ++++++++++++++-----------
 1 file changed, 62 insertions(+), 45 deletions(-)

diff --git a/gdb/testsuite/gdb.base/skip-inline.exp b/gdb/testsuite/gdb.base/skip-inline.exp
index f6e9926b66c..b7d519e60c1 100644
--- a/gdb/testsuite/gdb.base/skip-inline.exp
+++ b/gdb/testsuite/gdb.base/skip-inline.exp
@@ -15,6 +15,8 @@
 
 standard_testfile
 
+set epilogue [have_epilogue_line_info]
+
 if { [prepare_for_testing "failed to prepare" "skip-inline" \
 			  {skip-inline.c skip1.c } \
 			  {debug nowarnings}] } {
@@ -24,65 +26,80 @@ if { [prepare_for_testing "failed to prepare" "skip-inline" \
 set srcfile skip-inline.c
 set srcfile1 skip1.c
 
-if ![runto_main] {
-    return
-}
-
-# Create a skiplist entry for a specified file and function.
-
-gdb_test "skip function foo" "Function foo will be skipped when stepping\."
+proc_with_prefix single_step { } {
+    if ![runto_main] {
+	return
+    }
 
-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 "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"
-
-if ![runto_main] {
-    return
-}
-
-with_test_prefix "double step" {
     gdb_test "bt" "\\s*\\#0\\s+main.*" "in the main"
-    gdb_test "step 2" ".*" "step into baz, since foo will be skipped"
-    gdb_test "bt" "\\s*\\#0\\s+baz.*" "still in the baz"
-    gdb_test "step" ".*" "step back to 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_step_until ".*x = 0; x = baz \\(foo \\(\\)\\).*"
     gdb_test "bt" "\\s*\\#0\\s+main.*" "again in the main"
-    gdb_test "step 2" ".*" "step again into baz, since foo will be skipped"
+    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 back to main, again"
-    gdb_test "bt" "\\s*\\#0\\s+main.*" "again back to main"
+    gdb_step_until "main \\(\\) at .*" "step back to main, again"
+    gdb_test "bt" "\\s*\\#0.*main.*" "again back to main"
 }
 
-if ![runto_main] {
-    return
-}
+proc_with_prefix double_step { } {
+    if ![runto_main] {
+	return
+    }
 
-with_test_prefix "triple step" {
-    gdb_test "bt" "\\s*\\#0\\s+main.*" "in the main"
-    gdb_test "step 3" ".*" "step over baz"
-    gdb_test "bt" "\\s*\\#0\\s+main.*" "again in the main"
-    gdb_test "step 3" ".*" "step over baz, again"
-    gdb_test "bt" "\\s*\\#0\\s+main.*" "again back to main"
+    with_test_prefix "double step" {
+	gdb_test "bt" "\\s*\\#0\\s+main.*" "in the main"
+	gdb_test "step 2" ".*" "step into baz, since foo will be skipped"
+	gdb_test "bt" "\\s*\\#0\\s+baz.*" "still in the baz"
+	gdb_test "step" ".*" "step back to main"
+	gdb_test "bt" "\\s*\\#0\\s+main.*" "again in the main"
+	gdb_test "step 2" ".*" "step again into baz, since foo will be skipped"
+	gdb_test "bt" "\\s*\\#0\\s+baz.*" "again in the baz"
+	gdb_test "step" ".*" "step back to main, again"
+	gdb_test "bt" "\\s*\\#0\\s+main.*" "again back to main"
+    }
 }
 
-if ![runto_main] {
-    return
+proc_with_prefix triple_step { } {
+    if ![runto_main] {
+	return
+    }
+
+    with_test_prefix "triple step" {
+	gdb_test "bt" "\\s*\\#0\\s+main.*" "in the main"
+	gdb_test "step 3" ".*" "step over baz"
+	gdb_test "bt" "\\s*\\#0\\s+main.*" "again in the main"
+	gdb_test "step 3" ".*" "step over baz, again"
+	gdb_test "bt" "\\s*\\#0\\s+main.*" "again back to main"
+    }
 }
 
-gdb_test "skip delete" ".*" "skip delete"
+proc_with_prefix skip_current_frame { } {
+    if ![runto_main] {
+	return
+    }
+
+    gdb_test "skip delete" ".*" "skip delete"
 
-with_test_prefix "skip current frame" {
     gdb_test "bt" "\\s*\\#0\\s+main.*" "in the main"
     gdb_test "step" ".*" "step into foo"
     gdb_test "bt" "\\s*\\#0\\s+foo.*" "in the foo"
     gdb_test "skip" "Function foo will be skipped when stepping\." "skip"
 }
+
+# Create a skiplist entry for a specified file and function.
+
+gdb_test "skip function foo" "Function foo will be skipped when stepping\."
+
+single_step
+
+# Some compilers link the epilogue of functions to the line containing the
+# closing brace, while others dont.  The behavior ends up so out of sync
+# with multiple steps at a time, that completely different procs would need
+# to be made, and it is frankly unnecessary.
+if {$epilogue} {
+    double_step
+    triple_step
+}
+
+skip_current_frame
-- 
2.37.3


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v5 4/7] gdb/testsuite: fix gdb.base/msym-bp-shl when running with Clang
  2022-09-14 13:14 [PATCH v5 0/7] Clean gdb.base when testing with clang Bruno Larsen
                   ` (2 preceding siblings ...)
  2022-09-14 13:14 ` [PATCH v5 3/7] gdb/testsuite: fix testing gdb.base/skip-inline.exp with clang Bruno Larsen
@ 2022-09-14 13:14 ` Bruno Larsen
  2022-09-14 13:14 ` [PATCH v5 5/7] fix gdb.base/jit-elf.exp when testing with clang Bruno Larsen
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Bruno Larsen @ 2022-09-14 13:14 UTC (permalink / raw)
  To: gdb-patches

When trying to test gdb.base/msym-bp-shl.exp using clang, it would have
many failures because one of the version of the foo function was being
optimized away. Adding __attribute__ ((used)) to it fixed this.
---
 gdb/testsuite/gdb.base/msym-bp-shl-main-2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.base/msym-bp-shl-main-2.c b/gdb/testsuite/gdb.base/msym-bp-shl-main-2.c
index e047ac1145a..009656fd9ea 100644
--- a/gdb/testsuite/gdb.base/msym-bp-shl-main-2.c
+++ b/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)
 {
 }
-- 
2.37.3


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v5 5/7] fix gdb.base/jit-elf.exp when testing with clang
  2022-09-14 13:14 [PATCH v5 0/7] Clean gdb.base when testing with clang Bruno Larsen
                   ` (3 preceding siblings ...)
  2022-09-14 13:14 ` [PATCH v5 4/7] gdb/testsuite: fix gdb.base/msym-bp-shl when running with Clang Bruno Larsen
@ 2022-09-14 13:14 ` Bruno Larsen
  2022-09-14 13:14 ` [PATCH v5 6/7] gdb.base/skip.exp: Use finish to exit functions Bruno Larsen
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Bruno Larsen @ 2022-09-14 13:14 UTC (permalink / raw)
  To: gdb-patches

When using clang as the compiler for the target, gdb.base/jit-elf.exp
was failing because the filename displayed when GDB attached to the
inferior was only showing up as with a relative path, like so:

       (gdb) attach 3674146
       Attaching to program: /home/blarsen/Documents/gdb-build/gdb/testsuite/outputs/gdb.base/jit-elf/jit-elf-main, process 3674146
       Reading symbols from /lib64/libm.so.6...
       Reading symbols from .gnu_debugdata for /lib64/libm.so.6...
       (No debugging symbols found in .gnu_debugdata for /lib64/libm.so.6)
       Reading symbols from /lib64/libc.so.6...
       (No debugging symbols found in /lib64/libc.so.6)
       Reading symbols from /lib64/ld-linux-x86-64.so.2...
       [Thread debugging using libthread_db enabled]
       Using host libthread_db library "/lib64/libthread_db.so.1".
       0x00000000004013ff in main (argc=3, argv=0x7fffffffd820) at ../../../common/git-repos/binutils-gdb/gdb/testsuite/gdb.base/jit-elf-main.c:118
       118|  WAIT_FOR_GDB; i = 0;  /* gdb break here 1 */
       (gdb) FAIL: gdb.base/jit-elf.exp: attach: one_jit_test-2: break here 1: attach

While gcc's output is as follows:

       (gdb) attach 3592961
       Attaching to program: /home/blarsen/Documents/gdb-build/gdb/testsuite/outputs/gdb.base/jit-elf/jit-elf-main, process 3592961
       Reading symbols from /lib64/libm.so.6...
       Reading symbols from .gnu_debugdata for /lib64/libm.so.6...
       (No debugging symbols found in .gnu_debugdata for /lib64/libm.so.6)
       Reading symbols from /lib64/libc.so.6...
       (No debugging symbols found in /lib64/libc.so.6)
       Reading symbols from /lib64/ld-linux-x86-64.so.2...
       [Thread debugging using libthread_db enabled]
       Using host libthread_db library "/lib64/libthread_db.so.1".
       main (argc=3, argv=0x7fffffffd860) at /home/blarsen/Documents/gdb-build/gdb/testsuite/../../../common/git-repos/binutils-gdb/gdb/testsuite/gdb.base/jit-elf-main.c:118
       118|  WAIT_FOR_GDB; i = 0;  /* gdb break here 1 */
       (gdb) PASS: gdb.base/jit-elf.exp: attach: one_jit_test-2: break here 1: attach

This difference only happens when GDB's configure is ran using a
relative path, but seeing as testing the full path is not important for
this specific test, it feels worth fixing anyway.  To fix the false
positive, the regexp for checking where gdb has stopped was relaxed a
little to allow the relative path.
---
 gdb/testsuite/gdb.base/jit-elf.exp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/testsuite/gdb.base/jit-elf.exp b/gdb/testsuite/gdb.base/jit-elf.exp
index 38d541f74b9..0753072974f 100644
--- a/gdb/testsuite/gdb.base/jit-elf.exp
+++ b/gdb/testsuite/gdb.base/jit-elf.exp
@@ -54,7 +54,7 @@ proc clean_reattach {} {
     clean_restart ${main_binfile}
 
     if { ![gdb_attach $testpid \
-	      -pattern "main.*at .*$::main_srcfile:.*"] } {
+	      -pattern "main.*at .*$::main_basename.c:.*"] } {
 	return 0
     }
 
-- 
2.37.3


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v5 6/7] gdb.base/skip.exp: Use finish to exit functions
  2022-09-14 13:14 [PATCH v5 0/7] Clean gdb.base when testing with clang Bruno Larsen
                   ` (4 preceding siblings ...)
  2022-09-14 13:14 ` [PATCH v5 5/7] fix gdb.base/jit-elf.exp when testing with clang Bruno Larsen
@ 2022-09-14 13:14 ` Bruno Larsen
  2022-09-14 13:14 ` [PATCH v5 7/7] gdb/testsuite: Add test to step through function epilogue Bruno Larsen
  2022-09-21 16:05 ` [PATCH v5 0/7] Clean gdb.base when testing with clang Tom Tromey
  7 siblings, 0 replies; 10+ messages in thread
From: Bruno Larsen @ 2022-09-14 13:14 UTC (permalink / raw)
  To: gdb-patches

gdb.base/skip.exp was making use of a fixed number of step commands to
exit some functions.  This caused some problems when using clang to test
GDB, as GDB would need fewer steps to reach the desired spots.  For
instance, when testing in the section "step after disabling 3", the log
looks like this:

    Breakpoint 4, main () at binutils-gdb/gdb/testsuite/gdb.base/skip.c:32
    32        x = baz ((bar (), foo ()));
    (gdb) step
    bar () at binutils-gdb/gdb/testsuite/gdb.base/skip1.c:21
    21        return 1;
    (gdb) PASS: gdb.base/skip.exp: step after disabling 3: step 1
    step
    foo () at binutils-gdb/gdb/testsuite/gdb.base/skip.c:42
    42        return 0;
    (gdb) PASS: gdb.base/skip.exp: step after disabling 3: step 2
    step
    main () at binutils-gdb/gdb/testsuite/gdb.base/skip.c:34
    34        test_skip_file_and_function ();
    (gdb) step
    test_skip_file_and_function () at binutils-gdb/gdb/testsuite/gdb.base/skip.c:59
    59        test_skip ();
    (gdb) FAIL: gdb.base/skip.exp: step after disabling 3: step 3
    step
    test_skip () at binutils-gdb/gdb/testsuite/gdb.base/skip.c:48
    48      }
    (gdb) PASS: gdb.base/skip.exp: step after disabling 3: step 4
    step
    test_skip_file_and_function () at binutils-gdb/gdb/testsuite/gdb.base/skip.c:60
    60        skip1_test_skip_file_and_function ();
    (gdb) FAIL: gdb.base/skip.exp: step after disabling 3: step 5

This shows that the feature is working but because the inferior lands in
a different location, it registers as a failure.  Seeing as along with
this difference, there are also some differences that depend on gcc
versions (where gdb might stop back at line 32 before entering foo), it
would not be easy to test for this behavior using steps and analzing
where the inferior stops at each point. On the other hand, using
gdb_step_until is not feasible because we'd possibly gloss over stepping
into baz and rendering the whole test useless.  Instead, skip.exp now
uses finish to leave functions, synchronizing through compilers and
compiler versions.  Some test names were also changed to be a bit more
descriptive.  The new log looks like this, independently of compiler used:

    Breakpoint 4, main () at binutils-gdb/gdb/testsuite/gdb.base/skip.c:32
    32        x = baz ((bar (), foo ()));
    (gdb) step
    bar () at binutils-gdb/gdb/testsuite/gdb.base/skip1.c:21
    21        return 1;
    (gdb) PASS: gdb.base/skip.exp: step after disabling 3: step into bar
    finish
    Run till exit from #0  bar () at binutils-gdb/gdb/testsuite/gdb.base/skip1.c:21
    main () at binutils-gdb/gdb/testsuite/gdb.base/skip.c:32
    32        x = baz ((bar (), foo ()));
    Value returned is $2 = 1
    (gdb) PASS: gdb.base/skip.exp: step after disabling 3: return from bar
    step
    foo () at binutils-gdb/gdb/testsuite/gdb.base/skip.c:42
    42        return 0;
    (gdb) PASS: gdb.base/skip.exp: step after disabling 3: step into foo
    finish
    Run till exit from #0  foo () at binutils-gdb/gdb/testsuite/gdb.base/skip.c:42
    main () at binutils-gdb/gdb/testsuite/gdb.base/skip.c:32
    32        x = baz ((bar (), foo ()));
    Value returned is $3 = 0
    (gdb) PASS: gdb.base/skip.exp: step after disabling 3: Return from foo
    step
    34        test_skip_file_and_function ();
    (gdb) PASS: gdb.base/skip.exp: step after disabling 3: step and skip baz
---

Andrew OK'd this patch in a previous version, but asked for this to only
be pushed together with patch number 7

 gdb/testsuite/gdb.base/skip.exp | 34 ++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/gdb/testsuite/gdb.base/skip.exp b/gdb/testsuite/gdb.base/skip.exp
index e6b660004d9..0864e445f13 100644
--- a/gdb/testsuite/gdb.base/skip.exp
+++ b/gdb/testsuite/gdb.base/skip.exp
@@ -100,6 +100,14 @@ if ![runto_main] {
 gdb_test "step" ".*" "step in the main"
 gdb_test "bt" "\\s*\\#0\\s+main.*" "step after all ignored"
 
+# This proc tests that GDB can step into the function foo, exit it
+# and skip the functions bar and baz.
+proc step_foo_skip_bar_baz {} {
+    gdb_test "step" "foo \\(\\) at.*" "step and skip bar"
+    gdb_test "finish" ".*" "return from bar"
+    gdb_test "step" ".*test_skip_file_and_function.*" "step and skip baz"
+}
+
 # Now remove skip.c from the skiplist.  Our first step should take us
 # into foo(), and our second step should take us to the next line in main().
 
@@ -117,21 +125,19 @@ 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"
+    step_foo_skip_bar_baz
 }
 
 # Test that we step into foo(), then into bar(), but not into baz().
 proc step_bar_foo_skip_baz {} {
-    gdb_test "step" "bar \\(\\) at.*" "step 1"
-    gdb_test "step" ".*" "step 2"; # Return from bar()
+    gdb_test "step" "bar \\(\\) at.*" "step into bar"
+    gdb_test "finish" ".*" "return from bar"
 
     # With at least gcc 6.5.0 and 9.2.0, we jump once back to main
     # before entering foo here.  If that happens try to step a second
     # time.
     set stepped_again 0
-    gdb_test_multiple "step" "step 3" {
+    gdb_test_multiple "step" "step into foo" {
 	-re -wrap "foo \\(\\) at.*" {
 	    pass $gdb_test_name
 	}
@@ -144,8 +150,8 @@ proc step_bar_foo_skip_baz {} {
 	}
     }
 
-    gdb_test "step" ".*" "step 4"; # Return from foo()
-    gdb_test "step" "main \\(\\) at.*" "step 5"
+    gdb_test "finish" ".*" "Return from foo"
+    gdb_test "step" ".*test_skip_file_and_function.*" "step and skip baz"
 }
 
 # Now disable the skiplist entry for  skip1.c.  We should now
@@ -178,9 +184,7 @@ 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"
+    step_foo_skip_bar_baz
 }
 
 # Admin tests (disable,enable,delete).
@@ -249,9 +253,7 @@ 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"
+    step_foo_skip_bar_baz
 }
 
 with_test_prefix "step using -gfi" {
@@ -261,9 +263,7 @@ 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"
+    step_foo_skip_bar_baz
 }
 
 with_test_prefix "step using -fu for baz" {
-- 
2.37.3


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v5 7/7] gdb/testsuite: Add test to step through function epilogue
  2022-09-14 13:14 [PATCH v5 0/7] Clean gdb.base when testing with clang Bruno Larsen
                   ` (5 preceding siblings ...)
  2022-09-14 13:14 ` [PATCH v5 6/7] gdb.base/skip.exp: Use finish to exit functions Bruno Larsen
@ 2022-09-14 13:14 ` Bruno Larsen
  2022-09-21 16:05 ` [PATCH v5 0/7] Clean gdb.base when testing with clang Tom Tromey
  7 siblings, 0 replies; 10+ messages in thread
From: Bruno Larsen @ 2022-09-14 13:14 UTC (permalink / raw)
  To: gdb-patches

The testsuite implicitly tests GDB's ability to step through epilogues
in multiple tests, without doing it explicitly anywhere.  This is
unfortunate, as clang does not emit epilogue information, so using clang
on our testsuite makes many tests fail.  This patch adds a central,
explicit test for walking through the epilogue so we can safely remove
this from other tests and have them working with clang.

The test created attempts to step through a simple epilogue, an
epilogue that ends on another epilogue, and epilogues leading to other
function calls.
---
 .../gdb.base/step-through-epilogue.c          | 38 +++++++
 .../gdb.base/step-through-epilogue.exp        | 98 +++++++++++++++++++
 2 files changed, 136 insertions(+)
 create mode 100644 gdb/testsuite/gdb.base/step-through-epilogue.c
 create mode 100644 gdb/testsuite/gdb.base/step-through-epilogue.exp

diff --git a/gdb/testsuite/gdb.base/step-through-epilogue.c b/gdb/testsuite/gdb.base/step-through-epilogue.c
new file mode 100644
index 00000000000..532c1ee01b4
--- /dev/null
+++ b/gdb/testsuite/gdb.base/step-through-epilogue.c
@@ -0,0 +1,38 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 1992-2022 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+int
+multiply (int a, int b)
+{
+  return a * b;
+} /* Epilogue line of multiply.  */
+
+int
+square (int x)
+{
+  return multiply (x, x);
+} /* Epilogue line of square.  */
+
+int
+main(void)
+{
+  int x;
+  x = multiply (1, 2);
+  x = square (2);
+  x = multiply (square (1), square (2));
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/step-through-epilogue.exp b/gdb/testsuite/gdb.base/step-through-epilogue.exp
new file mode 100644
index 00000000000..a9a9b82a4c3
--- /dev/null
+++ b/gdb/testsuite/gdb.base/step-through-epilogue.exp
@@ -0,0 +1,98 @@
+#   Copyright 2022 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# This test is used to confirm that GDB is able to step, stopping at an
+# epilogue line, then step out of the function.
+
+standard_testfile
+
+if { ![have_epilogue_line_info] } {
+    untested "This test doesn't work with this compiler"
+    return
+}
+
+if { [prepare_for_testing "failed to prepare" $testfile \
+			  {step-through-epilogue.c}] } {
+    untested "failed to prepare"
+    return
+}
+
+if { ![runto_main] } {
+    untested "couldn't run to main"
+    return
+}
+
+proc step_till_epilogue_multiply {} {
+    gdb_test "step" ".*return a . b;.*" "step into multiply"
+    gdb_test "step" \
+	     "$::decimal\\s+\\\}\[^\r\n\]+Epilogue line of multiply.*" \
+	     "stop at the epilogue of multiply"
+}
+
+proc step_till_epilogue_square {} {
+    gdb_test "step" ".*return multiply.*" "step into square"
+    step_till_epilogue_multiply
+    gdb_test "step" \
+	     "$::decimal\\s+\\\}\[^\r\n\]+Epilogue line of square.*" \
+	     "stop at epilogue of square"
+}
+
+with_test_prefix "multiply" {
+    step_till_epilogue_multiply
+    gdb_test "step" "x = square \\(2\\);" "leave function"
+}
+
+with_test_prefix "square" {
+    step_till_epilogue_square
+    gdb_test "step" ".*x = multiply \\(square \\(1\\), square \\(2\\)\\);.*"\
+	"leave function"
+}
+
+# Some gcc versions can mess this test by requiring extra steps in a
+# few locations.  We dynamically test if we're in one such versions in the
+# the first argument, and use that to see if extra steps are needed to
+# finish the second argument
+set midway_return false
+
+with_test_prefix "square, first argument" {
+    step_till_epilogue_square
+    gdb_test_multiple "step" "step back to main" {
+	-re -wrap "multiply \\(square \\(1\\), square \\(2\\)\\);" {
+	    set midway_return true
+	    gdb_send "step\n"
+	    exp_continue
+	}
+
+	-re -wrap "return multiply.*" {
+	    pass $gdb_test_name
+	}
+    }
+}
+with_test_prefix "square, second argument" {
+    step_till_epilogue_multiply
+    gdb_test "step" \
+	     "$::decimal\\s+\\\}\[^\r\n\]+Epilogue line of square.*" \
+	     "stop at epilogue of square"
+    if {$midway_return} {
+	gdb_test "step"\
+	    ".*x = multiply \\(square \\(1\\), square \\(2\\)\\);.*"\
+	    "step back to main"
+    }
+}
+
+with_test_prefix "multiply with function args" {
+    step_till_epilogue_multiply
+    gdb_test "step" ".*return 0;.*" "leave last function"
+}
-- 
2.37.3


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v5 0/7] Clean gdb.base when testing with clang
  2022-09-14 13:14 [PATCH v5 0/7] Clean gdb.base when testing with clang Bruno Larsen
                   ` (6 preceding siblings ...)
  2022-09-14 13:14 ` [PATCH v5 7/7] gdb/testsuite: Add test to step through function epilogue Bruno Larsen
@ 2022-09-21 16:05 ` Tom Tromey
  2022-09-22  9:05   ` Bruno Larsen
  7 siblings, 1 reply; 10+ messages in thread
From: Tom Tromey @ 2022-09-21 16:05 UTC (permalink / raw)
  To: Bruno Larsen via Gdb-patches

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

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

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

Thanks.  I read through this and it looked good to me.

The use of __attribute__((used)) gave me some pause but I see it's
already in the test suite, so it seemed harmless to add another use.

Tom

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v5 0/7] Clean gdb.base when testing with clang
  2022-09-21 16:05 ` [PATCH v5 0/7] Clean gdb.base when testing with clang Tom Tromey
@ 2022-09-22  9:05   ` Bruno Larsen
  0 siblings, 0 replies; 10+ messages in thread
From: Bruno Larsen @ 2022-09-22  9:05 UTC (permalink / raw)
  To: Tom Tromey, Bruno Larsen via Gdb-patches

On 21/09/2022 18:05, Tom Tromey wrote:
>>>>>> "Bruno" == Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:
> Bruno> When testing GDB with clang, gdb.base had over 50 more failures than when
> Bruno> testing with gcc.  Examining the failed tests led to a few clang bugs, a
> Bruno> few GDB bugs, and many testsuite assumptions that could be changed.
>
> Bruno> After this patch series, nothing should be changed for testing with gcc,
> Bruno> and testing with clang should only show non-trivial failures for
> Bruno> maint.exp and macscp.exp, along with the same GCC failures.
>
> Thanks.  I read through this and it looked good to me.

Thanks, pushed!

Cheers,
Bruno

>
> The use of __attribute__((used)) gave me some pause but I see it's
> already in the test suite, so it seemed harmless to add another use.
>
> Tom
>


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-09-22  9:05 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-14 13:14 [PATCH v5 0/7] Clean gdb.base when testing with clang Bruno Larsen
2022-09-14 13:14 ` [PATCH v5 1/7] gdb/testsuite: Add a proc to test where compiler links the epilogue Bruno Larsen
2022-09-14 13:14 ` [PATCH v5 2/7] Change gdb.base/skip-solib.exp deal with lack of epilogue information Bruno Larsen
2022-09-14 13:14 ` [PATCH v5 3/7] gdb/testsuite: fix testing gdb.base/skip-inline.exp with clang Bruno Larsen
2022-09-14 13:14 ` [PATCH v5 4/7] gdb/testsuite: fix gdb.base/msym-bp-shl when running with Clang Bruno Larsen
2022-09-14 13:14 ` [PATCH v5 5/7] fix gdb.base/jit-elf.exp when testing with clang Bruno Larsen
2022-09-14 13:14 ` [PATCH v5 6/7] gdb.base/skip.exp: Use finish to exit functions Bruno Larsen
2022-09-14 13:14 ` [PATCH v5 7/7] gdb/testsuite: Add test to step through function epilogue Bruno Larsen
2022-09-21 16:05 ` [PATCH v5 0/7] Clean gdb.base when testing with clang Tom Tromey
2022-09-22  9:05   ` Bruno Larsen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).