[PATCH v3] Harden gdb.base/step-over-syscall.exp
Luis Machado
luis.machado@linaro.org
Mon Jan 27 18:48:00 GMT 2020
I decided to take another shot at validating the syscall numbers. It was a good
testsuite exercise and nice to see some of the differences between targets.
Updated to check the syscall numbers and confirmed it works by running
tests on the buildbot.
New in v3:
- Verify if the syscall number matches what is expected for the target.
- Used gdb_assert for one more check.
---
New in v2:
- Set initial values to -1 instead of 0.
- Rewrote RE to prevent unexpected matching when parsing one character at a
time.
- Used gdb_assert for an additional check.
- Validated with check-read1
---
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
gdb/testsuite/ChangeLog:
2020-01-24 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 | 135 ++++++++++++++-----
1 file changed, 102 insertions(+), 33 deletions(-)
diff --git a/gdb/testsuite/gdb.base/step-over-syscall.exp b/gdb/testsuite/gdb.base/step-over-syscall.exp
index b373c169c0..190b8fb066 100644
--- a/gdb/testsuite/gdb.base/step-over-syscall.exp
+++ b/gdb/testsuite/gdb.base/step-over-syscall.exp
@@ -16,13 +16,27 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.
set syscall_insn ""
+set syscall_register ""
+array set syscall_number {}
-# Define the syscall instruction for each target.
+# Define the syscall instructions, registers and numbers for each target.
if { [istarget "i\[34567\]86-*-linux*"] || [istarget "x86_64-*-linux*"] } {
set syscall_insn "\[ \t\](int|syscall|sysenter)\[ \t\]"
+ set syscall_register "eax"
+ array set syscall_number {fork "(56|120)" vfork "(58|190)" \
+ clone "(56|120)"}
} elseif { [istarget "aarch64*-*-linux*"] || [istarget "arm*-*-linux*"] } {
set syscall_insn "\[ \t\](swi|svc)\[ \t\]"
+
+ if { [istarget "aarch64*-*-linux*"] } {
+ set syscall_register "x8"
+ } else {
+ set syscall_register "r7"
+ }
+
+ array set syscall_number {fork "(120|220)" vfork "(190|220)" \
+ clone "(120|220)"}
} else {
return -1
}
@@ -30,13 +44,22 @@ if { [istarget "i\[34567\]86-*-linux*"] || [istarget "x86_64-*-linux*"] } {
proc_with_prefix check_pc_after_cross_syscall { syscall syscall_insn_next_addr } {
set syscall_insn_next_addr_found [get_hexadecimal_valueof "\$pc" "0"]
- set test "single step over $syscall final pc"
- if {$syscall_insn_next_addr != 0
- && $syscall_insn_next_addr == $syscall_insn_next_addr_found} {
- pass $test
- } else {
- fail $test
- }
+ gdb_assert {$syscall_insn_next_addr != 0 \
+ && $syscall_insn_next_addr == $syscall_insn_next_addr_found} \
+ "single step over $syscall final pc"
+}
+
+# Verify the syscall number is the correct one.
+
+proc syscall_number_matches { syscall } {
+ global syscall_register syscall_number
+
+ if {[gdb_test "p \$$syscall_register" ".*= $syscall_number($syscall)" \
+ "syscall number matches"] != 0} {
+ return 0
+ }
+
+ return 1
}
# Restart GDB and set up the test. Return a list in which the first one
@@ -47,6 +70,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 -1
set testfile "step-over-$syscall"
clean_restart $testfile
@@ -62,7 +87,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,39 +100,78 @@ 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" ".*"
+ # Check if the first instruction we stopped at is the syscall one.
+ set syscall_insn_addr -1
+ gdb_test_multiple "display/i \$pc" "fetch first stop pc" {
+ -re "display/i .*: x/i .*=> ($hex) .*:.*$syscall_insn.*$gdb_prompt $" {
+ set insn_addr $expect_out(1,string)
- # 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
+ # Is the syscall number the correct one?
+ if {[syscall_number_matches $syscall]} {
+ set syscall_insn_addr $insn_addr
+ }
+ pass $gdb_test_name
}
- -re "x/i .*=>.*\r\n$gdb_prompt $" {
- incr steps
- if {$steps == $max_steps} {
- fail $msg
- } else {
- send_gdb "stepi\n"
- exp_continue
+ -re ".*$gdb_prompt $" {
+ pass $gdb_test_name
+ }
+ }
+
+ # If we are not at the syscall instruction yet, keep looking for it with
+ # stepi commands.
+ if {$syscall_insn_addr == -1} {
+ # 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 $" {
+ # Is the syscall number the correct one?
+ if {[syscall_number_matches $syscall]} {
+ pass $gdb_test_name
+ } else {
+ exp_continue
+ }
}
+ -re "x/i .*=>.*\r\n$gdb_prompt $" {
+ incr steps
+ if {$steps == $max_steps} {
+ fail $gdb_test_name
+ } else {
+ send_gdb "stepi\n"
+ exp_continue
+ }
+ }
+ }
+
+ if {$steps == $max_steps} {
+ return { -1, -1 }
}
}
- 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.
+ gdb_test_multiple "x/2i \$pc" "pc before/after syscall instruction" {
+ -re "x/2i .*=> ($hex) .*:.*$syscall_insn.* ($hex) .*:.*$gdb_prompt $" {
+ set syscall_insn_addr $expect_out(1,string)
+ set next_insn_addr $expect_out(3,string)
+ pass $gdb_test_name
+ }
}
- 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 }
}
- return [list $syscall_insn_addr [get_hexadecimal_valueof "\$pc" \
- "0" "pc after stepi"]]
+
+ set pc_after_stepi [get_hexadecimal_valueof "\$pc" "0" \
+ "pc after stepi"]
+
+ gdb_assert {$next_insn_addr == $pc_after_stepi} \
+ "pc after stepi matches insn addr after syscall"
+
+ return [list $syscall_insn_addr $pc_after_stepi]
}
proc step_over_syscall { syscall } {
@@ -156,8 +220,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
More information about the Gdb-patches
mailing list