diff options
author | Luis Machado <luis.machado@linaro.org> | 2020-01-14 13:46:07 -0300 |
---|---|---|
committer | Luis Machado <luis.machado@linaro.org> | 2020-01-27 17:28:06 -0300 |
commit | 16b10d6e61bfa0940333354e8144b3924dc86e56 (patch) | |
tree | a3844d67c6879b914a8e67c6e1197e1e2d9b2666 /gdb | |
parent | 82ef9cad78b74ee9176e059e8c8dec1bdb4a1efe (diff) | |
download | gdb-16b10d6e61bfa0940333354e8144b3924dc86e56.zip gdb-16b10d6e61bfa0940333354e8144b3924dc86e56.tar.gz gdb-16b10d6e61bfa0940333354e8144b3924dc86e56.tar.bz2 |
Harden gdb.base/step-over-syscall.exp
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-27 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.
Validate syscall number.
(step_over_syscall): Don't continue to the syscall instruction if
we're already there.
Diffstat (limited to 'gdb')
-rw-r--r-- | gdb/testsuite/ChangeLog | 10 | ||||
-rw-r--r-- | gdb/testsuite/gdb.base/step-over-syscall.exp | 134 |
2 files changed, 111 insertions, 33 deletions
diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 70cbb75..4fd5dc8 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,13 @@ +2020-01-27 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. + Validate syscall number. + (step_over_syscall): Don't continue to the syscall instruction if + we're already there. + 2020-01-25 Philippe Waroquiers <philippe.waroquiers@skynet.be> * gdb.base/attach.exp: Test 'set exec-file-mismatch'. diff --git a/gdb/testsuite/gdb.base/step-over-syscall.exp b/gdb/testsuite/gdb.base/step-over-syscall.exp index b373c16..0d0c31a 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,77 @@ 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 steps 0 + set max_steps 1000 + gdb_test_multiple "stepi" "find syscall insn in $syscall" { + -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 +219,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" |