Re: [PATCH v1] gdb/testsuite: Fix skip-tree.exp for PowerPC post-call NOP

[Andrew Burgess <aburgess@redhat.com>]

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" \