Re: [PATCH] Dynamic printf for a target agent

[Stan Shebs <stanshebs at earthlink dot net>]

On 5/30/12 11:02 PM, Yao Qi wrote:


Thanks for the comments!

> On 05/30/2012 09:09 AM, Stan Shebs wrote:
>
> +      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."

Good idea!

> > @@ -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.

Done, and the nargs variables are moved down.

> > 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);

Ideally, remote.c shouldn't have to know about dprintf details, so I put 
an equivalent test at the top of build_target_command_list, which is 
what one would expect to modify when the time comes to generalize.  (I 
tried to get a contract to do the generalization this year, but no luck 
- maybe next year. :-) )

> +
> +    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.

Yeah, that's a head-scratcher; I looked around and didn't see any good 
examples of something doing that already.  It seems like it ought to be 
possible though, seeing as how the agent dprintf output does end up in 
testsuite/gdb.log.  In the meantime, I tweaked the test to just expect 
the usual result from a continue.

I'll commit the updated patch below later this week if there are no 
obstacles.

Stan
stan@codesourcery.com

2012-06-25  Stan Shebs  <stan@codesourcery.com>

    Add target-side support for dynamic printf.
    * NEWS: Mention the additional style.
    * breakpoint.h (struct bp_target_info): New fields tcommands, persist.
    (struct bp_location): New field cmd_bytecode.
    * breakpoint.c: Include format.h.
    (disconnected_dprintf): New global.
    (parse_cmd_to_aexpr): New function.
    (build_target_command_list): New function.
    (insert_bp_location): Call it.
    (remove_breakpoints_pid): Skip dprintf breakpoints.
    (print_one_breakpoint_location): Ditto.
    (dprintf_style_agent): New global.
    (dprintf_style_enums): Add dprintf_style_agent.
    (update_dprintf_command_list): Add agent case.
    (agent_printf_command): New function.
    (_initialize_breakpoint): Add new commands.
    * common/ax.def (printf): New bytecode.
    * ax.h (ax_string): Declare.
    * ax-gdb.h (gen_printf): Declare.
    * ax-gdb.c: Include cli-utils.h, format.h.
    (gen_printf): New function.
    (maint_agent_print_command): New function.
    (_initialize_ax_gdb): Add maint agent-printf command.
    * ax-general.c (ax_string): New function.
    (ax_print): Add printf disassembly.
    * Makefile.in (SFILES): Add format.c
    (COMMON_OBS): Add format.o.
    * common/format.h: New file.
    * common/format.c: New file.
    * printcmd.c: Include format.h.
    (ui_printf): Call parse_format_string.
    * remote.c (remote_state): New field breakpoint_commands.
    (PACKET_BreakpointCommands): New enum.
    (remote_breakpoint_commands_feature): New function.
    (remote_protocol_features): Add new BreakpointCommands entry.
    (remote_can_run_breakpoint_commands): New function.
    (remote_add_target_side_commands): New function.
    (remote_insert_breakpoint): Call it.
    (remote_insert_hw_breakpoint): Ditto.
    (_initialize_remote): Add new packet configuration for
    target-side breakpoint commands.
    * target.h (struct target_ops): New field
    to_can_run_breakpoint_commands.
    (target_can_run_breakpoint_commands): New macro.
    * target.c (update_current_target): Handle
    to_can_run_breakpoint_commands.

    [gdbserver]
    * Makefile.in (WARN_CFLAGS_NO_FORMAT): Define.
    (ax.o): Add it to build rule.
    (ax-ipa.o): Ditto.
    (OBS): Add format.o.
    (IPA_OBS): Add format.o.
    * server.c (handle_query): Claim support for breakpoint commands.
    (process_point_options): Add command case.
    (process_serial_event): Leave running if there are printfs in
    effect.
    * mem-break.h (any_persistent_commands): Declare.
    (add_breakpoint_commands): Declare.
    (gdb_no_commands_at_breakpoint): Declare.
    (run_breakpoint_commands): Declare.
    * mem-break.c (struct point_command_list): New struct.
    (struct breakpoint): New field command_list.
    (any_persistent_commands): New function.
    (add_commands_to_breakpoint): New function.
    (add_breakpoint_commands): New function.
    (gdb_no_commands_at_breakpoint): New function.
    (run_breakpoint_commands): New function.
    * linux-low.c (linux_wait_1): Test for and run breakpoint commands
    locally.
    * ax.c: Include format.h.
    (ax_printf): New function.
    (gdb_eval_agent_expr): Add printf opcode.

    [doc]
    * gdb.texinfo (Dynamic Printf): Mention agent style and
    disconnected dprintf.
    (Maintenance Commands): Describe maint agent-printf.
    (General Query Packets): Mention BreakpointCommands feature.
    (Packets): Document commands extension to Z0 packet.
    * agentexpr.texi (Bytecode Descriptions): Document printf
    bytecode.

    [testsuite]
    * gdb.base/dprintf.exp: Add agent style tests.

Attachment: dprintf-patch-5
Description: Text document