> On 22 Jan 2020, at 13:30, Luis Machado wrote: > > ping? > > https://sourceware.org/ml/gdb-patches/2020-01/msg00432.html > > On 1/15/20 5:36 PM, Luis Machado wrote: >> 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. I’m happy with leaving it this way too. >> 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 >> * 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 { I don’t like the use of display/i here because it will be displaying the pc after every command, so could break other tests. Alternatively, you could stepi then x/i $pc. However, the test already worked this way, so I’m ok with it. >> + -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"]] Minor nit. In the return, you could just use $next_insn_addr instead of calling get_hexadecimal_valueof again. >> } >> @@ -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" >> &j!z޶׭Yb֫rnr