This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Dynamic printf for a target agent
- From: Yao Qi <yao at codesourcery dot com>
- To: Stan Shebs <stanshebs at earthlink dot net>
- Cc: <gdb-patches at sourceware dot org>
- Date: Thu, 31 May 2012 14:02:29 +0800
- Subject: Re: [PATCH] Dynamic printf for a target agent
- References: <4FC57340.6070306@earthlink.net>
On 05/30/2012 09:09 AM, Stan Shebs wrote:
> Index: breakpoint.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/breakpoint.c,v
> retrieving revision 1.677
> diff -u -p -r1.677 breakpoint.c
> --- breakpoint.c 24 May 2012 16:51:34 -0000 1.677
> +++ breakpoint.c 30 May 2012 00:20:56 -0000
>
> +/* Parses a command described by string CMD into an agent expression
> + bytecode suitable for evaluation by the bytecode interpreter.
> + Return NULL if there was any error during parsing. */
> +
> +static struct agent_expr *
> +parse_cmd_to_aexpr (CORE_ADDR scope, char *cmd)
> +{
> + struct cleanup *old_cleanups = 0;
> + struct expression *expr, **argvec;
> + struct agent_expr *aexpr = NULL;
> + struct cleanup *old_chain = NULL;
> + volatile struct gdb_exception ex;
> + char *cmdrest;
> + char *format_start, *format_end;
> + struct format_piece *fpieces;
> + int nargs;
> + struct gdbarch *gdbarch = get_current_arch ();
> +
> + if (!cmd)
> + return NULL;
> +
> + cmdrest = cmd;
> +
> + if (*cmdrest == ',')
> + ++cmdrest;
> + cmdrest = skip_spaces (cmdrest);
> +
> + if (*cmdrest++ != '"')
> + error (_("No format string following the location"));
> +
> + format_start = cmdrest;
> +
> + fpieces = parse_format_string (&cmdrest);
> +
> + old_cleanups = make_cleanup (free_format_pieces_cleanup, &fpieces);
> +
> + format_end = cmdrest;
> +
> + if (*cmdrest++ != '"')
> + error (_("Bad format string, non-terminated '\"'."));
> +
> + cmdrest = skip_spaces (cmdrest);
> +
> + if (!(*cmdrest == ',' || *cmdrest == '\0'))
> + error (_("Invalid argument syntax"));
> +
> + if (*cmdrest == ',')
> + cmdrest++;
> + cmdrest = skip_spaces (cmdrest);
> +
> + /* For each argument, make an expression. */
> +
> + argvec = (struct expression **) alloca (strlen (cmd)
> + * sizeof (struct expression *));
> +
> + nargs = 0;
> + while (*cmdrest != '\0')
> + {
> + char *cmd1;
> +
> + cmd1 = cmdrest;
> + expr = parse_exp_1 (&cmd1, (struct block *) 0, 1);
We need an instance of block, like
expr = parse_exp_1 (&cmd1, block_for_pc (scope), 1);
otherwise, we'll see some fails when running dprintf.exp with gdbserver,
because of the warning below,
"No symbol "arg" in specified context."
> + argvec[nargs++] = expr;
> + cmdrest = cmd1;
> + if (*cmdrest == ',')
> + ++cmdrest;
> + }
> +
> + /* We don't want to stop processing, so catch any errors
> + that may show up. */
> + TRY_CATCH (ex, RETURN_MASK_ERROR)
> + {
> + aexpr = gen_printf (scope, gdbarch, 0, 0,
> + format_start, format_end - format_start,
> + fpieces, nargs, argvec);
> + }
> +
> + if (ex.reason < 0)
> + {
> + /* If we got here, it means the command could not be parsed to a valid
> + bytecode expression and thus can't be evaluated on the target's side.
> + It's no use iterating through the other commands. */
> + return NULL;
> + }
> +
> + do_cleanups (old_cleanups);
> +
> + /* We have a valid agent expression, return it. */
> + return aexpr;
> +}
> +
> @@ -2001,15 +1995,15 @@ print_variable_and_value (const char *na
> static void
> ui_printf (char *arg, struct ui_file *stream)
> {
> + struct format_piece *fpieces;
> char *f = NULL;
> char *s = arg;
> char *string = NULL;
local variables 'f' and 'string' can be removed.
> struct value **val_args;
> - char *substrings;
> - char *current_substring;
> int nargs = 0;
local variable 'nargs' can be defined in ...
> int allocated_args = 20;
> struct cleanup *old_cleanups;
> + int nargs_wanted;
local variable 'nargs_wanted' can be defined in ....
>
> val_args = xmalloc (allocated_args * sizeof (struct value *));
> old_cleanups = make_cleanup (free_current_contents, &val_args);
> @@ -2023,64 +2017,13 @@ ui_printf (char *arg, struct ui_file *st
> if (*s++ != '"')
> error (_("Bad format string, missing '\"'."));
>
> - /* Parse the format-control string and copy it into the string STRING,
> - processing some kinds of escape sequence. */
> -
> - f = string = (char *) alloca (strlen (s) + 1);
> -
> - while (*s != '"')
> - {
> - int c = *s++;
> - switch (c)
> - {
> - case '\0':
> - error (_("Bad format string, non-terminated '\"'."));
> + fpieces = parse_format_string (&s);
>
> - case '\\':
> - switch (c = *s++)
> - {
> - case '\\':
> - *f++ = '\\';
> - break;
> - case 'a':
> - *f++ = '\a';
> - break;
> - case 'b':
> - *f++ = '\b';
> - break;
> - case 'f':
> - *f++ = '\f';
> - break;
> - case 'n':
> - *f++ = '\n';
> - break;
> - case 'r':
> - *f++ = '\r';
> - break;
> - case 't':
> - *f++ = '\t';
> - break;
> - case 'v':
> - *f++ = '\v';
> - break;
> - case '"':
> - *f++ = '"';
> - break;
> - default:
> - /* ??? TODO: handle other escape sequences. */
> - error (_("Unrecognized escape character \\%c in format string."),
> - c);
> - }
> - break;
> + make_cleanup (free_format_pieces_cleanup, &fpieces);
>
> - default:
> - *f++ = c;
> - }
> - }
> -
> - /* Skip over " and following space and comma. */
> - s++;
> - *f++ = '\0';
> + if (*s++ != '"')
> + error (_("Bad format string, non-terminated '\"'."));
> +
> s = skip_spaces (s);
>
> if (*s != ',' && *s != 0)
> @@ -2090,240 +2033,14 @@ ui_printf (char *arg, struct ui_file *st
> s++;
> s = skip_spaces (s);
>
> - /* Need extra space for the '\0's. Doubling the size is sufficient. */
> - substrings = alloca (strlen (string) * 2);
> - current_substring = substrings;
> -
> {
> - /* Now scan the string for %-specs and see what kinds of args they want.
> - argclass[I] classifies the %-specs so we can give printf_filtered
> - something of the right size. */
> -
> - enum argclass
> - {
> - int_arg, long_arg, long_long_arg, ptr_arg,
> - string_arg, wide_string_arg, wide_char_arg,
> - double_arg, long_double_arg, decfloat_arg
> - };
> - enum argclass *argclass;
> - enum argclass this_argclass;
> - char *last_arg;
> - int nargs_wanted;
> - int i;
> + int i, fr;
> + char *current_substring;
... this block.
>
> - argclass = (enum argclass *) alloca (strlen (s) * sizeof *argclass);
> nargs_wanted = 0;
> - f = string;
> - last_arg = string;
> - while (*f)
> - if (*f++ == '%')
> - {
> - int seen_hash = 0, seen_zero = 0, lcount = 0, seen_prec = 0;
> - int seen_space = 0, seen_plus = 0;
> - int seen_big_l = 0, seen_h = 0, seen_big_h = 0;
> - int seen_big_d = 0, seen_double_big_d = 0;
> - int bad = 0;
> -
> - /* Check the validity of the format specifier, and work
> - out what argument it expects. We only accept C89
> - format strings, with the exception of long long (which
> - we autoconf for). */
> -
> - /* Skip over "%%". */
> - if (*f == '%')
> - {
> - f++;
> - continue;
> - }
> -
> - /* The first part of a format specifier is a set of flag
> - characters. */
> - while (strchr ("0-+ #", *f))
> - {
> - if (*f == '#')
> - seen_hash = 1;
> - else if (*f == '0')
> - seen_zero = 1;
> - else if (*f == ' ')
> - seen_space = 1;
> - else if (*f == '+')
> - seen_plus = 1;
> - f++;
> - }
> -
> - /* The next part of a format specifier is a width. */
> - while (strchr ("0123456789", *f))
> - f++;
> -
> - /* The next part of a format specifier is a precision. */
> - if (*f == '.')
> - {
> - seen_prec = 1;
> - f++;
> - while (strchr ("0123456789", *f))
> - f++;
> - }
> -
> - /* The next part of a format specifier is a length modifier. */
> - if (*f == 'h')
> - {
> - seen_h = 1;
> - f++;
> - }
> - else if (*f == 'l')
> - {
> - f++;
> - lcount++;
> - if (*f == 'l')
> - {
> - f++;
> - lcount++;
> - }
> - }
> - else if (*f == 'L')
> - {
> - seen_big_l = 1;
> - f++;
> - }
> - /* Decimal32 modifier. */
> - else if (*f == 'H')
> - {
> - seen_big_h = 1;
> - f++;
> - }
> - /* Decimal64 and Decimal128 modifiers. */
> - else if (*f == 'D')
> - {
> - f++;
> -
> - /* Check for a Decimal128. */
> - if (*f == 'D')
> - {
> - f++;
> - seen_double_big_d = 1;
> - }
> - else
> - seen_big_d = 1;
> - }
> -
> - switch (*f)
> - {
> - case 'u':
> - if (seen_hash)
> - bad = 1;
> - /* FALLTHROUGH */
> -
> - case 'o':
> - case 'x':
> - case 'X':
> - if (seen_space || seen_plus)
> - bad = 1;
> - /* FALLTHROUGH */
> -
> - case 'd':
> - case 'i':
> - if (lcount == 0)
> - this_argclass = int_arg;
> - else if (lcount == 1)
> - this_argclass = long_arg;
> - else
> - this_argclass = long_long_arg;
> -
> - if (seen_big_l)
> - bad = 1;
> - break;
> -
> - case 'c':
> - this_argclass = lcount == 0 ? int_arg : wide_char_arg;
> - if (lcount > 1 || seen_h || seen_big_l)
> - bad = 1;
> - if (seen_prec || seen_zero || seen_space || seen_plus)
> - bad = 1;
> - break;
> -
> - case 'p':
> - this_argclass = ptr_arg;
> - if (lcount || seen_h || seen_big_l)
> - bad = 1;
> - if (seen_prec || seen_zero || seen_space || seen_plus)
> - bad = 1;
> - break;
> -
> - case 's':
> - this_argclass = lcount == 0 ? string_arg : wide_string_arg;
> - if (lcount > 1 || seen_h || seen_big_l)
> - bad = 1;
> - if (seen_zero || seen_space || seen_plus)
> - bad = 1;
> - break;
> -
> - case 'e':
> - case 'f':
> - case 'g':
> - case 'E':
> - case 'G':
> - if (seen_big_h || seen_big_d || seen_double_big_d)
> - this_argclass = decfloat_arg;
> - else if (seen_big_l)
> - this_argclass = long_double_arg;
> - else
> - this_argclass = double_arg;
> -
> - if (lcount || seen_h)
> - bad = 1;
> - break;
> -
> - case '*':
> - error (_("`*' not supported for precision or width in printf"));
> -
> - case 'n':
> - error (_("Format specifier `n' not supported in printf"));
> -
> - case '\0':
> - error (_("Incomplete format specifier at end of format string"));
> -
> - default:
> - error (_("Unrecognized format specifier '%c' in printf"), *f);
> - }
> -
> - if (bad)
> - error (_("Inappropriate modifiers to "
> - "format specifier '%c' in printf"),
> - *f);
> -
> - f++;
> -
> - if (lcount > 1 && USE_PRINTF_I64)
> - {
> - /* Windows' printf does support long long, but not the usual way.
> - Convert %lld to %I64d. */
> - int length_before_ll = f - last_arg - 1 - lcount;
> -
> - strncpy (current_substring, last_arg, length_before_ll);
> - strcpy (current_substring + length_before_ll, "I64");
> - current_substring[length_before_ll + 3] =
> - last_arg[length_before_ll + lcount];
> - current_substring += length_before_ll + 4;
> - }
> - else if (this_argclass == wide_string_arg
> - || this_argclass == wide_char_arg)
> - {
> - /* Convert %ls or %lc to %s. */
> - int length_before_ls = f - last_arg - 2;
> -
> - strncpy (current_substring, last_arg, length_before_ls);
> - strcpy (current_substring + length_before_ls, "s");
> - current_substring += length_before_ls + 2;
> - }
> - else
> - {
> - strncpy (current_substring, last_arg, f - last_arg);
> - current_substring += f - last_arg;
> - }
> - *current_substring++ = '\0';
> - last_arg = f;
> - argclass[nargs_wanted++] = this_argclass;
> - }
> + for (fr = 0; fpieces[fr].string != NULL; fr++)
> + if (fpieces[fr].argclass != literal_piece)
> + ++nargs_wanted;
>
> /* Now, parse all arguments and evaluate them.
> Store the VALUEs in VAL_ARGS. */
> Index: remote.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/remote.c,v
> retrieving revision 1.499
> diff -u -p -r1.499 remote.c
> --- remote.c 24 May 2012 16:51:35 -0000 1.499
> +++ remote.c 30 May 2012 00:20:56 -0000
> @@ -7884,6 +7934,9 @@ remote_insert_breakpoint (struct gdbarch
> if (remote_supports_cond_breakpoints ())
> remote_add_target_side_condition (gdbarch, bp_tgt, p, endbuf);
>
> + if (remote_can_run_breakpoint_commands ())
> + remote_add_target_side_commands (gdbarch, bp_tgt, p);
> +
If dprintf_style is "gdb", we shouldn't add target side commands,
otherwise print is executed in target side, rather than GDB side. This
causes two regressions in testing with gdbserver,
FAIL: gdb.base/dprintf.exp: 1st dprintf, gdb
FAIL: gdb.base/dprintf.exp: 2nd dprintf, gdb
We only add target side commands only if target supports breakpoint
commands, and dprintf_style is not "gdb".
if (remote_can_run_breakpoint_commands ()
&& strcmp (dprintf_style, "gdb") != 0)
remote_add_target_side_commands (gdbarch, bp_tgt, p);
> putpkt (rs->buf);
> getpkt (&rs->buf, &rs->buf_size, 0);
>
> @@ -8125,6 +8178,9 @@ remote_insert_hw_breakpoint (struct gdba
> if (remote_supports_cond_breakpoints ())
> remote_add_target_side_condition (gdbarch, bp_tgt, p, endbuf);
>
> + if (remote_can_run_breakpoint_commands ())
> + remote_add_target_side_commands (gdbarch, bp_tgt, p);
> +
> putpkt (rs->buf);
> getpkt (&rs->buf, &rs->buf_size, 0);
>
> @@ -10063,6 +10119,14 @@ remote_supports_string_tracing (void)
> return rs->string_tracing;
> }
>
> Index: testsuite/gdb.base/dprintf.exp
> ===================================================================
> RCS file: /cvs/src/src/gdb/testsuite/gdb.base/dprintf.exp,v
> retrieving revision 1.3
> diff -u -p -r1.3 dprintf.exp
> --- testsuite/gdb.base/dprintf.exp 15 May 2012 13:36:18 -0000 1.3
> +++ testsuite/gdb.base/dprintf.exp 30 May 2012 00:20:59 -0000
> @@ -85,6 +85,30 @@ if ![target_info exists gdb,noinferiorio
> "2nd dprintf, fprintf"
> }
>
> +set target_can_dprintf 1
> +set msg "Set dprintf style to agent"
> +gdb_test_multiple "set dprintf-style agent" $msg {
> + -re "warning: Target cannot run dprintf commands.*" {
> + set target_can_dprintf 0
> + pass "$msg - cannot do"
> + }
> + -re ".*$gdb_prompt $" {
> + pass "$msg - can do"
> + }
> +}
> +
> +if $target_can_dprintf {
> +
> + gdb_run_cmd
> +
> + gdb_test "" "Breakpoint"
> +
> + gdb_test "continue" "At foo entry.*arg=1234, g=1234.*" "1st dprintf, agent"
> +
> + gdb_test "continue" "At foo entry.*arg=1235, g=2222.*" "2nd dprintf, agent"
When we set dprintf style to agent, GDB can't receive such outputs, so
these two tests above should fail. We may need more thoughts on testing
output from agent.
--
Yao (éå)