From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22934 invoked by alias); 4 Nov 2011 16:18:32 -0000 Received: (qmail 22918 invoked by uid 22791); 4 Nov 2011 16:18:28 -0000 X-SWARE-Spam-Status: No, hits=-1.5 required=5.0 tests=AWL,BAYES_00,FROM_12LTRDOM,TW_BP 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; Fri, 04 Nov 2011 16:18:08 +0000 Received: from nat-jpt.mentorg.com ([192.94.33.2] helo=PR1-MAIL.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1RMMT1-0002gZ-Vz from Yao_Qi@mentor.com for gdb-patches@sourceware.org; Fri, 04 Nov 2011 09:18:08 -0700 Received: from [127.0.0.1] ([172.16.63.104]) by PR1-MAIL.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.1830); Sat, 5 Nov 2011 01:18:05 +0900 Message-ID: <4EB41054.3070706@codesourcery.com> Date: Fri, 04 Nov 2011 16:18:00 -0000 From: Yao Qi User-Agent: Mozilla/5.0 (X11; Linux i686; rv:7.0.1) Gecko/20110930 Thunderbird/7.0.1 MIME-Version: 1.0 To: gdb-patches@sourceware.org Subject: [patch/gdbserver] Fix a bug when setting two fast tracepoints at the same place Content-Type: multipart/mixed; boundary="------------090901050100020007010906" 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-11/txt/msg00114.txt.bz2 This is a multi-part message in MIME format. --------------090901050100020007010906 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Content-length: 1622 Some days ago, I sent a test case (trace-break.exp) and a fix about setting breakpoint and fast tracepoint at the same the same address, [patch, gdbserver] Uninsert bpkt when regular and fast tracepoint are set at the same address http://sourceware.org/ml/gdb-patches/2011-10/msg00714.html I read trace-break.exp again, and find we didn't test setting two fast tracepoints at the same address. This test can be done easily by adding one line in trace-brea.exp (see below in my patch), however, it reveals one bug about tracepoint. FAIL: gdb.trace/trace-break.exp: 2 ftrace ftrace on: tfind frame 1 FAIL: gdb.trace/trace-break.exp: 2 ftrace ftrace off: tfind frame 1 The problem is that when setting two fast tracepoints at the same place, only one works. When setting fast tracepoint at an address, a jump pad is created to connect a jmp insn written at tracepoint place and collection routine. Even multiple fast tracepoints are set at the same address, there is only jump pad associated with this address. Inferior will jump to gdb_collect from jump pad with the first tracepoint information, rather than all tracepoints information at that address. This is cause of this problem. Fortunately, tracepoints list are sorted, and jump pad is associated with the "first" tracepoint of them. So tracepoint passed to gdb_collect is the "first" tracepoint of them which have the same address, then we can loop over tracepoints started from "first" tracepoint, and call collect_data_at_tracepoint in the loop. This is how this written. Tested on x86_64-linux native-gdbserver, OK to apply? -- Yao (齐尧) --------------090901050100020007010906 Content-Type: text/x-patch; name="0001-two-ftrace-point-at-the-same-addr.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0001-two-ftrace-point-at-the-same-addr.patch" Content-length: 4545 gdb/gdbserver: 2011-11-03 Yao Qi * tracepoint.c (gdb_collect): Loop over TPOINT. gdb/testsuite: 2011-11-03 Yao Qi * gdb.trace/trace-break.exp: Add test on setting two fast tracepoints at the same address. --- gdb/gdbserver/tracepoint.c | 83 ++++++++++++++++++------------- gdb/testsuite/gdb.trace/trace-break.exp | 1 + 2 files changed, 50 insertions(+), 34 deletions(-) diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c index 0a64104..f09a7d8 100644 --- a/gdb/gdbserver/tracepoint.c +++ b/gdb/gdbserver/tracepoint.c @@ -5417,47 +5417,62 @@ gdb_collect (struct tracepoint *tpoint, unsigned char *regs) if (!tracing) return; - if (!tpoint->enabled) - return; - ctx.base.type = fast_tracepoint; ctx.regs = regs; ctx.regcache_initted = 0; - ctx.tpoint = tpoint; - /* Wrap the regblock in a register cache (in the stack, we don't - want to malloc here). */ - ctx.regspace = alloca (register_cache_size ()); - if (ctx.regspace == NULL) + ctx.tpoint = tpoint; + for (; ctx.tpoint && ctx.tpoint->address == tpoint->address; + ctx.tpoint = ctx.tpoint->next) { - trace_debug ("Trace buffer block allocation failed, skipping"); - return; - } + if (!ctx.tpoint->enabled) + continue; - /* Test the condition if present, and collect if true. */ - if (tpoint->cond == NULL - || condition_true_at_tracepoint ((struct tracepoint_hit_ctx *) &ctx, - tpoint)) - { - collect_data_at_tracepoint ((struct tracepoint_hit_ctx *) &ctx, - tpoint->address, tpoint); + /* Multiple tracepoints of different types, such as fast tracepoint and + static tracepoint, can be set at the same address. */ + if (ctx.tpoint->type != tpoint->type) + continue; - /* Note that this will cause original insns to be written back - to where we jumped from, but that's OK because we're jumping - back to the next whole instruction. This will go badly if - instruction restoration is not atomic though. */ - if (stopping_tracepoint - || trace_buffer_is_full - || expr_eval_result != expr_eval_no_error) - stop_tracing (); - } - else - { - /* If there was a condition and it evaluated to false, the only - way we would stop tracing is if there was an error during - condition expression evaluation. */ - if (expr_eval_result != expr_eval_no_error) - stop_tracing (); + /* Wrap the regblock in a register cache (in the stack, we don't + want to malloc here). */ + ctx.regspace = alloca (register_cache_size ()); + if (ctx.regspace == NULL) + { + trace_debug ("Trace buffer block allocation failed, skipping"); + continue; + } + + /* Test the condition if present, and collect if true. */ + if (ctx.tpoint->cond == NULL + || condition_true_at_tracepoint ((struct tracepoint_hit_ctx *) &ctx, + ctx.tpoint)) + { + collect_data_at_tracepoint ((struct tracepoint_hit_ctx *) &ctx, + ctx.tpoint->address, ctx.tpoint); + + /* Note that this will cause original insns to be written back + to where we jumped from, but that's OK because we're jumping + back to the next whole instruction. This will go badly if + instruction restoration is not atomic though. */ + if (stopping_tracepoint + || trace_buffer_is_full + || expr_eval_result != expr_eval_no_error) + { + stop_tracing (); + break; + } + } + else + { + /* If there was a condition and it evaluated to false, the only + way we would stop tracing is if there was an error during + condition expression evaluation. */ + if (expr_eval_result != expr_eval_no_error) + { + stop_tracing (); + break; + } + } } } diff --git a/gdb/testsuite/gdb.trace/trace-break.exp b/gdb/testsuite/gdb.trace/trace-break.exp index 4a14d32..c2d7b2c 100644 --- a/gdb/testsuite/gdb.trace/trace-break.exp +++ b/gdb/testsuite/gdb.trace/trace-break.exp @@ -234,6 +234,7 @@ if { [gdb_test "info sharedlibrary" ".*libinproctrace\.so.*" "IPA loaded"] != 0 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_2 "ftrace" "ftrace" ${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 --------------090901050100020007010906--