From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23202 invoked by alias); 2 Mar 2012 01:00:55 -0000 Received: (qmail 22756 invoked by uid 22791); 2 Mar 2012 01:00:47 -0000 X-SWARE-Spam-Status: No, hits=-1.2 required=5.0 tests=AWL,BAYES_00,FROM_12LTRDOM,TW_CP,TW_EG 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, 02 Mar 2012 01:00:29 +0000 Received: from svr-orw-fem-01.mgc.mentorg.com ([147.34.98.93]) by relay1.mentorg.com with esmtp id 1S3GrF-0001rD-1S from Yao_Qi@mentor.com for gdb-patches@sourceware.org; Thu, 01 Mar 2012 17:00:29 -0800 Received: from SVR-ORW-FEM-05.mgc.mentorg.com ([147.34.97.43]) by svr-orw-fem-01.mgc.mentorg.com over TLS secured channel with Microsoft SMTPSVC(6.0.3790.4675); Thu, 1 Mar 2012 17:00:28 -0800 Received: from localhost.localdomain (147.34.91.1) by svr-orw-fem-05.mgc.mentorg.com (147.34.97.43) with Microsoft SMTP Server id 14.1.289.1; Thu, 1 Mar 2012 17:00:26 -0800 From: Yao Qi To: Subject: [PATCH 2/2] OO tracepoint_action Date: Fri, 02 Mar 2012 01:00:00 -0000 Message-ID: <1330650011-31899-3-git-send-email-yao@codesourcery.com> In-Reply-To: <1330650011-31899-1-git-send-email-yao@codesourcery.com> References: <1330650011-31899-1-git-send-email-yao@codesourcery.com> MIME-Version: 1.0 Content-Type: text/plain 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: 2012-03/txt/msg00065.txt.bz2 This is the major part of this patch series, which replace several switch/case blocks in code with function vectors. It is easier to add new operation to tracepoint_action in the future. gdb/gdbserver: * tracepoint.c (struct tracepoint_action_ops): New. (struct tracepoint_action) : New field. (record_tracepoint_error): Removed. (record_expr_eval_error): New. (add_tracepoint_action): Move code to ... (m_tracepoint_action_init, r_tracepoint_action_init): (x_tracepoint_action_init, l_tracepoint_action_init: ... here. New. (download_tracepoint_1): Move code to ... (m_tracepoint_action_download, r_tracepoint_action_download): (x_tracepoint_action_download, l_tracepoint_action_download): .. .here. New. (do_action_at_tracepoint): Move code to ... (m_tracepoint_action_execute, r_tracepoint_action_execute): (x_tracepoint_action_execute, l_tracepoint_action_execute): ... here. New. (condition_true_at_tracepoint): Call record_expr_eval_error instead. --- gdb/gdbserver/tracepoint.c | 560 ++++++++++++++++++++++++++++---------------- 1 files changed, 358 insertions(+), 202 deletions(-) diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c index 5dcc7a4..bc23825 100644 --- a/gdb/gdbserver/tracepoint.c +++ b/gdb/gdbserver/tracepoint.c @@ -469,12 +469,38 @@ write_inferior_uinteger (CORE_ADDR symaddr, unsigned int val) return write_inferior_memory (symaddr, (unsigned char *) &val, sizeof (val)); } +static CORE_ADDR target_malloc (ULONGEST size); +static int write_inferior_data_ptr (CORE_ADDR where, CORE_ADDR ptr); #endif +/* Operations on various types of tracepoint actions. */ + +struct tracepoint_action; +struct tracepoint_hit_ctx; + +struct tracepoint_action_ops +{ +#ifndef IN_PROCESS_AGENT + /* Parse rsp packet PACKET and initialize ACTION. Return the updated + pointer to rsp packet.. */ + char* (*init) (char *packet, struct tracepoint_action* action); + + /* Download tracepoint action ACTION to IPA. Return the address of action + in IPA/inferior. */ + CORE_ADDR (*download) (struct tracepoint_action *action); +#endif + + /* Do tracepoint action ACTION. Return 0 if success otherwise return + non-zero. */ + int (*execute) (struct tracepoint_action *action, + struct tracepoint_hit_ctx *ctx, struct traceframe *tframe); +}; + /* Base action. Concrete actions inherit this. */ struct tracepoint_action { + struct tracepoint_action_ops *ops; char type; }; @@ -1247,12 +1273,8 @@ static LONGEST get_timestamp (void); /* Record that an error occurred during expression evaluation. */ static void -record_tracepoint_error (struct tracepoint *tpoint, const char *which, - enum eval_result_type rtype) +record_expr_eval_error (enum eval_result_type rtype) { - trace_debug ("Tracepoint %d at %s %s eval reports error %d", - tpoint->number, paddress (tpoint->address), which, rtype); - #ifdef IN_PROCESS_AGENT /* Only record the first error we get. */ if (cmpxchg (&expr_eval_result, @@ -1263,8 +1285,6 @@ record_tracepoint_error (struct tracepoint *tpoint, const char *which, if (expr_eval_result != expr_eval_no_error) return; #endif - - error_tracepoint = tpoint; } /* Trace buffer management. */ @@ -1603,6 +1623,298 @@ trace_buffer_alloc (size_t amt) } #ifndef IN_PROCESS_AGENT +static char* +m_tracepoint_action_init (char *packet, struct tracepoint_action* action) +{ + ULONGEST basereg; + int is_neg; + struct collect_memory_action *maction + = (struct collect_memory_action *) action; + + maction->base.type = *packet; + ++packet; + + is_neg = (*packet == '-'); + if (*packet == '-') + ++packet; + packet = unpack_varlen_hex (packet, &basereg); + ++packet; + packet = unpack_varlen_hex (packet, &maction->addr); + ++packet; + packet = unpack_varlen_hex (packet, &maction->len); + maction->basereg = (is_neg ? - (int) basereg : (int) basereg); + trace_debug ("Want to collect %s bytes at 0x%s (basereg %d)", + pulongest (maction->len), + paddress (maction->addr), maction->basereg); + + return packet; +} + +static CORE_ADDR +m_tracepoint_action_download (struct tracepoint_action *action) +{ + CORE_ADDR ipa_action = target_malloc (sizeof (struct collect_memory_action)); + + write_inferior_memory (ipa_action, (unsigned char *) action, + sizeof (struct collect_memory_action)); + /* Write NULL to field `ops'. */ + write_inferior_data_ptr(ipa_action + offsetof (struct tracepoint_action, ops), + 0); + + return ipa_action; +} +#endif /* !IN_PROCESS_AGENT */ + +static int +m_tracepoint_action_execute (struct tracepoint_action *action, + struct tracepoint_hit_ctx *ctx, + struct traceframe *tframe) +{ + struct collect_memory_action *maction; + + maction = (struct collect_memory_action *) action; + + trace_debug ("Want to collect %s bytes at 0x%s (basereg %d)", + pulongest (maction->len), paddress (maction->addr), + maction->basereg); + /* (should use basereg) */ + agent_mem_read (tframe, NULL, + (CORE_ADDR) maction->addr, maction->len); + + return 0; +} + +static struct tracepoint_action_ops m_tracepoint_action_ops = +{ +#ifndef IN_PROCESS_AGENT + m_tracepoint_action_init, + m_tracepoint_action_download, +#endif + m_tracepoint_action_execute, +}; + +#ifndef IN_PROCESS_AGENT +static char* +r_tracepoint_action_init (char * packet, struct tracepoint_action* action) +{ + action->type = *packet; + ++packet; + + trace_debug ("Want to collect registers"); + + /* skip past hex digits of mask for now */ + while (isxdigit(*packet)) + ++packet; + + return packet; +} + +static CORE_ADDR +r_tracepoint_action_download (struct tracepoint_action *action) +{ + CORE_ADDR ipa_action + = target_malloc (sizeof (struct collect_registers_action)); + + write_inferior_memory (ipa_action, (unsigned char *) action, + sizeof (struct collect_registers_action)); + /* Write NULL to field `ops'. */ + write_inferior_data_ptr(ipa_action + offsetof (struct tracepoint_action, ops), + 0); + + return ipa_action; +} +#endif /* !IN_PROCESS_AGENT */ + +static unsigned char* add_traceframe_block (struct traceframe *tframe, int amt); +static struct regcache* get_context_regcache (struct tracepoint_hit_ctx *ctx); + +static int +r_tracepoint_action_execute (struct tracepoint_action *action, + struct tracepoint_hit_ctx *ctx, + struct traceframe *tframe) +{ + unsigned char *regspace; + struct regcache tregcache; + struct regcache *context_regcache; + + trace_debug ("Want to collect registers"); + + /* Collect all registers for now. */ + regspace = add_traceframe_block (tframe, 1 + register_cache_size ()); + if (regspace == NULL) + { + trace_debug ("Trace buffer block allocation failed, skipping"); + return 1; + } + /* Identify a register block. */ + *regspace = 'R'; + + context_regcache = get_context_regcache (ctx); + + /* Wrap the regblock in a register cache (in the stack, we don't want + to malloc here). */ + init_register_cache (&tregcache, regspace + 1); + + /* Copy the register data to the regblock. */ + regcache_cpy (&tregcache, context_regcache); + +#ifndef IN_PROCESS_AGENT + /* On some platforms, trap-based tracepoints will have the PC pointing + to the next instruction after the trap, but we don't want the user + or GDB trying to guess whether the saved PC needs adjusting; so + always record the adjusted stop_pc. Note that we can't use + tpoint->address instead, since it will be wrong for while-stepping + actions. This adjustment is a nop for fast tracepoints collected from + the in-process lib (but not if GDBserver is collecting one preemptively), + since the PC had already been adjusted to contain the tracepoint's + address by the jump pad. */ + trace_debug ("Storing stop pc (0x%s) in regblock", + paddress (ctx->stop_pc)); + + /* This changes the regblock, not the thread's regcache. */ + regcache_write_pc (&tregcache, ctx->stop_pc); +#endif + + return 0; +} + +static struct tracepoint_action_ops r_tracepoint_action_ops = +{ +#ifndef IN_PROCESS_AGENT + r_tracepoint_action_init, + r_tracepoint_action_download, +#endif + r_tracepoint_action_execute, +}; + +#ifndef IN_PROCESS_AGENT +static char* +x_tracepoint_action_init (char *packet, struct tracepoint_action* action) +{ + action->type = *packet; + + trace_debug ("Want to evaluate expression"); + ((struct eval_expr_action *) action)->expr = gdb_parse_agent_expr (&packet); + + return packet; +} + +static CORE_ADDR download_agent_expr (struct agent_expr *expr); + +static CORE_ADDR +x_tracepoint_action_download (struct tracepoint_action *action) +{ + CORE_ADDR ipa_action = target_malloc (sizeof (struct eval_expr_action)); + CORE_ADDR expr; + + write_inferior_memory (ipa_action, (unsigned char *) action, + sizeof (struct eval_expr_action)); + expr = download_agent_expr (((struct eval_expr_action *)action)->expr); + write_inferior_data_ptr(ipa_action + offsetof (struct eval_expr_action, expr), + expr); + /* Write NULL to field `ops'. */ + write_inferior_data_ptr(ipa_action + offsetof (struct tracepoint_action, ops), + 0); + + return ipa_action; +} +#endif /* !IN_PROCESS_AGENT */ + +static void record_expr_eval_error (enum eval_result_type rtype); + +static int +x_tracepoint_action_execute (struct tracepoint_action *action, + struct tracepoint_hit_ctx *ctx, + struct traceframe *tframe) +{ + struct eval_expr_action *eaction = (struct eval_expr_action *) action; + enum eval_result_type err; + + trace_debug ("Want to evaluate expression"); + + err = eval_tracepoint_agent_expr (ctx, tframe, eaction->expr, NULL); + + if (err != expr_eval_no_error) + { + trace_debug ("Tracepoint at %s action expression eval reports error %d", + paddress (ctx->stop_pc), err); + + record_expr_eval_error (err); + + return 1; + } + + return 0; +} + +static struct tracepoint_action_ops x_tracepoint_action_ops = +{ +#ifndef IN_PROCESS_AGENT + x_tracepoint_action_init, + x_tracepoint_action_download, +#endif + x_tracepoint_action_execute, +}; + +#ifndef IN_PROCESS_AGENT +static char* +l_tracepoint_action_init (char *packet, struct tracepoint_action* action) +{ + action->type = *packet; + packet++; + + trace_debug ("Want to collect static trace data"); + + return packet; +} + +static CORE_ADDR +l_tracepoint_action_download (struct tracepoint_action *action) +{ + CORE_ADDR ipa_action + = target_malloc (sizeof (struct collect_static_trace_data_action)); + + write_inferior_memory (ipa_action, (unsigned char *) action, + sizeof (struct collect_static_trace_data_action)); + /* Write NULL to field `ops'. */ + write_inferior_data_ptr(ipa_action + offsetof (struct tracepoint_action, ops), + 0); + + return ipa_action; +} +#endif /* !IN_PROCESS_AGENT */ + +#if defined IN_PROCESS_AGENT && defined HAVE_UST +static void collect_ust_data_at_tracepoint (struct tracepoint_hit_ctx *ctx, + struct traceframe *tframe); +#endif + +static int +l_tracepoint_action_execute (struct tracepoint_action *action, + struct tracepoint_hit_ctx *ctx, + struct traceframe *tframe) +{ +#if defined IN_PROCESS_AGENT && defined HAVE_UST + trace_debug ("Want to collect static trace data"); + collect_ust_data_at_tracepoint (ctx, tframe); + return 0; +#else + trace_debug ("warning: collecting static trace data, " + "but static tracepoints are not supported"); + return 1; +#endif +} + +static struct tracepoint_action_ops l_tracepoint_action_ops = +{ +#ifndef IN_PROCESS_AGENT + l_tracepoint_action_init, + l_tracepoint_action_download, +#endif + l_tracepoint_action_execute, +}; + +#ifndef IN_PROCESS_AGENT /* Return the total free space. This is not necessarily the largest block we can allocate, because of the two-part case. */ @@ -1755,56 +2067,23 @@ add_tracepoint_action (struct tracepoint *tpoint, char *packet) { case 'M': { - struct collect_memory_action *maction; - ULONGEST basereg; - int is_neg; - - maction = xmalloc (sizeof *maction); - maction->base.type = *act; - action = &maction->base; - - ++act; - is_neg = (*act == '-'); - if (*act == '-') - ++act; - act = unpack_varlen_hex (act, &basereg); - ++act; - act = unpack_varlen_hex (act, &maction->addr); - ++act; - act = unpack_varlen_hex (act, &maction->len); - maction->basereg = (is_neg - ? - (int) basereg - : (int) basereg); - trace_debug ("Want to collect %s bytes at 0x%s (basereg %d)", - pulongest (maction->len), - paddress (maction->addr), maction->basereg); + action = xmalloc (sizeof (struct collect_memory_action)); + action->ops = &m_tracepoint_action_ops; + break; } case 'R': { - struct collect_registers_action *raction; - - raction = xmalloc (sizeof *raction); - raction->base.type = *act; - action = &raction->base; + action = xmalloc (sizeof (struct collect_registers_action)); + action->ops = &r_tracepoint_action_ops; - trace_debug ("Want to collect registers"); - ++act; - /* skip past hex digits of mask for now */ - while (isxdigit(*act)) - ++act; break; } case 'L': { - struct collect_static_trace_data_action *raction; + action = xmalloc (sizeof (struct collect_static_trace_data_action)); + action->ops = &l_tracepoint_action_ops; - raction = xmalloc (sizeof *raction); - raction->base.type = *act; - action = &raction->base; - - trace_debug ("Want to collect static trace data"); - ++act; break; } case 'S': @@ -1813,14 +2092,8 @@ add_tracepoint_action (struct tracepoint *tpoint, char *packet) break; case 'X': { - struct eval_expr_action *xaction; - - xaction = xmalloc (sizeof (*xaction)); - xaction->base.type = *act; - action = &xaction->base; - - trace_debug ("Want to evaluate expression"); - xaction->expr = gdb_parse_agent_expr (&act); + action = xmalloc (sizeof (struct eval_expr_action)); + action->ops = &x_tracepoint_action_ops; break; } default: @@ -1833,6 +2106,8 @@ add_tracepoint_action (struct tracepoint *tpoint, char *packet) if (action == NULL) break; + act = action->ops->init (act, action); + if (seen_step_action_flag) { tpoint->num_step_actions++; @@ -4263,8 +4538,6 @@ tracepoint_was_hit (struct thread_info *tinfo, CORE_ADDR stop_pc) #if defined IN_PROCESS_AGENT && defined HAVE_UST struct ust_marker_data; -static void collect_ust_data_at_tracepoint (struct tracepoint_hit_ctx *ctx, - struct traceframe *tframe); #endif /* Create a trace frame for the hit of the given tracepoint in the @@ -4412,105 +4685,31 @@ do_action_at_tracepoint (struct tracepoint_hit_ctx *ctx, struct traceframe *tframe, struct tracepoint_action *taction) { - enum eval_result_type err; - - switch (taction->type) +#ifdef IN_PROCESS_AGENT + if (taction->ops == NULL) { - case 'M': - { - struct collect_memory_action *maction; - - maction = (struct collect_memory_action *) taction; - - trace_debug ("Want to collect %s bytes at 0x%s (basereg %d)", - pulongest (maction->len), - paddress (maction->addr), maction->basereg); - /* (should use basereg) */ - agent_mem_read (tframe, NULL, - (CORE_ADDR) maction->addr, maction->len); - break; - } - case 'R': - { - unsigned char *regspace; - struct regcache tregcache; - struct regcache *context_regcache; - - - trace_debug ("Want to collect registers"); - - /* Collect all registers for now. */ - regspace = add_traceframe_block (tframe, - 1 + register_cache_size ()); - if (regspace == NULL) - { - trace_debug ("Trace buffer block allocation failed, skipping"); - break; - } - /* Identify a register block. */ - *regspace = 'R'; - - context_regcache = get_context_regcache (ctx); - - /* Wrap the regblock in a register cache (in the stack, we - don't want to malloc here). */ - init_register_cache (&tregcache, regspace + 1); - - /* Copy the register data to the regblock. */ - regcache_cpy (&tregcache, context_regcache); - -#ifndef IN_PROCESS_AGENT - /* On some platforms, trap-based tracepoints will have the PC - pointing to the next instruction after the trap, but we - don't want the user or GDB trying to guess whether the - saved PC needs adjusting; so always record the adjusted - stop_pc. Note that we can't use tpoint->address instead, - since it will be wrong for while-stepping actions. This - adjustment is a nop for fast tracepoints collected from the - in-process lib (but not if GDBserver is collecting one - preemptively), since the PC had already been adjusted to - contain the tracepoint's address by the jump pad. */ - trace_debug ("Storing stop pc (0x%s) in regblock", - paddress (ctx->stop_pc)); - - /* This changes the regblock, not the thread's - regcache. */ - regcache_write_pc (&tregcache, ctx->stop_pc); + switch (taction->type) + { + case 'M': + taction->ops = &m_tracepoint_action_ops; + break; + case 'X': + taction->ops = &x_tracepoint_action_ops; + break; + case 'R': + taction->ops = &r_tracepoint_action_ops; + break; + case 'L': + taction->ops = &l_tracepoint_action_ops; + break; + default: + return; + }; + } #endif - } - break; - case 'X': - { - struct eval_expr_action *eaction; - - eaction = (struct eval_expr_action *) taction; - trace_debug ("Want to evaluate expression"); - - err = eval_tracepoint_agent_expr (ctx, tframe, eaction->expr, NULL); - - if (err != expr_eval_no_error) - { - record_tracepoint_error (tpoint, "action expression", err); - return; - } - } - break; - case 'L': - { -#if defined IN_PROCESS_AGENT && defined HAVE_UST - trace_debug ("Want to collect static trace data"); - collect_ust_data_at_tracepoint (ctx, tframe); -#else - trace_debug ("warning: collecting static trace data, " - "but static tracepoints are not supported"); -#endif - } - break; - default: - trace_debug ("unknown trace action '%c', ignoring", taction->type); - break; - } + if (taction->ops->execute (taction, ctx, tframe)) + error_tracepoint = tpoint; } static int @@ -4543,7 +4742,11 @@ condition_true_at_tracepoint (struct tracepoint_hit_ctx *ctx, if (err != expr_eval_no_error) { - record_tracepoint_error (tpoint, "condition", err); + trace_debug ("Tracepoint %d at %s condition eval reports error %d", + tpoint->number, paddress (tpoint->address), err); + + record_expr_eval_error (err); + error_tracepoint = tpoint; /* The error case must return false. */ return 0; } @@ -5666,55 +5869,8 @@ download_tracepoint_1 (struct tracepoint *tpoint) /* Now for each pointer, download the action. */ for (i = 0; i < tpoint->numactions; i++) { - CORE_ADDR ipa_action = 0; struct tracepoint_action *action = tpoint->actions[i]; - - switch (action->type) - { - case 'M': - ipa_action - = target_malloc (sizeof (struct collect_memory_action)); - write_inferior_memory (ipa_action, - (unsigned char *) action, - sizeof (struct collect_memory_action)); - break; - case 'R': - ipa_action - = target_malloc (sizeof (struct collect_registers_action)); - write_inferior_memory (ipa_action, - (unsigned char *) action, - sizeof (struct collect_registers_action)); - break; - case 'X': - { - CORE_ADDR expr; - struct eval_expr_action *eaction - = (struct eval_expr_action *) action; - - ipa_action = target_malloc (sizeof (*eaction)); - write_inferior_memory (ipa_action, - (unsigned char *) eaction, - sizeof (*eaction)); - - expr = download_agent_expr (eaction->expr); - write_inferior_data_ptr - (ipa_action + offsetof (struct eval_expr_action, expr), - expr); - break; - } - case 'L': - ipa_action = target_malloc - (sizeof (struct collect_static_trace_data_action)); - write_inferior_memory - (ipa_action, - (unsigned char *) action, - sizeof (struct collect_static_trace_data_action)); - break; - default: - trace_debug ("unknown trace action '%c', ignoring", - action->type); - break; - } + CORE_ADDR ipa_action = action->ops->download (action); if (ipa_action != 0) write_inferior_data_ptr -- 1.7.0.4