Insufficient documentation of struct thread_info

Insufficient documentation of struct thread_info

[Eli Zaretskii]

As result of this thread:

  https://sourceware.org/ml/gdb/2015-03/msg00024.html

I looked at the above-mentioned structure and found there this:

  struct thread_info
  {
    [...]

    /* Non-zero means the thread is executing.  Note: this is different
       from saying that there is an active target and we are stopped at
       a breakpoint, for instance.  This is a real indicator whether the
       thread is off and running.  */
    int executing;

    /* Frontend view of the thread state.  Note that the THREAD_RUNNING/
       THREAD_STOPPED states are different from EXECUTING.  When the
       thread is stopped internally while handling an internal event,
       like a software single-step breakpoint, EXECUTING will be false,
       but STATE will still be THREAD_RUNNING.  */
    enum thread_state state;

I think the semantics of these two fields and the difference between
'executing' being non-zero and 'state' being THREAD_RUNNING are
insufficiently and even confusingly explained in the comments.  At
least I couldn't figure out why we set each one from different parts
of code and under different conditions.  The significance of "software
single-step breakpoint" mentioned in the comment and what exactly
counts as "internal events" is left unclear.

Could someone please clarify these comments?  TIA.

Re: Insufficient documentation of struct thread_info

[Pedro Alves]

On 06/10/2015 04:23 PM, Eli Zaretskii wrote:
> As result of this thread:
> 
>   https://sourceware.org/ml/gdb/2015-03/msg00024.html
> 
> I looked at the above-mentioned structure and found there this:
> 
>   struct thread_info
>   {
>     [...]
> 
>     /* Non-zero means the thread is executing.  Note: this is different
>        from saying that there is an active target and we are stopped at
>        a breakpoint, for instance.  This is a real indicator whether the
>        thread is off and running.  */
>     int executing;
> 
>     /* Frontend view of the thread state.  Note that the THREAD_RUNNING/
>        THREAD_STOPPED states are different from EXECUTING.  When the
>        thread is stopped internally while handling an internal event,
>        like a software single-step breakpoint, EXECUTING will be false,
>        but STATE will still be THREAD_RUNNING.  */
>     enum thread_state state;
> 
> I think the semantics of these two fields and the difference between
> 'executing' being non-zero and 'state' being THREAD_RUNNING are
> insufficiently and even confusingly explained in the comments.  At
> least I couldn't figure out why we set each one from different parts
> of code and under different conditions.  The significance of "software
> single-step breakpoint" mentioned in the comment and what exactly
> counts as "internal events" is left unclear.
> 
> Could someone please clarify these comments?  TIA.

Let's try with an example, based on "(gdb) step".

First thing GDB does after "step" is make sure that the thread isn't
already marked as THREAD_RUNNING.  If so, then error out.  That's in
ensure_not_running).   If not running yet, then the "step" goes forward.
thread->state is switches from THREAD_STOPPED to THREAD_RUNNING.  From
that point on, the user can't apply other commands to the thread
until it finishes the "step" (either PC moves to a different line
or it trips on some unconditional breakpoint, etc.).  Once the
step finishes, the thread goes back to THREAD_STOPPED.

The THREAD_RUNNING/THREAD_STOPPED states are visible
in "info threads", and the corresponding state transitions
result in emitting *running and *stopped MI events to the GUI.

Now, "step" is internally implemented as a sequence of single steps.  For
each single-step, the thread will execute one instruction, stop, execute
another instruction, stop, etc.  Each time the thread is single-stepped
it is marked as executing.  Each time it reports a stop event to infrun,
it is marked as not-executing.  The t->executing true/false transitions are
not visible to the user anywhere.  Each of the single-step stops are
considered internal events; they are internal detail.

So we put this in a flow chart where times flows downward,
it goes like:

|      |      external         |   internal     |
| time | (public/user-visible) |    state       |
|      |      state            |(thr->executing)|
|------+-----------------------+----------------|
| t0   | THREAD_STOPPED        |              0 | user does "step".
| t1   | THREAD_RUNNING        |              1 | >> proceed
| t2   | "                     |              0 | << internal single-step stop
| t3   | "                     |              1 | >> still same line, single-step
| t4   | "                     |              0 | << internal single-step stop
| t5   | "                     |              1 | >> still same line, single-step
| t6   | "                     |              0 | etc.
| t7   | "                     |              1 |
| t8   | "                     |              0 |
| t9   | "                     |              1 |
| t10  | "                     |              0 |
| t11  | "                     |              1 |
| t12  | THREAD_STOPPED        |              0 | << moved to a different line.  done.

Hope that helps.

Thanks,
Pedro Alves

Re: Insufficient documentation of struct thread_info

[Eli Zaretskii]

> Date: Mon, 15 Jun 2015 15:31:29 +0100
> From: Pedro Alves <palves@redhat.com>
> 
> Let's try with an example, based on "(gdb) step".
> [...]
> Hope that helps.

It does, thanks.  But you just gave an example, and the question is
now how to generalize it in order to have a better documentation that
doesn't stop at an example.

First, AFAIU, if the state is THREAD_STOPPED, then EXECUTING is
definitely zero, is that right?  (The converse is false.)

Next, what other situations, in addition to single-stepping, are
"internal details" that flip the EXECUTING flag, but leave the state
at THREAD_RUNNING?

And finally, why do we need to pretend on the user level that the
thread is running, when it really is stopped?  Is it just because we
don't want to allow application of commands to that thread, or are
there other reasons?

TIA