From patchwork Sat Jun 27 16:21:38 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Wei-cheng, Wang" X-Patchwork-Id: 7410 Received: (qmail 44792 invoked by alias); 27 Jun 2015 16:21:58 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 44766 invoked by uid 89); 27 Jun 2015 16:21:56 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=0.3 required=5.0 tests=AWL, BAYES_00, FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-pa0-f54.google.com Received: from mail-pa0-f54.google.com (HELO mail-pa0-f54.google.com) (209.85.220.54) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Sat, 27 Jun 2015 16:21:54 +0000 Received: by pactm7 with SMTP id tm7so83306900pac.2 for ; Sat, 27 Jun 2015 09:21:52 -0700 (PDT) X-Received: by 10.70.55.165 with SMTP id t5mr2544131pdp.102.1435422112807; Sat, 27 Jun 2015 09:21:52 -0700 (PDT) Received: from localhost.localdomain (114-32-204-230.HINET-IP.hinet.net. [114.32.204.230]) by mx.google.com with ESMTPSA id ju3sm27788766pbc.33.2015.06.27.09.21.51 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Sat, 27 Jun 2015 09:21:52 -0700 (PDT) From: Wei-cheng Wang To: uweigand@de.ibm.com, gdb-patches@sourceware.org Cc: Wei-cheng Wang Subject: [PATCH 1/5 v4] Remove tracepoint_action ops. Date: Sun, 28 Jun 2015 00:21:38 +0800 Message-Id: <1435422102-39438-1-git-send-email-cole945@gmail.com> Hi, Ulrich Weigand wrote: > It would be good to verify this doesn't break Intel by running the > regression test suite there. The testing result on my x86_64 Ubuntu 14.04.2 before after PASS 2625 2625 FAIL 33 33 XFAIL 16 16 KFAIL 2 2 UNTESTED 1 1 UNSUPPORTED 3 3 Ulrich Weigand wrote: > I guess there's not much need to have these "size_in_ipa" variables > any more, subsequent code might as well just use the sizeof directly. Ok, I have replaced all the "size_in_ipa" to just sizeof. Thanks, Wei-cheng --- This patch removes 'ops' in tracepoint, and uses helper functions to call action handler instead. The object layout of tracepoint_action may differ in gdbserver and inferior depend on the alignment rule of target ABI, so gdbserver cannot simply copy the object from its memory to inferior memory. For example, struct collect_memory_action { struct tracepoint_action base; { #ifndef IN_PROCESS_AGENT const struct tracepoint_action_ops *ops; #if - char type; | } | ULONGEST addr; | ULONGEST len; - int32_t basereg; }; and on PowerPC, Wihtout ops with ops 0 1 2 3 0 1 2 3 0 |type| PADDING... 0 |ops-------------| 4 ................. 4 |type|PADDING....| 8 |addr------------ 8 |addr------------- c ----------------| c -----------------| 10 |len------------- 10 |len-------------- 14 ----------------| 14 -----------------| 18 |basereg--------| 18 |basereg---------| so we cannot directly copy the object. In this patch, 'ops' is removed in order to make the objects identical. gdbserver/ChangeLog 2015-06-27 Wei-cheng Wang * tracepoint.c (struct tracepoint_action): Remove ops. (m_tracepoint_action_download, r_tracepoint_action_download, x_tracepoint_action_download, l_tracepoint_action_download): Adjust size and offset accordingly. (m_tracepoint_action_ops, r_tracepoint_action_ops, x_tracepoint_action_ops, l_tracepoint_action_ops): Delete (tracepoint_action_send, tracepoint_action_download): New functions. Helpers for tracetion action handlers. (add_tracepoint_action): Remove setup actions ops. (download_tracepoint_1, tracepoint_send_agent): Call helper functions. --- gdb/gdbserver/tracepoint.c | 103 +++++++++++++++++++++------------------------ 1 file changed, 49 insertions(+), 54 deletions(-) diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c index a934866..57b329d9 100644 --- a/gdb/gdbserver/tracepoint.c +++ b/gdb/gdbserver/tracepoint.c @@ -483,9 +483,6 @@ struct tracepoint_action_ops struct tracepoint_action { -#ifndef IN_PROCESS_AGENT - const struct tracepoint_action_ops *ops; -#endif char type; }; @@ -525,12 +522,10 @@ struct collect_static_trace_data_action static CORE_ADDR m_tracepoint_action_download (const struct tracepoint_action *action) { - int size_in_ipa = (sizeof (struct collect_memory_action) - - offsetof (struct tracepoint_action, type)); - CORE_ADDR ipa_action = target_malloc (size_in_ipa); + CORE_ADDR ipa_action = target_malloc (sizeof (struct collect_memory_action)); - write_inferior_memory (ipa_action, (unsigned char *) &action->type, - size_in_ipa); + write_inferior_memory (ipa_action, (unsigned char *) action, + sizeof (struct collect_memory_action)); return ipa_action; } @@ -547,21 +542,13 @@ m_tracepoint_action_send (char *buffer, const struct tracepoint_action *action) return buffer; } -static const struct tracepoint_action_ops m_tracepoint_action_ops = -{ - m_tracepoint_action_download, - m_tracepoint_action_send, -}; - static CORE_ADDR r_tracepoint_action_download (const struct tracepoint_action *action) { - int size_in_ipa = (sizeof (struct collect_registers_action) - - offsetof (struct tracepoint_action, type)); - CORE_ADDR ipa_action = target_malloc (size_in_ipa); + CORE_ADDR ipa_action = target_malloc (sizeof (struct collect_registers_action)); - write_inferior_memory (ipa_action, (unsigned char *) &action->type, - size_in_ipa); + write_inferior_memory (ipa_action, (unsigned char *) action, + sizeof (struct collect_registers_action)); return ipa_action; } @@ -572,28 +559,19 @@ r_tracepoint_action_send (char *buffer, const struct tracepoint_action *action) return buffer; } -static const struct tracepoint_action_ops r_tracepoint_action_ops = -{ - r_tracepoint_action_download, - r_tracepoint_action_send, -}; - static CORE_ADDR download_agent_expr (struct agent_expr *expr); static CORE_ADDR x_tracepoint_action_download (const struct tracepoint_action *action) { - int size_in_ipa = (sizeof (struct eval_expr_action) - - offsetof (struct tracepoint_action, type)); - CORE_ADDR ipa_action = target_malloc (size_in_ipa); + CORE_ADDR ipa_action = target_malloc (sizeof (struct eval_expr_action)); CORE_ADDR expr; - write_inferior_memory (ipa_action, (unsigned char *) &action->type, - size_in_ipa); - expr = download_agent_expr (((struct eval_expr_action *)action)->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_pointer (ipa_action - + offsetof (struct eval_expr_action, expr) - - offsetof (struct tracepoint_action, type), + + offsetof (struct eval_expr_action, expr), expr); return ipa_action; @@ -631,21 +609,14 @@ x_tracepoint_action_send ( char *buffer, const struct tracepoint_action *action) return agent_expr_send (buffer, eaction->expr); } -static const struct tracepoint_action_ops x_tracepoint_action_ops = -{ - x_tracepoint_action_download, - x_tracepoint_action_send, -}; - static CORE_ADDR l_tracepoint_action_download (const struct tracepoint_action *action) { - int size_in_ipa = (sizeof (struct collect_static_trace_data_action) - - offsetof (struct tracepoint_action, type)); - CORE_ADDR ipa_action = target_malloc (size_in_ipa); + CORE_ADDR ipa_action = + target_malloc (sizeof (struct collect_static_trace_data_action)); - write_inferior_memory (ipa_action, (unsigned char *) &action->type, - size_in_ipa); + write_inferior_memory (ipa_action, (unsigned char *) action, + sizeof (struct collect_static_trace_data_action)); return ipa_action; } @@ -656,11 +627,39 @@ l_tracepoint_action_send (char *buffer, const struct tracepoint_action *action) return buffer; } -static const struct tracepoint_action_ops l_tracepoint_action_ops = +static char * +tracepoint_action_send (char *buffer, const struct tracepoint_action *action) { - l_tracepoint_action_download, - l_tracepoint_action_send, -}; + switch (action->type) + { + case 'M': + return m_tracepoint_action_send (buffer, action); + case 'R': + return r_tracepoint_action_send (buffer, action); + case 'X': + return x_tracepoint_action_send (buffer, action); + case 'L': + return l_tracepoint_action_send (buffer, action); + } + error ("Unknown trace action '%c'.", action->type); +} + +static CORE_ADDR +tracepoint_action_download (const struct tracepoint_action *action) +{ + switch (action->type) + { + case 'M': + return m_tracepoint_action_download (action); + case 'R': + return r_tracepoint_action_download (action); + case 'X': + return x_tracepoint_action_download (action); + case 'L': + return l_tracepoint_action_download (action); + } + error ("Unknown trace action '%c'.", action->type); +} #endif /* This structure describes a piece of the source-level definition of @@ -1944,7 +1943,6 @@ add_tracepoint_action (struct tracepoint *tpoint, char *packet) maction = xmalloc (sizeof *maction); maction->base.type = *act; - maction->base.ops = &m_tracepoint_action_ops; action = &maction->base; ++act; @@ -1970,7 +1968,6 @@ add_tracepoint_action (struct tracepoint *tpoint, char *packet) raction = xmalloc (sizeof *raction); raction->base.type = *act; - raction->base.ops = &r_tracepoint_action_ops; action = &raction->base; trace_debug ("Want to collect registers"); @@ -1986,7 +1983,6 @@ add_tracepoint_action (struct tracepoint *tpoint, char *packet) raction = xmalloc (sizeof *raction); raction->base.type = *act; - raction->base.ops = &l_tracepoint_action_ops; action = &raction->base; trace_debug ("Want to collect static trace data"); @@ -2003,7 +1999,6 @@ add_tracepoint_action (struct tracepoint *tpoint, char *packet) xaction = xmalloc (sizeof (*xaction)); xaction->base.type = *act; - xaction->base.ops = &x_tracepoint_action_ops; action = &xaction->base; trace_debug ("Want to evaluate expression"); @@ -6046,7 +6041,7 @@ download_tracepoint_1 (struct tracepoint *tpoint) for (i = 0; i < tpoint->numactions; i++) { struct tracepoint_action *action = tpoint->actions[i]; - CORE_ADDR ipa_action = action->ops->download (action); + CORE_ADDR ipa_action = tracepoint_action_download (action); if (ipa_action != 0) write_inferior_data_pointer @@ -6096,7 +6091,7 @@ tracepoint_send_agent (struct tracepoint *tpoint) struct tracepoint_action *action = tpoint->actions[i]; p[0] = action->type; - p = action->ops->send (&p[1], action); + p = tracepoint_action_send (&p[1], action); } get_jump_space_head ();