Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH v1] gdb/testsuite: Fix skip-tree.exp for PowerPC post-call NOP
@ 2026-04-15  5:51 Abhay Kandpal
  2026-04-16 16:21 ` Andrew Burgess
  0 siblings, 1 reply; 2+ messages in thread
From: Abhay Kandpal @ 2026-04-15  5:51 UTC (permalink / raw)
  To: gdb-patches; +Cc: aburgess, Ulrich.Weigand, cel, abhay.k, Abhay Kandpal

The skip-tree.exp test was recently added in commit 5ed98c177f9a
and fails on PowerPC due to architectural differences in function
return handling.

On PowerPC64 ELFv2, calls to external functions are followed by a
`nop` instruction that serves as a placeholder for TOC pointer
restoration. Since this `nop` typically has no DWARF line entry,
`finish` reports the call line instead of the next line.

This patch adds a PowerPC-specific `next` command after `finish` in
skip-tree.exp to advance to the expected source line. This handles
the architectural difference without changing GDB's core behavior.

Tested on ppc64le, x86_64-linux.

gdb/testsuite/
	* gdb.base/skip-tree.exp (run_test): Add PowerPC-specific
	handling after `finish` command by issuing an extra `next`.
---
This patch is reg tested.

 gdb/testsuite/gdb.base/skip-tree.exp | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/gdb/testsuite/gdb.base/skip-tree.exp b/gdb/testsuite/gdb.base/skip-tree.exp
index dc93c4cef84..c2504e75979 100644
--- a/gdb/testsuite/gdb.base/skip-tree.exp
+++ b/gdb/testsuite/gdb.base/skip-tree.exp
@@ -140,8 +140,18 @@ proc run_test { file_glob skip_func1 skip_func2 skip_func3 skip_func4 \
 		"step into func$i"
 
 	    # Now finish the function, returning to the caller.
-	    gdb_test "finish" ".*" \
-		"finish from func$i"
+	    if { [istarget "powerpc*-*-*"] } {
+	    # On PowerPC, finish may land on call line due to post-call NOP
+	    # that lacks a DWARF line entry. We need an extra 'next' to reach
+	    # the next statement.
+		gdb_test "finish" ".*" \
+		    "finish from func$i"
+		gdb_test "next" ".*" \
+		    "next after finish from func$i"
+	    } else {
+		gdb_test "finish" ".*" \
+		    "finish from func$i"
+	    }
 	} else {
 	    # After skipping the function, on which line should we
 	    # stop, and what does the line look like.
-- 
2.47.3


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

* Re: [PATCH v1] gdb/testsuite: Fix skip-tree.exp for PowerPC post-call NOP
  2026-04-15  5:51 [PATCH v1] gdb/testsuite: Fix skip-tree.exp for PowerPC post-call NOP Abhay Kandpal
@ 2026-04-16 16:21 ` Andrew Burgess
  0 siblings, 0 replies; 2+ messages in thread
From: Andrew Burgess @ 2026-04-16 16:21 UTC (permalink / raw)
  To: Abhay Kandpal, gdb-patches; +Cc: Ulrich.Weigand, cel, abhay.k, Abhay Kandpal

Abhay Kandpal <abhay@linux.ibm.com> writes:

> The skip-tree.exp test was recently added in commit 5ed98c177f9a
> and fails on PowerPC due to architectural differences in function
> return handling.
>
> On PowerPC64 ELFv2, calls to external functions are followed by a
> `nop` instruction that serves as a placeholder for TOC pointer
> restoration. Since this `nop` typically has no DWARF line entry,
> `finish` reports the call line instead of the next line.
>
> This patch adds a PowerPC-specific `next` command after `finish` in
> skip-tree.exp to advance to the expected source line. This handles
> the architectural difference without changing GDB's core behavior.
>
> Tested on ppc64le, x86_64-linux.

Abhay,

Thanks for looking at this, and apologies for adding a failing PPC test.

While I think your proposal is fine, I'd like to suggest the patch below
as an alternative.  While your change is PPC specific, the patch below
is more general and works by spotting the 'finish' returns us to the
call line, or the next line.  This way, if there are other targets that
have the same problem, they will also be fixed by this patch.

Would you be OK with me pushing this instead?

Thanks,
Andrew

---

commit 3feb7845032e48bd010291f80591cead065ece40
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Thu Apr 16 17:55:27 2026 +0200

    gdb/testsuite fix skip-tree.exp test for PowerPC
    
    The gdb.base/skip-tree.exp test was added in commit:
    
      commit 5ed98c177f9a2381fe5c6ab873d05f1e5dcb8f7f
      Date:   Wed Feb 4 20:31:31 2026 +0000
    
          gdb: introduce '**' for skip globs
    
    This test currently fails on PowerPC due to architectural differences
    in function return handling.
    
    On PowerPC64 ELFv2, calls to external functions are followed by a
    `nop` instruction that serves as a placeholder for TOC pointer
    restoration. Since this `nop` typically has no DWARF line entry,
    `finish` reports the call line instead of the next line.
    
    This issue was reported here:
    
      https://inbox.sourceware.org/gdb-patches/20260415055144.2729164-1-abhay@linux.ibm.com
    
    The fix proposed there was to add a PowerPC specific check so that the
    test would send a `next` command after using `finish` to advance to
    the expected source line.
    
    This commit actually takes a more general approach.  After calling
    `finish` we check which line we ended up on.  If it was the caller
    line then we send GDB a `next` to move to the expected line.  If we
    arrive directly at the next line, then no `next` is needed.
    
    I've tested this change on x86-64, where no `next` is needed, and
    PowerPC, where `next` is now used as required to get the test
    passing.
    
    To simplify the pattern matching in the exp file, I tweaked the source
    file slightly.  But adding a call to a dummy 'func5' it's now easy to
    pattern match as we step through 'func1', 'func2', etc.
    
    Co-Authored-By: Abhay Kandpal <abhay@linux.ibm.com>

diff --git a/gdb/testsuite/gdb.base/skip-tree.c b/gdb/testsuite/gdb.base/skip-tree.c
index 4a254b860b1..3bedc397a85 100644
--- a/gdb/testsuite/gdb.base/skip-tree.c
+++ b/gdb/testsuite/gdb.base/skip-tree.c
@@ -22,17 +22,26 @@ extern void func4 (void);
 
 static volatile int global_var;
 
+void
+func5 (void)
+{
+  /* Nothing.  */
+}
+
 int
 main (void)
 {
+  /* Some filler before the real work starts.  */
   ++global_var;
 
   func1 ();	/* In aa/bb/file.c */
   func2 ();	/* In cc/bb/file.c */
   func3 ();	/* In dd/ee/file.c */
   func4 ();	/* In dd/ee/ff/file.c */
+  func5 ();	/* Makes exp file simpler.  */
 
-  ++global_var;	/* End marker.  */
+  /* Some filler before the function ends.  */
+  ++global_var;
 
   return 0;
 }
diff --git a/gdb/testsuite/gdb.base/skip-tree.exp b/gdb/testsuite/gdb.base/skip-tree.exp
index dc93c4cef84..b1d42ffa87e 100644
--- a/gdb/testsuite/gdb.base/skip-tree.exp
+++ b/gdb/testsuite/gdb.base/skip-tree.exp
@@ -48,7 +48,7 @@ set lineno_f1 [gdb_get_line_number "func1 ();" $srcfile]
 set lineno_f2 [gdb_get_line_number "func2 ();" $srcfile]
 set lineno_f3 [gdb_get_line_number "func3 ();" $srcfile]
 set lineno_f4 [gdb_get_line_number "func4 ();" $srcfile]
-set lineno_end [gdb_get_line_number "End marker" $srcfile]
+set lineno_f5 [gdb_get_line_number "func5 ();" $srcfile]
 
 # Compile all the source files into DESTFILE.  If RELATIVE_FILENAMES
 # is true then the compilation is done using relative filenames to the
@@ -139,19 +139,29 @@ proc run_test { file_glob skip_func1 skip_func2 skip_func3 skip_func4 \
 		     "$::decimal\\s+\\+\\+global_var_$i;"] \
 		"step into func$i"
 
-	    # Now finish the function, returning to the caller.
-	    gdb_test "finish" ".*" \
-		"finish from func$i"
+	    # Now finish the function, returning to the caller.  Some
+	    # targets, after the finish, will place the inferior back
+	    # on the calling line.  We spot this and issue a 'next' to
+	    # move forward to the next function call.
+	    set saw_next_line false
+	    set saw_previous_line false
+	    gdb_test_multiple "finish" "finish from func$i" {
+		-re -wrap "\r\n$::decimal\\s+func[expr {$i + 1}] \\(\\);\[^\r\n\]*" {
+		    pass $gdb_test_name
+		}
+		-re -wrap "\r\n$::decimal\\s+func$i \\(\\);\[^\r\n\]*" {
+		    # After 'finish' this target reports the inferior
+		    # as still on the calling line.  Use 'next' to
+		    # prepare for the next test.
+		    send_gdb "next\n"
+		    exp_continue
+		}
+	    }
 	} else {
 	    # After skipping the function, on which line should we
 	    # stop, and what does the line look like.
-	    if { $j < 5 } {
-		set lineno [set "::lineno_f$j"]
-		set after_skip_re "func$j \\(\\);\[^\r\n\]+"
-	    } else {
-		set lineno $::lineno_end
-		set after_skip_re "\\+\\+global_var;\[^\r\n\]+"
-	    }
+	    set lineno [set "::lineno_f$j"]
+	    set after_skip_re "func$j \\(\\);\[^\r\n\]+"
 
 	    # Step the inferior, we expect to skip the function.
 	    gdb_test "step" \


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

end of thread, other threads:[~2026-04-16 16:21 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-04-15  5:51 [PATCH v1] gdb/testsuite: Fix skip-tree.exp for PowerPC post-call NOP Abhay Kandpal
2026-04-16 16:21 ` Andrew Burgess

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox