[PATCH] Fix error when gdb connect to a stub that tracepoint is running[1/2] reset current_trace_status in the begin of remote_start_remote

[PATCH] Fix error when gdb connect to a stub that tracepoint is running[1/2] reset current_trace_status in the begin of remote_start_remote

[Hui Zhu]

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

Hi,

For the re-insert breakpoint, the stack dump is:
#0  remote_download_tracepoint (loc=0x35040b0) at 
../../src/gdb/remote.c:9982
#1  0x00000000006727c1 in download_tracepoint_locations () at 
../../src/gdb/breakpoint.c:10670
#2  0x0000000000673111 in update_global_location_list (should_insert=1) 
at ../../src/gdb/breakpoint.c:11021
#3  0x0000000000663752 in create_overlay_event_breakpoint () at 
../../src/gdb/breakpoint.c:2322
#4  0x0000000000675c9f in breakpoint_re_set () at 
../../src/gdb/breakpoint.c:12621
#5  0x00000000007efe65 in solib_add (pattern=0x0, from_tty=1, 
target=0x1d1e860, readsyms=1)
     at ../../src/gdb/solib.c:928
#6  0x000000000057b452 in enable_break (info=0x3307890, from_tty=1) at 
../../src/gdb/solib-svr4.c:1624
#7  0x000000000057c428 in svr4_solib_create_inferior_hook (from_tty=1) 
at ../../src/gdb/solib-svr4.c:2234
#8  0x00000000007f04a9 in solib_create_inferior_hook (from_tty=1) at 
../../src/gdb/solib.c:1172
#9  0x00000000006cc8e5 in post_create_inferior (target=0x1d1e860, 
from_tty=1) at ../../src/gdb/infcmd.c:431
#10 0x00000000006d479d in start_remote (from_tty=1) at 
../../src/gdb/infrun.c:2309
#11 0x00000000005d80c9 in remote_start_remote (from_tty=1, 
target=0x1cefce0, extended_p=0)
     at ../../src/gdb/remote.c:3367

About this patch, what I do is:
1. If we try to connect an new stub, reset current_trace_status to unknown.
2. Change remote_can_download_tracepoint to if the current_trace_status 
is unknown, return false.
After this patch, the tracepoint insered after:
   /* Possibly the target has been engaged in a trace run started
      previously; find out where things are at.  */
   if (remote_get_trace_status (current_trace_status ()) != -1)
     {
I think that will make us handle tracepoint issue more easy.

2012-01-31  Hui Zhu  <hui_zhu@mentor.com>

	* remote.c (remote_start_remote): Set running_known of
	current_trace_status to 0 in the begin of this function.
	(remote_can_download_tracepoint): If running_known of
	current_trace_status is 0, return false.

Thanks,
Hui

[-- Attachment #2: reset-trace-status.txt --]
[-- Type: text/plain, Size: 1301 bytes --]

---
 remote.c |   13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

--- a/remote.c
+++ b/remote.c
@@ -3203,6 +3203,7 @@ remote_start_remote (int from_tty, struc
   struct remote_state *rs = get_remote_state ();
   struct packet_config *noack_config;
   char *wait_status = NULL;
+  struct trace_status *ts = current_trace_status ();
 
   immediate_quit++;		/* Allow user to interrupt it.  */
 
@@ -3212,6 +3213,9 @@ remote_start_remote (int from_tty, struc
   /* Ack any packet which the remote side has already sent.  */
   serial_write (remote_desc, "+", 1);
 
+  /* Clear current status of trace.  */
+  ts->running_known = 0;
+
   /* The first packet we send to the target is the optional "supported
      packets" request.  If the target can answer this, it will tell us
      which later probes to skip.  */
@@ -10122,9 +10126,14 @@ static int
 remote_can_download_tracepoint (void)
 {
   struct trace_status *ts = current_trace_status ();
-  int status = remote_get_trace_status (ts);
+  int status;
 
-  if (status == -1 || !ts->running_known || !ts->running)
+  if (!ts->running_known)
+    return 0;
+
+  status = remote_get_trace_status (ts);
+
+  if (status == -1 || !ts->running)
     return 0;
 
   /* If we are in a tracing experiment, but remote stub doesn't support

Re: [PATCH] Fix error when gdb connect to a stub that tracepoint is running[1/2] reset current_trace_status in the begin of remote_start_remote

[Yao Qi]

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

On 01/31/2012 10:04 AM, Hui Zhu wrote:
> #9  0x00000000006cc8e5 in post_create_inferior (target=0x1d1e860,
> from_tty=1) at ../../src/gdb/infcmd.c:431
> #10 0x00000000006d479d in start_remote (from_tty=1) at
> ../../src/gdb/infrun.c:2309
> #11 0x00000000005d80c9 in remote_start_remote (from_tty=1,
> target=0x1cefce0, extended_p=0)
>     at ../../src/gdb/remote.c:3367
> 
> About this patch, what I do is:
> 1. If we try to connect an new stub, reset current_trace_status to unknown.
> 2. Change remote_can_download_tracepoint to if the current_trace_status
> is unknown, return false.
> After this patch, the tracepoint insered after:
>   /* Possibly the target has been engaged in a trace run started
>      previously; find out where things are at.  */
>   if (remote_get_trace_status (current_trace_status ()) != -1)
>     {
> I think that will make us handle tracepoint issue more easy.

It is caused by setting `inserted' flag improperly, but I have some
different understanding on the cause of improper-set-of-inserted-flag.

As you posted, the call stack is,

remote_start_remote
        |
        +--> start_remote
        |          |
        |          `--> .... [1]
        `--> merge_uploaded_tracepoints

Error occurs in the callees of [1].  In fact, the `inserted' flag will
be set in merge_uploaded_tracepoints, if it has been in remote target
(See merge_uploaded_tracepoints).  The intention of these bits is to
mark tracepoint bp_locations as `inserted' to avoid inserting them
again.  The root cause is that we use the `inserted' flag (in calleees
of [1]) prior to setting it properly (in merge_uploaded_tracepoints), so
the fix to this problem is moving merge_uploaded_tracepoints prior to
start_remote.  I drafted a patch in this way, and fixed the problem you
posted.

Patch attached is used to illustrate my thought to fix this problem.  I
am not confident on it because I don't know it is correct to change the
order of function calls in remote_start_remote.  The "non stop" path in
remote_start_remote is not affected by this patch.  In the "stop" path,
the order of some functions call is changed, but don't know they are
equivalent.

   Original                            Patched

   start_remote                  merge_uploaded_tracepoints
   remote_check_symbols          start_remote
   merge_uploaded_tracepoints    remote_check_symbols

-- 
Yao (齐尧)

[-- Attachment #2: 0007-upload-merge-prior-use.patch --]
[-- Type: text/x-patch, Size: 3066 bytes --]


	* remote.c (remote_upload_merge_tp_tsv): New.
	(remote_start_remote):  Move some bits to
	remote_upload_merge_tp_tsv and call it.
---
 gdb/remote.c |   52 ++++++++++++++++++++++++++++++----------------------
 1 files changed, 30 insertions(+), 22 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index 60d7ecd..090f68c 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -3150,6 +3150,32 @@ send_interrupt_sequence (void)
 }
 
 static void
+remote_upload_merge_tp_tsv (void)
+{
+  /* Possibly the target has been engaged in a trace run started
+     previously; find out where things are at.  */
+  if (remote_get_trace_status (current_trace_status ()) != -1)
+    {
+      struct uploaded_tp *uploaded_tps = NULL;
+      struct uploaded_tsv *uploaded_tsvs = NULL;
+
+      if (current_trace_status ()->running)
+	printf_filtered (_("Trace is already running on the target.\n"));
+
+      /* Get trace state variables first, they may be checked when
+	 parsing uploaded commands.  */
+
+      remote_upload_trace_state_variables (&uploaded_tsvs);
+
+      merge_uploaded_trace_state_variables (&uploaded_tsvs);
+
+      remote_upload_tracepoints (&uploaded_tps);
+
+      merge_uploaded_tracepoints (&uploaded_tps);
+    }
+}
+
+static void
 remote_start_remote (int from_tty, struct target_ops *target, int extended_p)
 {
   struct remote_state *rs = get_remote_state ();
@@ -3313,6 +3339,8 @@ remote_start_remote (int from_tty, struct target_ops *target, int extended_p)
 	  target_find_description ();
 	}
 
+      remote_upload_merge_tp_tsv ();
+
       /* Use the previously fetched status.  */
       gdb_assert (wait_status != NULL);
       strcpy (rs->buf, wait_status);
@@ -3387,6 +3415,8 @@ remote_start_remote (int from_tty, struct target_ops *target, int extended_p)
 
       /* Report all signals during attach/startup.  */
       remote_pass_signals (0, NULL);
+
+      remote_upload_merge_tp_tsv ();
     }
 
   /* If we connected to a live target, do some additional setup.  */
@@ -3396,28 +3426,6 @@ remote_start_remote (int from_tty, struct target_ops *target, int extended_p)
 	remote_check_symbols (symfile_objfile);
     }
 
-  /* Possibly the target has been engaged in a trace run started
-     previously; find out where things are at.  */
-  if (remote_get_trace_status (current_trace_status ()) != -1)
-    {
-      struct uploaded_tp *uploaded_tps = NULL;
-      struct uploaded_tsv *uploaded_tsvs = NULL;
-
-      if (current_trace_status ()->running)
-	printf_filtered (_("Trace is already running on the target.\n"));
-
-      /* Get trace state variables first, they may be checked when
-	 parsing uploaded commands.  */
-
-      remote_upload_trace_state_variables (&uploaded_tsvs);
-
-      merge_uploaded_trace_state_variables (&uploaded_tsvs);
-
-      remote_upload_tracepoints (&uploaded_tps);
-
-      merge_uploaded_tracepoints (&uploaded_tps);
-    }
-
   /* If breakpoints are global, insert them now.  */
   if (gdbarch_has_global_breakpoints (target_gdbarch)
       && breakpoints_always_inserted_mode ())
-- 
1.7.0.4

Re: [PATCH] Fix error when gdb connect to a stub that tracepoint is running[1/2] reset current_trace_status in the begin of remote_start_remote

[Hui Zhu]

Hi Yao,

Thanks for you review.
I have 2 ideas with your mail:
1. If we want move merge_uploaded_tracepoints (&uploaded_tps); to the 
begin, I think all the code about tracepoint should move to the begin of 
this function.

   /* Possibly the target has been engaged in a trace run started
      previously; find out where things are at.  */
   if (remote_get_trace_status (current_trace_status ()) != -1)
     {
       struct uploaded_tp *uploaded_tps = NULL;
       struct uploaded_tsv *uploaded_tsvs = NULL;

       if (current_trace_status ()->running)
	printf_filtered (_("Trace is already running on the target.\n"));

       /* Get trace state variables first, they may be checked when
	 parsing uploaded commands.  */

       remote_upload_trace_state_variables (&uploaded_tsvs);

       merge_uploaded_trace_state_variables (&uploaded_tsvs);

       remote_upload_tracepoints (&uploaded_tps);

       merge_uploaded_tracepoints (&uploaded_tps);
     }

2.  But I think move the code before start_remote is not very well.
Because the code before tracepoint code do a lot of init work, maybe 
move it will bring more issues to current code.

Best,
Hui

On 01/31/12 16:22, Yao Qi wrote:
> On 01/31/2012 10:04 AM, Hui Zhu wrote:
>> #9  0x00000000006cc8e5 in post_create_inferior (target=0x1d1e860,
>> from_tty=1) at ../../src/gdb/infcmd.c:431
>> #10 0x00000000006d479d in start_remote (from_tty=1) at
>> ../../src/gdb/infrun.c:2309
>> #11 0x00000000005d80c9 in remote_start_remote (from_tty=1,
>> target=0x1cefce0, extended_p=0)
>>      at ../../src/gdb/remote.c:3367
>>
>> About this patch, what I do is:
>> 1. If we try to connect an new stub, reset current_trace_status to unknown.
>> 2. Change remote_can_download_tracepoint to if the current_trace_status
>> is unknown, return false.
>> After this patch, the tracepoint insered after:
>>    /* Possibly the target has been engaged in a trace run started
>>       previously; find out where things are at.  */
>>    if (remote_get_trace_status (current_trace_status ()) != -1)
>>      {
>> I think that will make us handle tracepoint issue more easy.
>
> It is caused by setting `inserted' flag improperly, but I have some
> different understanding on the cause of improper-set-of-inserted-flag.
>
> As you posted, the call stack is,
>
> remote_start_remote
>          |
>          +-->  start_remote
>          |          |
>          |          `-->  .... [1]
>          `-->  merge_uploaded_tracepoints
>
> Error occurs in the callees of [1].  In fact, the `inserted' flag will
> be set in merge_uploaded_tracepoints, if it has been in remote target
> (See merge_uploaded_tracepoints).  The intention of these bits is to
> mark tracepoint bp_locations as `inserted' to avoid inserting them
> again.  The root cause is that we use the `inserted' flag (in calleees
> of [1]) prior to setting it properly (in merge_uploaded_tracepoints), so
> the fix to this problem is moving merge_uploaded_tracepoints prior to
> start_remote.  I drafted a patch in this way, and fixed the problem you
> posted.
>
> Patch attached is used to illustrate my thought to fix this problem.  I
> am not confident on it because I don't know it is correct to change the
> order of function calls in remote_start_remote.  The "non stop" path in
> remote_start_remote is not affected by this patch.  In the "stop" path,
> the order of some functions call is changed, but don't know they are
> equivalent.
>
>     Original                            Patched
>
>     start_remote                  merge_uploaded_tracepoints
>     remote_check_symbols          start_remote
>     merge_uploaded_tracepoints    remote_check_symbols
>

Re: [PATCH] Fix error when gdb connect to a stub that tracepoint is running[1/2] reset current_trace_status in the begin of remote_start_remote

[Yao Qi]

On 01/31/2012 07:26 PM, Hui Zhu wrote:
> I have 2 ideas with your mail:
> 1. If we want move merge_uploaded_tracepoints (&uploaded_tps); to the
> begin, I think all the code about tracepoint should move to the begin of
> this function.
> 
>   /* Possibly the target has been engaged in a trace run started
>      previously; find out where things are at.  */
>   if (remote_get_trace_status (current_trace_status ()) != -1)
>     {
>       struct uploaded_tp *uploaded_tps = NULL;
>       struct uploaded_tsv *uploaded_tsvs = NULL;
> 
>       if (current_trace_status ()->running)
>     printf_filtered (_("Trace is already running on the target.\n"));
> 
>       /* Get trace state variables first, they may be checked when
>      parsing uploaded commands.  */
> 
>       remote_upload_trace_state_variables (&uploaded_tsvs);
> 
>       merge_uploaded_trace_state_variables (&uploaded_tsvs);
> 
>       remote_upload_tracepoints (&uploaded_tps);
> 
>       merge_uploaded_tracepoints (&uploaded_tps);
>     }
> 

Yes, that is what I did in my patch.  They should be moved as a whole.

> 2.  But I think move the code before start_remote is not very well.
> Because the code before tracepoint code do a lot of init work, maybe
> move it will bring more issues to current code.

Changing the order of code in a large function (remote_start_remote)
makes me feel a little uncomfortable.  I'll think of it carefully.

-- 
Yao (齐尧)

Re: [PATCH] Fix error when gdb connect to a stub that tracepoint is running[1/2] reset current_trace_status in the begin of remote_start_remote

[Hui Zhu]

Ping.

Thanks,
Hui

On 01/31/12 10:04, Hui Zhu wrote:
> Hi,
>
> For the re-insert breakpoint, the stack dump is:
> #0 remote_download_tracepoint (loc=0x35040b0) at
> ../../src/gdb/remote.c:9982
> #1 0x00000000006727c1 in download_tracepoint_locations () at
> ../../src/gdb/breakpoint.c:10670
> #2 0x0000000000673111 in update_global_location_list (should_insert=1)
> at ../../src/gdb/breakpoint.c:11021
> #3 0x0000000000663752 in create_overlay_event_breakpoint () at
> ../../src/gdb/breakpoint.c:2322
> #4 0x0000000000675c9f in breakpoint_re_set () at
> ../../src/gdb/breakpoint.c:12621
> #5 0x00000000007efe65 in solib_add (pattern=0x0, from_tty=1,
> target=0x1d1e860, readsyms=1)
> at ../../src/gdb/solib.c:928
> #6 0x000000000057b452 in enable_break (info=0x3307890, from_tty=1) at
> ../../src/gdb/solib-svr4.c:1624
> #7 0x000000000057c428 in svr4_solib_create_inferior_hook (from_tty=1) at
> ../../src/gdb/solib-svr4.c:2234
> #8 0x00000000007f04a9 in solib_create_inferior_hook (from_tty=1) at
> ../../src/gdb/solib.c:1172
> #9 0x00000000006cc8e5 in post_create_inferior (target=0x1d1e860,
> from_tty=1) at ../../src/gdb/infcmd.c:431
> #10 0x00000000006d479d in start_remote (from_tty=1) at
> ../../src/gdb/infrun.c:2309
> #11 0x00000000005d80c9 in remote_start_remote (from_tty=1,
> target=0x1cefce0, extended_p=0)
> at ../../src/gdb/remote.c:3367
>
> About this patch, what I do is:
> 1. If we try to connect an new stub, reset current_trace_status to unknown.
> 2. Change remote_can_download_tracepoint to if the current_trace_status
> is unknown, return false.
> After this patch, the tracepoint insered after:
> /* Possibly the target has been engaged in a trace run started
> previously; find out where things are at. */
> if (remote_get_trace_status (current_trace_status ()) != -1)
> {
> I think that will make us handle tracepoint issue more easy.
>
> 2012-01-31 Hui Zhu <hui_zhu@mentor.com>
>
> * remote.c (remote_start_remote): Set running_known of
> current_trace_status to 0 in the begin of this function.
> (remote_can_download_tracepoint): If running_known of
> current_trace_status is 0, return false.
>
> Thanks,
> Hui

Re: [PATCH] Fix error when gdb connect to a stub that tracepoint is running[1/2] reset current_trace_status in the begin of remote_start_remote

[Pedro Alves]

On 01/31/2012 08:22 AM, Yao Qi wrote:
> Patch attached is used to illustrate my thought to fix this problem.  I
> am not confident on it because I don't know it is correct to change the
> order of function calls in remote_start_remote.  The "non stop" path in
> remote_start_remote is not affected by this patch.  In the "stop" path,
> the order of some functions call is changed, but don't know they are
> equivalent.
> 
>    Original                            Patched
> 
>    start_remote                  merge_uploaded_tracepoints
>    remote_check_symbols          start_remote
>    merge_uploaded_tracepoints    remote_check_symbols

The tracepoints module depends on remote_check_symbols (qSymbols), in
order to detect the IPA is loaded, and so that anything related
to fast tracepoints works.  On the other hand, if there are already
fast tracepoints on the target, then the qSymbols business must have
already have been done in the previous time gdb was connected.

I don't think we're okay with this in non-stop mode though.  We should
relocate our symbols before we try to merge tracepoints.  Yet, we need to
merge tracepoints before any breakpoint re-set.  Thing is in non-stop mode,
you find new inferiors and possibly do re-sets early in remote_start_remote
(find threads -> remote_notice_new_inferior -> notice_new_inferior can do a lot
behind the scenes).

It may be simpler and safer to just have a way for update_global_location_list
to know that it shouldn't try to install tracepoints yet (cause we're still going
through startup)?

-- 
Pedro Alves

Re: [PATCH] Fix error when gdb connect to a stub that tracepoint is running[1/2] reset current_trace_status in the begin of remote_start_remote

[Hui Zhu]

Hi Pedro,

What about my patch in 
http://sourceware.org/ml/gdb-patches/2012-01/msg01007.html ?
It make tracepoint cannot be install before remote_get_trace_status.
And after remote_get_trace_status, merge_uploaded_tracepoints can handle 
tracepoint.

Best,
Hui

On 02/09/12 02:31, Pedro Alves wrote:
> On 01/31/2012 08:22 AM, Yao Qi wrote:
>> Patch attached is used to illustrate my thought to fix this problem.  I
>> am not confident on it because I don't know it is correct to change the
>> order of function calls in remote_start_remote.  The "non stop" path in
>> remote_start_remote is not affected by this patch.  In the "stop" path,
>> the order of some functions call is changed, but don't know they are
>> equivalent.
>>
>>     Original                            Patched
>>
>>     start_remote                  merge_uploaded_tracepoints
>>     remote_check_symbols          start_remote
>>     merge_uploaded_tracepoints    remote_check_symbols
>
> The tracepoints module depends on remote_check_symbols (qSymbols), in
> order to detect the IPA is loaded, and so that anything related
> to fast tracepoints works.  On the other hand, if there are already
> fast tracepoints on the target, then the qSymbols business must have
> already have been done in the previous time gdb was connected.
>
> I don't think we're okay with this in non-stop mode though.  We should
> relocate our symbols before we try to merge tracepoints.  Yet, we need to
> merge tracepoints before any breakpoint re-set.  Thing is in non-stop mode,
> you find new inferiors and possibly do re-sets early in remote_start_remote
> (find threads ->  remote_notice_new_inferior ->  notice_new_inferior can do a lot
> behind the scenes).
>
> It may be simpler and safer to just have a way for update_global_location_list
> to know that it shouldn't try to install tracepoints yet (cause we're still going
> through startup)?
>

Re: [PATCH] Fix error when gdb connect to a stub that tracepoint is running[1/2] reset current_trace_status in the begin of remote_start_remote

[Hui Zhu]

And what about this patch.  :)

http://sourceware.org/ml/gdb-patches/2012-01/msg01008.html

Thanks,
Hui

On 02/10/12 15:49, Hui Zhu wrote:
> Hi Pedro,
>
> What about my patch in
> http://sourceware.org/ml/gdb-patches/2012-01/msg01007.html ?
> It make tracepoint cannot be install before remote_get_trace_status.
> And after remote_get_trace_status, merge_uploaded_tracepoints can handle
> tracepoint.
>
> Best,
> Hui
>
> On 02/09/12 02:31, Pedro Alves wrote:
>> On 01/31/2012 08:22 AM, Yao Qi wrote:
>>> Patch attached is used to illustrate my thought to fix this problem. I
>>> am not confident on it because I don't know it is correct to change the
>>> order of function calls in remote_start_remote. The "non stop" path in
>>> remote_start_remote is not affected by this patch. In the "stop" path,
>>> the order of some functions call is changed, but don't know they are
>>> equivalent.
>>>
>>> Original Patched
>>>
>>> start_remote merge_uploaded_tracepoints
>>> remote_check_symbols start_remote
>>> merge_uploaded_tracepoints remote_check_symbols
>>
>> The tracepoints module depends on remote_check_symbols (qSymbols), in
>> order to detect the IPA is loaded, and so that anything related
>> to fast tracepoints works. On the other hand, if there are already
>> fast tracepoints on the target, then the qSymbols business must have
>> already have been done in the previous time gdb was connected.
>>
>> I don't think we're okay with this in non-stop mode though. We should
>> relocate our symbols before we try to merge tracepoints. Yet, we need to
>> merge tracepoints before any breakpoint re-set. Thing is in non-stop
>> mode,
>> you find new inferiors and possibly do re-sets early in
>> remote_start_remote
>> (find threads -> remote_notice_new_inferior -> notice_new_inferior can
>> do a lot
>> behind the scenes).
>>
>> It may be simpler and safer to just have a way for
>> update_global_location_list
>> to know that it shouldn't try to install tracepoints yet (cause we're
>> still going
>> through startup)?
>>
>

Re: [PATCH] Fix error when gdb connect to a stub that tracepoint is running[1/2] reset current_trace_status in the begin of remote_start_remote

[Hui Zhu]

Ping.

Thanks,
Hui

On 02/10/12 15:49, Hui Zhu wrote:
> Hi Pedro,
>
> What about my patch in
> http://sourceware.org/ml/gdb-patches/2012-01/msg01007.html ?
> It make tracepoint cannot be install before remote_get_trace_status.
> And after remote_get_trace_status, merge_uploaded_tracepoints can handle
> tracepoint.
>
> Best,
> Hui
>
> On 02/09/12 02:31, Pedro Alves wrote:
>> On 01/31/2012 08:22 AM, Yao Qi wrote:
>>> Patch attached is used to illustrate my thought to fix this problem. I
>>> am not confident on it because I don't know it is correct to change the
>>> order of function calls in remote_start_remote. The "non stop" path in
>>> remote_start_remote is not affected by this patch. In the "stop" path,
>>> the order of some functions call is changed, but don't know they are
>>> equivalent.
>>>
>>> Original Patched
>>>
>>> start_remote merge_uploaded_tracepoints
>>> remote_check_symbols start_remote
>>> merge_uploaded_tracepoints remote_check_symbols
>>
>> The tracepoints module depends on remote_check_symbols (qSymbols), in
>> order to detect the IPA is loaded, and so that anything related
>> to fast tracepoints works. On the other hand, if there are already
>> fast tracepoints on the target, then the qSymbols business must have
>> already have been done in the previous time gdb was connected.
>>
>> I don't think we're okay with this in non-stop mode though. We should
>> relocate our symbols before we try to merge tracepoints. Yet, we need to
>> merge tracepoints before any breakpoint re-set. Thing is in non-stop
>> mode,
>> you find new inferiors and possibly do re-sets early in
>> remote_start_remote
>> (find threads -> remote_notice_new_inferior -> notice_new_inferior can
>> do a lot
>> behind the scenes).
>>
>> It may be simpler and safer to just have a way for
>> update_global_location_list
>> to know that it shouldn't try to install tracepoints yet (cause we're
>> still going
>> through startup)?
>>
>

Re: [PATCH] Fix error when gdb connect to a stub that tracepoint is running[1/2] reset current_trace_status in the begin of remote_start_remote

[Hui Zhu]

On 02/10/12 15:54, Hui Zhu wrote:
> And what about this patch. :)
>
> http://sourceware.org/ml/gdb-patches/2012-01/msg01008.html
>
> Thanks,
> Hui
>

Ping.

Thanks,
Hui


> On 02/10/12 15:49, Hui Zhu wrote:
>> Hi Pedro,
>>
>> What about my patch in
>> http://sourceware.org/ml/gdb-patches/2012-01/msg01007.html ?
>> It make tracepoint cannot be install before remote_get_trace_status.
>> And after remote_get_trace_status, merge_uploaded_tracepoints can handle
>> tracepoint.
>>
>> Best,
>> Hui
>>
>> On 02/09/12 02:31, Pedro Alves wrote:
>>> On 01/31/2012 08:22 AM, Yao Qi wrote:
>>>> Patch attached is used to illustrate my thought to fix this problem. I
>>>> am not confident on it because I don't know it is correct to change the
>>>> order of function calls in remote_start_remote. The "non stop" path in
>>>> remote_start_remote is not affected by this patch. In the "stop" path,
>>>> the order of some functions call is changed, but don't know they are
>>>> equivalent.
>>>>
>>>> Original Patched
>>>>
>>>> start_remote merge_uploaded_tracepoints
>>>> remote_check_symbols start_remote
>>>> merge_uploaded_tracepoints remote_check_symbols
>>>
>>> The tracepoints module depends on remote_check_symbols (qSymbols), in
>>> order to detect the IPA is loaded, and so that anything related
>>> to fast tracepoints works. On the other hand, if there are already
>>> fast tracepoints on the target, then the qSymbols business must have
>>> already have been done in the previous time gdb was connected.
>>>
>>> I don't think we're okay with this in non-stop mode though. We should
>>> relocate our symbols before we try to merge tracepoints. Yet, we need to
>>> merge tracepoints before any breakpoint re-set. Thing is in non-stop
>>> mode,
>>> you find new inferiors and possibly do re-sets early in
>>> remote_start_remote
>>> (find threads -> remote_notice_new_inferior -> notice_new_inferior can
>>> do a lot
>>> behind the scenes).
>>>
>>> It may be simpler and safer to just have a way for
>>> update_global_location_list
>>> to know that it shouldn't try to install tracepoints yet (cause we're
>>> still going
>>> through startup)?
>>>
>>
>