From: Hui Zhu <teawater@gmail.com>
To: Tom Tromey <tromey@redhat.com>,
gdb-patches ml <gdb-patches@sourceware.org>,
Stan Shebs <stan@codesourcery.com>
Subject: Re: [PATCH] tracepoint: add new trace command "printf"[0] gdb
Date: Mon, 21 Feb 2011 08:18:00 -0000 [thread overview]
Message-ID: <AANLkTi=6V7Ao9Oc=vFfrnreYjzdXBsF1XsgPByMaZfmL@mail.gmail.com> (raw)
In-Reply-To: <AANLkTin=-CisfrJu2kmhmfvfJEntgG-EZLsoWjs2DX1h@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 4786 bytes --]
On Thu, Feb 17, 2011 at 16:15, Hui Zhu <teawater@gmail.com> wrote:
> On Sat, Feb 12, 2011 at 02:45, Tom Tromey <tromey@redhat.com> wrote:
>>>>>>> ">" == Hui Zhu <teawater@gmail.com> writes:
>>
>>>> + gen_expr (expr, &pc, aexpr, &value);
>>>> +
>>>> +
>>>> + if (value.optimized_out)
>>
>> There's no need to have 2 blank lines here.
>>
>> This function and some other new ones have no documentation.
>>
>>>> + {"printf", 0, 0, 0, 0}, /* 0x31 */
>> [...]
>>>> + aop_printf = 0x31,
>>
>> If you add a new AX expression, you must update agentexpr.texi.
>>
>> I think any AX addition should also require a corresponding addition to
>> gdbserver.
>>
>>>> +typedef void (printf_callback) (char *fbuf, char **expp,
>>>> + struct bp_location *loc,
>>>> + struct agent_expr *aexpr);
>>
>> From what I can see, the `loc' and `aexpr' arguments are passed through
>> string_printf without interpretation.
>>
>> In a case like this it is customary to just add a single `void *data'
>> argument and have the caller and callback agree on the type. Here, that
>> type would be an instance of a struct wrapping the two values.
>>
>> This definition should not be here.
>>
>>>> static void
>>>> +ui_printf (char *arg, struct ui_file *stream)
>>>> +{
>>>> + string_printf (arg, stream, NULL, NULL, NULL);
>>>> +}
>>
>> There's no reason to keep ui_printf around, just inline this into the 2
>> callers.
>>
>>>> +extern void printf_command (char *arg, int from_tty);
>>>> +typedef void (printf_callback) (char **expp, struct bp_location *loc,
>>>> + struct agent_expr *aexpr);
>>>> +extern void string_printf (char *arg, struct ui_file *stream,
>>>> + printf_callback callback, struct bp_location *loc,
>>>> + struct agent_expr *aexpr);
>>
>> These should be in some appropriate header, not tracepoint.c.
>>
>>>> + /* The agent expr include expr for arguments, format string, 1 byte
>>>> + * for aop_printf, 1 byte for the number of arguments, 1 byte for
>>>> + * size of format string, 1 byte for blank after format string
>>>> + * and 1 byte for aop_end. */
>>
>> Wrong comment format, no leading "*"s.
>>
>>
>> This new feature needs a documentation patch and probably also a patch
>> to NEWS.
>>
>> From what I can tell from the patch, the idea here is to add a printf to
>> a tracepoint's actions, with the printf evaluated on the remote side. I
>> guess that is an ok idea, though I don't use this stuff enough to really
>> have an opinion.
>>
>> I think it would be good for other maintainers to speak up now. If it
>> is left to me, I will allow this if you fix up the problems and write
>> the documentation.
>>
>> Tom
>>
>
> Thanks for your help Tom.
>
> I make a new patch according to your comments. And I have send the
> patch for doc in prev mail.
>
> Best,
> Hui
>
> 2011-02-17 Hui Zhu <teawater@gmail.com>
>
> * Makefile.in (HFILES_NO_SRCDIR): Add printcmd.h.
> * ax-gdb.c (gen_printf_expr_callback): New function.
> * ax-general.c (ax_memcpy): New function.
> (aop_map): Add new entry for "printf".
> (ax_print): Handle "printf".
> (ax_reqs): Ditto.
> * ax.h (agent_op): Add aop_printf.
> (ax_memcpy): Forward declare.
> * printcmd.c (printf_callback): New typedef.
> (string_printf): New function from ui_printf.
> (ui_printf): Call string_printf.
> (printf_command): Remove static.
> * printcmd.h: New file.
> * tracepoint.c (printf_command, gen_printf_expr_callback,
> printf_callback, string_printf): Forward declares.
> (validate_actionline, encode_actions_1): handle printf_command.
>
Hi guys,
Patch in attachment checked in.
Thanks,
Hui
2011-02-17 Hui Zhu <teawater@gmail.com>
* Makefile.in (HFILES_NO_SRCDIR): Add printcmd.h.
* ax-gdb.c (gen_printf_expr_callback): New function.
* ax-gdb.h (gen_printf_expr_callback): Forward declare.
* ax-general.c (ax_memcpy): New function.
(aop_map): Add new entry for "printf".
(ax_print): Handle "printf".
(ax_reqs): Ditto.
* ax.h (ax_memcpy): Forward declare.
* common/ax.def (invalid2): Removed.
(printf): New entry.
* printcmd.c (printcmd.h): New include.
(string_printf): New function.
(ui_printf): Removed.
(printf_command): Remove static. Call string_printf.
(eval_command): Call string_printf.
* printcmd.h: New file.
* tracepoint.c (printf_command, gen_printf_expr_callback,
printf_callback, string_printf): Forward declares.
(validate_actionline, encode_actions_1): handle printf_command.
[-- Attachment #2: tp_print.txt --]
[-- Type: text/plain, Size: 12813 bytes --]
---
Makefile.in | 2 -
ax-gdb.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
ax-gdb.h | 2 +
ax-general.c | 53 +++++++++++++++++++++++++++++++++++++++++++++-------
ax.h | 2 +
common/ax.def | 6 -----
printcmd.c | 38 ++++++++++++++++++++++++++++---------
printcmd.h | 30 +++++++++++++++++++++++++++++
tracepoint.c | 39 ++++++++++++++++++++++++++++++++++++++
9 files changed, 209 insertions(+), 22 deletions(-)
--- a/Makefile.in
+++ b/Makefile.in
@@ -813,7 +813,7 @@ annotate.h sim-regno.h dictionary.h dfp.
remote-fileio.h i386-linux-tdep.h vax-tdep.h objc-lang.h \
sentinel-frame.h bcache.h symfile.h windows-tdep.h linux-tdep.h \
gdb_usleep.h jit.h xml-syscall.h microblaze-tdep.h \
-psymtab.h psympriv.h progspace.h bfin-tdep.h ia64-hpux-tdep.h
+psymtab.h psympriv.h progspace.h bfin-tdep.h ia64-hpux-tdep.h printcmd.h
# Header files that already have srcdir in them, or which are in objdir.
--- a/ax-gdb.c
+++ b/ax-gdb.c
@@ -2445,6 +2445,65 @@ gen_eval_for_expr (CORE_ADDR scope, stru
return ax;
}
+void
+gen_printf_expr_callback (char *fbuf, char **expp, void *loc_v, void *aexpr_v)
+{
+ struct bp_location *loc = loc_v;
+ struct agent_expr *aexpr = aexpr_v;
+
+ if (expp)
+ {
+ struct cleanup *old_chain = NULL;
+ struct expression *expr = NULL;
+ union exp_element *pc;
+ struct axs_value value;
+
+ expr = parse_exp_1 (expp, block_for_pc (loc->address), 1);
+ old_chain = make_cleanup (free_current_contents, &expr);
+
+ pc = expr->elts;
+ trace_kludge = 0;
+ value.optimized_out = 0;
+ gen_expr (expr, &pc, aexpr, &value);
+
+ if (value.optimized_out)
+ error (_("value has been optimized out"));
+ switch (value.kind)
+ {
+ case axs_lvalue_memory:
+ if (TYPE_CODE (value.type) != TYPE_CODE_ARRAY)
+ {
+ int length = TYPE_LENGTH (check_typedef (value.type));
+ switch (length)
+ {
+ case 4:
+ ax_simple (aexpr, aop_ref32);
+ break;
+ case 8:
+ ax_simple (aexpr, aop_ref64);
+ break;
+ default:
+ error (_("Size of value is not OK."));
+ break;
+ }
+ }
+ break;
+ case axs_lvalue_register:
+ ax_reg (aexpr, value.u.reg);
+ break;
+ }
+
+ do_cleanups (old_chain);
+ }
+
+ ax_simple (aexpr, aop_printf);
+ if (expp)
+ ax_simple (aexpr, 1);
+ else
+ ax_simple (aexpr, 0);
+ ax_memcpy (aexpr, fbuf, strlen (fbuf) + 1);
+}
+
static void
agent_command (char *exp, int from_tty)
{
--- a/ax-gdb.h
+++ b/ax-gdb.h
@@ -108,6 +108,8 @@ extern struct agent_expr *gen_trace_for_
extern struct agent_expr *gen_eval_for_expr (CORE_ADDR, struct expression *);
+extern void gen_printf_expr_callback (char *, char **, void *, void *);
+
extern int trace_kludge;
#endif /* AX_GDB_H */
--- a/ax-general.c
+++ b/ax-general.c
@@ -330,6 +330,14 @@ ax_tsv (struct agent_expr *x, enum agent
x->buf[x->len + 2] = (num) & 0xff;
x->len += 3;
}
+
+void
+ax_memcpy (struct agent_expr *x, const void *src, size_t n)
+{
+ grow_expr (x, n);
+ memcpy (x->buf + x->len, src, n);
+ x->len += n;
+}
\f
@@ -368,6 +376,7 @@ ax_print (struct ui_file *f, struct agen
for (i = 0; i < x->len;)
{
enum agent_op op = x->buf[i];
+ int op_size;
if (op >= (sizeof (aop_map) / sizeof (aop_map[0]))
|| !aop_map[op].name)
@@ -376,7 +385,19 @@ ax_print (struct ui_file *f, struct agen
i++;
continue;
}
- if (i + 1 + aop_map[op].op_size > x->len)
+ if (op == aop_printf)
+ {
+ if (i + 2 >= x->len)
+ {
+ fprintf_filtered (f, _("%3d <bad opcode %02x>\n"), i, op);
+ i++;
+ continue;
+ }
+ op_size = 1 + strlen (x->buf + i + 2) + 1;
+ }
+ else
+ op_size = aop_map[op].op_size;
+ if (i + 1 + op_size > x->len)
{
fprintf_filtered (f, _("%3d <incomplete opcode %s>\n"),
i, aop_map[op].name);
@@ -384,15 +405,15 @@ ax_print (struct ui_file *f, struct agen
}
fprintf_filtered (f, "%3d %s", i, aop_map[op].name);
- if (aop_map[op].op_size > 0)
+ if (op_size > 0)
{
fputs_filtered (" ", f);
print_longest (f, 'd', 0,
- read_const (x, i + 1, aop_map[op].op_size));
+ read_const (x, i + 1, op_size));
}
fprintf_filtered (f, "\n");
- i += 1 + aop_map[op].op_size;
+ i += 1 + op_size;
is_float = (op == aop_float);
}
@@ -460,6 +481,8 @@ ax_reqs (struct agent_expr *ax)
/* Pointer to a description of the present op. */
struct aop_map *op;
+ int op_size = 0, consumed = 0;
+
memset (targets, 0, ax->len * sizeof (targets[0]));
memset (boundary, 0, ax->len * sizeof (boundary[0]));
@@ -467,7 +490,7 @@ ax_reqs (struct agent_expr *ax)
ax->flaw = agent_flaw_none;
ax->max_data_size = 0;
- for (i = 0; i < ax->len; i += 1 + op->op_size)
+ for (i = 0; i < ax->len; i += 1 + op_size)
{
if (ax->buf[i] > (sizeof (aop_map) / sizeof (aop_map[0])))
{
@@ -483,7 +506,23 @@ ax_reqs (struct agent_expr *ax)
return;
}
- if (i + 1 + op->op_size > ax->len)
+ if (ax->buf[i] == aop_printf)
+ {
+ if (i + 2 >= ax->len)
+ {
+ ax->flaw = agent_flaw_incomplete_instruction;
+ return;
+ }
+ consumed = ax->buf[i + 1];
+ op_size = 1 + strlen (ax->buf + i + 2) + 1;
+ }
+ else
+ {
+ op_size = op->op_size;
+ consumed = op->consumed;
+ }
+
+ if (i + 1 + op_size > ax->len)
{
ax->flaw = agent_flaw_incomplete_instruction;
return;
@@ -501,7 +540,7 @@ ax_reqs (struct agent_expr *ax)
boundary[i] = 1;
heights[i] = height;
- height -= op->consumed;
+ height -= consumed;
if (height < ax->min_height)
ax->min_height = height;
height += op->produced;
--- a/ax.h
+++ b/ax.h
@@ -213,6 +213,8 @@ extern void ax_reg_mask (struct agent_ex
/* Assemble code to operate on a trace state variable. */
extern void ax_tsv (struct agent_expr *expr, enum agent_op op, int num);
+
+extern void ax_memcpy (struct agent_expr *x, const void *src, size_t n);
\f
/* Functions for printing out expressions, and otherwise debugging
--- a/common/ax.def
+++ b/common/ax.def
@@ -86,12 +86,8 @@ DEFOP (swap, 0, 0, 2, 2, 0x2b)
DEFOP (getv, 2, 0, 0, 1, 0x2c)
DEFOP (setv, 2, 0, 0, 1, 0x2d)
DEFOP (tracev, 2, 0, 0, 1, 0x2e)
-/* We need something here just to make the tables come out ok. */
DEFOP (invalid, 0, 0, 0, 0, 0x2f)
DEFOP (trace16, 2, 0, 1, 1, 0x30)
-/* We need something here just to make the tables come out ok. */
-DEFOP (invalid2, 0, 0, 0, 0, 0x2f)
-/* The "consumed" number for pick is wrong, but there's no way to
- express the right thing. */
+DEFOP (printf, 0, 0, 0, 0, 0x31)
DEFOP (pick, 1, 0, 0, 1, 0x32)
DEFOP (rot, 0, 0, 3, 3, 0x33)
--- a/printcmd.c
+++ b/printcmd.c
@@ -49,6 +49,7 @@
#include "parser-defs.h"
#include "charset.h"
#include "arch-utils.h"
+#include "printcmd.h"
#ifdef TUI
#include "tui/tui.h" /* For tui_active et al. */
@@ -1960,10 +1961,9 @@ print_variable_and_value (const char *na
fprintf_filtered (stream, "\n");
}
-/* printf "printf format string" ARG to STREAM. */
-
-static void
-ui_printf (char *arg, struct ui_file *stream)
+void
+string_printf (char *arg, struct ui_file *stream, printf_callback callback,
+ void *loc_v, void *aexpr_v)
{
char *f = NULL;
char *s = arg;
@@ -1974,6 +1974,8 @@ ui_printf (char *arg, struct ui_file *st
int nargs = 0;
int allocated_args = 20;
struct cleanup *old_cleanups;
+ struct bp_location *loc = loc_v;
+ struct agent_expr *aexpr = aexpr_v;
val_args = xmalloc (allocated_args * sizeof (struct value *));
old_cleanups = make_cleanup (free_current_contents, &val_args);
@@ -2296,26 +2298,42 @@ ui_printf (char *arg, struct ui_file *st
/* Now, parse all arguments and evaluate them.
Store the VALUEs in VAL_ARGS. */
+ if (callback)
+ current_substring = substrings;
while (*s != '\0')
{
char *s1;
+ s1 = s;
if (nargs == allocated_args)
val_args = (struct value **) xrealloc ((char *) val_args,
(allocated_args *= 2)
* sizeof (struct value *));
- s1 = s;
- val_args[nargs] = parse_to_comma_and_eval (&s1);
+ if (callback)
+ {
+ if (nargs >= nargs_wanted)
+ error (_("Wrong number of arguments for specified "
+ "format-string"));
+ callback (current_substring, &s1, loc, aexpr);
+ current_substring += strlen (current_substring) + 1;
+ }
+ else
+ val_args[nargs] = parse_to_comma_and_eval (&s1);
nargs++;
s = s1;
if (*s == ',')
s++;
}
+ if (callback)
+ callback (last_arg, NULL, loc, aexpr);
if (nargs != nargs_wanted)
error (_("Wrong number of arguments for specified format-string"));
+ if (!stream)
+ goto after_print;
+
/* Now actually print them. */
current_substring = substrings;
for (i = 0; i < nargs; i++)
@@ -2670,15 +2688,17 @@ ui_printf (char *arg, struct ui_file *st
by default, which will warn here if there is no argument. */
fprintf_filtered (stream, last_arg, 0);
}
+
+after_print:
do_cleanups (old_cleanups);
}
/* Implement the "printf" command. */
-static void
+void
printf_command (char *arg, int from_tty)
{
- ui_printf (arg, gdb_stdout);
+ string_printf (arg, gdb_stdout, NULL, NULL, NULL);
}
/* Implement the "eval" command. */
@@ -2690,7 +2710,7 @@ eval_command (char *arg, int from_tty)
struct cleanup *cleanups = make_cleanup_ui_file_delete (ui_out);
char *expanded;
- ui_printf (arg, ui_out);
+ string_printf (arg, ui_out, NULL, NULL, NULL);
expanded = ui_file_xstrdup (ui_out, NULL);
make_cleanup (xfree, expanded);
--- /dev/null
+++ b/printcmd.h
@@ -0,0 +1,30 @@
+/* Print values for GNU debugger GDB.
+
+ Copyright (C) 2011 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ 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/>. */
+
+#ifndef _PRINTCMD_H_
+#define _PRINTCMD_H_
+
+extern void printf_command (char *arg, int from_tty);
+typedef void (printf_callback) (char *fbuf, char **expp, void *loc_v,
+ void *aexpr_v);
+extern void string_printf (char *arg, struct ui_file *stream,
+ printf_callback callback, void *loc_v,
+ void *aexpr_v);
+
+#endif /* _PRINTCMD_H_ */
--- a/tracepoint.c
+++ b/tracepoint.c
@@ -51,6 +51,7 @@
#include "ax.h"
#include "ax-gdb.h"
#include "memrange.h"
+#include "printcmd.h"
/* readline include files */
#include "readline/readline.h"
@@ -763,6 +764,28 @@ validate_actionline (char **line, struct
error (_("while-stepping step count `%s' is malformed."), *line);
}
+ else if (cmd_cfunc_eq (c, printf_command))
+ {
+ char fbuf[101];
+
+ for (loc = t->loc; loc; loc = loc->next)
+ {
+ int nargs;
+ aexpr = new_agent_expr (loc->gdbarch, loc->address);
+ old_chain = make_cleanup_free_agent_expr (aexpr);
+ string_printf (p, NULL, gen_printf_expr_callback,
+ loc, aexpr);
+ ax_simple (aexpr, aop_end);
+ /* The agent expr include expr for arguments, format string, 1 byte
+ for aop_printf, 1 byte for the number of arguments, 1 byte for
+ size of format string, 1 byte for blank after format string
+ and 1 byte for aop_end. */
+ if (aexpr->len > MAX_AGENT_EXPR_LEN)
+ error (_("Expression is too complicated."));
+ do_cleanups (old_chain);
+ }
+ }
+
else if (cmd_cfunc_eq (c, end_actions_pseudocommand))
;
@@ -1474,6 +1497,22 @@ encode_actions_1 (struct command_line *a
encode_actions_1 (action->body_list[0], t, tloc, frame_reg,
frame_offset, stepping_list, NULL);
}
+ else if (cmd_cfunc_eq (cmd, printf_command))
+ {
+ char fbuf[101];
+ struct cleanup *old_chain = NULL;
+
+ aexpr = new_agent_expr (tloc->gdbarch, tloc->address);
+ old_chain = make_cleanup_free_agent_expr (aexpr);
+ string_printf (action_exp, NULL, gen_printf_expr_callback,
+ tloc, aexpr);
+ ax_simple (aexpr, aop_end);
+
+ ax_reqs (aexpr);
+ report_agent_reqs_errors (aexpr);
+ discard_cleanups (old_chain);
+ add_aexpr (collect, aexpr);
+ }
else
error (_("Invalid tracepoint command '%s'"), action->line);
} /* for */
prev parent reply other threads:[~2011-02-21 8:17 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-01-03 16:29 Hui Zhu
2011-01-03 19:21 ` Doug Evans
2011-01-04 4:34 ` Hui Zhu
2011-01-04 6:19 ` Doug Evans
2011-01-04 12:07 ` Hui Zhu
2011-01-05 17:24 ` Doug Evans
2011-01-05 18:18 ` Michael Snyder
2011-01-06 6:42 ` Hui Zhu
2011-01-05 20:51 ` Stan Shebs
2011-01-06 6:43 ` Hui Zhu
2011-01-28 5:54 ` Hui Zhu
2011-02-04 15:59 ` Hui Zhu
2011-02-11 3:49 ` Hui Zhu
2011-02-11 18:45 ` Tom Tromey
2011-02-17 8:16 ` Hui Zhu
2011-02-21 8:18 ` Hui Zhu [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='AANLkTi=6V7Ao9Oc=vFfrnreYjzdXBsF1XsgPByMaZfmL@mail.gmail.com' \
--to=teawater@gmail.com \
--cc=gdb-patches@sourceware.org \
--cc=stan@codesourcery.com \
--cc=tromey@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).