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

[Pedro Alves <pedro at codesourcery dot com>]

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 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2011 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#include <stdlib.h>
> +int

Newline between #include and int, please.

> +main (void)
> +{
> +  int  pid;
> +
> +  pid = fork ();
> +  if (pid == 0) /* child */
> +    {
> +      exit (0); /* at exit */
> +    }
> +  else
> +    {
> +    }
> +
> +  pid = fork ();
> +  if (pid == 0) /* child */
> +    {
> +      exit (0); /* at exit */
> +    }
> +  else
> +    {
> +    }
> +
> +}
> diff --git a/gdb/testsuite/gdb.base/disp-step-fork.exp b/gdb/testsuite/gdb.base/disp-step-fork.exp
> new file mode 100644
> index 0000000..a044d3e
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/disp-step-fork.exp
> @@ -0,0 +1,115 @@
> +# This testcase is part of GDB, the GNU debugger.
> +
> +# Copyright 2011 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +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.

> +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.

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.

> +
> +if { ![runto main] } then {
> +    fail "run to main ($teststr)"

teststr doesn't exist in this test.  Looks like a pasto.

> +    return
> +}
> +
> +gdb_test "break fork" "Breakpoint.*at.*"
> +gdb_test "continue" "Continuing\\..*Breakpoint \[0-9\]+, .* in fork ().*" \
> +    "continue to fork (1st time)"
> +
> +# Hit the breakpoint on fork for the first time.  In this time, the address
> +# of syscall insn and next insn of syscall are recorded.
> +
> +gdb_test "display/i \$pc" ".*"
> +
> +set see_syscall_insn 0
> +
> +# 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.

> +
> +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"]

> +    -re "\\$\[0-9\]+ = (\[^\r\n\]+).*$gdb_prompt $" {
> +       if [regexp "0x\[0-9a-fA-F\]+" $expect_out(0,string) syscall_insn_addr] {
> +           pass "$test"
> +       } else {
> +           fail "$test"
> +       }
> +    }
> +    -re ".*$gdb_prompt $" {
> +       fail "$test"
> +    }
> +}
> +
> +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"]

> +
> +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.

> diff --git a/gdb/testsuite/gdb.base/disp-step-vfork.c b/gdb/testsuite/gdb.base/disp-step-vfork.c
> new file mode 100644
> index 0000000..84e79b0
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/disp-step-vfork.c
> @@ -0,0 +1,47 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2011 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
> +
> +#include <unistd.h>
> +
> +int global = 0;
> +
> +static void
> +marker () {}
> +
> +int
> +main (void)
> +{
> +  int pid;
> +
> +  pid = vfork ();
> +  if (pid == -1)
> +    {
> +      return 1;
> +    }
> +  else if (pid != 0)
> +    {
> +    }
> +  else
> +    {
> +      global = 1;
> +      _exit (0);
> +    }
> +
> +  marker ();
> +  return 0;
> +
> +}
> diff --git a/gdb/testsuite/gdb.base/disp-step-vfork.exp b/gdb/testsuite/gdb.base/disp-step-vfork.exp
> new file mode 100644
> index 0000000..65b7bdc
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/disp-step-vfork.exp
> @@ -0,0 +1,81 @@
> +# This testcase is part of GDB, the GNU debugger.
> +
> +# Copyright 2011 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +if { ![support_displaced_stepping] } {
> +    unsupported "displaced stepping"
> +    return -1
> +}
> +
> +global srcfile
> +set testfile "disp-step-vfork"
> +
> +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
> +}
> +
> +if { ![runto main] } then {
> +    fail "run to main ($teststr)"
> +    return
> +}
> +
> +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?

> +
> +gdb_test "break \*$syscall_insn_addr" "Breakpoint \[0-9\]+ at .*" "break on syscall insn"
> +gdb_test "continue" "Continuing\\..*Breakpoint \[0-9\]+, .* in vfork ().*" \
> +    "continue to vfork"
> +
> +gdb_test_no_output "set displaced-stepping on"
> +gdb_test "stepi" ".*" "single step over vfork"
> +gdb_test "continue" "Continuing\\..*Breakpoint \[0-9\]+, marker \\(\\) at.*" \
> +    "continue to marker"
> +# Make sure child process is executed correctly
> +gdb_test "print global" ".* = 1"
> \ No newline at end of file
> -- 
> 1.7.0.4

-- 
Pedro Alves