[patch] Turn on dbg msg of IPA along with gdbserver's

[patch] Turn on dbg msg of IPA along with gdbserver's

[Yao Qi]

[-- Attachment #1: Type: text/plain, Size: 586 bytes --]

Hi,
We are still unable to change IPA's debug flag `debug_threads', because
it is within inferior's address space.  Since gdbserver has been a good
mechanism to access gdb_agent_XXX variables in inferior's space, we can
do similar thing to turn on IPA's debug flag.

This patch renames debug flag `debug_threads' to `gdb_agent_debug', and
let gdbserver to set it if `debug_threads' (in gdbserver's space) is
true.  For the user's perspective, if gdbserver option --debug is on,
IPA's debugging message comes into the gdbserver's debug message.

OK for mainline?

-- 
Yao (齐尧)

[-- Attachment #2: 0003-set-debug-flag-for-ipa.patch --]
[-- Type: text/x-patch, Size: 1962 bytes --]

2012-01-04  Yao Qi  <yao@codesourcery.com>

	* tracepoint.c [IN_PROCESS_AGENT] (debug_threads): Macro
	defined.
	(struct ipa_sym_addresses): New field `addr_debug'.
	(symbol_list): New element IPA_SYM(debug).
	(tracepoint_look_up_symbols): Set `gdb_agent_debug'
	if `debug_threads' is true.
	[IN_PROCESS_AGENT] (gdb_agent_debug): New global
	variable.
---
 gdb/gdbserver/tracepoint.c |   18 ++++++++++++++++--
 1 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
index 524fa77..360c14b 100644
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -67,9 +67,13 @@ trace_vdebug (const char *fmt, ...)
   va_end (ap);
 }
 
+#ifdef IN_PROCESS_AGENT
+#define debug_threads gdb_agent_debug
+#endif
+
 #define trace_debug_1(level, fmt, args...)	\
   do {						\
-    if (level <= debug_threads)			\
+    if (level <= debug_threads)		\
       trace_vdebug ((fmt), ##args);		\
   } while (0)
 
@@ -179,6 +183,7 @@ struct ipa_sym_addresses
   CORE_ADDR addr_ust_loaded;
   CORE_ADDR addr_helper_thread_id;
   CORE_ADDR addr_cmd_buf;
+  CORE_ADDR addr_debug;
 };
 
 #define STRINGIZE_1(STR) #STR
@@ -226,6 +231,7 @@ static struct
   IPA_SYM(ust_loaded),
   IPA_SYM(helper_thread_id),
   IPA_SYM(cmd_buf),
+  IPA_SYM(debug),
 };
 
 struct ipa_sym_addresses ipa_sym_addrs;
@@ -348,6 +354,14 @@ tracepoint_look_up_symbols (void)
     }
 
   all_tracepoint_symbols_looked_up = 1;
+
+  /* If the debug option of gdbserver is turned on, turn IPA's debug
+     option on as well.  */
+  if (debug_threads)
+    write_inferior_memory (ipa_sym_addrs.addr_debug,
+			   (unsigned char *) &debug_threads,
+			   sizeof (debug_threads));
+
 }
 
 #endif
@@ -366,7 +380,7 @@ tracepoint_look_up_symbols (void)
    GDBserver side.  */
 
 #ifdef IN_PROCESS_AGENT
-int debug_threads = 0;
+int gdb_agent_debug = 0;
 
 int
 read_inferior_memory (CORE_ADDR memaddr, unsigned char *myaddr, int len)
-- 
1.7.0.4

Re: [patch] Turn on dbg msg of IPA along with gdbserver's

[Pedro Alves]

On 01/04/2012 03:40 AM, Yao Qi wrote:
> Hi,
> We are still unable to change IPA's debug flag `debug_threads', because
> it is within inferior's address space.

Since it's in the inferior, I just do "print debug_threads = 1"
from gdb.   It won't work if the ipa is stripped though, so I won't
object to something like this.

> Since gdbserver has been a good
> mechanism to access gdb_agent_XXX variables in inferior's space, we can
> do similar thing to turn on IPA's debug flag.
>
> This patch renames debug flag `debug_threads' to `gdb_agent_debug', and
> let gdbserver to set it if `debug_threads' (in gdbserver's space) is
> true.  For the user's perspective, if gdbserver option --debug is on,
> IPA's debugging message comes into the gdbserver's debug message.
>
> OK for mainline?

Sorry, not in this form.  It's quite useful to be able to turn
gdbserver's and the IPA's debug output independently.  I'd like to
retain that possibility.

-- 
Pedro Alves

Re: [patch] Turn on dbg msg of IPA along with gdbserver's

[Yao Qi]

[-- Attachment #1: Type: text/plain, Size: 1111 bytes --]

On 01/04/2012 08:02 PM, Pedro Alves wrote:
> Since it's in the inferior, I just do "print debug_threads = 1"
> from gdb.   It won't work if the ipa is stripped though, so I won't
> object to something like this.
> 

Well, I don't mind to turn debug flag on in this way.  However,
`debug_threads' is confusing here.  How about change this flag to
`debug_agent'?

>> Since gdbserver has been a good
>> mechanism to access gdb_agent_XXX variables in inferior's space, we can
>> do similar thing to turn on IPA's debug flag.
>>
>> This patch renames debug flag `debug_threads' to `gdb_agent_debug', and
>> let gdbserver to set it if `debug_threads' (in gdbserver's space) is
>> true.  For the user's perspective, if gdbserver option --debug is on,
>> IPA's debugging message comes into the gdbserver's debug message.
>>
>> OK for mainline?
> 
> Sorry, not in this form.  It's quite useful to be able to turn
> gdbserver's and the IPA's debug output independently.  I'd like to
> retain that possibility.

That is fine to me.  It is reasonable to decouple gdbserver's and IPA's
debug output.

-- 
Yao (齐尧)

[-- Attachment #2: 0003-set-debug-flag-for-ipa.patch --]
[-- Type: text/x-patch, Size: 1014 bytes --]

2012-01-04  Yao Qi  <yao@codesourcery.com>

	* tracepoint.c [IN_PROCESS_AGENT] (debug_threads): Macro
	defined.
	[IN_PROCESS_AGENT] (debug_agent): New global variable.
---
 gdb/gdbserver/tracepoint.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/gdb/gdbserver/tracepoint.c b/gdb/gdbserver/tracepoint.c
index 524fa77..ba0bc1f 100644
--- a/gdb/gdbserver/tracepoint.c
+++ b/gdb/gdbserver/tracepoint.c
@@ -67,9 +67,13 @@ trace_vdebug (const char *fmt, ...)
   va_end (ap);
 }
 
+#ifdef IN_PROCESS_AGENT
+#define debug_threads debug_agent
+#endif
+
 #define trace_debug_1(level, fmt, args...)	\
   do {						\
-    if (level <= debug_threads)			\
+    if (level <= debug_threads)		\
       trace_vdebug ((fmt), ##args);		\
   } while (0)
 
@@ -366,7 +370,7 @@ tracepoint_look_up_symbols (void)
    GDBserver side.  */
 
 #ifdef IN_PROCESS_AGENT
-int debug_threads = 0;
+int debug_agent = 0;
 
 int
 read_inferior_memory (CORE_ADDR memaddr, unsigned char *myaddr, int len)
-- 
1.7.0.4

Re: [patch] Turn on dbg msg of IPA along with gdbserver's

[Pedro Alves]

On 01/04/2012 02:55 PM, Yao Qi wrote:
> On 01/04/2012 08:02 PM, Pedro Alves wrote:
>> Since it's in the inferior, I just do "print debug_threads = 1"
>> from gdb.   It won't work if the ipa is stripped though, so I won't
>> object to something like this.
>>
>
> Well, I don't mind to turn debug flag on in this way.  However,
> `debug_threads' is confusing here.  How about change this flag to
> `debug_agent'?

Fine with me.  `debug_threads' is confusing for gdbserver too.  It
is just historical and doesn't make sense anymore, AFAIK.  We should probably
rename it to something more generic, like `debug_gdbserver', `gdbserver_debug'
or so.

 > 2012-01-04  Yao Qi<yao@codesourcery.com>
 >
 > 	* tracepoint.c [IN_PROCESS_AGENT] (debug_threads): Macro
 > 	defined.
 > 	[IN_PROCESS_AGENT] (debug_agent): New global variable.

Okay.

-- 
Pedro Alves

[committed] : [patch] Turn on dbg msg of IPA along with gdbserver's

[Yao Qi]

On 01/05/2012 10:46 PM, Pedro Alves wrote:
>> 2012-01-04  Yao Qi<yao@codesourcery.com>
>>
>>     * tracepoint.c [IN_PROCESS_AGENT] (debug_threads): Macro
>>     defined.
>>     [IN_PROCESS_AGENT] (debug_agent): New global variable.
> 
> Okay.

Thanks for the review.  Committed.
http://sourceware.org/ml/gdb-cvs/2012-01/msg00078.html

-- 
Yao (齐尧)