Re: [PATCH 3/9] command set agent on and off.

[Pedro Alves <palves@redhat.com>]

On 02/17/2012 02:54 AM, Yao Qi wrote:
> This patch adds new command `set agent on|off'.
> 
> 2012-02-15  Yao Qi  <yao@codesourcery.com>
> 
> 	* Makefile.in (SFILES): Add agent.c and common/agent.c.
> 	(COMMON_OBS): Add agent.o and common/agent.o.
> 	(common-agent.o): New rule.
> 	* agent.c: New.
> 
> gdb/testsuite:
> 
> 2012-02-15  Yao Qi  <yao@codesourcery.com>
> 
>         * gdb.trace/strace.exp (strace_info_marker): Set agent on.
>         (strace_probe_marker, strace_trace_on_same_addr): Likewise.
>         (strace_trace_on_diff_addr): Likewise.
> ---
>  gdb/Makefile.in                    |   10 ++++-
>  gdb/agent.c                        |   70 ++++++++++++++++++++++++++++++++++++
>  gdb/testsuite/gdb.trace/strace.exp |    4 ++
>  3 files changed, 82 insertions(+), 2 deletions(-)
>  create mode 100644 gdb/agent.c
> 
> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index 38c93c9..edd616f 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -682,6 +682,7 @@ TARGET_FLAGS_TO_PASS = \
>  SFILES = ada-exp.y ada-lang.c ada-typeprint.c ada-valprint.c ada-tasks.c \
>  	addrmap.c \
>  	auxv.c ax-general.c ax-gdb.c \
> +	agent.c \
>  	bcache.c \
>  	bfd-target.c \
>  	block.c blockframe.c breakpoint.c buildsym.c \
> @@ -738,7 +739,7 @@ SFILES = ada-exp.y ada-lang.c ada-typeprint.c ada-valprint.c ada-tasks.c \
>  	annotate.c common/signals.c copying.c dfp.c gdb.c inf-child.c \
>  	regset.c sol-thread.c windows-termcap.c \
>  	common/common-utils.c common/xml-utils.c \
> -	common/ptid.c common/buffer.c gdb-dlfcn.c
> +	common/ptid.c common/buffer.c gdb-dlfcn.c common/agent.c
>  
>  LINTFILES = $(SFILES) $(YYFILES) $(CONFIG_SRCS) init.c
>  
> @@ -852,6 +853,7 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \
>  	annotate.o \
>  	addrmap.o \
>  	auxv.o \
> +	agent.o \
>  	bfd-target.o \
>  	blockframe.o breakpoint.o findvar.o regcache.o \
>  	charset.o continuations.o corelow.o disasm.o dummy-frame.o dfp.o \
> @@ -906,7 +908,7 @@ COMMON_OBS = $(DEPFILES) $(CONFIG_OBS) $(YYOBJ) \
>  	target-descriptions.o target-memory.o xml-tdesc.o xml-builtin.o \
>  	inferior.o osdata.o gdb_usleep.o record.o gcore.o \
>  	jit.o progspace.o skip.o \
> -	common-utils.o buffer.o ptid.o gdb-dlfcn.o
> +	common-utils.o buffer.o ptid.o gdb-dlfcn.o common-agent.o
>  
>  TSOBS = inflow.o
>  
> @@ -1923,6 +1925,10 @@ linux-procfs.o: $(srcdir)/common/linux-procfs.c
>  	$(COMPILE) $(srcdir)/common/linux-procfs.c
>  	$(POSTCOMPILE)
>  
> +common-agent.o: $(srcdir)/common/agent.c
> +	$(COMPILE) $(srcdir)/common/agent.c
> +	$(POSTCOMPILE)
> +
>  #
>  # gdb/tui/ dependencies
>  #
> diff --git a/gdb/agent.c b/gdb/agent.c
> new file mode 100644
> index 0000000..c10ed89
> --- /dev/null
> +++ b/gdb/agent.c
> @@ -0,0 +1,70 @@
> +/* Copyright (C) 2012 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/>.  */
> +
> +#include "defs.h"
> +#include "command.h"
> +#include "gdbcmd.h"
> +#include "target.h"
> +#include "agent.h"
> +
> +/* Enum strings for "set|show agent".  */
> +
> +static const char can_use_agent_on[] = "on";
> +static const char can_use_agent_off[] = "off";
> +static const char *can_use_agent_enum[] =
> +{
> +  can_use_agent_on,
> +  can_use_agent_off,
> +  NULL,
> +};
> +
> +static const char *can_use_agent = can_use_agent_off;
> +
> +static void
> +show_can_use_agent (struct ui_file *file, int from_tty,
> +		    struct cmd_list_element *c, const char *value)
> +{
> +  fprintf_filtered (file,
> +		    _("Debugger's willingness to use agent in inferior "
> +		      "as a helper is %s.\n"), value);
> +}
> +
> +static void
> +set_can_use_agent (char *args, int from_tty, struct cmd_list_element *c)
> +{
> +  if (target_use_agent (can_use_agent == can_use_agent_on) == 0)
> +    /* Something wrong during setting, set flag to default value.  */
> +    can_use_agent = can_use_agent_off;
> +}
> +
> +void
> +_initialize_agent (void)
> +{
> +  add_setshow_enum_cmd ("agent", class_run,
> +			can_use_agent_enum,
> +			&can_use_agent, _("\
> +Set debugger's willingness to use agent as a helper."), _("\
> +Show debugger's willingness to use agent as a helper."), _("\
> +If on, GDB will delegate some of the debugging operations to the\n\
> +agent, if the target supports it.  This will speed up those\n\
> +operations that are supported by the agent.\n\
> +If off, GDB will not use agent, even if such is supported by the\n\
> +target."),
> +			set_can_use_agent,
> +			show_can_use_agent,
> +			&setlist, &showlist);
> +}
> diff --git a/gdb/testsuite/gdb.trace/strace.exp b/gdb/testsuite/gdb.trace/strace.exp
> index 4d6ea10..8b0c4ef 100644
> --- a/gdb/testsuite/gdb.trace/strace.exp
> +++ b/gdb/testsuite/gdb.trace/strace.exp
> @@ -66,6 +66,7 @@ proc strace_info_marker { } {
>  	set pf_prefix $old_pf_prefix
>  	return -1
>      }
> +    gdb_test_no_output "set agent on"
>  
>      # List the markers in program.  They should be disabled.
>      gdb_test "info static-tracepoint-markers" \
> @@ -91,6 +92,7 @@ proc strace_probe_marker { } {
>  	set pf_prefix $old_pf_prefix
>  	return -1
>      }
> +    gdb_test_no_output "set agent on"
>  
>      gdb_test "strace -m ust/bar" "Static tracepoint \[0-9\]+ at ${hex}: file.*"
>      gdb_test "strace -m ust/bar2" "Static tracepoint \[0-9\]+ at ${hex}: file.*"
> @@ -127,6 +129,7 @@ proc strace_trace_on_same_addr { type } {
>  	set pf_prefix $old_pf_prefix
>  	return -1
>      }
> +    gdb_test_no_output "set agent on"
>  
>      set marker_bar_addr ""
>      set marker_bar2_addr ""
> @@ -223,6 +226,7 @@ proc strace_trace_on_diff_addr { } {
>  	set pf_prefix $old_pf_prefix
>  	return -1
>      }
> +    gdb_test_no_output "set agent on"
>  
>      set marker_bar_addr ""
>      set marker_bar2_addr ""

Hmm, is it really the right thing to do to require turning this
on for static tracepoints?  We didn't need it before, and there's
not way for doing static tracepoints without the in-process agent,
so it just looks like unnecessary extra trouble for the user.

-- 
Pedro Alves