[PATCH 7/9] move in_process_agent_loaded to agent_loaded_p.

Pedro Alves palves@redhat.com
Thu Feb 23 22:11:00 GMT 2012


On 02/17/2012 02:54 AM, Yao Qi wrote:
> This patch moves in_process_agent_loaded from gdbserver to gdb/common,
> and renmame it to agent_loaded_p, then gdb can use it as well.  This
> is a refacotr patch, no function is changed.
> 
> gdb:
> 
> 2012-02-14  Yao Qi  <yao@codesourcery.com>
> 
> 	* common/agent.c (agent_loaded_p): New.
>         (agent_look_up_symbols): New global.
> 	* common/agent.h: Declare agent_loaded_p.
> 
> gdb/gdbserver:
> 
> 2012-02-14  Yao Qi  <yao@codesourcery.com>
> 
> 	* Makefile.in (linux-low.o): Keep dependence on agent.h.
> 	(linux-x86-low.o): Likewise.
> 	* server.h: Remove in_process_agent_loaded.
> 	* tracepoint.c (in_process_agent_loaded): Removed.  Moved it
> 	common/agent.c.
> 	Update callers.
> ---
>  gdb/common/agent.c            |   11 +++++++++++
>  gdb/common/agent.h            |    2 ++
>  gdb/gdbserver/Makefile.in     |    4 ++--
>  gdb/gdbserver/linux-low.c     |    7 ++++---
>  gdb/gdbserver/linux-x86-low.c |    3 ++-
>  gdb/gdbserver/server.h        |    2 --
>  gdb/gdbserver/tracepoint.c    |   37 +++++++++++++------------------------
>  7 files changed, 34 insertions(+), 32 deletions(-)
> 
> diff --git a/gdb/common/agent.c b/gdb/common/agent.c
> index 074efea..d2f59db 100644
> --- a/gdb/common/agent.c
> +++ b/gdb/common/agent.c
> @@ -71,6 +71,14 @@ static struct
>  
>  static struct ipa_sym_addresses ipa_sym_addrs;
>  
> +static int all_agent_symbols_looked_up = 0;
> +
> +int
> +agent_loaded_p (void)
> +{
> +  return all_agent_symbols_looked_up;
> +}
> +
>  /* Look up all symbols needed by agent.  Return 0 if all the symbols are
>     found, return non-zero otherwise.  */
>  
> @@ -79,6 +87,8 @@ agent_look_up_symbols (void)
>  {
>    int i;
>  
> +  all_agent_symbols_looked_up = 0;
> +
>    for (i = 0; i < sizeof (symbol_list) / sizeof (symbol_list[0]); i++)
>      {
>        CORE_ADDR *addrp =
> @@ -100,6 +110,7 @@ agent_look_up_symbols (void)
>  	}
>      }
>  
> +  all_agent_symbols_looked_up = 1;
>    return 0;
>  }
>  
> diff --git a/gdb/common/agent.h b/gdb/common/agent.h
> index a1ac9b2..b89d111 100644
> --- a/gdb/common/agent.h
> +++ b/gdb/common/agent.h
> @@ -33,6 +33,8 @@ int agent_look_up_symbols (void);
>     thread.  */
>  #define IPA_CMD_BUF_SIZE 1024
>  
> +int agent_loaded_p (void);
> +
>  extern int debug_agent;
>  
>  extern int use_agent;
> diff --git a/gdb/gdbserver/Makefile.in b/gdb/gdbserver/Makefile.in
> index e840db2..b7f1454 100644
> --- a/gdb/gdbserver/Makefile.in
> +++ b/gdb/gdbserver/Makefile.in
> @@ -451,7 +451,7 @@ i386-low.o: i386-low.c $(i386_low_h) $(server_h) $(target_h)
>  i387-fp.o: i387-fp.c $(server_h)
>  
>  linux-low.o: linux-low.c $(linux_low_h) $(linux_ptrace_h) $(linux_procfs_h) \
> -	$(server_h) $(linux_osdata_h)
> +	$(server_h) $(linux_osdata_h) $(agent_h)
>  	$(CC) -c $(CPPFLAGS) $(INTERNAL_CFLAGS) $< @USE_THREAD_DB@
>  
>  linux-arm-low.o: linux-arm-low.c $(linux_low_h) $(server_h) \
> @@ -468,7 +468,7 @@ linux-s390-low.o: linux-s390-low.c $(linux_low_h) $(server_h)
>  linux-sh-low.o: linux-sh-low.c $(linux_low_h) $(server_h)
>  linux-tic6x-low.o: linux-tic6x-low.c $(linux_low_h) $(server_h)
>  linux-x86-low.o: linux-x86-low.c $(linux_low_h) $(server_h) \
> -	$(gdb_proc_service_h) $(i386_low_h)
> +	$(gdb_proc_service_h) $(i386_low_h) $(agent_h)
>  linux-xtensa-low.o: linux-xtensa-low.c xtensa-xtregs.c $(linux_low_h) $(server_h)
>  
>  lynx-low.o: lynx-low.c $(server_h) $(target_h) $(lynx_low_h)
> diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
> index c9b8a3b..d9f4b12 100644
> --- a/gdb/gdbserver/linux-low.c
> +++ b/gdb/gdbserver/linux-low.c
> @@ -19,6 +19,7 @@
>  #include "server.h"
>  #include "linux-low.h"
>  #include "linux-osdata.h"
> +#include "agent.h"
>  
>  #include <sys/wait.h>
>  #include <stdio.h>
> @@ -1320,7 +1321,7 @@ maybe_move_out_of_jump_pad (struct lwp_info *lwp, int *wstat)
>    if ((wstat == NULL
>         || (WIFSTOPPED (*wstat) && WSTOPSIG (*wstat) != SIGTRAP))
>        && supports_fast_tracepoints ()
> -      && in_process_agent_loaded ())
> +      && agent_loaded_p ())

But the checks aren't equivalent.  in_process_agent_loaded
checks for a bunch of libinproctrace.so symbols, agent_loaded_p
only checks for a few symbols in common/agent.c.

>      {
>        struct fast_tpoint_collect_status status;
>        int r;
> @@ -2251,7 +2252,7 @@ retry:
>    if (WIFSTOPPED (w)
>        && WSTOPSIG (w) != SIGTRAP
>        && supports_fast_tracepoints ()
> -      && in_process_agent_loaded ())
> +      && agent_loaded_p ())
>      {
>        if (debug_threads)
>  	fprintf (stderr,
> @@ -2795,7 +2796,7 @@ stuck_in_jump_pad_callback (struct inferior_list_entry *entry, void *data)
>  
>    /* Allow debugging the jump pad, gdb_collect, etc..  */
>    return (supports_fast_tracepoints ()
> -	  && in_process_agent_loaded ()
> +	  && agent_loaded_p ()
>  	  && (gdb_breakpoint_here (lwp->stop_pc)
>  	      || lwp->stopped_by_watchpoint
>  	      || thread->last_resume_kind == resume_step)
> diff --git a/gdb/gdbserver/linux-x86-low.c b/gdb/gdbserver/linux-x86-low.c
> index 365cd52..a662780 100644
> --- a/gdb/gdbserver/linux-x86-low.c
> +++ b/gdb/gdbserver/linux-x86-low.c
> @@ -28,6 +28,7 @@
>  #include "elf/common.h"
>  
>  #include "gdb_proc_service.h"
> +#include "agent.h"
>  
>  /* Defined in auto-generated file i386-linux.c.  */
>  void init_registers_i386_linux (void);
> @@ -1586,7 +1587,7 @@ x86_get_min_fast_tracepoint_insn_len (void)
>      return 5;
>  #endif
>  
> -  if (in_process_agent_loaded ())
> +  if (agent_loaded_p ())
>      {
>        char errbuf[IPA_BUFSIZ];
>  
> diff --git a/gdb/gdbserver/server.h b/gdb/gdbserver/server.h
> index 5c798c8..390f629 100644
> --- a/gdb/gdbserver/server.h
> +++ b/gdb/gdbserver/server.h
> @@ -436,8 +436,6 @@ char *pfildes (gdb_fildes_t fd);
>     agent back to GDBserver.  */
>  #define IPA_BUFSIZ 100
>  
> -int in_process_agent_loaded (void);
> -
>  void initialize_tracepoint (void);
>  
>  extern int tracing;
> diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
> index 0e1f9ed..0004ff2 100644
> --- a/gdb/gdbserver/tracepoint.c
> +++ b/gdb/gdbserver/tracepoint.c
> @@ -228,14 +228,6 @@ static struct
>  
>  static struct ipa_sym_addresses ipa_sym_addrs;
>  
> -int all_tracepoint_symbols_looked_up;
> -
> -int
> -in_process_agent_loaded (void)
> -{
> -  return all_tracepoint_symbols_looked_up;
> -}
> -
>  static int read_inferior_integer (CORE_ADDR symaddr, int *val);
>  
>  /* Returns true if both the in-process agent library and the static
> @@ -247,7 +239,7 @@ in_process_agent_supports_ust (void)
>  {
>    int loaded = 0;
>  
> -  if (!in_process_agent_loaded ())
> +  if (!agent_loaded_p ())
>      {
>        warning ("In-process agent not loaded");
>        return 0;
> @@ -294,7 +286,7 @@ write_e_ust_not_loaded (char *buffer)
>  static int
>  maybe_write_ipa_not_loaded (char *buffer)
>  {
> -  if (!in_process_agent_loaded ())
> +  if (!agent_loaded_p ())
>      {
>        write_e_ipa_not_loaded (buffer);
>        return 1;
> @@ -309,7 +301,7 @@ maybe_write_ipa_not_loaded (char *buffer)
>  static int
>  maybe_write_ipa_ust_not_loaded (char *buffer)
>  {
> -  if (!in_process_agent_loaded ())
> +  if (!agent_loaded_p ())
>      {
>        write_e_ipa_not_loaded (buffer);
>        return 1;
> @@ -333,7 +325,7 @@ tracepoint_look_up_symbols (void)
>  {
>    int i;
>  
> -  if (all_tracepoint_symbols_looked_up)
> +  if (agent_loaded_p ())
>      return;
>  
>    for (i = 0; i < sizeof (symbol_list) / sizeof (symbol_list[0]); i++)
> @@ -349,10 +341,7 @@ tracepoint_look_up_symbols (void)
>  	}
>      }
>  
> -  if (agent_look_up_symbols () != 0)
> -    return;
> -
> -  all_tracepoint_symbols_looked_up = 1;
> +  agent_look_up_symbols ();
>  }
>  
>  #endif
> @@ -2965,7 +2954,7 @@ install_tracepoint (struct tracepoint *tpoint, char *own_buf)
>      {
>        struct tracepoint *tp;
>  
> -      if (!in_process_agent_loaded ())
> +      if (!agent_loaded_p ())
>  	{
>  	  trace_debug ("Requested a %s tracepoint, but fast "
>  		       "tracepoints aren't supported.",
> @@ -3041,7 +3030,7 @@ cmd_qtstart (char *packet)
>    pause_all (1);
>  
>    /* Sync the fast tracepoints list in the inferior ftlib.  */
> -  if (in_process_agent_loaded ())
> +  if (agent_loaded_p ())
>      {
>        download_tracepoints ();
>        download_trace_state_variables ();
> @@ -3142,7 +3131,7 @@ cmd_qtstart (char *packet)
>    /* Tracing is now active, hits will now start being logged.  */
>    tracing = 1;
>  
> -  if (in_process_agent_loaded ())
> +  if (agent_loaded_p ())
>      {
>        if (write_inferior_integer (ipa_sym_addrs.addr_tracing, 1))
>  	fatal ("Error setting tracing variable in lib");
> @@ -3201,7 +3190,7 @@ stop_tracing (void)
>    /* Stop logging. Tracepoints can still be hit, but they will not be
>       recorded.  */
>    tracing = 0;
> -  if (in_process_agent_loaded ())
> +  if (agent_loaded_p ())
>      {
>        if (write_inferior_integer (ipa_sym_addrs.addr_tracing, 0))
>  	fatal ("Error clearing tracing variable in lib");
> @@ -3249,7 +3238,7 @@ stop_tracing (void)
>    /* Clear out the tracepoints.  */
>    clear_installed_tracepoints ();
>  
> -  if (in_process_agent_loaded ())
> +  if (agent_loaded_p ())
>      {
>        /* Pull in fast tracepoint trace frames from the inferior lib
>  	 buffer into our buffer, even if our buffer is already full,
> @@ -3412,7 +3401,7 @@ cmd_qtstatus (char *packet)
>    trace_debug ("Returning trace status as %d, stop reason %s",
>  	       tracing, tracing_stop_reason);
>  
> -  if (in_process_agent_loaded ())
> +  if (agent_loaded_p ())
>      {
>        pause_all (1);
>  
> @@ -4086,7 +4075,7 @@ tracepoint_finished_step (struct thread_info *tinfo, CORE_ADDR stop_pc)
>  
>    /* Pull in fast tracepoint trace frames from the inferior lib buffer into
>       our buffer.  */
> -  if (in_process_agent_loaded ())
> +  if (agent_loaded_p ())
>      upload_fast_traceframes ();
>  
>    /* Check if we were indeed collecting data for one of more
> @@ -4187,7 +4176,7 @@ handle_tracepoint_bkpts (struct thread_info *tinfo, CORE_ADDR stop_pc)
>    /* Pull in fast tracepoint trace frames from the inferior in-process
>       agent's buffer into our buffer.  */
>  
> -  if (!in_process_agent_loaded ())
> +  if (!agent_loaded_p ())
>      return 0;
>  
>    upload_fast_traceframes ();

Okay, though I'd've preferred if we weren't dropping the in-process qualifier
everywhere...  Also, "_p" as suffix means "predicate".  It's not really
necessary if the symbol's name is already clearly a predicate.

-- 
Pedro Alves



More information about the Gdb-patches mailing list