[PATCH] Harden gdb.base/step-over-syscall.exp

[Luis Machado <luis.machado@linaro.org>]

There are a couple problems with this test.

First
--

gdb.base/step-over-syscall.exp records the address of a syscall instruction
within fork/vfork/clone functions and also the address of the instruction
after that syscall instruction.

It uses these couples addresses to make sure we stepped over a syscall
instruction (fork/vfork/clone events) correctly.

The way the test fetches the addresses of the instructions is by stepi-ing
its way through the fork/vfork/clone functions until it finds a match for
a syscall. Then it stepi's once again to get the address of the next
instruction.

This assumes that stepi-ing over a syscall is working correctly and landing
in the right PC. This is not the case for AArch64/Linux, where we're
landing a couple instructions after the syscall in some cases.

The following patch lets the test execute as before, but adds a new instruction
address check using the x command as opposed to stepi.

I didn't want to change how the test works since we may also be
interested in checking if stepi-ing over the syscall under different
conditions (displaced stepping on/off) yields the same results. I don't
feel strongly about this, so i'm OK with changing how we compare PC's for
the entire test if folks decide it is reasonable.

Second
--

FAIL: gdb.base/step-over-syscall.exp: vfork: displaced=off: continue to vfork (3rd time) (the program exited)
FAIL: gdb.base/step-over-syscall.exp: vfork: displaced=off: continue to syscall insn vfork (the program is no longer running)
FAIL: gdb.base/step-over-syscall.exp: vfork: displaced=off: single step over vfork (the program is no longer running)

Depending on the glibc version we may have different code generated for the
fork/vfork/clone functions.

I ran into the situation where vfork for newer glibc's on AArch64/Linux is
very short, so "break vfork" will put a breakpoint right at the syscall
instruction, which is something the testcase isn't expecting (a off-by-1
of sorts).

The patch adds extra code to handle this case. If the test detects we're
already sitting at a syscall instruction, it records the address and moves
on to record the address after that particular instruction.

Another measure is to "break *$syscall" instead of "break $syscall". That
guarantees we're stopping at the first instruction of the syscall function,
if it ever happens that the syscall instruction is the first instruction of
those functions.

With these changes i can fix some failures for aarch64-linux-gnu and also
expose the problems i've reported here:

https://sourceware.org/ml/gdb-patches/2019-12/msg01071.html

These tests now fail for aarch64-linux-gnu (patch for this is going through
reviews):

FAIL: gdb.base/step-over-syscall.exp: vfork: displaced=off: pc after stepi matches insn addr after syscall
FAIL: gdb.base/step-over-syscall.exp: vfork: displaced=on: pc after stepi matches insn addr after syscall

I've queued a test run on the buildbot to make sure things are sane for other
architectures, and i've tested it on aarch64-linux-gnu.

gdb/testsuite/ChangeLog:

2020-01-15  Luis Machado  <luis.machado@linaro.org>

	* gdb.base/step-over-syscall.exp (setup): Check if we're already
	sitting at a syscall instruction when we hit the syscall function's
	breakpoint.
	Check PC against one obtained with the x command.
	(step_over_syscall): Don't continue to the syscall instruction if
	we're already there.
---
 gdb/testsuite/gdb.base/step-over-syscall.exp | 91 ++++++++++++++------
 1 file changed, 66 insertions(+), 25 deletions(-)

diff --git a/gdb/testsuite/gdb.base/step-over-syscall.exp b/gdb/testsuite/gdb.base/step-over-syscall.exp
index b373c169c0..4d9488b1d4 100644
--- a/gdb/testsuite/gdb.base/step-over-syscall.exp
+++ b/gdb/testsuite/gdb.base/step-over-syscall.exp
@@ -46,7 +46,8 @@ proc_with_prefix check_pc_after_cross_syscall { syscall syscall_insn_next_addr }
 
 proc setup { syscall } {
     global gdb_prompt syscall_insn
-
+    global hex
+    set next_insn_addr 0
     set testfile "step-over-$syscall"
 
     clean_restart $testfile
@@ -62,7 +63,7 @@ proc setup { syscall } {
     gdb_test_no_output "set displaced-stepping off" \
 	"set displaced-stepping off during test setup"
 
-    gdb_test "break $syscall" "Breakpoint \[0-9\]* at .*"
+    gdb_test "break \*$syscall" "Breakpoint \[0-9\]* at .*"
 
     gdb_test "continue" "Continuing\\..*Breakpoint \[0-9\]+, (.* in |__libc_|)$syscall \\(\\).*" \
 	"continue to $syscall (1st time)"
@@ -75,37 +76,72 @@ proc setup { syscall } {
     # Hit the breakpoint on $syscall for the second time.  In this time,
     # the address of syscall insn and next insn of syscall are recorded.
 
-    gdb_test "display/i \$pc" ".*"
-
-    # Single step until we see a syscall insn or we reach the
-    # upper bound of loop iterations.
-    set msg "find syscall insn in $syscall"
-    set steps 0
-    set max_steps 1000
-    gdb_test_multiple "stepi" $msg {
-	-re ".*$syscall_insn.*$gdb_prompt $" {
-	    pass $msg
+    # Check if the first instruction we stopped at is the syscall one.
+    set syscall_insn_addr 0
+    set test "fetch first stop pc"
+    gdb_test_multiple "display/i \$pc" $test {
+	-re "display/i .*: x/i .*=> ($hex) .*:.*$syscall_insn.*$gdb_prompt $" {
+	    set syscall_insn_addr $expect_out(1,string)
+	    pass $test
 	}
-	-re "x/i .*=>.*\r\n$gdb_prompt $" {
-	    incr steps
-	    if {$steps == $max_steps} {
-		fail $msg
-	    } else {
-		send_gdb "stepi\n"
-		exp_continue
+	-re "display/i.*" {
+	    pass $test
+	}
+    }
+
+    # If we are not at the syscall instruction yet, keep looking for it with
+    # stepi commands.
+    if {$syscall_insn_addr == 0} {
+	# Single step until we see a syscall insn or we reach the
+	# upper bound of loop iterations.
+	set msg "find syscall insn in $syscall"
+	set steps 0
+	set max_steps 1000
+	gdb_test_multiple "stepi" $msg {
+	    -re ".*$syscall_insn.*$gdb_prompt $" {
+		pass $msg
+	    }
+	    -re "x/i .*=>.*\r\n$gdb_prompt $" {
+		incr steps
+		if {$steps == $max_steps} {
+		    fail $msg
+		} else {
+		    send_gdb "stepi\n"
+		    exp_continue
+		}
 	    }
 	}
+
+	if {$steps == $max_steps} {
+	    return { -1, -1 }
+	}
+
+	set syscall_insn_addr [get_hexadecimal_valueof "\$pc" "0" \
+				  "pc before stepi"]
     }
 
-    if {$steps == $max_steps} {
-	return { -1, -1 }
+    # We have found the syscall instruction.  Now record the next instruction.
+    # Use the X command instead of stepi since we can't guarantee
+    # stepi is working properly.
+    set test "pc after syscall instruction"
+    gdb_test_multiple "x/2i \$pc" $test {
+	-re "x/2i .*=> $hex .*:.*$syscall_insn.* ($hex) .*:.*$gdb_prompt $" {
+	    set next_insn_addr $expect_out(2,string)
+	    pass $test
+	}
     }
 
-    set syscall_insn_addr [get_hexadecimal_valueof "\$pc" "0" \
-			       "pc before stepi"]
     if {[gdb_test "stepi" "x/i .*=>.*" "stepi $syscall insn"] != 0} {
 	return { -1, -1 }
     }
+
+    set pc_after_stepi [get_hexadecimal_valueof "\$pc" "0" \
+			    "pc after stepi with x command"]
+
+    if {$next_insn_addr != $pc_after_stepi} {
+      fail "pc after stepi matches insn addr after syscall"
+    }
+
     return [list $syscall_insn_addr [get_hexadecimal_valueof "\$pc" \
 					 "0" "pc after stepi"]]
 }
@@ -156,8 +192,13 @@ proc step_over_syscall { syscall } {
 		}
 	    }
 
-	    gdb_test "continue" "Continuing\\..*Breakpoint \[0-9\]+, .*" \
-		"continue to syscall insn $syscall"
+	    # Check if the syscall breakpoint is at the syscall instruction
+	    # address.  If so, no need to continue, otherwise we will run the
+	    # inferior to completion.
+	    if {$syscall_insn_addr != [get_hexadecimal_valueof "\$pc" "0"]} {
+		gdb_test "continue" "Continuing\\..*Breakpoint \[0-9\]+, .*" \
+		    "continue to syscall insn $syscall"
+	    }
 
 	    gdb_test_no_output "set displaced-stepping $displaced"
 
-- 
2.17.1