[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
hui_zhu@mentor.com
Tue Jan 31 13:06:00 GMT 2012
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
>
More information about the Gdb-patches
mailing list