This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch, gdbserver] Uninsert bpkt when regular and fast tracepoint are set at the same address


On Thursday 27 October 2011 03:56:17, Yao Qi wrote:

> I find a program will receive segv fault when I set a regular tracepoint
> and a fast tracepoint at the same address, start tracing and resume program.
> 
> gdbserver has taken care of this situation in many places of the code,
> when uninserting breakpoint or fast tracepoint, write_inferior_memory is
> called to take care of layering breakpoints on top of fast tracepoints.
>  However, it is not right to me.  Here is an example to illustrate this
> problem.
> 
> Supposing I set a regular tracepoint and a fast tracepoint on 0x080484fc,
> 
>     0x080484fc <+3>:     e8 f3 ff ff ff  call   0x80484f4 <func>
> 
> During insertion, trap insn (for regular tracepoint) and jmp insn (for
> fast tracepoint) are inserted, and gdbserver takes care of them to make
> sure trap insn is *always* inserted on top of jmp insn.  In my example,
> gdbserver will first insert a jmp insn (0xe9XXXXXXXX), save original
> insn in jump shadow (0xe8f3ffffff), and then insert trap insn on top of
> jump insn.  So, gdbserver writes 0xcc to 0x080484fc and save the first
> byte (0xe9) in bp->old_data.  Memory content on 0x080484fc is
> 0xccXXXXXXXX.  

> Note that bp->old_data saves the first byte of jmp insn
> rather than original insn.

_This_ is the bug.  I must have introduced it as a last minute change
before pushing fast tracepoints upstream, because I'm sure this used
to work, proof being all the comments in the code about it.  :-/

The idea is that the shadow of all breakpoints and fast tracepoint
jumps always has the contents of the memory as if no breakpoint
of fast tracepoint was inserted.

The problem is that check_mem_write does:

 - loop over fast tracepoint jumps, updating their shadow buffers,
   _and_ clobbers the input buffer.
 - loop over raw breakpoints, updating their shadow buffers, but
   at this point, the input buffer is already overwritten with
   fast tracepoint jumps...

The fix is to split updating the shadow buffers from stapling fast
tracepoint jumps and breakpoints on top of the input buffer.

While at it, I did several changes to the new tests.  Thanks a lot
for writing them BTW.  They're awesome.  I wish I had done so before...
I'll point out the main issues below.

> diff --git a/gdb/testsuite/gdb.trace/trace-break.c b/gdb/testsuite/gdb.trace/trace-break.c
> new file mode 100644
> index 0000000..0eda029
> --- /dev/null
> +++ b/gdb/testsuite/gdb.trace/trace-break.c
> @@ -0,0 +1,46 @@
> +/* 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/>.  */
> +
> +
> +static void
> +func ()
> +{}

Only called from asm, should have __attribute__((used)), as some
compilers can get rid of it even without optimizations on.

> +
> +static void
> +marker ()
> +{
> +  int a = 0;
> +  int b = a;
> +
> +  asm (".global set_point");
> +  asm (" set_point:"); /* The lable that we'll set tracepoint on.  */

Some targets (cygwin/mingw, for example) need an underscore on asm
symbols to make them accessible to C.

> +#if (defined __x86_64__ || defined __i386__)
> +  /* It is a five-byte insn, so that fast trace point can be set on it.  */
> +  asm ("call func");
> +#endif
> +}
> +
> +static void
> +end ()
> +{}
> +
> +int main()
> +{
> +  marker ();
> +  end ();
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.trace/trace-break.exp b/gdb/testsuite/gdb.trace/trace-break.exp
> new file mode 100644
> index 0000000..6f8d9aa
> --- /dev/null
> +++ b/gdb/testsuite/gdb.trace/trace-break.exp
> @@ -0,0 +1,233 @@
> +# 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/>.
> +
> +load_lib "trace-support.exp";
> +
> +if $tracelevel then {
> +    strace $tracelevel
> +}
> +
> +set testfile "trace-break"
> +
> +set srcfile $testfile.c
> +set binfile $objdir/$subdir/$testfile
> +if { [gdb_compile "$srcdir/$subdir/$srcfile" $binfile \
> +         executable [list debug nowarnings] ] != "" } {
> +    untested trace-break.exp
> +    return -1
> +}
> +
> +gdb_start
> +gdb_load ${binfile}
> +
> +runto_main
> +gdb_reinitialize_dir $srcdir/$subdir

We can use prepare_for_testing for most of this.  Failure on
runto_main should be handled.

> +
> +# We generously give ourselves one "pass" if we successfully 
> +# detect that this test cannot be run on this target!
> +if { ![gdb_target_supports_trace] } then {
> +    pass "Current target does not support trace"
> +    return 1;
> +}

We should go through all the gdb.trace tests and get rid of
this funny pass.  Should be `unsupported' instead.

> +
> +# Set breakpoint and tracepoint at the same address.
> +
> +proc break_trace_same_addr_1 { trace_type option } {
> +    global srcdir
> +    global subdir
> +    global binfile
> +    global pf_prefix
> +    global srcfile
> +
> +    # Start with a fresh gdb.
> +    gdb_exit
> +    gdb_start
> +    gdb_load ${binfile}
> +    runto_main
> +    gdb_reinitialize_dir $srcdir/$subdir

clean_restart does all this for us.  runto_main failures
be handled.

> +
> +    set old_pf_prefix $pf_prefix
> +    set pf_prefix "$pf_prefix 1 $trace_type $option:"

The prefix should be changed _before_ compiling/running to main,
so if the latter fails, the FAIL's will have the correct prefix.

> +
> +    gdb_test_no_output "set breakpoint always-inserted  ${option}"

Spurious extra space.

> +
> +    gdb_test "break end" "Breakpoint \[0-9\] at 0x\[0-9a-fA-F\]+: file.*"

We can use $hex.

> +
> +    gdb_test "break set_point" "Breakpoint \[0-9\] at 0x\[0-9a-fA-F\]+: file.*"
> +    gdb_test "${trace_type} set_point" "\(Fast t|T\)racepoint \[0-9\] at 0x\[0-9a-fA-F\]+: file.*"
> +
> +    gdb_test_no_output "tstart"
> +
> +    gdb_test "continue" "Continuing\\.\[ \r\n\]+Breakpoint.*" "continue to set_point"
> +
> +    gdb_test "continue" "Continuing\\.\[ \r\n\]+Breakpoint.*" "continue to end"
> +    gdb_test_no_output "tstop"
> +
> +    gdb_test "tfind" "Found trace frame 0, tracepoint .*" "tfind frame 0"
> +    gdb_test "tfind" "Target failed to find requested trace frame\\..*"
> +
> +    set pf_prefix $old_pf_prefix 
> +}
> +
> +# Set multiple tracepoints at the same address.
> +
> +proc break_trace_same_addr_2 { trace_type1 trace_type2 option } {
> +    global srcdir
> +    global subdir
> +    global binfile
> +    global pf_prefix
> +    global srcfile
> +
> +    # Start with a fresh gdb.
> +    gdb_exit
> +    gdb_start
> +    gdb_load ${binfile}
> +    runto_main
> +    gdb_reinitialize_dir $srcdir/$subdir
> +
> +    set old_pf_prefix $pf_prefix
> +    set pf_prefix "$pf_prefix 2 $trace_type1 $trace_type2 $option:"
> +
> +    gdb_test_no_output "set breakpoint always-inserted  ${option}"
> +
> +    gdb_test "break end" "Breakpoint \[0-9\] at 0x\[0-9a-fA-F\]+: file.*"
> +    gdb_test "${trace_type1} set_point" "\(Fast t|T\)racepoint \[0-9\] at 0x\[0-9a-fA-F\]+: file.*"
> +    gdb_test "${trace_type2} set_point" "\(Fast t|T\)racepoint \[0-9\] at 0x\[0-9a-fA-F\]+: file.*"

These last two will result in the same message in gdb.sum when trace_type1 and
trace_type2 is the same.  Always do something like:

  $ cat testsuite/gdb.sum | grep PASS | sort | uniq -c | sort -n

to make sure all tests have unique messages.

> +
> +    gdb_test_no_output "tstart"
> +    gdb_test "continue" "Continuing\\.\[ \r\n\]+Breakpoint.*" "continue to end"
> +
> +    gdb_test_no_output "tstop"
> +
> +    gdb_test "tfind" "Found trace frame 0, tracepoint .*" "tfind frame 0"
> +    gdb_test "tfind" "Found trace frame 1, tracepoint .*" "tfind frame 1"
> +    gdb_test "tfind" "Target failed to find requested trace frame\\..*"
> +
> +    set pf_prefix $old_pf_prefix 
> +}
> +
> +# Set breakpoint and tracepoint at the same address.  Delete breakpoint, and verify
> +# that tracepoint still works.
> +
> +proc break_trace_same_addr_3 { trace_type option } {
> +    global srcdir
> +    global subdir
> +    global binfile
> +    global pf_prefix
> +    global srcfile
> +
> +    # Start with a fresh gdb.
> +    gdb_exit
> +    gdb_start
> +    gdb_load ${binfile}
> +    runto_main
> +    gdb_reinitialize_dir $srcdir/$subdir
> +
> +    set old_pf_prefix $pf_prefix
> +    set pf_prefix "$pf_prefix 3 $trace_type $option:"
> +
> +    gdb_test_no_output "set breakpoint always-inserted  ${option}"
> +    gdb_test "break marker" "Breakpoint \[0-9\] at 0x\[0-9a-fA-F\]+: file.*"
> +    gdb_test "break end" "Breakpoint \[0-9\] at 0x\[0-9a-fA-F\]+: file.*"
> +
> +    gdb_test "break set_point" "Breakpoint \[0-9\] at 0x\[0-9a-fA-F\]+: file.*"
> +    gdb_test "${trace_type} set_point" "\(Fast t|T\)racepoint \[0-9\] at 0x\[0-9a-fA-F\]+: file.*"
> +
> +    gdb_test_no_output "tstart"
> +
> +    gdb_test "continue" "Continuing\\.\[ \r\n\]+Breakpoint.*" "continue to marker"
> +    gdb_test "delete break 4"
> +
> +    gdb_test "continue" "Continuing\\.\[ \r\n\]+Breakpoint.*" "continue to end"
> +    gdb_test_no_output "tstop"
> +
> +    gdb_test "tfind" "Found trace frame 0, tracepoint .*" "tfind frame 0"
> +    gdb_test "tfind" "Target failed to find requested trace frame\\..*"
> +
> +    set pf_prefix $old_pf_prefix 
> +}
> +
> +# Set breakpoint and tracepoint at the same address.  Delete tracepoint, and verify
> +# that breakpoint still works.
> +
> +proc break_trace_same_addr_4 { trace_type option } {
> +    global srcdir
> +    global subdir
> +    global binfile
> +    global pf_prefix
> +    global srcfile
> +
> +    # Start with a fresh gdb.
> +    gdb_exit
> +    gdb_start
> +    gdb_load ${binfile}
> +    runto_main
> +    gdb_reinitialize_dir $srcdir/$subdir
> +
> +    set old_pf_prefix $pf_prefix
> +    set pf_prefix "$pf_prefix 4 $trace_type $option:"
> +
> +    gdb_test_no_output "set breakpoint always-inserted  ${option}"
> +    gdb_test "break marker" "Breakpoint \[0-9\] at 0x\[0-9a-fA-F\]+: file.*"
> +    gdb_test "break end" "Breakpoint \[0-9\] at 0x\[0-9a-fA-F\]+: file.*"
> +
> +    gdb_test "break set_point" "Breakpoint \[0-9\] at 0x\[0-9a-fA-F\]+: file.*"
> +    gdb_test "${trace_type} set_point" "\(Fast t|T\)racepoint \[0-9\] at 0x\[0-9a-fA-F\]+: file.*"
> +
> +    gdb_test "continue" "Continuing\\.\[ \r\n\]+Breakpoint.*" "continue to marker"
> +    # Detele tracepoint set on set_point.
> +    gdb_test "delete trace 5"
> +
> +    gdb_test "tstart" "No tracepoints defined, not starting trace.*"
> +
> +    gdb_test "continue" "Continuing\\.\[ \r\n\]+Breakpoint.*" "continue to set_point"
> +    gdb_test "continue" "Continuing\\.\[ \r\n\]+Breakpoint.*" "continue to end"
> +    gdb_test "tstop" "Trace is not running.*"
> +
> +    gdb_test "tfind" "Target failed to find requested trace frame\\..*"
> +
> +    set pf_prefix $old_pf_prefix 
> +}
> +
> +foreach break_always_inserted { "on" "off" } {
> +    break_trace_same_addr_1 "trace" ${break_always_inserted}
> +    break_trace_same_addr_2 "trace" "trace" ${break_always_inserted}
> +    break_trace_same_addr_3 "trace" ${break_always_inserted}
> +    break_trace_same_addr_4 "trace" ${break_always_inserted}
> +}
> +
> +gdb_exit
> +
> +set libipa $objdir/../gdbserver/libinproctrace.so

Needs uploading to the remote host.

> +
> +if { [gdb_compile "$srcdir/$subdir/$srcfile" $binfile \
> +         executable [list debug nowarnings shlib=$libipa] ] != "" } {
> +    untested trace-break.exp
> +    return -1
> +}

We can't use prepare_for_testing/build_executable for this one
(see comment in patch), but we can use clean_restart.

> +
> +gdb_start
> +gdb_load ${binfile}
> +
> +runto_main
> +gdb_reinitialize_dir $srcdir/$subdir
> +gdb_test "info sharedlibrary" ".*libinproctrace\.so.*" "check libinproctrace.so"

If this fails, we shouldn't proceed and run the test of the tests.  They'll
all fail/timeout.

> +
> +foreach break_always_inserted { "on" "off" } {
> +    break_trace_same_addr_1 "ftrace" ${break_always_inserted}
> +    break_trace_same_addr_2 "trace" "ftrace" ${break_always_inserted}
> +    break_trace_same_addr_2 "ftrace" "trace" ${break_always_inserted}
> +    break_trace_same_addr_3 "ftrace" ${break_always_inserted}
> +    break_trace_same_addr_4 "ftrace" ${break_always_inserted}
> +}

Several typos and spurious whitespace were fixed as well.

Below's my version of the patches.  WDYT?

-- 
Pedro Alves
2011-10-27  Yao Qi  <yao@codesourcery.com>
	    Pedro Alves  <pedro@codesourcery.com>

	gdb/testsuite/
        * gdb.trace/trace-break.c: New.
        * gdb.trace/trace-break.exp: New.

---
 gdb/testsuite/gdb.trace/trace-break.c   |   58 +++++++
 gdb/testsuite/gdb.trace/trace-break.exp |  240 ++++++++++++++++++++++++++++++++
 2 files changed, 298 insertions(+)
 create mode 100644 gdb/testsuite/gdb.trace/trace-break.c
 create mode 100644 gdb/testsuite/gdb.trace/trace-break.exp

Index: src/gdb/testsuite/gdb.trace/trace-break.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ src/gdb/testsuite/gdb.trace/trace-break.c	2011-10-27 16:32:41.905141841 +0100
@@ -0,0 +1,58 @@
+/* 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/>.  */
+
+#ifdef SYMBOL_PREFIX
+#define SYMBOL(str)     SYMBOL_PREFIX #str
+#else
+#define SYMBOL(str)     #str
+#endif
+
+/* Called from asm.  */
+static void __attribute__((used))
+func (void)
+{}
+
+static void
+marker (void)
+{
+  /* Some code to make sure `b marker' and `b set_point' set
+     breakpoints at different addresses.  */
+  int a = 0;
+  int b = a;
+
+  /* `set_point' is the label where we'll set multiple tracepoints and
+     breakpoints at.  The insn at the label must the large enough to
+     fit a fast tracepoint jump.  */
+  asm ("    .global " SYMBOL(set_point) "\n"
+       SYMBOL(set_point) ":\n"
+#if (defined __x86_64__ || defined __i386__)
+       "    call " SYMBOL(func) "\n"
+#endif
+       );
+}
+
+static void
+end (void)
+{}
+
+int
+main ()
+{
+  marker ();
+  end ();
+  return 0;
+}
Index: src/gdb/testsuite/gdb.trace/trace-break.exp
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ src/gdb/testsuite/gdb.trace/trace-break.exp	2011-10-27 16:39:03.545141899 +0100
@@ -0,0 +1,240 @@
+# 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/>.
+
+load_lib "trace-support.exp";
+
+set testfile "trace-break"
+set executable $testfile
+set srcfile $testfile.c
+set binfile $objdir/$subdir/$testfile
+set expfile $testfile.exp
+
+# Some targets have leading underscores on assembly symbols.
+set additional_flags [gdb_target_symbol_prefix_flags]
+
+if [prepare_for_testing $expfile $executable $srcfile \
+	[list debug $additional_flags]] {
+    untested "failed to prepare for trace tests"
+    return -1
+}
+
+if ![runto_main] {
+    fail "Can't run to main to check for trace support"
+    return -1
+}
+
+if ![gdb_target_supports_trace] {
+    unsupported "target does not support trace"
+    return -1;
+}
+
+# Set breakpoint and tracepoint at the same address.
+
+proc break_trace_same_addr_1 { trace_type option } {
+    global executable
+    global pf_prefix
+    global hex
+
+    set old_pf_prefix $pf_prefix
+    set pf_prefix "$pf_prefix 1 $trace_type $option:"
+
+    # Start with a fresh gdb.
+    clean_restart ${executable}
+    if ![runto_main] {
+	fail "Can't run to main"
+	set pf_prefix $old_pf_prefix
+	return -1
+    }
+
+    gdb_test_no_output "set breakpoint always-inserted ${option}"
+
+    gdb_test "break end" "Breakpoint \[0-9\] at $hex: file.*"
+
+    gdb_test "break set_point" "Breakpoint \[0-9\] at $hex: file.*"
+    gdb_test "${trace_type} set_point" "\(Fast t|T\)racepoint \[0-9\] at $hex: file.*"
+
+    gdb_test_no_output "tstart"
+
+    gdb_test "continue" "Continuing\\.\[ \r\n\]+Breakpoint.*" "continue to set_point"
+
+    gdb_test "continue" "Continuing\\.\[ \r\n\]+Breakpoint.*" "continue to end"
+    gdb_test_no_output "tstop"
+
+    gdb_test "tfind" "Found trace frame 0, tracepoint .*" "tfind frame 0"
+    gdb_test "tfind" "Target failed to find requested trace frame\\..*"
+
+    set pf_prefix $old_pf_prefix
+}
+
+# Set multiple tracepoints at the same address.
+
+proc break_trace_same_addr_2 { trace_type1 trace_type2 option } {
+    global executable
+    global pf_prefix
+    global hex
+
+    set old_pf_prefix $pf_prefix
+    set pf_prefix "$pf_prefix 2 $trace_type1 $trace_type2 $option:"
+
+    # Start with a fresh gdb.
+    clean_restart ${executable}
+    if ![runto_main] {
+	fail "Can't run to main"
+	set pf_prefix $old_pf_prefix
+	return -1
+    }
+
+    gdb_test_no_output "set breakpoint always-inserted ${option}"
+
+    gdb_test "break end" "Breakpoint \[0-9\] at $hex: file.*"
+
+    gdb_test "${trace_type1} set_point" \
+	"\(Fast t|T\)racepoint \[0-9\] at $hex: file.*" \
+	"${trace_type1} set_point (1)"
+
+    gdb_test "${trace_type2} set_point" \
+	"\(Fast t|T\)racepoint \[0-9\] at $hex: file.*" \
+	"${trace_type2} set_point (2)"
+
+    gdb_test_no_output "tstart"
+    gdb_test "continue" "Continuing\\.\[ \r\n\]+Breakpoint.*" "continue to end"
+
+    gdb_test_no_output "tstop"
+
+    gdb_test "tfind" "Found trace frame 0, tracepoint .*" "tfind frame 0"
+    gdb_test "tfind" "Found trace frame 1, tracepoint .*" "tfind frame 1"
+    gdb_test "tfind" "Target failed to find requested trace frame\\..*"
+
+    set pf_prefix $old_pf_prefix
+}
+
+# Set breakpoint and tracepoint at the same address.  Delete breakpoint, and verify
+# that tracepoint still works.
+
+proc break_trace_same_addr_3 { trace_type option } {
+    global executable
+    global pf_prefix
+    global hex
+
+    set old_pf_prefix $pf_prefix
+    set pf_prefix "$pf_prefix 3 $trace_type $option:"
+
+    # Start with a fresh gdb.
+    clean_restart ${executable}
+    if ![runto_main] {
+	fail "Can't run to main"
+	set pf_prefix $old_pf_prefix
+	return -1
+    }
+
+    gdb_test_no_output "set breakpoint always-inserted ${option}"
+    gdb_test "break marker" "Breakpoint \[0-9\] at $hex: file.*"
+    gdb_test "break end" "Breakpoint \[0-9\] at $hex: file.*"
+
+    gdb_test "break set_point" "Breakpoint \[0-9\] at $hex: file.*"
+    gdb_test "${trace_type} set_point" "\(Fast t|T\)racepoint \[0-9\] at $hex: file.*"
+
+    gdb_test_no_output "tstart"
+
+    gdb_test "continue" "Continuing\\.\[ \r\n\]+Breakpoint.*" "continue to marker"
+    gdb_test "delete break 4"
+
+    gdb_test "continue" "Continuing\\.\[ \r\n\]+Breakpoint.*" "continue to end"
+    gdb_test_no_output "tstop"
+
+    gdb_test "tfind" "Found trace frame 0, tracepoint .*" "tfind frame 0"
+    gdb_test "tfind" "Target failed to find requested trace frame\\..*"
+
+    set pf_prefix $old_pf_prefix
+}
+
+# Set breakpoint and tracepoint at the same address.  Delete tracepoint, and verify
+# that breakpoint still works.
+
+proc break_trace_same_addr_4 { trace_type option } {
+    global executable
+    global pf_prefix
+    global hex
+
+    set old_pf_prefix $pf_prefix
+    set pf_prefix "$pf_prefix 4 $trace_type $option:"
+
+    # Start with a fresh gdb.
+    clean_restart ${executable}
+    if ![runto_main] {
+	fail "Can't run to main"
+	set pf_prefix $old_pf_prefix
+	return -1
+    }
+
+    gdb_test_no_output "set breakpoint always-inserted ${option}"
+    gdb_test "break marker" "Breakpoint \[0-9\] at $hex: file.*"
+    gdb_test "break end" "Breakpoint \[0-9\] at $hex: file.*"
+
+    gdb_test "break set_point" "Breakpoint \[0-9\] at $hex: file.*"
+    gdb_test "${trace_type} set_point" "\(Fast t|T\)racepoint \[0-9\] at $hex: file.*"
+
+    gdb_test "continue" "Continuing\\.\[ \r\n\]+Breakpoint.*" "continue to marker"
+    # Delete tracepoint set on set_point.
+    gdb_test "delete trace 5"
+
+    gdb_test "tstart" "No tracepoints defined, not starting trace.*"
+
+    gdb_test "continue" "Continuing\\.\[ \r\n\]+Breakpoint.*" "continue to set_point"
+    gdb_test "continue" "Continuing\\.\[ \r\n\]+Breakpoint.*" "continue to end"
+    gdb_test "tstop" "Trace is not running.*"
+
+    gdb_test "tfind" "Target failed to find requested trace frame\\..*"
+
+    set pf_prefix $old_pf_prefix
+}
+
+foreach break_always_inserted { "on" "off" } {
+    break_trace_same_addr_1 "trace" ${break_always_inserted}
+    break_trace_same_addr_2 "trace" "trace" ${break_always_inserted}
+    break_trace_same_addr_3 "trace" ${break_always_inserted}
+    break_trace_same_addr_4 "trace" ${break_always_inserted}
+}
+
+set libipa $objdir/../gdbserver/libinproctrace.so
+gdb_load_shlibs $libipa
+
+# Can't use prepare_for_testing, because that splits compiling into
+# building objects and then linking, and we'd fail with "linker input
+# file unused because linking not done" when building the object.
+
+if { [gdb_compile "$srcdir/$subdir/$srcfile" $binfile \
+	  executable [list debug $additional_flags shlib=$libipa] ] != "" } {
+    untested "failed to compile ftrace tests"
+    return -1
+}
+clean_restart ${executable}
+
+if ![runto_main] {
+    fail "Can't run to main for ftrace tests"
+    return 0
+}
+
+gdb_reinitialize_dir $srcdir/$subdir
+if { [gdb_test "info sharedlibrary" ".*libinproctrace\.so.*" "IPA loaded"] != 0 } {
+    untested "Could not find IPA lib loaded"
+} else {
+    foreach break_always_inserted { "on" "off" } {
+	break_trace_same_addr_1 "ftrace" ${break_always_inserted}
+	break_trace_same_addr_2 "trace" "ftrace" ${break_always_inserted}
+	break_trace_same_addr_2 "ftrace" "trace" ${break_always_inserted}
+	break_trace_same_addr_3 "ftrace" ${break_always_inserted}
+	break_trace_same_addr_4 "ftrace" ${break_always_inserted}
+    }
+}
2011-10-27  Pedro Alves  <pedro@codesourcery.com>

	gdb/gdbserver/
	* mem-break.c (check_mem_write): Split updating shadows and
	filling the write buffer into two passes.  Don't clobber the
	breakpoints' shadows with fast tracepoint jumps.

---
 gdb/gdbserver/mem-break.c |   68 +++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 61 insertions(+), 7 deletions(-)

Index: src/gdb/gdbserver/mem-break.c
===================================================================
--- src.orig/gdb/gdbserver/mem-break.c	2011-10-27 15:40:38.195141376 +0100
+++ src/gdb/gdbserver/mem-break.c	2011-10-27 16:31:25.865141831 +0100
@@ -1032,14 +1032,15 @@ void
 check_mem_write (CORE_ADDR mem_addr, unsigned char *buf, int mem_len)
 {
   struct process_info *proc = current_process ();
-  struct raw_breakpoint *bp = proc->raw_breakpoints;
-  struct fast_tracepoint_jump *jp = proc->fast_tracepoint_jumps;
-  CORE_ADDR mem_end = mem_addr + mem_len;
+  struct raw_breakpoint *bp;
+  struct fast_tracepoint_jump *jp;
+  const CORE_ADDR mem_end = mem_addr + mem_len;
   int disabled_one = 0;
 
-  /* First fast tracepoint jumps, then breakpoint traps on top.  */
+  /* First update the shadows of breakpoints and fast tracepoint jumps
+     with the contents of BUF.  */
 
-  for (; jp != NULL; jp = jp->next)
+  for (jp = proc->fast_tracepoint_jumps; jp != NULL; jp = jp->next)
     {
       CORE_ADDR jp_end = jp->pc + jp->length;
       CORE_ADDR start, end;
@@ -1064,12 +1065,66 @@ check_mem_write (CORE_ADDR mem_addr, uns
 
       memcpy (fast_tracepoint_jump_shadow (jp) + copy_offset,
 	      buf + buf_offset, copy_len);
+    }
+
+  for (bp = proc->raw_breakpoints; bp != NULL; bp = bp->next)
+    {
+      CORE_ADDR bp_end = bp->pc + breakpoint_len;
+      CORE_ADDR start, end;
+      int copy_offset, copy_len, buf_offset;
+
+      if (mem_addr >= bp_end)
+	continue;
+      if (bp->pc >= mem_end)
+	continue;
+
+      start = bp->pc;
+      if (mem_addr > start)
+	start = mem_addr;
+
+      end = bp_end;
+      if (end > mem_end)
+	end = mem_end;
+
+      copy_len = end - start;
+      copy_offset = start - bp->pc;
+      buf_offset = start - mem_addr;
+
+      memcpy (bp->old_data + copy_offset, buf + buf_offset, copy_len);
+    }
+
+  /* Then fill in BUF with what we really want to write to memory.
+     First fast tracepoint jumps, then breakpoint traps on top.  */
+
+  for (jp = proc->fast_tracepoint_jumps; jp != NULL; jp = jp->next)
+    {
+      CORE_ADDR jp_end = jp->pc + jp->length;
+      CORE_ADDR start, end;
+      int copy_offset, copy_len, buf_offset;
+
+      if (mem_addr >= jp_end)
+	continue;
+      if (jp->pc >= mem_end)
+	continue;
+
+      start = jp->pc;
+      if (mem_addr > start)
+	start = mem_addr;
+
+      end = jp_end;
+      if (end > mem_end)
+	end = mem_end;
+
+      copy_len = end - start;
+      copy_offset = start - jp->pc;
+      buf_offset = start - mem_addr;
+
       if (jp->inserted)
 	memcpy (buf + buf_offset,
 		fast_tracepoint_jump_insn (jp) + copy_offset, copy_len);
     }
 
-  for (; bp != NULL; bp = bp->next)
+  for (bp = proc->raw_breakpoints; bp != NULL; bp = bp->next)
     {
       CORE_ADDR bp_end = bp->pc + breakpoint_len;
       CORE_ADDR start, end;
@@ -1092,7 +1147,6 @@ check_mem_write (CORE_ADDR mem_addr, uns
       copy_offset = start - bp->pc;
       buf_offset = start - mem_addr;
 
-      memcpy (bp->old_data + copy_offset, buf + buf_offset, copy_len);
       if (bp->inserted)
 	{
 	  if (validate_inserted_breakpoint (bp))

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]