From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1857 invoked by alias); 27 Oct 2011 02:56:52 -0000 Received: (qmail 1837 invoked by uid 22791); 27 Oct 2011 02:56:50 -0000 X-SWARE-Spam-Status: No, hits=-0.8 required=5.0 tests=AWL,BAYES_00,FROM_12LTRDOM,TW_BP,TW_EG,TW_XC X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 27 Oct 2011 02:56:31 +0000 Received: from nat-jpt.mentorg.com ([192.94.33.2] helo=PR1-MAIL.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1RJG8s-00014M-OA from Yao_Qi@mentor.com for gdb-patches@sourceware.org; Wed, 26 Oct 2011 19:56:31 -0700 Received: from [127.0.0.1] ([172.16.63.104]) by PR1-MAIL.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.1830); Thu, 27 Oct 2011 11:56:26 +0900 Message-ID: <4EA8C851.2080703@codesourcery.com> Date: Thu, 27 Oct 2011 07:04:00 -0000 From: Yao Qi User-Agent: Mozilla/5.0 (X11; Linux i686; rv:7.0) Gecko/20110923 Thunderbird/7.0 MIME-Version: 1.0 To: "gdb-patches@sourceware.org" Subject: [patch, gdbserver] Uninsert bpkt when regular and fast tracepoint are set at the same address Content-Type: multipart/mixed; boundary="------------070606000802010806080506" X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2011-10/txt/msg00715.txt.bz2 This is a multi-part message in MIME format. --------------070606000802010806080506 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Content-length: 4490 Hi, 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 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. When program hits trap insn, gdbserver will step-over trap insn. gdbserver will uninsert trap and uninsert fast tracepoint jumps (see linux-low.c:start_step_over). In uninsert_raw_breakpoint, bp->old_data will be written back to bp->pc, in my example, 0xe9 is written back to 0x080484fc. check_mem_write will be called, and check the overlap of memory to be write and breakpoint/tracepoint insn memory range. In my example, there is an overlap with fast tracepoint jump insn and trap tracepoint insn. So, 1) 0xe9 is written to the first byte of jump shadow and shadow becomes 0xe9f3ffffff, which is clobbered, 2) insn on 0x080484fc becomes 0xe9XXXXXXXX (right jump insn). Later, when uninserting fast tracepoint jumps, jump shadow will be written back, so 0xe9f3ffffff is written back, it is a wrong insn. Generally, when writing to memory, we need check_mem_write to take care of layering breakpoint and tracepoint, however, when writing memory during breakpoint/tracepoint uninsertion, we don't have to. Because 1) trap breakpoint is *always* inserted on top of fast tracepoint jump, 2) trap insn is not longer than jmp insn. During uninsertion, we don't have to worry about layering, and uninsert trap breakpoint and fast tracepoint jump one by one with simple *the_target->write_memory. Test case attached is about testing setting breakpoint and different kinds of tracepoint at the same address. In current trunk, there are 3 fails on x86_64-linux, FAIL: gdb.trace/trace-break.exp: 1 ftrace on: continue to end FAIL: gdb.trace/trace-break.exp: 2 trace ftrace on: continue to end FAIL: gdb.trace/trace-break.exp: 2 trace ftrace off: continue to end I also run trace-break.exp on commit de642393026eee797efdd1355c1913f8054dab64, which is the first revision for fast tracepoint. commit de642393026eee797efdd1355c1913f8054dab64 Author: Pedro Alves Date: Tue Jun 1 13:20:49 2010 +0000 gdb/gdbserver/ 2010-06-01 Pedro Alves Stan Shebs [...] * mem-break.c (set_raw_breakpoint_at): Use read_inferior_memory. (struct fast_tracepoint_jump): New. (fast_tracepoint_jump_insn): New. (fast_tracepoint_jump_shadow): New. (find_fast_tracepoint_jump_at): New. (fast_tracepoint_jump_here): New. (delete_fast_tracepoint_jump): New. (set_fast_tracepoint_jump): New. (uninsert_fast_tracepoint_jumps_at): New. (reinsert_fast_tracepoint_jumps_at): New. (set_breakpoint_at): Use write_inferior_memory. (uninsert_raw_breakpoint): Use write_inferior_memory. (check_mem_read): Mask out fast tracepoint jumps. (check_mem_write): Mask out fast tracepoint jumps. [...] FAIL: gdb.trace/trace-break.exp: 1 ftrace on: continue to end FAIL: gdb.trace/trace-break.exp: 2 trace ftrace on: continue to end FAIL: gdb.trace/trace-break.exp: 3 ftrace on: continue to end FAIL: gdb.trace/trace-break.exp: 2 trace ftrace off: continue to end FAIL: gdb.trace/trace-break.exp: 3 ftrace off: continue to end Test case fails as well, so these fails are not regression. Regression tested on x86_64-linux, no regression and fails in gdb.trace/trace-break.exp are fixed. OK for mainline? -- Yao (齐尧) --------------070606000802010806080506 Content-Type: text/x-patch; name="0003-new-testcase-trace-break.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0003-new-testcase-trace-break.patch" Content-length: 9815 2011-10-26 Yao Qi * gdb.trace/trace-break.c: New. * gdb.trace/trace-break.exp: New. --- gdb/testsuite/gdb.trace/trace-break.c | 46 ++++++ gdb/testsuite/gdb.trace/trace-break.exp | 233 +++++++++++++++++++++++++++++++ 3 files changed, 290 insertions(+), 0 deletions(-) create mode 100644 gdb/testsuite/gdb.trace/trace-break.c create mode 100644 gdb/testsuite/gdb.trace/trace-break.exp 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 . */ + + +static void +func () +{} + +static void +marker () +{ + int a = 0; + int b = a; + + asm (".global set_point"); + asm (" set_point:"); /* The lable that we'll set tracepoint on. */ +#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 . + +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 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; +} + +# 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 + + set old_pf_prefix $pf_prefix + set pf_prefix "$pf_prefix 1 $trace_type $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 "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.*" + + 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 + +if { [gdb_compile "$srcdir/$subdir/$srcfile" $binfile \ + executable [list debug nowarnings shlib=$libipa] ] != "" } { + untested trace-break.exp + return -1 +} + +gdb_start +gdb_load ${binfile} + +runto_main +gdb_reinitialize_dir $srcdir/$subdir +gdb_test "info sharedlibrary" ".*libinproctrace\.so.*" "check libinproctrace.so" + +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} +} -- 1.7.0.4 --------------070606000802010806080506 Content-Type: text/x-patch; name="0002-don-t-call-write_inferior_memory.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0002-don-t-call-write_inferior_memory.patch" Content-length: 3160 gdb/gdbserver * mem-break.c (uninsert_fast_tracepoint_jumps_at): Don't use write_inferior_memory. (delete_raw_breakpoint, uninsert_raw_breakpoint): Likewise. --- gdb/gdbserver/mem-break.c | 42 +++++++----------------------------------- 1 files changed, 7 insertions(+), 35 deletions(-) diff --git a/gdb/gdbserver/mem-break.c b/gdb/gdbserver/mem-break.c index 1a140f4..6ec2078 100644 --- a/gdb/gdbserver/mem-break.c +++ b/gdb/gdbserver/mem-break.c @@ -411,19 +411,9 @@ uninsert_fast_tracepoint_jumps_at (CORE_ADDR pc) if (jp->inserted) { jp->inserted = 0; - - /* Since there can be trap breakpoints inserted in the same - address range, we use use `write_inferior_memory', which - takes care of layering breakpoints on top of fast - tracepoints, and on top of the buffer we pass it. This works - because we've already marked the fast tracepoint fast - tracepoint jump uninserted above. Also note that we need to - pass the current shadow contents, because - write_inferior_memory updates any shadow memory with what we - pass here, and we want that to be a nop. */ - err = write_inferior_memory (jp->pc, - fast_tracepoint_jump_shadow (jp), - jp->length); + err = (*the_target->write_memory) (jp->pc, + fast_tracepoint_jump_shadow (jp), + jp->length); if (err != 0) { jp->inserted = 1; @@ -526,18 +516,8 @@ delete_raw_breakpoint (struct process_info *proc, struct raw_breakpoint *todel) struct raw_breakpoint *prev_bp_link = *bp_link; *bp_link = bp->next; - - /* Since there can be trap breakpoints inserted in the - same address range, we use `write_inferior_memory', - which takes care of layering breakpoints on top of - fast tracepoints, and on top of the buffer we pass - it. This works because we've already unlinked the - fast tracepoint jump above. Also note that we need - to pass the current shadow contents, because - write_inferior_memory updates any shadow memory with - what we pass here, and we want that to be a nop. */ - ret = write_inferior_memory (bp->pc, bp->old_data, - breakpoint_len); + ret = (*the_target->write_memory) (bp->pc, bp->old_data, + breakpoint_len); if (ret != 0) { /* Something went wrong, relink the breakpoint. */ @@ -747,16 +727,8 @@ uninsert_raw_breakpoint (struct raw_breakpoint *bp) int err; bp->inserted = 0; - /* Since there can be fast tracepoint jumps inserted in the same - address range, we use `write_inferior_memory', which takes - care of layering breakpoints on top of fast tracepoints, and - on top of the buffer we pass it. This works because we've - already unlinked the fast tracepoint jump above. Also note - that we need to pass the current shadow contents, because - write_inferior_memory updates any shadow memory with what we - pass here, and we want that to be a nop. */ - err = write_inferior_memory (bp->pc, bp->old_data, - breakpoint_len); + err = (*the_target->write_memory) (bp->pc, bp->old_data, breakpoint_len); + if (err != 0) { bp->inserted = 1; -- 1.7.0.4 --------------070606000802010806080506--