From: Pedro Alves <palves@redhat.com>
To: Yao Qi <yao@codesourcery.com>
Cc: gdb-patches@sourceware.org
Subject: Re: [PATCH 7/9] move in_process_agent_loaded to agent_loaded_p.
Date: Thu, 23 Feb 2012 22:11:00 -0000 [thread overview]
Message-ID: <4F46B852.2060502@redhat.com> (raw)
In-Reply-To: <1329447300-18841-8-git-send-email-yao@codesourcery.com>
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
next prev parent reply other threads:[~2012-02-23 22:06 UTC|newest]
Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-02-17 2:56 [patch v2] GDB/GDBserver talks with agents Yao Qi
2012-02-17 2:56 ` [PATCH 1/9] move agent related code from gdbserver to common/agent.c Yao Qi
2012-02-17 9:55 ` Mark Kettenis
2012-02-23 21:05 ` Pedro Alves
2012-02-23 21:17 ` Pedro Alves
2012-02-24 7:57 ` Yao Qi
2012-02-24 13:40 ` Yao Qi
2012-02-17 2:56 ` [PATCH 3/9] command set agent on and off Yao Qi
2012-02-23 21:51 ` Pedro Alves
2012-02-24 13:13 ` Yao Qi
2012-02-24 14:15 ` Pedro Alves
2012-02-17 2:57 ` [PATCH 7/9] move in_process_agent_loaded to agent_loaded_p Yao Qi
2012-02-23 22:11 ` Pedro Alves [this message]
2012-02-23 22:27 ` Pedro Alves
2012-02-17 2:57 ` [PATCH 5/9] agent capability Yao Qi
2012-02-17 2:57 ` [PATCH 4/9] agent doc Yao Qi
2012-02-17 11:38 ` Eli Zaretskii
2012-02-17 13:34 ` Yao Qi
2012-02-17 16:10 ` Pedro Alves
2012-02-18 17:11 ` Eli Zaretskii
2012-02-20 3:56 ` Yao Qi
2012-02-20 6:22 ` Eli Zaretskii
2012-02-21 18:18 ` Pedro Alves
2012-02-22 1:56 ` Yao Qi
2012-02-22 20:41 ` Pedro Alves
2012-02-22 23:43 ` Eli Zaretskii
2012-02-27 13:23 ` Yao Qi
2012-02-17 2:57 ` [PATCH 2/9] add target_ops fields use_agent and can_use_agent Yao Qi
2012-02-17 11:40 ` Eli Zaretskii
2012-02-23 21:21 ` Pedro Alves
2012-02-24 13:01 ` Yao Qi
2012-02-24 13:05 ` Pedro Alves
2012-02-17 2:57 ` [PATCH 8/9] impl of use_agent and can_use_agent in linux-nat Yao Qi
2012-02-23 22:15 ` Pedro Alves
2012-02-24 8:34 ` Yao Qi
2012-02-24 10:47 ` Pedro Alves
2012-02-24 13:31 ` Yao Qi
2012-02-24 14:10 ` Pedro Alves
2012-02-24 14:34 ` Yao Qi
2012-02-17 2:57 ` [PATCH 6/9] agent capability of static tracepoint Yao Qi
2012-02-23 22:06 ` Pedro Alves
2012-02-24 13:19 ` Yao Qi
2012-02-24 14:25 ` Pedro Alves
2012-02-17 3:02 ` [PATCH 9/9] info static tracepoint in linux-nat Yao Qi
2012-02-24 13:37 ` Yao Qi
2012-02-17 4:02 ` [patch v2] GDB/GDBserver talks with agents Yao Qi
2012-02-17 12:03 ` Eli Zaretskii
2012-02-27 9:43 ` Yao Qi
2012-02-27 18:04 ` Eli Zaretskii
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=4F46B852.2060502@redhat.com \
--to=palves@redhat.com \
--cc=gdb-patches@sourceware.org \
--cc=yao@codesourcery.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