[patch 2/2] Displaced stepping across fork/vfork : test case

[Yao Qi]

On 09/12/2011 10:59 PM, Pedro Alves wrote:
> On Sunday 11 September 2011 01:55:00, Yao Qi wrote:
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.base/disp-step-fork.c
>> @@ -0,0 +1,42 @@
>> +
>> +#include<stdlib.h>
>> +int
>
> Newline between #include and int, please.
>

Fixed.

>> +
>> +if { ![support_displaced_stepping] } {
>> +    unsupported "displaced stepping"
>> +    return -1
>> +}
>> +
>> +global srcfile
>
> Did you mean to do:
>
>   set srcfile ${testfile}.c
>
> and use $srcfile below instead?  Otherwise, this var looks write-only.
>

This line is copied from other test case.  It is not needed.  Removed.

>> +set testfile "disp-step-fork"
>> +
>> +if [prepare_for_testing ${testfile}.exp ${testfile} ${testfile}.c {debug}] {
>> +    untested ${testfile}.exp
>> +    return -1
>> +}
>> +
>> +set syscall_insn ""
>> +
>> +# Define the syscall instruction for each target.
>> +
>> +if { [istarget "i\[34567\]86-*-linux*"] } {
>> +    set syscall_insn "int"
>> +} elseif { [istarget "x86_64-*-linux*"] } {
>> +    set syscall_insn "syscall"
>> +} else {
>> +    return -1
>> +}
>
> You could do this before trying to compile the program.  I'll
> be cheaper on !x86-linux targets.
>

OK.

> Very recent kernels will allow choosing the syscall instruction
> in the vsyscall page.  It'll be better to make the test
> skip on !x86-linux at the top, and then, change the test
> to look for all alternatives of the syscall insn.
>

I read some pages on vsyscall, and thought we can fetch the value of 
AT_SYSINFO in auxv to get the address of __kernel_vsyscall.  However, 
another simpler way comes up in my mind is to set possible syscall insn 
to $syscall_insn like this,

   set syscall_insn "(int|syscall|sysenter)"

If we match either of them in instruction stream, we are sure program 
will enter to syscall, so we don't have to worry about vsyscall page.

>> +
>> +if { ![runto main] } then {
>> +    fail "run to main ($teststr)"
>
> teststr doesn't exist in this test.  Looks like a pasto.
>

Sorry.  Removed $teststr.

>> +
>> +# Single step until we see sysall insn.
>> +while { $see_syscall_insn == 0 } {
>> +    send_gdb "stepi\n"
>> +    gdb_expect {
>> +       -re ".*$syscall_insn.*$gdb_prompt $" {
>> +           set see_syscall_insn 1
>> +       }
>> +       -re ".*$gdb_prompt $" {}
>> +    }
>> +}
>
> Best put an upper cap on number of steps.  Do you need to
> step through PLT resolution within the dynamic linker
> this way?  That'll be a bunch more steps than required.
> You can work around it by instead letting the program
> call fork once (so the plt resolves), and then do this
> stepping dance on the second fork call.

The upper bound of steps is added in loop.

We don't need to step through PLT resolution.  Now, each syscall 
function is called three times.  For the first time, it is for PLT 
resolution, for the second time, it is for locating the address of 
syscall insn, for the third time, single-step over syscall insn.

>
>> +
>> +set syscall_insn_addr ""
>> +set test "extract syscall insn address"
>> +
>> +send_gdb "print \$pc\n"
>> +gdb_expect {
>
> gdb_test_multiple, please.  Or better still, just do:
>
> set pc [get_hexadecimal_valueof "\$pc" "0"]
>

Oh, get_hexadecimal_valueof is better.  I am not aware of it before.

>> +
>> +set syscall_insn_next_addr ""
>> +set test "extract syscall insn address"
>> +
>> +send_gdb "stepi\n"
>> +gdb_expect {
>> +    -re "0x\[0-9a-fA-F\]+ in .*$gdb_prompt $" {
>> +       if [regexp "0x\[0-9a-fA-F\]+" $expect_out(0,string) syscall_insn_next_addr] {
>> +           pass "$test"
>> +       } else {
>> +           fail "$test"
>> +       }
>> +    }
>> +}
>
> Does this work instead?
>
> gdb_test "stepi" "" "stepi over fork syscall"
> set syscall_insn_next_addr [get_hexadecimal_valueof "\$pc" "0"]
>

Yes, fixed like this.

>> +
>> +gdb_test "continue" "Continuing\\..*Breakpoint \[0-9\]+, .* in fork ().*" \
>> +    "continue to fork (2nd time)"
>> +
>> +# Hit the breakpoint on fork for the second time.  In this time, we'll set breakpoint
>> +# on the syscall insn we recorded previously, and single step over it.
>> +
>> +gdb_test "break \*$syscall_insn_addr" "Breakpoint \[0-9\]+ at.*" \
>> +    "break on syscall insn"
>> +
>> +gdb_test "continue" "Continuing\\..*Breakpoint \[0-9\]+, .*" \
>> +    "continue to syscall insn"
>> +
>> +gdb_test_no_output "set displaced-stepping on"
>> +# Check the address of next instruction of syscall.
>> +gdb_test "stepi" ".*$syscall_insn_next_addr.*" "single step over fork"
>> \ No newline at end of file
>
> Please add one.
>

Added a new line.

>> +
>> +gdb_test "break marker" "Breakpoint.*at.* file .*${testfile}.c, line.*"
>> +
>> +set syscall_insn_line ""
>> +set syscall_insn_addr ""
>> +
>> +# Disassemble vfork to extract the address of syscall instruction.
>> +gdb_test_multiple "disassemble vfork" "disassemble vfork" {
>> +    -re "Dump of assembler code for function vfork.*$gdb_prompt $" {
>> +       pass "disassemble vfork"
>> +       if [regexp "0x\[0-9a-fA-F\]+\[ \t\]<\\+\[0-9\]+>:\[ \t\]$syscall_insn" $expect_out(0,string) syscall_insn_line] {
>> +           pass "find syscall insn address"
>> +
>> +           regexp "0x\[0-9a-fA-F\]+" $syscall_insn_line syscall_insn_addr
>> +       } else {
>> +           fail "find syscall insn address"
>> +           return -1
>> +       }
>> +    }
>> +    -re ".*$gdb_prompt $" {
>> +       fail "disassemble vfork"
>> +       return -1
>> +    }
>> +}
>
> Why can you do a disassemble here, while you did the stepi thing
> for fork?  The comments to the fork tests apply equally to this
> file.  Just a suggestion, not use it'd be better: would it be possible to
> merge the .exp files, by putting the tests under a proc/function that
> takes fork/vfork and .c file to build as arg?
>

The reason I choose disassembly here is that disassembly is cheaper than 
multiple stepi to locate the address of syscall insn.

In this version, there is only one .exp file, which is 
disp-step-syscall.exp, and common test logic is moved to a single proc.

b.t.w, the test to TARGET_WAITKIND_EXECD event can be added into 
disp-step-syscall.exp easily, but that is the work in next step.

-- 
Yao (齐尧)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0015-disp-step-fork-testcase.patch
Type: text/x-patch
Size: 8258 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/gdb-patches/attachments/20110914/36dfbaf1/attachment.bin>