Re: Enhancement - show old and new thread info when switching during debugging

pfee
2011-08-09T15:25:53Z

>> Sorry, but current_inferior_ptid for this will be even more confusing.
>> There's several current_FOO globals that represent the currently selected
>> state. And one of them is "current_inferior". Having current_inferior()
>> return one thing, and current_inferior_ptid mean another thing will be a
>> recipe for long term confusion.
>
> No problem, I'll pick another name.

I've reverted current_inferior_ptid back to previous_inferior_ptid, which
makes the patch smaller.

The new variable is now previous_selected_ptid.

>> Any chance you can convince your mailer to attach patches
>> with "Content-Type: text/x-patch" or some other text mime
>> type?
>
> I'm using Yahoo webmail, it's handling of attachments could be better.
> I'll try using a traditional email client.

I tried kmail which allows setting the content-type and content-disposition, but
the email didn't reach the gdb-patches list. I'm reverting to using webmail and
I'll put the patch as plain text below.

>> Please no leading * on every comment line. See other
>> comments, and follow the same style.
>
> Ok

Done

>
>>
>> In order for this to be accepted, it will need some
>> documentation in the manual.

Let's consider documentation once the patch is acceptable.

>>
>> I'm not sure the variable is sufficiently well defined yet.
>> What does gdb print in this case?
>>
>> (gdb) thread 2
>> (gdb) thread 3
>> (gdb) p $_prev_thread
>>
>> It feels like it should print thread 2, that is, we'd
>> define it to the thread the user last had selected,
>> and so:
>>
>> (gdb) thread $_prev_thread
>> (gdb) thread $_prev_thread
>>
>> would cycle between thread 2 and 3.

I now also set previous_selected_ptid in do_captured_thread_select(), which
results in the behaviour you've asked for.

>>
>> This definition works for non-stop mode as well.
>> Otherwise, as is, we get to define it as "the thread that
>> the user had selected the last time an execution command
>> was ran" (and the $_prev_thread is undefined/meaningless in
>> non-stop mode).
>
> I appreciate the feedback, your summary of the current operation is
> correct. I'll take a look at making $_prev_thread be influenced by the
> user entering
> "thread" commands. Since that should then give us consistent operation in
> both all-stop and non-stop modes, the additional change will be
> worthwhile.

I tested the patch in non-stop mode. $_prev_thread only gets modified when
the user switches using the "thread" command. I think that's what you're
asking for also.

Thanks,
Paul

2011-08-09 Paul Fee <pfee@talk21.com>

* inferior.h: Add $_prev_thread convenience variable.
* infrun.c: Add $_prev_thread convenience variable, set it when current
thread changes.
* thread.c: Add $_prev_thread convenience variable, set it when user
switches threads.

Index: inferior.h
===================================================================
RCS file: /cvs/src/src/gdb/inferior.h,v
retrieving revision 1.161
diff -c -p -r1.161 inferior.h
*** inferior.h 21 Jul 2011 23:46:08 -0000 1.161
--- inferior.h 9 Aug 2011 14:44:23 -0000
*************** extern const char *get_inferior_io_termi
*** 94,99 ****
--- 94,105 ----

extern ptid_t inferior_ptid;

+ /* Values move from interior_ptid to previous_inferior_ptid to
+ * previous_selected_ptid. The previous value is exposed to the
+ * user through the $_prev_thread convenience variable.
+ */
+ extern ptid_t previous_selected_ptid;
+
/* Are we simulating synchronous execution? This is used in async gdb
to implement the 'run', 'continue' etc commands, which will not
redisplay the prompt until the execution is actually over. */
*************** extern int stop_on_solib_events;
*** 204,209 ****
--- 210,218 ----

extern void start_remote (int from_tty);

+ extern struct value *prev_thread_id_make_value (struct gdbarch *,
+ struct internalvar *);
+
extern void normal_stop (void);

extern int signal_stop_state (int);
Index: infrun.c
===================================================================
RCS file: /cvs/src/src/gdb/infrun.c,v
retrieving revision 1.498
diff -c -p -r1.498 infrun.c
*** infrun.c 4 Aug 2011 19:10:12 -0000 1.498
--- infrun.c 9 Aug 2011 14:44:29 -0000
*************** int sync_execution = 0;
*** 127,132 ****
--- 127,138 ----

static ptid_t previous_inferior_ptid;

+ /* Values move from interior_ptid to previous_inferior_ptid to
+ previous_selected_ptid. The previous selected value is exposed
+ to the user through the $_prev_thread convenience variable. */
+
+ ptid_t previous_selected_ptid;
+
/* Default behavior is to detach newly forked processes (legacy). */
int detach_fork = 1;

*************** print_no_history_reason (void)
*** 5730,5735 ****
--- 5736,5753 ----
ui_out_text (current_uiout, "\nNo more reverse-execution history.\n");
}

+ /* Return the previous thread's id. Return a value of 0 if
+ no previous thread was selected, or it doesn't exist. */
+
+ struct value *
+ prev_thread_id_make_value (struct gdbarch *gdbarch, struct internalvar *var)
+ {
+ struct thread_info *tp = find_thread_ptid (previous_selected_ptid);
+
+ return value_from_longest (builtin_type (gdbarch)->builtin_int,
+ (tp ? tp->num : 0));
+ }
+
/* Here to return control to GDB when the inferior stops for real.
Print appropriate messages, remove breakpoints, give terminal our modes.

*************** normal_stop (void)
*** 5777,5784 ****
{
target_terminal_ours_for_output ();
printf_filtered (_("[Switching to %s]\n"),
! target_pid_to_str (inferior_ptid));
annotate_thread_changed ();
previous_inferior_ptid = inferior_ptid;
}

--- 5795,5803 ----
{
target_terminal_ours_for_output ();
printf_filtered (_("[Switching to %s]\n"),
! target_pid_to_str (inferior_ptid));
annotate_thread_changed ();
+ previous_selected_ptid = previous_inferior_ptid;
previous_inferior_ptid = inferior_ptid;
}

Index: thread.c
===================================================================
RCS file: /cvs/src/src/gdb/thread.c,v
retrieving revision 1.142
diff -c -p -r1.142 thread.c
*** thread.c 4 Aug 2011 19:10:13 -0000 1.142
--- thread.c 9 Aug 2011 14:44:30 -0000
*************** do_captured_thread_select (struct ui_out
*** 1394,1399 ****
--- 1394,1400 ----
if (!thread_alive (tp))
error (_("Thread ID %d has terminated."), num);

+ previous_selected_ptid = inferior_ptid;
switch_to_thread (tp->ptid);

annotate_thread_changed ();
*************** Show printing of thread events (such as
*** 1498,1501 ****
--- 1499,1503 ----
&setprintlist, &showprintlist);

create_internalvar_type_lazy ("_thread", thread_id_make_value);
+ create_internalvar_type_lazy ("_prev_thread", prev_thread_id_make_value);
}