MI non-stop mode spec

MI non-stop mode spec

[Vladimir Prus]

Making good used of GDB in async mode, and especially in async non-stop
mode demands some changes in MI -- both general clarifications, and actual
work to allow most MI commands while the target is running and define
their behaviour.

I propose the changes as below. Please view this document as spec
of how MI should ideally operate. In the cases where MI currently
operates differently, there may be various transition plans, including
deciding that current frontends won't be affected by change anyway,
allowing earlier behaviour with a big warning in MI docs, or bumping
MI version and implementing new behaviour only for MI3. This spec does
not deal with such compatibility issues -- only with ideal MI 
behaviour.

MI command details
==================

Legend:
(*) Changes from current MI behaviour.
(**) Changes from original non-stop mode proposal in
http://sourceware.org/ml/gdb/2007-11/msg00198.html
-> Todo items

General clarifications
----------------------

When operating in MI mode, gdb prints a prompt if and only if it is ready to
immediately read and process the next command.  Of course, a frontend can
send a command without waiting for the prompt, and it will be eventually
processed.  However, if gdb has not printed a prompt after the previous
command yet, it means that it might take a lot time until the next command
will be executed.  The frontend may use the lack of prompt to disable
some commands, or to show "waiting for previous command to finish" 
indicator.

(*) Current MI syntax says that any result record must be followed by a
prompt. For sync targets, this is wrong -- when gdb prints ^running
and resumes the target, it does not check for input, so (gdb) is misleading.
When the target stops, the *stopped message, followed by the prompt is
printed -- and it's at this point that gdb starts to accept the input
again. So, I propose to remove the prompt right after ^running for the 
sync targets.
        
Each MI command results in either ^done, ^error, ^connected or ^running
response. The ^connected response is basically identical to ^done, 
and the naming is different for historic reasons.  All of those
except for ^running are immediately followed by prompt. The ^running
response means that the target has started running. Further events
from the target will be reported using async notifications.

The async notifications are for various interesting events that cannot
generally be reported as result of a command. For example, 
          
          =thread-created

is emitted whenever a new thread appears, and *stopped is emitted whenever
target stops for any reason (generally, a stop in the target is not
100% related to the execution command we were last doing, for example
-exec-next can cause a breakpoint to be hit in unrelated thread).

Because async notifications are not related to any given command,
they don't, generally speaking, include the token of any previous
MI command.

(*) Currently, the *stopped output does include the token of
the last command.  However, it's implemented in limited way --
if we allow any command except for -exec-interrupt in async
mode, the token printed for *stopped will be wrong. In non-stop
mode, associating *stopped with a command is just impossible.

Presently, MI spec says a command can output ^running just once.
However, it the presense of breakpoint commands, it's quite possible
that we resume one thread, hit a breakpoint, and breakpoint commands
resume all threads, or some other thread.

To handle this case we need a new async output for this case:

   *running,thread-id="xxx"

which is emitted whenever a previously stopped thread is resumed.
In case all threads are resumed, "xxx" will be "all".

Async/non-stop mode notes
-------------------------

The async mode basically means that gdb can process commands even while
the target is running.  This mode is enabled whenever the target is
async-capable.

In async mode, there are two further distinctions:
  - Should we allow an exec command while some other exec command
  is already in progress?
  - Should we stop all threads whenever an event is detected in one thread?

Those issues seem independent initially, so we might try to allow
two -exec-next at one time, and then stop all threads when any -exec-next
is finished.  However, that will require user to explicitly resume the
-exec-next that is not yet finished, which does not seem very helpful --
it's not much better than sending two -exec-next sequentally.
Therefore, for now we shall have only two modes:

  - "all-stop", when all threads are stopped if any thread stops,
  and when only one exec command is allowed to be active at any given time,
  - "non-stop", where individual threads are stopped, and several
  exec commands can be active.

Except for what is said above "all-stop" and "non-stop" async modes are
identical. 

Generally, most MI commands are allowed while a target is running, with the
following exceptions:

   - An execution command that logically applies to a single thread 
   cannot be used if the thread is already running.  Global -exec-continue is
   OK, and will resume all threads that are not presently running. In all-stop
   mode an execution command cannot be started if any thread is running.

   - Many commands in current MI operate on current thread.  In non-stop
   mode, this creates a potential for race condition -- between the
   frontend sending a command to gdb and gdb reading that command,
   gdb can detect a breakpoint and change the current thread. Furthermore, in
   case when a frontend sends a series of commands and gdb detects a thread
   stop in between, it's plain impossible to figure if the second command
   should be executed in the originally current thread, or in the thread
   were stop was detected. For that reason, in non-stop mode all MI commands
   that require a thread will require the frontend to specify the thread
   explicitly.  

          Note: stateless protocol will not only be more robust in
          non-stop mode, but actually simplify frontends. Say, in CDT
          there are several places were we switch a thread, do some operation,
          and then switch back to the original thread. Say, 
          Thread.getStackFrameCount or Target.evaluateExpressionToString.
          
   - Some data access commands are also implicitly using a specific thread --
   because they are using registers, or variables in specific thread/frame,
   or because they have to call a function in the inferior. Such data access
   commands are not allowed when the necessary thread is running. Wholesale
   update operations like -var-update will plain ignore variable objects
   that need now-running threads.

   - The commands that grab global state of the target are allowed when 
   some threads are running.  It should be understood that the result may
   be inaccurate -- in particular, the memory content can change while
   we read it, and the list of threads might not be updated until some
   event from the target arrives. In addition, some targets might not   
   even allow reading memory while all threads are running -- so memory
   access commands on such targets will require that at least one thread
   is stopped.

To simplify things, if GDB is started in MI mode, no CLI command is allowed
while the target is running, and -interpreter-exec is not allowed either.

   Note: in cases when to implement a command, GDB requires that at least
   one (or all) threads are stopped, and the command is issued when this
   condition is not met, we have two choices -- issue an error, or briefly
   stop the target, perform the operation, and resume. Clearly, doing
   that in GDB (or even in the target) will be less intrusive then doing
   that in frontend.  However, it might be still too intrusive. For now,
   we'll just emit error, should a real need arise, we can always implement
   automatic interruption of the target.   

MI commands changes
-------------------

    (**) There are two new options that a number of MI commands may take:

          --thread <id>

    option specifies the id of the thread the command should operate on.

           --global

    specifies that the command should operate on no thread, but on 
    global data.  This option is necessary to distinguish the case where
    the frontend has forgot to specify --thread, assuming that the current
    thread will be used, from the case when frontend explicitly wants
    to execute a command in global scope.  This clarify of intention
    is particularly important when the "current thread" is running.

    - Break commands. All breakpoints commands are allowed at any time
    and the changes are applied to the program immediately.
    The output of -break-list and -break-info in general should be considered
    invalid as soon as a stop in any thread is reported. The break commands
    generally don't care about current thread and frame.
    If -break-insert command specifies only line number, then it implicitly
    depends on the filename of the current thread/frame. In non-stop mode,
    the frontend is required to specify thread and frame explicitly.

    - Program context commands. Those are not allowed after the program
    has started execution.

    - Thread commands. The -thread-info command should be implemented (a
    patch is already posted).
    (**) The -thread-list-all-threads is not necessary with the current
    behaviour of -thread-info.
    (**) The -thread-select command is only allowed on the that that is
    currently stopped.  This command should not generally be used in
    non-stop mode.
   
    - Program execution. The -exec-next, -exec-step, -exec-finish, 
    -exec-until, -exec-return, -exec-step-instruction and 
    -exec-next-instruction command require --thread parameter. Also,
    those commands resume strictly the thread that is being stepped,
    equivalent to "scheduler-locking on". 
    The -exec-continue command with the --thread parameter will resume
    just one thread, whereas -exec-continue without a --thread parameter
    will resume all threads that are not presently running.

    - Stack commands. All are allowed. All require a thread parameter in
    non-stop mode.

    - Variable objects. The -var-create command requires either the
    --thread or --global option. If --thread is specified, then the
    referred thread must be stopped. With --global, 
    varobj's expression is evaluated in the global scope.
    If a variable object makes use of any local variable, the object 
    becomes bound to the
    thread where it was created.  An attempt to manipulate an
    individual variable obect when the thread it is bound to is not
    stopped results in an error. As a special exception, -var-update will
    ignore any variable object whose bound thread is not stopped.
    A variable object's expression might involve a function call.  If a
    variable object is not bound to any thread, the function call will be
    executed in any currently stopped thread.

        Note: this does allow creating a global varobj while all threads
        are running.  If the target does not allow accessing memory when
        all threads are running, such a varobj will only get any value
        at a later time.

        -> Should @ varobjs be bound to only thread, or to nothing at all.
    
    - Data manipulation. All commands are supported, except for
    data-list-changed-registers, which is presently thread-unaware and seems
    inferior to using variable objects anyway. 
    The -data-evaluate-expression and -data-list-register-values requires
    that either the --thread or the --global option be provided.

    - Symbol query commands. All are supported.
    
    - File commands. The -file-exec-and-symbols and -file-exec-file cannot
    be used when any thread is running.  The -file-list-shared-libraries
    can be used while inferior is running, but the results might be
    invalidated immediately. Same for -file-list-symbol-files. Others
    can be used without limitations.

    On some targets it might not be possible to get the list of shared 
    libraries from the target while it is running. In such cases, GDB will
    report an error.

    - Target manipulation. The -target-disconnect can be used while threads
    are running. Other (currently implemented) commands can only
    be used before we have an executable.

    - File transfer. Can be always used.

    - Misc. Case-by-case, nothing interesting.

Re: MI non-stop mode spec

[Nick Roberts]

 > (*) Current MI syntax says that any result record must be followed by a
 > prompt. For sync targets, this is wrong -- when gdb prints ^running
 > and resumes the target, it does not check for input, so (gdb) is misleading.
 > When the target stops, the *stopped message, followed by the prompt is
 > printed -- and it's at this point that gdb starts to accept the input
 > again. So, I propose to remove the prompt right after ^running for the 
 > sync targets.

It's not a prompt, just a delimiter.  For a start it has a newline after it.
Furthermore if you change the prompt with "set prompt", it doesn't change.

       
 > Each MI command results in either ^done, ^error, ^connected or ^running
 > response. The ^connected response is basically identical to ^done, 
 > and the naming is different for historic reasons.  All of those
 > except for ^running are immediately followed by prompt. The ^running
 > response means that the target has started running. Further events
 > from the target will be reported using async notifications.
 > 
 > The async notifications are for various interesting events that cannot
 > generally be reported as result of a command. For example, 
 >           
 >           =thread-created

This notification doesn't appear to be in the manual.  Why are there no
equivalent =thread-exited notifications?


 >...
 > Presently, MI spec says a command can output ^running just once.
 > However, it the presense of breakpoint commands, it's quite possible
 > that we resume one thread, hit a breakpoint, and breakpoint commands
 > resume all threads, or some other thread.
 > 
 > To handle this case we need a new async output for this case:
 > 
 >    *running,thread-id="xxx"

      ^running,thread-id="xxx" ?  ("running" isn't an out-of-bound record)


 > which is emitted whenever a previously stopped thread is resumed.
 > In case all threads are resumed, "xxx" will be "all".
 > To simplify things, if GDB is started in MI mode, no CLI command is allowed
 > while the target is running, and -interpreter-exec is not allowed either.

If you can make this work with MI commands, it should be easy to add CLI
commands to do the same thing.  I will do this.


 >... 
 >     - Thread commands. The -thread-info command should be implemented (a
 >     patch is already posted).

Notice that there is currently an inconsistency here:

  -exec-run
  ^running
  (gdb) 
  *stopped,reason="breakpoint-hit",bkptno="1",thread-id="0",...
                                              ^^^^^^^^^^^^
while

  (gdb) 
  -thread-info
  ^done,threads=[]
  (gdb) 
  -thread-info 0
  ^done,threads=[]
  (gdb) 
  b main thread 0 
  &"b main thread 0\n"
  &"Unknown thread 0.\n"
  ^error,msg="Unknown thread 0."
  (gdb) 

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: MI non-stop mode spec

[Vladimir Prus]

Nick Roberts wrote:

>  > (*) Current MI syntax says that any result record must be followed by a
>  > prompt. For sync targets, this is wrong -- when gdb prints ^running
>  > and resumes the target, it does not check for input, so (gdb) is misleading.
>  > When the target stops, the *stopped message, followed by the prompt is
>  > printed -- and it's at this point that gdb starts to accept the input
>  > again. So, I propose to remove the prompt right after ^running for the
>  > sync targets.
> 
> It's not a prompt, just a delimiter.  For a start it has a newline after it.
> Furthermore if you change the prompt with "set prompt", it doesn't change.

Let's call (gdb) a "MI prompt", then. Given that each MI output is already
terminated with a newline, (gdb) is not necessary to property parse MI
output. Then, the question is that does (gdb) mean? If it does not mean 
anything, it should be, ideally, just removed. And if it means anything,
then what? Current behaviour is not consistent, but the code suggests
that it's meant to indicate when GDB is ready for a new command. I think
such a behaviour will be useful for a frontend.


>  > Each MI command results in either ^done, ^error, ^connected or ^running
>  > response. The ^connected response is basically identical to ^done,
>  > and the naming is different for historic reasons.  All of those
>  > except for ^running are immediately followed by prompt. The ^running
>  > response means that the target has started running. Further events
>  > from the target will be reported using async notifications.
>  > 
>  > The async notifications are for various interesting events that cannot
>  > generally be reported as result of a command. For example,
>  >           
>  >           =thread-created
> 
> This notification doesn't appear to be in the manual.  

Because I'm still working for a doc patch for same.

> Why are there no 
> equivalent =thread-exited notifications?

Because it's not implemented. Note that current thread.c implementation
will only declare a thread as done when we do -thread-info (or anything
else that calls prune_threads, so the value of =thread-exited will be limited,
without some associated work on threads layer).

>  >...
>  > Presently, MI spec says a command can output ^running just once.
>  > However, it the presense of breakpoint commands, it's quite possible
>  > that we resume one thread, hit a breakpoint, and breakpoint commands
>  > resume all threads, or some other thread.
>  > 
>  > To handle this case we need a new async output for this case:
>  > 
>  >    *running,thread-id="xxx"
> 
>       ^running,thread-id="xxx" ?  ("running" isn't an out-of-bound record)

"*running" is the new async output proposed by this spec (and async-output is
a kind of out-of-bound record). We cannot use ^running, because ^running is 
emitted once for each command, and each command can resume the target several 
times, and possibly - different threads.

>  > which is emitted whenever a previously stopped thread is resumed.
>  > In case all threads are resumed, "xxx" will be "all".
>  > To simplify things, if GDB is started in MI mode, no CLI command is allowed
>  > while the target is running, and -interpreter-exec is not allowed either.
> 
> If you can make this work with MI commands, it should be easy to add CLI
> commands to do the same thing.  I will do this.

Ok. I have not fundamental objections to add CLI commands, however each
CLI command added must be individually examined and its behaviour in non-stop/async
case should be clarified -- which is what I'm trying to do for MI.

>  >...
>  >     - Thread commands. The -thread-info command should be implemented (a
>  >     patch is already posted).
> 
> Notice that there is currently an inconsistency here:
> 
>   -exec-run
>   ^running
>   (gdb)
>   *stopped,reason="breakpoint-hit",bkptno="1",thread-id="0",...
>                                               ^^^^^^^^^^^^
> while
> 
>   (gdb)
>   -thread-info
>   ^done,threads=[]
>   (gdb)
>   -thread-info 0
>   ^done,threads=[]
>   (gdb)
>   b main thread 0
>   &"b main thread 0\n"
>   &"Unknown thread 0.\n"
>   ^error,msg="Unknown thread 0."
>   (gdb)

It's everywhere. The problem is that current GDB does not agree with
the obvious fact that a single-threaded program has a single thread.
Instead, it believes such a program has zero threads. Above is just
one example of the inconsistency. Dan has just posted an in-progress 
patch that causes ptid not to change (gaining non-zero tid member) 
when program goes from ST to MT, and I have a patch to early add ptid
of the main thread to the thread table, so hopefully we'll have 
this sorted.

- Volodya

 

Re: MI non-stop mode spec

[Nick Roberts]

 > > It's not a prompt, just a delimiter.  For a start it has a newline after
 > > it.  Furthermore if you change the prompt with "set prompt", it doesn't
 > > change.
 > 
 > Let's call (gdb) a "MI prompt", then. Given that each MI output is already
 > terminated with a newline, (gdb) is not necessary to property parse MI
 > output. Then, the question is that does (gdb) mean?

Well I've not tried to parse the MI output on it's own, in earnest, yet but if
it's a delimiter then means that the parser can find the end of the output
record.

 >                                                     If it does not mean 
 > anything, it should be, ideally, just removed. And if it means anything,
 > then what? Current behaviour is not consistent, but the code suggests
 > that it's meant to indicate when GDB is ready for a new command. I think
 > such a behaviour will be useful for a frontend.

If it stays, the frontend can just use the rule that GDB is ready for a new
command after "(gdb)\n" unless it's preceded by ^running.

 > >  > Each MI command results in either ^done, ^error, ^connected or ^running
 > >  > response. The ^connected response is basically identical to ^done,
 > >  > and the naming is different for historic reasons.  All of those
 > >  > except for ^running are immediately followed by prompt. The ^running
 > >  > response means that the target has started running. Further events
 > >  > from the target will be reported using async notifications.
 > >  > 
 > >  > The async notifications are for various interesting events that cannot
 > >  > generally be reported as result of a command. For example,
 > >  >           
 > >  >           =thread-created
 > > 
 > > This notification doesn't appear to be in the manual.  
 > 
 > Because I'm still working for a doc patch for same.

According to the syntax, as above, this should be:

     =thread-created,id="3"
     (gdb)

 > > Why are there no 
 > > equivalent =thread-exited notifications?
 > 
 > Because it's not implemented. 

Does that mean that you think that =thread-created is more useful?

 > Note that current thread.c implementation will only declare a thread as done
 > when we do -thread-info (or anything else that calls prune_threads, so the
 > value of =thread-exited will be limited, without some associated work on
 > threads layer).

I'm not sure what you mean.  If I run Gdb normally with a multi-threaded
application, I get:

[New Thread -1210639472 (LWP 7235)]

when a thread is created and:

[Thread -1210639472 (LWP 7235) exited]

when it is terminated.


 > >  > Presently, MI spec says a command can output ^running just once.
 > >  > However, it the presense of breakpoint commands, it's quite possible
 > >  > that we resume one thread, hit a breakpoint, and breakpoint commands
 > >  > resume all threads, or some other thread.
 > >  > 
 > >  > To handle this case we need a new async output for this case:
 > >  > 
 > >  >    *running,thread-id="xxx"
 > > 
 > >       ^running,thread-id="xxx" ?  ("running" isn't an out-of-bound record)
 > 
 > "*running" is the new async output proposed by this spec (and async-output
 > is a kind of out-of-bound record). We cannot use ^running, because ^running
 > is emitted once for each command, and each command can resume the target
 > several times, and possibly - different threads.

Breakpoint command lists don't currently work in MI, so your scenario is a bit
hypothetical, but if they did then it's quite possible that we hit a breakpoint
on one thread and breakpoint commands resume all threads without async mode.

It probably doesn't matter that much if you use "*running" or "^running" and
you can probably define asynchronous in different ways but I think starting
execution and detecting stopped execution different in this respect.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: MI non-stop mode spec

[Vladimir Prus]

On Wednesday 19 March 2008 12:14:19 Nick Roberts wrote:
>  > > It's not a prompt, just a delimiter.  For a start it has a newline after
>  > > it.  Furthermore if you change the prompt with "set prompt", it doesn't
>  > > change.
>  > 
>  > Let's call (gdb) a "MI prompt", then. Given that each MI output is already
>  > terminated with a newline, (gdb) is not necessary to property parse MI
>  > output. Then, the question is that does (gdb) mean?
> 
> Well I've not tried to parse the MI output on it's own, in earnest, yet but if
> it's a delimiter then means that the parser can find the end of the output
> record.

And what's the practical value? Both KDevelop and CDT parse MI output line-by-line,
and it appears to be just enough.

>  >                                                     If it does not mean 
>  > anything, it should be, ideally, just removed. And if it means anything,
>  > then what? Current behaviour is not consistent, but the code suggests
>  > that it's meant to indicate when GDB is ready for a new command. I think
>  > such a behaviour will be useful for a frontend.
> 
> If it stays, the frontend can just use the rule that GDB is ready for a new
> command after "(gdb)\n" unless it's preceded by ^running.

Which happens to be wrong. In async mode, gdb *may* be ready for a new
command even if ^running is output. And whether it's ready or not, generally
depends on the command

>  > >  > Each MI command results in either ^done, ^error, ^connected or ^running
>  > >  > response. The ^connected response is basically identical to ^done,
>  > >  > and the naming is different for historic reasons.  All of those
>  > >  > except for ^running are immediately followed by prompt. The ^running
>  > >  > response means that the target has started running. Further events
>  > >  > from the target will be reported using async notifications.
>  > >  > 
>  > >  > The async notifications are for various interesting events that cannot
>  > >  > generally be reported as result of a command. For example,
>  > >  >           
>  > >  >           =thread-created
>  > > 
>  > > This notification doesn't appear to be in the manual.  
>  > 
>  > Because I'm still working for a doc patch for same.
> 
> According to the syntax, as above, this should be:
> 
>      =thread-created,id="3"
>      (gdb)

And what is the value of (gdb) here?
 
>  > > Why are there no 
>  > > equivalent =thread-exited notifications?
>  > 
>  > Because it's not implemented. 
> 
> Does that mean that you think that =thread-created is more useful?

It's easy to implement.
 
>  > Note that current thread.c implementation will only declare a thread as done
>  > when we do -thread-info (or anything else that calls prune_threads, so the
>  > value of =thread-exited will be limited, without some associated work on
>  > threads layer).
> 
> I'm not sure what you mean.  If I run Gdb normally with a multi-threaded
> application, I get:
> 
> [New Thread -1210639472 (LWP 7235)]
> 
> when a thread is created and:
> 
> [Thread -1210639472 (LWP 7235) exited]
> 
> when it is terminated.

At which point, and where in code is that message printed? It is
printed by linux-thread-db.c:detach_thread, so it's not good for
generic code. And generic code will hold on to thread until 
"info thread". It's not very good if you need to issue "info thread"
to get notifications about exited thread.

So, thread.c and its interaction with linux-thread-db.c have to
be fixed.

>  > >  > Presently, MI spec says a command can output ^running just once.
>  > >  > However, it the presense of breakpoint commands, it's quite possible
>  > >  > that we resume one thread, hit a breakpoint, and breakpoint commands
>  > >  > resume all threads, or some other thread.
>  > >  > 
>  > >  > To handle this case we need a new async output for this case:
>  > >  > 
>  > >  >    *running,thread-id="xxx"
>  > > 
>  > >       ^running,thread-id="xxx" ?  ("running" isn't an out-of-bound record)
>  > 
>  > "*running" is the new async output proposed by this spec (and async-output
>  > is a kind of out-of-bound record). We cannot use ^running, because ^running
>  > is emitted once for each command, and each command can resume the target
>  > several times, and possibly - different threads.
> 
> Breakpoint command lists don't currently work in MI, 

Do you suggest they should never work? They probably don't work because nobody
tried to make them -- we have a call to bpstat_do_actions in 
mi_interpreter_exec_continuation so we try to do them only for CLI commands,
and only in async mode.

> so your scenario is a bit 
> hypothetical, but if they did then it's quite possible that we hit a breakpoint
> on one thread and breakpoint commands resume all threads without async mode.

Yes, and what's wrong about that? We still need to accurately report to
the frontend which threads are running, so that the frontend does not
allow the user to mess with the currently running threads.

> It probably doesn't matter that much if you use "*running" or "^running" and
> you can probably define asynchronous in different ways but I think starting
> execution and detecting stopped execution different in this respect.

I have a simple goal -- at any point, the frontend should know exactly:

1. Which threads are there.
2. Which threads are running or not

The second requires that we always report when currently-stopped thread
gets running, and this might require more than one notification. Since ^running
is now specified as emitted once we have the following choices:

1. Say that ^running can be emitted several time. But then, why can't be allow
^done to be emitted many times? And if we do, the point of ^ responses disppear.
2. Say that ^running is bad design to begin with. Replace ^running with *running.
Declare that -exec commands result in ^done.
3. Leave ^running along. Add *running that naturally can be printed several
times.

I personally prefer (2), but it's more drastic change for frontends, so
I've suggested (3) for now.

- Volodya

Re: MI non-stop mode spec

[Bob Rossi]

On Wed, Mar 19, 2008 at 12:16:01AM +0300, Vladimir Prus wrote:
> 
> Making good used of GDB in async mode, and especially in async non-stop
> mode demands some changes in MI -- both general clarifications, and actual
> work to allow most MI commands while the target is running and define
> their behaviour.

Do you mind posting an updated grammar for the GDB/MI changes that you
are making? or at least just a diff of it?

I currently maintain a gdb/mi bison parser that i have not put into 
production use yet. However, the time for this is coming, I will 
probably start working on this again after all of these changes get 
through.

At one point, I would parse all of the output of gdb/mi that came
from running 'make check' in the gdb.mi testsuite. If you would like,
I could do that again after your patch, and let you know where it
breaks.

Thanks,
Bob Rossi

Re: MI non-stop mode spec

[Bob Rossi]

On Wed, Mar 19, 2008 at 09:25:39AM +0300, Vladimir Prus wrote:
> Nick Roberts wrote:
> 
> >  > (*) Current MI syntax says that any result record must be followed by a
> >  > prompt. For sync targets, this is wrong -- when gdb prints ^running
> >  > and resumes the target, it does not check for input, so (gdb) is misleading.
> >  > When the target stops, the *stopped message, followed by the prompt is
> >  > printed -- and it's at this point that gdb starts to accept the input
> >  > again. So, I propose to remove the prompt right after ^running for the
> >  > sync targets.
> > 
> > It's not a prompt, just a delimiter.  For a start it has a newline after it.
> > Furthermore if you change the prompt with "set prompt", it doesn't change.
> 
> Let's call (gdb) a "MI prompt", then. Given that each MI output is already
> terminated with a newline, (gdb) is not necessary to property parse MI
> output. Then, the question is that does (gdb) mean? If it does not mean 
> anything, it should be, ideally, just removed. And if it means anything,
> then what? Current behaviour is not consistent, but the code suggests
> that it's meant to indicate when GDB is ready for a new command. I think
> such a behaviour will be useful for a frontend.

Exactly, "(gdb)" means nothing except, 
  output ==>
      ( out-of-band-record )* [ result-record ] "(gdb)" nl
exactly that. It is the end of the output record. I can tell you that my
parser can probably handle not having (gdb) in it, because the "(gdb)"
isn't the end symbol, the newline is. In fact, look at my grammar here,
  output: 
    opt_oob_record_list opt_result_record OPEN_PAREN variable CLOSED_PAREN NEWLINE {

You can see that I could probably remove the 
  OPEN_PAREN variable CLOSED_PAREN
part, and simply look for the NEWLINE. In fact, if I remove them, I do
not get an error from bison.

So, yes, it is there for absolutely no reason. However, if you are using
a more adhoc parser than mine, I bet you might find that you just do
need that "(gdb)" there!

Bob Rossi

Re: MI non-stop mode spec

[Vladimir Prus]

On Wednesday 19 March 2008 14:09:38 Bob Rossi wrote:
> On Wed, Mar 19, 2008 at 12:16:01AM +0300, Vladimir Prus wrote:
> > 
> > Making good used of GDB in async mode, and especially in async non-stop
> > mode demands some changes in MI -- both general clarifications, and actual
> > work to allow most MI commands while the target is running and define
> > their behaviour.
> 
> Do you mind posting an updated grammar for the GDB/MI changes that you
> are making? or at least just a diff of it?

The following changes will happen:

	output ==> 
	( out-of-band-record )* [ result-record ] "(gdb)" nl

becomes:

	output ==>

		( out-of-band-record | result-record | "(gdb)" ) nl


then

	async-class ==> 
		"stopped" | others (where others will be added depending on the needs--this is still in development).

becomes:

	async-class ==> 
	 "stopped" | "running" | "thread-created" | others (where others will be added depending on the needs--this is still in development).


> I currently maintain a gdb/mi bison parser that i have not put into 
> production use yet. However, the time for this is coming, I will 
> probably start working on this again after all of these changes get 
> through.

Except for the 'output' change -- which essentially codifies that
MI output is a sequence of almost independent lines, I don't think
there are further changes to the grammar planned except for adding
new async-classes.
 
> At one point, I would parse all of the output of gdb/mi that came
> from running 'make check' in the gdb.mi testsuite. If you would like,
> I could do that again after your patch, and let you know where it
> breaks.

That would be good, thanks.

- Volodya

Re: MI non-stop mode spec

[Nick Roberts]

 > > Well I've not tried to parse the MI output on it's own, in earnest, yet
 > > but if it's a delimiter then means that the parser can find the end of the
 > > output record.
 > 
 > And what's the practical value? Both KDevelop and CDT parse MI output
 > line-by-line, and it appears to be just enough.

Maybe you're right but the "(gdb) \n" doesn't seem to cost much.

 > >  >                                                     If it does not mean 
 > >  > anything, it should be, ideally, just removed. And if it means anything,
 > >  > then what? Current behaviour is not consistent, but the code suggests
 > >  > that it's meant to indicate when GDB is ready for a new command. I think
 > >  > such a behaviour will be useful for a frontend.
 > > 
 > > If it stays, the frontend can just use the rule that GDB is ready for a new
 > > command after "(gdb)\n" unless it's preceded by ^running.
 > 
 > Which happens to be wrong. In async mode, gdb *may* be ready for a new
 > command even if ^running is output. And whether it's ready or not, generally
 > depends on the command

In async mode Gdb is pretty much always ready for a new command isn't it?

 >  > >  >...
 > > Breakpoint command lists don't currently work in MI, 
 > 
 > Do you suggest they should never work? 

No, but breaking my sentence here might make it appear that I did.

 >                                         They probably don't work because
 > nobody tried to make them -- we have a call to bpstat_do_actions in
 > mi_interpreter_exec_continuation so we try to do them only for CLI commands,
 > and only in async mode.

CLI commands like "commands" that query the user don't fit well with MI.
Apple have created an MI command called -break-commands which gets round
this problem for "commands".

 > > so your scenario is a bit hypothetical, but if they did then it's quite
 > > possible that we hit a breakpoint on one thread and breakpoint commands
 > > resume all threads without async mode.
 > 
 > Yes, and what's wrong about that? We still need to accurately report to
 > the frontend which threads are running, so that the frontend does not
 > allow the user to mess with the currently running threads.

There's nothing "wrong" about that, just that apart from adding the thread id,
I can't see much difference to the present case.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: MI non-stop mode spec

[Vladimir Prus]

On Wednesday 19 March 2008 15:00:29 Nick Roberts wrote:
>  > > Well I've not tried to parse the MI output on it's own, in earnest, yet
>  > > but if it's a delimiter then means that the parser can find the end of the
>  > > output record.
>  > 
>  > And what's the practical value? Both KDevelop and CDT parse MI output
>  > line-by-line, and it appears to be just enough.
> 
> Maybe you're right but the "(gdb) \n" doesn't seem to cost much.

It make it impossible to know if gdb is ready for input.

>  > >  >                                                     If it does not mean 
>  > >  > anything, it should be, ideally, just removed. And if it means anything,
>  > >  > then what? Current behaviour is not consistent, but the code suggests
>  > >  > that it's meant to indicate when GDB is ready for a new command. I think
>  > >  > such a behaviour will be useful for a frontend.
>  > > 
>  > > If it stays, the frontend can just use the rule that GDB is ready for a new
>  > > command after "(gdb)\n" unless it's preceded by ^running.
>  > 
>  > Which happens to be wrong. In async mode, gdb *may* be ready for a new
>  > command even if ^running is output. And whether it's ready or not, generally
>  > depends on the command
> 
> In async mode Gdb is pretty much always ready for a new command isn't it?

Not necessary. For example, presently calling of function in inferior is
not async, and making it async is fairly complex.

>  >  > >  >...
>  > > Breakpoint command lists don't currently work in MI, 
>  > 
>  > Do you suggest they should never work? 
> 
> No, but breaking my sentence here might make it appear that I did.
> 
>  >                                         They probably don't work because
>  > nobody tried to make them -- we have a call to bpstat_do_actions in
>  > mi_interpreter_exec_continuation so we try to do them only for CLI commands,
>  > and only in async mode.
> 
> CLI commands like "commands" that query the user don't fit well with MI.

Well, I can set breakpoint commands in MI just fine, it's just they
are not run.

> Apple have created an MI command called -break-commands which gets round
> this problem for "commands".

Yes, it's not very hard to grab, in fact.
 
>  > > so your scenario is a bit hypothetical, but if they did then it's quite
>  > > possible that we hit a breakpoint on one thread and breakpoint commands
>  > > resume all threads without async mode.
>  > 
>  > Yes, and what's wrong about that? We still need to accurately report to
>  > the frontend which threads are running, so that the frontend does not
>  > allow the user to mess with the currently running threads.
> 
> There's nothing "wrong" about that, just that apart from adding the thread id,
> I can't see much difference to the present case.
 
Is it either:
1. You don't believe that a single command can result in more than one resumption,
of different threads?
2. You think it's fine to emit ^running several times?

- Volodya

Re: MI non-stop mode spec

[Bob Rossi]

On Wed, Mar 19, 2008 at 02:19:39PM +0300, Vladimir Prus wrote:
> On Wednesday 19 March 2008 14:09:38 Bob Rossi wrote:
> > On Wed, Mar 19, 2008 at 12:16:01AM +0300, Vladimir Prus wrote:
> > > 
> > > Making good used of GDB in async mode, and especially in async non-stop
> > > mode demands some changes in MI -- both general clarifications, and actual
> > > work to allow most MI commands while the target is running and define
> > > their behaviour.
> > 
> > Do you mind posting an updated grammar for the GDB/MI changes that you
> > are making? or at least just a diff of it?
> 
> The following changes will happen:
> 	output ==> 
> 	( out-of-band-record )* [ result-record ] "(gdb)" nl
> becomes:
> 	output ==>
> 		( out-of-band-record | result-record | "(gdb)" ) nl

OK, something seems wrong here. 

Previously, you could have 0 or more out-of-band-records. That could be
followed by a result-record, but didn't have to be, and then the obvious
"(gdb)" nl.

Now you are saying you must have 1 out-of-band-record followed by 1 result-record. 
Is this what you intended to say? I mean, currently, how would that
support handling multiples lines of output from the inferior on stdout?

> then
> 	async-class ==> 
> 		"stopped" | others (where others will be added depending on the needs--this is still in development).
> becomes:
> 	async-class ==> 
> 	 "stopped" | "running" | "thread-created" | others (where others will be added depending on the needs--this is still in development).

OK, makes perfect sense.

> > I currently maintain a gdb/mi bison parser that i have not put into 
> > production use yet. However, the time for this is coming, I will 
> > probably start working on this again after all of these changes get 
> > through.
> 
> Except for the 'output' change -- which essentially codifies that
> MI output is a sequence of almost independent lines, I don't think
> there are further changes to the grammar planned except for adding
> new async-classes.

OK, that's great. Please think about the request I'm going to make, I
think it's very important. I think the first thing gdb should do is
output a single line (a header) that tells the version that mi is going
to use during this communication, and the current versions that the 
particular gdb supports.

The more we bump the MI revisions, the more it is going to take time for
front ends to continually start gdb, probing for the best version it 
supports.

I can submit a patch for this, if you don't feel like doing it.

> > At one point, I would parse all of the output of gdb/mi that came
> > from running 'make check' in the gdb.mi testsuite. If you would like,
> > I could do that again after your patch, and let you know where it
> > breaks.
> 
> That would be good, thanks.

Ok, I'll do that once we have resolved what you expect the grammar to
be.

Bob Rossi

Re: MI non-stop mode spec

[Vladimir Prus]

On Wednesday 19 March 2008 16:41:34 Bob Rossi wrote:
> On Wed, Mar 19, 2008 at 02:19:39PM +0300, Vladimir Prus wrote:
> > On Wednesday 19 March 2008 14:09:38 Bob Rossi wrote:
> > > On Wed, Mar 19, 2008 at 12:16:01AM +0300, Vladimir Prus wrote:
> > > > 
> > > > Making good used of GDB in async mode, and especially in async non-stop
> > > > mode demands some changes in MI -- both general clarifications, and actual
> > > > work to allow most MI commands while the target is running and define
> > > > their behaviour.
> > > 
> > > Do you mind posting an updated grammar for the GDB/MI changes that you
> > > are making? or at least just a diff of it?
> > 
> > The following changes will happen:
> > 	output ==> 
> > 	( out-of-band-record )* [ result-record ] "(gdb)" nl
> > becomes:
> > 	output ==>
> > 		( out-of-band-record | result-record | "(gdb)" ) nl
> 
> OK, something seems wrong here. 
> 
> Previously, you could have 0 or more out-of-band-records. That could be
> followed by a result-record, but didn't have to be, and then the obvious
> "(gdb)" nl.
> 
> Now you are saying you must have 1 out-of-band-record followed by 1 result-record. 
> Is this what you intended to say? I mean, currently, how would that
> support handling multiples lines of output from the inferior on stdout?

The above grammar say that gdb outputs either:

1. out-of-band record,
2. result record
3. prompt

and that any of those is followed by a newline. So, you can
have 100 out-of-band-records, followed by result record, followed by
prompt, followed by some more out-of-band records.

The 'output' nonterminal does not describe all the output
that gdb might produce during entire run, it describes a single line
that gdb might output.

> 
> > then
> > 	async-class ==> 
> > 		"stopped" | others (where others will be added depending on the needs--this is still in development).
> > becomes:
> > 	async-class ==> 
> > 	 "stopped" | "running" | "thread-created" | others (where others will be added depending on the needs--this is still in development).
> 
> OK, makes perfect sense.
> 
> > > I currently maintain a gdb/mi bison parser that i have not put into 
> > > production use yet. However, the time for this is coming, I will 
> > > probably start working on this again after all of these changes get 
> > > through.
> > 
> > Except for the 'output' change -- which essentially codifies that
> > MI output is a sequence of almost independent lines, I don't think
> > there are further changes to the grammar planned except for adding
> > new async-classes.
> 
> OK, that's great. Please think about the request I'm going to make, I
> think it's very important. I think the first thing gdb should do is
> output a single line (a header) that tells the version that mi is going
> to use during this communication, and the current versions that the 
> particular gdb supports.
> 
> The more we bump the MI revisions, the more it is going to take time for
> front ends to continually start gdb, probing for the best version it 
> supports.
> 
> I can submit a patch for this, if you don't feel like doing it.

We should see what the final changes will be. Probably, we can announce
them using the existing -list-features command and then add a mechanism
to enable new behaviour with a MI command.

Of course, if that proves unsufficient, we can implement a command
to query the available MI versions, and switch to the desired one.

- Volodya

Re: MI non-stop mode spec

[Bob Rossi]

On Wed, Mar 19, 2008 at 04:50:27PM +0300, Vladimir Prus wrote:
> > > The following changes will happen:
> > > 	output ==> 
> > > 	( out-of-band-record )* [ result-record ] "(gdb)" nl
> > > becomes:
> > > 	output ==>
> > > 		( out-of-band-record | result-record | "(gdb)" ) nl
> > 

> The above grammar say that gdb outputs either:
> 
> 1. out-of-band record,
> 2. result record
> 3. prompt
> 
> and that any of those is followed by a newline. So, you can
> have 100 out-of-band-records, followed by result record, followed by
> prompt, followed by some more out-of-band records.
> 
> The 'output' nonterminal does not describe all the output
> that gdb might produce during entire run, it describes a single line
> that gdb might output.

Hmm, that's interesting. So you are suggesting that output will be 
called over and over, instead of once. Am I correct?

Is it no longer possible to add a top level rule?
  ie. 
    output ==> output_line* (some terminator)

    output_line ==> 
	( out-of-band-record | result-record | "(gdb)" ) nl

I'm not sure how to rationalize about getting line after line, with no
final response. Perhaps I need to entirely rethink what gdb/mi is, and
how to model an api on top of it. Could you give me some pointers?

> > OK, that's great. Please think about the request I'm going to make, I
> > think it's very important. I think the first thing gdb should do is
> > output a single line (a header) that tells the version that mi is going
> > to use during this communication, and the current versions that the 
> > particular gdb supports.
> > 
> > The more we bump the MI revisions, the more it is going to take time for
> > front ends to continually start gdb, probing for the best version it 
> > supports.
> > 
> > I can submit a patch for this, if you don't feel like doing it.
> 
> We should see what the final changes will be. Probably, we can announce
> them using the existing -list-features command and then add a mechanism
> to enable new behaviour with a MI command.
> 
> Of course, if that proves unsufficient, we can implement a command
> to query the available MI versions, and switch to the desired one.

hehe. Think of it like this, xml, html, all of these other great
languages always tell you first what the protocol you are communicating
with is using. I just want mi to do the same thing. It's WAY to late in
the game to call -list-features. For all you know, the user has some
stuff in his .gdbinit that get run, and MI starts spewing out.

I'm sure we could play some games with the command line, to put our own
.gdbinit command to run first, but that just seems overly complicated.

We should, at a minimum, print out the current version of mi in a
prolouge mi statement.

Bob Rossi

Re: MI non-stop mode spec

[Vladimir Prus]

On Wednesday 19 March 2008 17:05:31 Bob Rossi wrote:
> On Wed, Mar 19, 2008 at 04:50:27PM +0300, Vladimir Prus wrote:
> > > > The following changes will happen:
> > > > 	output ==> 
> > > > 	( out-of-band-record )* [ result-record ] "(gdb)" nl
> > > > becomes:
> > > > 	output ==>
> > > > 		( out-of-band-record | result-record | "(gdb)" ) nl
> > > 
> 
> > The above grammar say that gdb outputs either:
> > 
> > 1. out-of-band record,
> > 2. result record
> > 3. prompt
> > 
> > and that any of those is followed by a newline. So, you can
> > have 100 out-of-band-records, followed by result record, followed by
> > prompt, followed by some more out-of-band records.
> > 
> > The 'output' nonterminal does not describe all the output
> > that gdb might produce during entire run, it describes a single line
> > that gdb might output.
> 
> Hmm, that's interesting. So you are suggesting that output will be 
> called over and over, instead of once. Am I correct?

"called"? If you mean, printed, then yes. Which is also the case
today -- each (gdb) is part of new 'output' nonterminal.

> Is it no longer possible to add a top level rule?
>   ie. 
>     output ==> output_line* (some terminator)
> 
>     output_line ==> 
> 	( out-of-band-record | result-record | "(gdb)" ) nl

I probably miss something -- why would we want to add such a top-level
rule?

> I'm not sure how to rationalize about getting line after line, with no
> final response. Perhaps I need to entirely rethink what gdb/mi is, and
> how to model an api on top of it. Could you give me some pointers?

There are no pointers, I'm afraid. But basically, MI output is a number
of lines. Frontend first parses each line individually. Then
depending on what kind of line it was it can either:

1. For a result-record line, it declares that some command is done,
and invokes processing relevant to that command.
2. For out-of-band-record it updates the internal state, accordingly
to the out-of-band-record.
3. For prompt, it records that it's now OK to send new commands to GDB
(and expect GDB to process a command immediately).

> 
> > > OK, that's great. Please think about the request I'm going to make, I
> > > think it's very important. I think the first thing gdb should do is
> > > output a single line (a header) that tells the version that mi is going
> > > to use during this communication, and the current versions that the 
> > > particular gdb supports.
> > > 
> > > The more we bump the MI revisions, the more it is going to take time for
> > > front ends to continually start gdb, probing for the best version it 
> > > supports.
> > > 
> > > I can submit a patch for this, if you don't feel like doing it.
> > 
> > We should see what the final changes will be. Probably, we can announce
> > them using the existing -list-features command and then add a mechanism
> > to enable new behaviour with a MI command.
> > 
> > Of course, if that proves unsufficient, we can implement a command
> > to query the available MI versions, and switch to the desired one.
> 
> hehe. Think of it like this, xml, html, all of these other great
> languages always tell you first what the protocol you are communicating
> with is using. I just want mi to do the same thing. It's WAY to late in
> the game to call -list-features. For all you know, the user has some
> stuff in his .gdbinit that get run, and MI starts spewing out.

Sorry, what problem is here? The MI output is designed in such a way that
the frontend can, and should, ignore things it does not understand. So,
if .gdbinit causes program to run, and some async notification to be printed,
and the frontend does not understand those, well, it does not understand those.
Things only matter if we change the meaning of some of MI output in a backward
incompatible way.

> 
> I'm sure we could play some games with the command line, to put our own
> .gdbinit command to run first, but that just seems overly complicated.
> 
> We should, at a minimum, print out the current version of mi in a
> prolouge mi statement.

What should MI2 capable frontend to is GDB reports MI7?

- Volodya

Re: MI non-stop mode spec

[Michael Snyder]

>  > Note that current thread.c implementation will only declare a thread as done
>  > when we do -thread-info (or anything else that calls prune_threads, so the
>  > value of =thread-exited will be limited, without some associated work on
>  > threads layer).
> 
> I'm not sure what you mean.  If I run Gdb normally with a multi-threaded
> application, I get:
> 
> [New Thread -1210639472 (LWP 7235)]
> 
> when a thread is created and:
> 
> [Thread -1210639472 (LWP 7235) exited]
> 
> when it is terminated.

This is historical.  In the earliest gdb thread debugging
implementations, there was no notification on thread exit, 
and gdb only removed a thread from its internal list when
prune_threads was called (which was usually on the "info
threads" command).

More recently, we have SOME thread debugging interfaces that
give us a thread exit notification, but we retain the prune_threads
mechanism as insurance.  So, depending on whose threads you are
using, you may be able to detect thread exit when it happens, 
or you may not.

RE: MI non-stop mode spec

[Marc Khouzam]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; 	charset="utf-8", Size: 4098 bytes --]

Some comments from the DSF frontend point-of-view:

> So, I propose to remove the prompt right after ^running for the 
> sync targets.

DSF also ignores the "(gdb)", except at startup when it uses it
to know GDB is ready.

> (*) Currently, the *stopped output does include the token of
> the last command.  However, it's implemented in limited way --
> if we allow any command except for -exec-interrupt in async
> mode, the token printed for *stopped will be wrong. In non-stop
> mode, associating *stopped with a command is just impossible.

DSF ignores the token for all out-of-band events, so removing
it from *stopped shouldn't be a problem.


> To simplify things, if GDB is started in MI mode, no CLI command is allowed
> while the target is running, and -interpreter-exec is not allowed either.

This would mean that unless all threads are stop, the user will not be able
to use the CLI on top of MI.  It would be nice if the user could interact
with the stopped threads using the CLI, although, I agree with you, that
this needs to be done carefully.  

>    (**) There are two new options that a number of MI commands may take:
>
>          --thread <id>
>
>    option specifies the id of the thread the command should operate on.
>
>           --global
>
>    specifies that the command should operate on no thread, but on 
>    global data.  This option is necessary to distinguish the case where
>    the frontend has forgot to specify --thread, assuming that the current
>    thread will be used, from the case when frontend explicitly wants
>    to execute a command in global scope.  This clarify of intention
>    is particularly important when the "current thread" is running.

Does this mean that MI will still accept commands without --thread or --global,
and interpret them to mean 'use current thread'?
For some reason, I don't like that too much.  I think the frontend should
always use --thread or --global, so we make sure the frontend did not really
'forget' to specify the thread. (I'm not considering any backwards compatibly
issues here.)
But it would be nice to have a way to specifically indicate to use  the current
thread.  Maybe a special thread id could be used ( - or * for example).


>    - Thread commands. The -thread-info command should be implemented (a
>    patch is already posted).
>    (**) The -thread-list-all-threads is not necessary with the current
>    behaviour of -thread-info.
>    (**) The -thread-select command is only allowed on the that that is
>    currently stopped.  This command should not generally be used in
>    non-stop mode.

As suggested above, if we always use --thread or --global, then
-thread-select could be removed -entirely.  Or, it could be disabled in
non-stop mode, if it really should be kept for all-stop mode (although I
don't think it does; but again, not considering backward compatibility.)
   
>    - Program execution. The -exec-next, -exec-step, -exec-finish, 
>    -exec-until, -exec-return, -exec-step-instruction and 
>    -exec-next-instruction command require --thread parameter. Also,
>    those commands resume strictly the thread that is being stepped,
>    equivalent to "scheduler-locking on". 
>    The -exec-continue command with the --thread parameter will resume
>    just one thread, whereas -exec-continue without a --thread parameter
>    will resume all threads that are not presently running.

Again, I get the feeling we should always use --thread.  But I would
like your opinion on that.  We could have 'all' as a thread id, or use
--global for -exec-continue all threads.


>   -> Should @ varobjs be bound to only thread, or to nothing at all.

This is an interesting question.  It brings the possibility of supporting
both those options.  That would mean three types of variable objects:
1- bound to a frame
2- bound to a thread
3- not bound

DSF does not make use of @ variable objects, so I don't know what would be
more useful between #2 and #3.
    

Regards,

Marc
\x16º&ÖëzÛ«Ÿ}x×yb²Ö«r\x18\x1d

Re: MI non-stop mode spec

[Vladimir Prus]

On Thursday 20 March 2008 20:58:20 Marc Khouzam wrote:
> Some comments from the DSF frontend point-of-view:
> > So, I propose to remove the prompt right after ^running for the
> > sync targets.
>
> DSF also ignores the "(gdb)", except at startup when it uses it
> to know GDB is ready.

Ok, good.

> > (*) Currently, the *stopped output does include the token of
> > the last command.  However, it's implemented in limited way --
> > if we allow any command except for -exec-interrupt in async
> > mode, the token printed for *stopped will be wrong. In non-stop
> > mode, associating *stopped with a command is just impossible.
>
> DSF ignores the token for all out-of-band events, so removing
> it from *stopped shouldn't be a problem.

Excellent.

>
> > To simplify things, if GDB is started in MI mode, no CLI command is
> > allowed while the target is running, and -interpreter-exec is not
> > allowed either.
>
> This would mean that unless all threads are stop, the user will not
> be able to use the CLI on top of MI.  It would be nice if the user
> could interact with the stopped threads using the CLI, although, I
> agree with you, that this needs to be done carefully.

Yes, my biggest concern is that if we enable all CLI commands, user
can easily messup everything. OTOH, maybe we should just allow the
frontend to recover:

1. GDB will accept commands at all times.
2. I have a patch for -thread-info to report which threads are running,
and which are stopped. I might also add information about which command
"step/continue/finish/etc" is being run in each thread.

Then, even if user's CLI command does something bad, we should be able
to see the state of the target, stop all running threads, and get 
situation under control.

> >    (**) There are two new options that a number of MI commands may
> > take:
> >
> >          --thread <id>
> >
> >    option specifies the id of the thread the command should operate
> > on.
> >
> >           --global
> >
> >    specifies that the command should operate on no thread, but on
> >    global data.  This option is necessary to distinguish the case
> > where the frontend has forgot to specify --thread, assuming that
> > the current thread will be used, from the case when frontend
> > explicitly wants to execute a command in global scope.  This
> > clarify of intention is particularly important when the "current
> > thread" is running.
>
> Does this mean that MI will still accept commands without --thread or
> --global, and interpret them to mean 'use current thread'?
> For some reason, I don't like that too much.  I think the frontend
> should always use --thread or --global, so we make sure the frontend
> did not really 'forget' to specify the thread. (I'm not considering
> any backwards compatibly issues here.)
> But it would be nice to have a way to specifically indicate to use 
> the current thread.  Maybe a special thread id could be used ( - or *
> for example).

I'm not actually sure what's the best way here. On one hand, allowing
implicit current thread provides some backward compatibility. However,
it seems that converting either CDT or KDevelop should be relatively
easy. 

> >    - Thread commands. The -thread-info command should be
> > implemented (a patch is already posted).
> >    (**) The -thread-list-all-threads is not necessary with the
> > current behaviour of -thread-info.
> >    (**) The -thread-select command is only allowed on the that that
> > is currently stopped.  This command should not generally be used in
> > non-stop mode.
>
> As suggested above, if we always use --thread or --global, then
> -thread-select could be removed -entirely.  Or, it could be disabled
> in non-stop mode, if it really should be kept for all-stop mode
> (although I don't think it does; but again, not considering backward
> compatibility.)

Yes, removing -thread-select would be very nice.

> >    - Program execution. The -exec-next, -exec-step, -exec-finish,
> >    -exec-until, -exec-return, -exec-step-instruction and
> >    -exec-next-instruction command require --thread parameter. Also,
> >    those commands resume strictly the thread that is being stepped,
> >    equivalent to "scheduler-locking on".
> >    The -exec-continue command with the --thread parameter will
> > resume just one thread, whereas -exec-continue without a --thread
> > parameter will resume all threads that are not presently running.
>
> Again, I get the feeling we should always use --thread.  But I would
> like your opinion on that.  We could have 'all' as a thread id, or
> use --global for -exec-continue all threads.

I think that for -exec-continue, we are relatively safe, as the
current thread is relatively unimportant in its operation -- it 
basically work the same way no matter what thread is current.

> >   -> Should @ varobjs be bound to only thread, or to nothing at
> > all.
>
> This is an interesting question.  It brings the possibility of
> supporting both those options.  That would mean three types of
> variable objects: 1- bound to a frame
> 2- bound to a thread
> 3- not bound
>
> DSF does not make use of @ variable objects, so I don't know what
> would be more useful between #2 and #3.

I don't know either. The purpose of @ varobj appears to be for 
expressions that have to be reevaluated in the current scope -- the
"watch" functionality. But the meaning of current scope is a question. 
I'd presume that frontend would keep the current thread (from user 
point of view, just whatever is selected in thread list) and inside 
that the current frame, and when user selects either different thread
or different frame, watch should be updated. This suggests that @ 
varobjs should not be bound to thread. 

It should be noted that for  @ varobj to work as described, each change
of user-visible current thread/frame should result in -var-update 
command for each @ varobj, since -var-update * updates all varobj, and 
this might be slow. Maybe, we need "-var-update @" that will update
all @ varobjs.

- Volodya

RE: MI non-stop mode spec

[Nick Roberts]

 > >   -> Should @ varobjs be bound to only thread, or to nothing at all.
 > 
 > This is an interesting question.  It brings the possibility of supporting
 > both those options.  That would mean three types of variable objects:
 > 1- bound to a frame
 > 2- bound to a thread
 > 3- not bound

I think there are more than three possibilities:

1) bound to the frame in which varobj is created (*).
2) bound to the selected frame (@)
3) bound to the thread in which varobj is created and 1)
4) bound to the thread in which varobj is created and 2)
5) bound to the selected thread and 1)
6) bound to the selected thread and 2)

Maybe there are more, e.g, all threads (I've not really thought them through)

Currently only 1) works and 2) has a broken implementation.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: MI non-stop mode spec

[Vladimir Prus]

On Thursday 20 March 2008 23:53:02 Nick Roberts wrote:
>  > >   -> Should @ varobjs be bound to only thread, or to nothing at
>  > > all.
>  >
>  > This is an interesting question.  It brings the possibility of
>  > supporting both those options.  That would mean three types of
>  > variable objects: 1- bound to a frame
>  > 2- bound to a thread
>  > 3- not bound
>
> I think there are more than three possibilities:
>
> 1) bound to the frame in which varobj is created (*).
> 2) bound to the selected frame (@)
> 3) bound to the thread in which varobj is created and 1)
> 4) bound to the thread in which varobj is created and 2)
> 5) bound to the selected thread and 1)
> 6) bound to the selected thread and 2)
>
> Maybe there are more, e.g, all threads (I've not really thought them
> through)
>
> Currently only 1) works and 2) has a broken implementation.

Didn't you check in a patch to make *-varobjs be found to a thread? 
Furthermore, are (1) and (2) actually separate options? You cannot
evaluate varobj in a frame without also specifying a thread.

- Volodya

Re: MI non-stop mode spec

[Vladimir Prus]

On Wednesday 19 March 2008 00:16:01 Vladimir Prus wrote:

> MI commands changes
> -------------------
>
>     (**) There are two new options that a number of MI commands may
> take:
>
>           --thread <id>
>
>     option specifies the id of the thread the command should operate
> on.
>
>            --global
>
>     specifies that the command should operate on no thread, but on
>     global data.  This option is necessary to distinguish the case
> where the frontend has forgot to specify --thread, assuming that the
> current thread will be used, from the case when frontend explicitly
> wants to execute a command in global scope.  This clarify of
> intention is particularly important when the "current thread" is
> running.

It seems like I have over-engineered this one. The --global option
was meant so that we can evaluate expressions while the target is 
running, but:

- Generally, we cannot access target memory when all threads are
running
- GDB is full with assumptions that we have current thread and current
frame, so even if target can access memory when all threads is running,
gdb will still try to grab selected frame, and that will try to grab
target's register, and that will result in error.

So, I'd like to remove the --global option. This means that the --thread
option must be used to specify which thread to operate on, and that
thread must be stopped.

In fact, a mechanism to evaluate expression in an arbitrary scope 
(including global scope) might be actually good. Now, implementing
variable tooltips via MI is impossible, as we cannot evaluate an
expression in a given scope (defined by source line). However, that
appears to be orthogonal issue to the non-stop mode.

- Volodya



 

Re: MI non-stop mode spec

[Nick Roberts]

 > > I think there are more than three possibilities:
 > >
 > > 1) bound to the frame in which varobj is created (*).
 > > 2) bound to the selected frame (@)
 > > 3) bound to the thread in which varobj is created and 1)
 > > 4) bound to the thread in which varobj is created and 2)
 > > 5) bound to the selected thread and 1)
 > > 6) bound to the selected thread and 2)
 > >
 > > Maybe there are more, e.g, all threads (I've not really thought them
 > > through)
 > >
 > > Currently only 1) works and 2) has a broken implementation.
 > 
 > Didn't you check in a patch to make *-varobjs be found to a thread?

I submitted a patch earlier this year that stopped thinking that a variable
object had gone out of scope if the thread changed but nothing happened.

 > Furthermore, are (1) and (2) actually separate options? You cannot
 > evaluate varobj in a frame without also specifying a thread.

Hmm, perhaps I typed that too quickly, it looks like 3-6 are just
multi-threaded cases of 1 and 2, so there are four in total.

It appears that Totalview call 1) FIXED compilation scope and 2) FLOATING
compilation scope.  Gdb calls it USE_CURRENT_FRAME and USE_SELECTED_FRAME which
I find very confusing.  Particularly (as I've said before) the manual mixes the
meaning of current frame with selected frame.  With USE_SELECTED_FRAME, the
value can change without execution, e.g. after an up or down.  It would be nice
to change these enum values to USE_FIXED_FRAME and USE_FLOATING_FRAME.  WDYT?

In general I guess threads don't traverse the same frames so watch expressions
wouldn't always work for all threads. I don't know how GDB would know if they
did but I see that Totalview has something that they call a laminated view
which views variables across threads (and processes).  In fact their online
manual must be a good guideline for some of the non-stop mode spec.

Also GDB loses sense of the selected frame: if you change to a different thread
and back again you always get back to the innermost (= current) frame.  So
that makes it difficult to get USE_SELECTED_FRAME to work in the
multi-threaded case.



-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: MI non-stop mode spec

[Vladimir Prus]

On Friday 21 March 2008 12:48:15 Nick Roberts wrote:
>  > > I think there are more than three possibilities:
>  > >
>  > > 1) bound to the frame in which varobj is created (*).
>  > > 2) bound to the selected frame (@)
>  > > 3) bound to the thread in which varobj is created and 1)
>  > > 4) bound to the thread in which varobj is created and 2)
>  > > 5) bound to the selected thread and 1)
>  > > 6) bound to the selected thread and 2)
>  > >
>  > > Maybe there are more, e.g, all threads (I've not really thought
>  > > them through)
>  > >
>  > > Currently only 1) works and 2) has a broken implementation.
>  >
>  > Didn't you check in a patch to make *-varobjs be found to a
>  > thread?
>
> I submitted a patch earlier this year that stopped thinking that a
> variable object had gone out of scope if the thread changed but
> nothing happened.

Can you resend the current version of that patch, and I'll take a look.

>  > Furthermore, are (1) and (2) actually separate options? You cannot
>  > evaluate varobj in a frame without also specifying a thread.
>
> Hmm, perhaps I typed that too quickly, it looks like 3-6 are just
> multi-threaded cases of 1 and 2, so there are four in total.
>
> It appears that Totalview call 1) FIXED compilation scope and 2)
> FLOATING compilation scope.  Gdb calls it USE_CURRENT_FRAME and
> USE_SELECTED_FRAME which I find very confusing.  Particularly (as
> I've said before) the manual mixes the meaning of current frame with
> selected frame.  With USE_SELECTED_FRAME, the value can change
> without execution, e.g. after an up or down.  It would be nice to
> change these enum values to USE_FIXED_FRAME and USE_FLOATING_FRAME. 
> WDYT?

There's also USE_SPECIFIED_FRAME, to make the whole thing
funnier. Those values are only used in the call to varobj_create,
and struct varobj has a field named 'use_selected_frame'. Probably
the name of field of struct varobj is fine, whereas those USE_*
are badly named indeed. Maybe, the varobj_create interface should
be redone to accept a frame and a "floating" flag?

> In general I guess threads don't traverse the same frames so watch
> expressions wouldn't always work for all threads. 

Right, so when user switches UI to a different thread, we get to
reevaluate watches. Something like:

	-var-update --thread 2 --frame 5 @

seems like appropriate solution.

> I don't know how 
> GDB would know if they did but I see that Totalview has something
> that they call a laminated view which views variables across threads
> (and processes).  In fact their online manual must be a good
> guideline for some of the non-stop mode spec.
>
> Also GDB loses sense of the selected frame: if you change to a
> different thread and back again you always get back to the innermost
> (= current) frame.  So that makes it difficult to get
> USE_SELECTED_FRAME to work in the multi-threaded case.

I think that the syntax mentioned above will get around that. When
GDB evaluates

	-var-update --thread 2 --frame 5 @

it switches to thread (which selects frame 0) and then immediately
selects frame 5, so by the time we evaluate expression, we're
in the right thread and frame.

- Volodya

Re: MI non-stop mode spec

[Pawel Piech]

Hi Vladimir,
Thank you for putting together this proposal for extending the MI 
protocol to allow non-stop debugging.  In general I think the changes 
you are proposing are positive I just have two criticisms when looking 
at this proposal from the client's point of view.  BTW, I work with Marc 
on the Eclipse DSF-GDB integration.  One disclaimer I should make is 
that I'm not familiar with any of the GDB internals so I don't know what 
is easier/more difficult.  I'm only speaking from a client's point of view.

1) Changing the protocol to be (partially) stateless by adding -thread 
and -global options to multitude of commands is not backwards compatible. 

 From the clients point of view, keeping the protocol backward 
compatible is a great benefit.  I'm not a big fan of -thread-select, but 
it's there and trying to remove it now I think will cause more problems 
than it solves. 

You mentioned that when switching to non-stop mode there is an inherent 
race condition in using -thread-select because GDB may switch current 
threads due to an async stopped event.  A less jarring change to the 
protocol than adding -thread/-global to all the commands would be to 
change the behavior of *stopped not to switch the current thread.  To 
remain backward compatible with older GDB versions, i.e. to work with 
old *stopped event behavior, MI clients can make the assumption that 
after a stopped event the current thread is not known and send a 
-thread-select before sending new commands.  One objection to this 
change may be that it's not forward compatible and older clients will 
not work with the new MI protocol version, but I think older clients 
will not work with GDB in non-stop mode anyhow with the protocol changes 
that you propose, so that point should make no difference in your decision.

I don't fully understand why disabling CLI commands is desired, but I'm 
guessing it's because the CLI commands would still rely on the current 
thread.  If so, then keeping the MI protocol stateful would hopefully 
address that concern and not force you to disable the CLI interface, 
which would be unfortunate.

2) The proposal does not address multi-process debugging in any way.  I 
understand that multi-process (or multi-core) debugging is not part of 
this proposal, but I think keeping that use case in mind when making 
this change would make it less painful for the clients when we actually 
get to that point.  In context of multi-process debugging, options such 
as -global and fields such as "all" are rather ambigeous.  Instead, I 
have the following suggestion:

Assign an ID to a process in the same name space as threads.  Until 
multi-process debugging is implemented you could just reserve a thread 
id, such as "0" to be the process thread ID.  For completeness this ID 
would be returned already in the thread-id field of the response to 
-target-select command  In order to operate on a whole process context 
use a -thread-select command with the process context id (followed by 
the command).  -thread-list-ids would list threads within that process, 
normally starting with "1".  In reporting stopped events, (in non-stop 
debugging mode) if a thread has suspended, use the thread-id field to 
indicate which thread has triggered the stop.  Use a separate field: 
container-id, to indicate whether the whole process stopped, assuming 
that some events still cause all threads to stop.  In all-stop mode, the 
container-id field would always be present in the stopped event.  I 
think that making this change would allow clients to still work with 
older versions of GDB in all-stop mode, since thread 0 in all-stop mode 
would act like a container thread anyway, resuming and suspending all 
threads.


The Wind River debugger which implements the MI protocol, supprots 
multi-core/multi-process debugging, non-stop and all-stop debugging 
modes simultaneously (for different targets), uses the above protocol 
extensions for several years now rather successfully.  So I hope you 
consider these suggestions seriously even if they are not easiest to 
implement given the GDB architecture.

Cheers,
Pawel

Re: MI non-stop mode spec

[Nick Roberts]

 > >  > Didn't you check in a patch to make *-varobjs be found to a
 > >  > thread?
 > >
 > > I submitted a patch earlier this year that stopped thinking that a
 > > variable object had gone out of scope if the thread changed but
 > > nothing happened.
 > 
 > Can you resend the current version of that patch, and I'll take a look.

I used pid_to_thread_id (inferior_ptid) for the thread_id, just as
infrun.c does for *stopped.  I think that the thread_id problem for
single/multi-threaded programs should be fixed first (perhaps along the
lines Daniel suggested).


 > > Also GDB loses sense of the selected frame: if you change to a
 > > different thread and back again you always get back to the innermost
 > > (= current) frame.  So that makes it difficult to get
 > > USE_SELECTED_FRAME to work in the multi-threaded case.
 > 
 > I think that the syntax mentioned above will get around that. When
 > GDB evaluates
 > 
 > 	-var-update --thread 2 --frame 5 @
 > 
 > it switches to thread (which selects frame 0) and then immediately
 > selects frame 5, so by the time we evaluate expression, we're
 > in the right thread and frame.

Perhaps we're miscommunicating.  I mean if the user wants the variable object
to "float" with the selected frame, he can't change threads because GDB will
forget which frame was the selected frame.  In other words, I think that, in a
stateful GDB, get_selected_frame needs to be fixed.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: MI non-stop mode spec

[Vladimir Prus]

On Saturday 22 March 2008 03:32:53 Nick Roberts wrote:
>  > >  > Didn't you check in a patch to make *-varobjs be found to a
>  > >  > thread?
>  > >
>  > > I submitted a patch earlier this year that stopped thinking that
>  > > a variable object had gone out of scope if the thread changed
>  > > but nothing happened.
>  >
>  > Can you resend the current version of that patch, and I'll take a
>  > look.
>
> I used pid_to_thread_id (inferior_ptid) for the thread_id, just as
> infrun.c does for *stopped.  I think that the thread_id problem for
> single/multi-threaded programs should be fixed first (perhaps along
> the lines Daniel suggested).

Dan's patch is in, however, it only affects Linux. I suspect in might
be a long time till the problem is solved on all system. Until then,
we probably should assume that if thread list is empty, then the 
program is ST and never will become MT.

I happen to be working on variable objects for non-stop now, so I'll
play with the last version of your patch that you have posted.

>  > > Also GDB loses sense of the selected frame: if you change to a
>  > > different thread and back again you always get back to the
>  > > innermost (= current) frame.  So that makes it difficult to get
>  > > USE_SELECTED_FRAME to work in the multi-threaded case.
>  >
>  > I think that the syntax mentioned above will get around that. When
>  > GDB evaluates
>  >
>  > 	-var-update --thread 2 --frame 5 @
>  >
>  > it switches to thread (which selects frame 0) and then immediately
>  > selects frame 5, so by the time we evaluate expression, we're
>  > in the right thread and frame.
>
> Perhaps we're miscommunicating.  I mean if the user wants the
> variable object to "float" with the selected frame, he can't change
> threads because GDB will forget which frame was the selected frame. 
> In other words, I think that, in a stateful GDB, get_selected_frame
> needs to be fixed.

Maybe we're looking at it from different perspectives. I think
that frontend can, and probably should, maintain its own notion
of selected frame for each thread. Then, the @ variables can
be re-evaluated in whatever thread/frame is current from frontend
point of view.

- Volodya

Re: MI non-stop mode spec

[Nick Roberts]

 > >  > >  > Didn't you check in a patch to make *-varobjs be found to a
 > >  > >  > thread?
 > >  > >
 > >  > > I submitted a patch earlier this year that stopped thinking that
 > >  > > a variable object had gone out of scope if the thread changed
 > >  > > but nothing happened.
 > >  >
 > >  > Can you resend the current version of that patch, and I'll take a
 > >  > look.
 > >
 > > I used pid_to_thread_id (inferior_ptid) for the thread_id, just as
 > > infrun.c does for *stopped.  I think that the thread_id problem for
 > > single/multi-threaded programs should be fixed first (perhaps along
 > > the lines Daniel suggested).
 > 
 > Dan's patch is in, however, it only affects Linux. I suspect in might
 > be a long time till the problem is solved on all system. Until then,
 > we probably should assume that if thread list is empty, then the 
 > program is ST and never will become MT.
 > 
 > I happen to be working on variable objects for non-stop now, so I'll
 > play with the last version of your patch that you have posted.

I've made some changes since then so here's my latest patch updated to current
sources.  I found that after all I did need thread_id == -2 for global
variables (used in mi-cmd-var which doesn't print the thread_id field for them
since their value is the same in all threads).

-- 
Nick                                           http://www.inet.net.nz/~nickrob


2008-03-23  Nick Roberts  <nickrob@snap.net.nz>

	* thread.c (make_cleanup_restore_current_thread)
	(do_restore_current_thread_cleanup): Don't make static
	(struct current_thread_cleanup): Move to gdbthread.h

	* gdbthread.h: Declare above functions as externs here.

	* varobj.c  (struct varobj_root): New component thread_id.
	(varobj_get_thread_id, check_scope): New functions.
	(c_value_of_root): Use it to iterate over threads.

	* varobj.h (varobj_get_thread_id): New extern.

	* Makefile.in (varobj_h): Update dependencies.

	* mi/mi-cmd-var.c (print_varobj): Add thread-id field.


Index: thread.c
===================================================================
RCS file: /cvs/src/src/gdb/thread.c,v
retrieving revision 1.64
diff -p -r1.64 thread.c
*** thread.c	21 Mar 2008 15:44:53 -0000	1.64
--- thread.c	23 Mar 2008 04:36:10 -0000
*************** static void info_threads_command (char *
*** 61,68 ****
  static void thread_apply_command (char *, int);
  static void restore_current_thread (ptid_t);
  static void prune_threads (void);
- static struct cleanup *make_cleanup_restore_current_thread (ptid_t,
-                                                             struct frame_id);
  
  void
  delete_step_resume_breakpoint (void *arg)
--- 61,66 ----
*************** restore_selected_frame (struct frame_id 
*** 555,567 ****
      }
  }
  
! struct current_thread_cleanup
! {
!   ptid_t inferior_ptid;
!   struct frame_id selected_frame_id;
! };
! 
! static void
  do_restore_current_thread_cleanup (void *arg)
  {
    struct current_thread_cleanup *old = arg;
--- 553,559 ----
      }
  }
  
! void
  do_restore_current_thread_cleanup (void *arg)
  {
    struct current_thread_cleanup *old = arg;
*************** do_restore_current_thread_cleanup (void 
*** 570,576 ****
    xfree (old);
  }
  
! static struct cleanup *
  make_cleanup_restore_current_thread (ptid_t inferior_ptid, 
                                       struct frame_id a_frame_id)
  {
--- 562,568 ----
    xfree (old);
  }
  
! struct cleanup *
  make_cleanup_restore_current_thread (ptid_t inferior_ptid, 
                                       struct frame_id a_frame_id)
  {


Index: gdbthread.h
===================================================================
RCS file: /cvs/src/src/gdb/gdbthread.h,v
retrieving revision 1.21
diff -p -r1.21 gdbthread.h
*** gdbthread.h	21 Mar 2008 15:44:53 -0000	1.21
--- gdbthread.h	23 Mar 2008 04:36:10 -0000
*************** extern void load_infrun_state (ptid_t pt
*** 149,154 ****
--- 149,165 ----
  /* Switch from one thread to another.  */
  extern void switch_to_thread (ptid_t ptid);
  
+ struct current_thread_cleanup
+ {
+   ptid_t inferior_ptid;
+   struct frame_id selected_frame_id;
+ };
+ 
+ extern void do_restore_current_thread_cleanup (void *arg);
+ 
+ extern struct cleanup * make_cleanup_restore_current_thread
+                           (ptid_t inferior_ptid, struct frame_id a_frame_id);
+ 
  /* Commands with a prefix of `thread'.  */
  extern struct cmd_list_element *thread_cmd_list;
  

Index: varobj.c
===================================================================
RCS file: /cvs/src/src/gdb/varobj.c,v
retrieving revision 1.103
diff -p -r1.103 varobj.c
*** varobj.c	4 Feb 2008 07:49:04 -0000	1.103
--- varobj.c	23 Mar 2008 04:36:12 -0000
***************
*** 24,30 ****
--- 24,32 ----
  #include "language.h"
  #include "wrapper.h"
  #include "gdbcmd.h"
+ #include "gdbthread.h"
  #include "block.h"
+ #include "inferior.h"
  
  #include "gdb_assert.h"
  #include "gdb_string.h"
*************** struct varobj_root
*** 65,70 ****
--- 67,78 ----
    /* The frame for this expression */
    struct frame_id frame;
  
+   /* GDB thread id.
+      0 means that the application is single-threaded.
+     -1 means that the thread is dead.
+     -2 means that this expression is a global variable.  */
+   int thread_id;
+ 
    /* If 1, "update" always recomputes the frame & valid block
       using the currently selected frame. */
    int use_selected_frame;
*************** varobj_create (char *objname,
*** 488,493 ****
--- 496,506 ----
  
        var->format = variable_default_display (var);
        var->root->valid_block = innermost_block;
+       if (innermost_block)
+  	var->root->thread_id = pid_to_thread_id (inferior_ptid);
+       else
+  	/* Give global variables a thread_id of -2.  */
+  	var->root->thread_id = -2;
        expr_len = strlen (expression);
        var->name = savestring (expression, expr_len);
        /* For a root var, the name and the expr are the same.  */
*************** varobj_get_display_format (struct varobj
*** 689,694 ****
--- 702,713 ----
    return var->format;
  }
  
+ int
+ varobj_get_thread_id (struct varobj *var)
+ {
+   return var->root->thread_id;
+ }
+ 
  void
  varobj_set_frozen (struct varobj *var, int frozen)
  {
*************** c_path_expr_of_child (struct varobj *chi
*** 2145,2181 ****
    return child->path_expr;
  }
  
  static struct value *
  c_value_of_root (struct varobj **var_handle)
  {
    struct value *new_val = NULL;
    struct varobj *var = *var_handle;
!   struct frame_info *fi;
!   int within_scope;
  
    /*  Only root variables can be updated... */
    if (!is_root_p (var))
      /* Not a root var */
      return NULL;
  
- 
    /* Determine whether the variable is still around. */
    if (var->root->valid_block == NULL || var->root->use_selected_frame)
      within_scope = 1;
    else
      {
!       fi = frame_find_by_id (var->root->frame);
!       within_scope = fi != NULL;
!       /* FIXME: select_frame could fail */
!       if (fi)
  	{
! 	  CORE_ADDR pc = get_frame_pc (fi);
! 	  if (pc <  BLOCK_START (var->root->valid_block) ||
! 	      pc >= BLOCK_END (var->root->valid_block))
! 	    within_scope = 0;
! 	  else
! 	    select_frame (fi);
! 	}	  
      }
  
    if (within_scope)
--- 2164,2228 ----
    return child->path_expr;
  }
  
+ static int
+ check_scope (struct varobj *var)
+ {
+   struct frame_info *fi;
+   int scope;
+ 
+   fi = frame_find_by_id (var->root->frame);
+   scope = fi != NULL;
+ 
+   /* FIXME: select_frame could fail */
+   if (fi)
+     {
+       CORE_ADDR pc = get_frame_pc (fi);
+       if (pc <  BLOCK_START (var->root->valid_block) ||
+ 	  pc >= BLOCK_END (var->root->valid_block))
+ 	scope = 0;
+       else
+ 	select_frame (fi);
+     }
+   return scope;
+ }
+ 
  static struct value *
  c_value_of_root (struct varobj **var_handle)
  {
    struct value *new_val = NULL;
    struct varobj *var = *var_handle;
!   struct frame_id saved_frame_id;
!   struct cleanup *old_cleanups = NULL;
!   int within_scope, thread_id;
!   ptid_t ptid;
  
    /*  Only root variables can be updated... */
    if (!is_root_p (var))
      /* Not a root var */
      return NULL;
  
    /* Determine whether the variable is still around. */
    if (var->root->valid_block == NULL || var->root->use_selected_frame)
      within_scope = 1;
    else
      {
!       thread_id = var->root->thread_id;
!       ptid = thread_id_to_pid (thread_id);
!       if (thread_id == 0)
! 	/* Single-threaded application.  */
! 	within_scope = check_scope (var);
!       else if (thread_id != -1 && target_thread_alive (ptid))
  	{
! 
! 	  saved_frame_id = get_frame_id (get_selected_frame (NULL));
! 	  old_cleanups = make_cleanup_restore_current_thread (inferior_ptid,
! 							      saved_frame_id);
! 	  switch_to_thread (ptid);
! 	  within_scope = check_scope (var);
! 	}
!       else
! 	/* Mark it as dead.  */
! 	var->root->thread_id = -1;
      }
  
    if (within_scope)
*************** c_value_of_root (struct varobj **var_han
*** 2186,2191 ****
--- 2233,2240 ----
        return new_val;
      }
  
+   do_cleanups (old_cleanups);
+ 
    return NULL;
  }
  

Index: varobj.h
===================================================================
RCS file: /cvs/src/src/gdb/varobj.h,v
retrieving revision 1.15
diff -p -r1.15 varobj.h
*** varobj.h	30 Jan 2008 07:17:31 -0000	1.15
--- varobj.h	23 Mar 2008 04:36:12 -0000
*************** extern enum varobj_display_formats varob
*** 89,94 ****
--- 89,96 ----
  extern enum varobj_display_formats varobj_get_display_format (
  							struct varobj *var);
  
+ extern int varobj_get_thread_id (struct varobj *var);
+ 
  extern void varobj_set_frozen (struct varobj *var, int frozen);
  
  extern int varobj_get_frozen (struct varobj *var);


Index: mi/mi-cmd-var.c
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-cmd-var.c,v
retrieving revision 1.45
diff -p -r1.45 mi-cmd-var.c
*** mi/mi-cmd-var.c	30 Jan 2008 07:17:31 -0000	1.45
--- mi/mi-cmd-var.c	23 Mar 2008 04:36:15 -0000
*************** print_varobj (struct varobj *var, enum p
*** 50,55 ****
--- 50,56 ----
  {
    struct type *gdb_type;
    char *type;
+   int thread_id;
  
    ui_out_field_string (uiout, "name", varobj_get_objname (var));
    if (print_expression)
*************** print_varobj (struct varobj *var, enum p
*** 66,71 ****
--- 67,76 ----
        xfree (type);
      }
  
+   thread_id = varobj_get_thread_id (var);
+   if (thread_id > 0)
+     ui_out_field_int (uiout, "thread-id", thread_id);
+ 
    if (varobj_get_frozen (var))
      ui_out_field_int (uiout, "frozen", 1);
  }


Index: Makefile.in
===================================================================
RCS file: /cvs/src/src/gdb/Makefile.in,v
retrieving revision 1.994
diff -p -r1.994 Makefile.in
*** Makefile.in	21 Mar 2008 17:09:35 -0000	1.994
--- Makefile.in	23 Mar 2008 04:36:15 -0000
*************** user_regs_h = user-regs.h
*** 901,907 ****
  valprint_h = valprint.h
  value_h = value.h $(doublest_h) $(frame_h) $(symtab_h) $(gdbtypes_h) \
  	$(expression_h)
! varobj_h = varobj.h $(symtab_h) $(gdbtypes_h) $(vec_h)
  vax_tdep_h = vax-tdep.h
  vec_h = vec.h $(gdb_assert_h) $(gdb_string_h)
  version_h = version.h
--- 901,908 ----
  valprint_h = valprint.h
  value_h = value.h $(doublest_h) $(frame_h) $(symtab_h) $(gdbtypes_h) \
  	$(expression_h)
! varobj_h = varobj.h $(symtab_h) $(gdbtypes_h) $(vec_h) $(inferior_h) \
! 	$(gdbthread_h)
  vax_tdep_h = vax-tdep.h
  vec_h = vec.h $(gdb_assert_h) $(gdb_string_h)
  version_h = version.h

Re: MI non-stop mode spec

[Nick Roberts]

 > I don't fully understand why disabling CLI commands is desired, but I'm 
 > guessing it's because the CLI commands would still rely on the current 
 > thread.  If so, then keeping the MI protocol stateful would hopefully 
 > address that concern and not force you to disable the CLI interface, 
 > which would be unfortunate.

It's probably not desired but it's certainly more effort.  Perhaps the customer
in question can be persuaded of it's value: it's certainly important for any
frontend that wants to keep the console.  This still provides access to a lot
of funtionality in GDB that hasn't yet been properly implemented in MI.

Currently commands like -break-insert gives synchronous MI output for the
frontend to parse while the CLI command "break" issued under MI,
"-interpreter-exec console break", say, just gives CLI output.  I think this is
the wrong approach and that -break-insert shouldn't generate the output
directly but instead MI should generate event notifications informing the
frontend of changes:

=breakpoints-changed,...

This could be done with observers like Vladimir has done for threads and
would produce the same notifications regardless of whether the breakpoint
was created with a CLI or MI command.

Likewise with the new commands for non-stop mode.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: MI non-stop mode spec

[Vladimir Prus]

On Sunday 23 March 2008 07:40:17 Nick Roberts wrote:
>  > >  > >  > Didn't you check in a patch to make *-varobjs be found to a
>  > >  > >  > thread?
>  > >  > >
>  > >  > > I submitted a patch earlier this year that stopped thinking that
>  > >  > > a variable object had gone out of scope if the thread changed
>  > >  > > but nothing happened.
>  > >  >
>  > >  > Can you resend the current version of that patch, and I'll take a
>  > >  > look.
>  > >
>  > > I used pid_to_thread_id (inferior_ptid) for the thread_id, just as
>  > > infrun.c does for *stopped.  I think that the thread_id problem for
>  > > single/multi-threaded programs should be fixed first (perhaps along
>  > > the lines Daniel suggested).
>  > 
>  > Dan's patch is in, however, it only affects Linux. I suspect in might
>  > be a long time till the problem is solved on all system. Until then,
>  > we probably should assume that if thread list is empty, then the 
>  > program is ST and never will become MT.
>  > 
>  > I happen to be working on variable objects for non-stop now, so I'll
>  > play with the last version of your patch that you have posted.
> 
> I've made some changes since then so here's my latest patch updated to current
> sources.  I found that after all I did need thread_id == -2 for global
> variables (used in mi-cmd-var which doesn't print the thread_id field for them
> since their value is the same in all threads).

I don't think we need it. A global variable has a NULL valid_block, so we can
use that. I'll work on adjusting your patch this way.

>   static struct value *
>   c_value_of_root (struct varobj **var_handle)
>   {
>     struct value *new_val = NULL;
>     struct varobj *var = *var_handle;
> !   struct frame_id saved_frame_id;
> !   struct cleanup *old_cleanups = NULL;
> !   int within_scope, thread_id;
> !   ptid_t ptid;
>   
>     /*  Only root variables can be updated... */
>     if (!is_root_p (var))
>       /* Not a root var */
>       return NULL;
>   
>     /* Determine whether the variable is still around. */
>     if (var->root->valid_block == NULL || var->root->use_selected_frame)
>       within_scope = 1;
>     else
>       {
> !       thread_id = var->root->thread_id;
> !       ptid = thread_id_to_pid (thread_id);
> !       if (thread_id == 0)
> !       /* Single-threaded application.  */
> !       within_scope = check_scope (var);
> !       else if (thread_id != -1 && target_thread_alive (ptid))
>         {
> ! 
> !         saved_frame_id = get_frame_id (get_selected_frame (NULL));
> !         old_cleanups = make_cleanup_restore_current_thread (inferior_ptid,
> !                                                             saved_frame_id);
> !         switch_to_thread (ptid);
> !         within_scope = check_scope (var);
> !       }
> !       else
> !       /* Mark it as dead.  */
> !       var->root->thread_id = -1;
>       }
>   
>     if (within_scope)
> *************** c_value_of_root (struct varobj **var_han
> *** 2186,2191 ****
> --- 2233,2240 ----
>         return new_val;
>       }
>   
> +   do_cleanups (old_cleanups);
> + 
>     return NULL;
>   }

I think the use of cleanups above is wrong. You basically have:

	struct cleanups *old_cleanups = NULL;
	if (whatever)
		old_cleanups = ...
	do_cleanups (old_cleanups);

so, if 'whatever' evaluates to false, all cleanups, including those set
in parent, will be executed.

That's what we get for using a language that does not have exceptions
and proper destructors. I'll fix this too.

- Volodya

Thread bound variable objects [was: Re: MI non-stop mode spec]

[Nick Roberts]

 > > +   do_cleanups (old_cleanups);
 > > + 
 > >     return NULL;
 > >   }
 > 
 > I think the use of cleanups above is wrong. You basically have:
 > 
 > 	struct cleanups *old_cleanups = NULL;
 > 	if (whatever)
 > 		old_cleanups = ...
 > 	do_cleanups (old_cleanups);
 > 
 > so, if 'whatever' evaluates to false, all cleanups, including those set
 > in parent, will be executed.

OK, it should be:

  +  if (old_cleanups != NULL)
  +    do_cleanups (old_cleanups);

 > That's what we get for using a language that does not have exceptions
 > and proper destructors. I'll fix this too.

I see now from the ChangeLog that you've committed your own change without
posting to the list first or explaining what it does.

My patch does two things:

1) It stops a variable object from being considered automatically out of
   scope when the selected thread changes.
2) It associates a thread-id field with the variable object so that the
   front end can organise the display of watch expressions accordingly.

AFAICS your patch does neither of these.  Could you please say what it
does do?

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: Thread bound variable objects [was: Re: MI non-stop mode spec]

[Vladimir Prus]

On Monday 24 March 2008 07:02:48 Nick Roberts wrote:
>  > > +   do_cleanups (old_cleanups);
>  > > + 
>  > >     return NULL;
>  > >   }
>  > 
>  > I think the use of cleanups above is wrong. You basically have:
>  > 
>  > 	struct cleanups *old_cleanups = NULL;
>  > 	if (whatever)
>  > 		old_cleanups = ...
>  > 	do_cleanups (old_cleanups);
>  > 
>  > so, if 'whatever' evaluates to false, all cleanups, including those set
>  > in parent, will be executed.
> 
> OK, it should be:
> 
>   +  if (old_cleanups != NULL)
>   +    do_cleanups (old_cleanups);

I think that's also wrong. In the event that no cleanups were installed
before calling this function, this code will fail to run the
cleanups installed by this function. In the event that a cleanup
should be really installed conditionally, the right code is:

	struct cleanups *back_to  = make_cleanup (null_cleanup, NULL);

	if (...)
		make_cleanup ();

	do_cleanups (back_to);

>  > That's what we get for using a language that does not have exceptions
>  > and proper destructors. I'll fix this too.
> 
> I see now from the ChangeLog that you've committed your own change without
> posting to the list first or explaining what it does.

Sorry, no:
1. I've checked in two changes, not one.
2. Both are posted to gdb-patches.
3. Both are general cleanups, and don't implement anything that your patch
tries to implement.

> My patch does two things:
> 
> 1) It stops a variable object from being considered automatically out of
>    scope when the selected thread changes.
> 2) It associates a thread-id field with the variable object so that the
>    front end can organise the display of watch expressions accordingly.
> 
> AFAICS your patch does neither of these.  Could you please say what it
> does do?

Please see my gdb-patches posts.

- Volodya

Re: Thread bound variable objects [was: Re: MI non-stop mode spec]

[Nick Roberts]

 > > OK, it should be:
 > > 
 > >   +  if (old_cleanups != NULL)
 > >   +    do_cleanups (old_cleanups);
 > 
 > I think that's also wrong. In the event that no cleanups were installed
 > before calling this function, this code will fail to run the
 > cleanups installed by this function. In the event that a cleanup
 > should be really installed conditionally, the right code is:
 > 
 > 	struct cleanups *back_to  = make_cleanup (null_cleanup, NULL);
 > 
 > 	if (...)
 > 		make_cleanup ();
 > 
 > 	do_cleanups (back_to);

I don't really see that: if nothing is added to cleanup_chain then no cleanups
are done.

 >...
 > > I see now from the ChangeLog that you've committed your own change without
 > > posting to the list first or explaining what it does.
 > 
 > Sorry, no:
 > 1. I've checked in two changes, not one.
 > 2. Both are posted to gdb-patches.

OK, my mistake.  I saw the first one.

 > 3. Both are general cleanups, and don't implement anything that your patch
 > tries to implement.

It's just that the second one includes changes in and overlaps with my patch so
I thought it was part of that thread.  I'm not sure where that leaves things
now.

 > > My patch does two things:
 > > 
 > > 1) It stops a variable object from being considered automatically out of
 > >    scope when the selected thread changes.
 > > 2) It associates a thread-id field with the variable object so that the
 > >    front end can organise the display of watch expressions accordingly.
 > > 
 > > AFAICS your patch does neither of these.  Could you please say what it
 > > does do?
 > 
 > Please see my gdb-patches posts.

With regard to the second patch, why you are using
make_cleanup_restore_current_thread when you aren't switching the thread in the
first place?

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: Thread bound variable objects [was: Re: MI non-stop mode spec]

[Vladimir Prus]

On Monday 24 March 2008 09:57:24 Nick Roberts wrote:
>  > > OK, it should be:
>  > > 
>  > >   +  if (old_cleanups != NULL)
>  > >   +    do_cleanups (old_cleanups);
>  > 
>  > I think that's also wrong. In the event that no cleanups were installed
>  > before calling this function, this code will fail to run the
>  > cleanups installed by this function. In the event that a cleanup
>  > should be really installed conditionally, the right code is:
>  > 
>  > 	struct cleanups *back_to  = make_cleanup (null_cleanup, NULL);
>  > 
>  > 	if (...)
>  > 		make_cleanup ();
>  > 
>  > 	do_cleanups (back_to);
> 
> I don't really see that: if nothing is added to cleanup_chain then no cleanups
> are done.

Suppose that no cleanups were installed before this function is called. Then, the
first call to make_cleanup will return NULL, which will be stored in 'old_cleanups'.
Then, at the end of the function you will execute:

	if (old_cleanups != NULL)
	   do_cleanups (old_cleanups);

Here, old_cleanups is NULL, so cleanups added in the function are not run.

>  > > I see now from the ChangeLog that you've committed your own change without
>  > > posting to the list first or explaining what it does.
>  > 
>  > Sorry, no:
>  > 1. I've checked in two changes, not one.
>  > 2. Both are posted to gdb-patches.
> 
> OK, my mistake.  I saw the first one.
> 
>  > 3. Both are general cleanups, and don't implement anything that your patch
>  > tries to implement.
> 
> It's just that the second one includes changes in and overlaps with my patch so
> I thought it was part of that thread.  I'm not sure where that leaves things
> now.

I've started looking at your patch, and saw various cleanups that can
be done first, which I've done. I'll now get back to your patch proper, and hopefully
will get it in today.

>  > > My patch does two things:
>  > > 
>  > > 1) It stops a variable object from being considered automatically out of
>  > >    scope when the selected thread changes.
>  > > 2) It associates a thread-id field with the variable object so that the
>  > >    front end can organise the display of watch expressions accordingly.
>  > > 
>  > > AFAICS your patch does neither of these.  Could you please say what it
>  > > does do?
>  > 
>  > Please see my gdb-patches posts.
> 
> With regard to the second patch, why you are using
> make_cleanup_restore_current_thread when you aren't switching the thread in the
> first place?

Because make_cleanup_restore_current_thread is handy pre-existing solution 
that will restore both thread and frame, and won't cause any harm if the 
thread is not actually changed. I probably could have introduced another 
cleanup to restore just frame, but that will be less safe, and only 
insignificantly faster.

- Volodya

Re: MI non-stop mode spec

[Pawel Piech]

Hi Nick,
Thank you for the reply.  I'm still rather curious what you, Vladimir, 
and anyone else think of the rest of my suggestions. 

Nick Roberts wrote:
>  > I don't fully understand why disabling CLI commands is desired, but I'm 
>  > guessing it's because the CLI commands would still rely on the current 
>  > thread.  If so, then keeping the MI protocol stateful would hopefully 
>  > address that concern and not force you to disable the CLI interface, 
>  > which would be unfortunate.
>
> It's probably not desired but it's certainly more effort.  Perhaps the customer
> in question can be persuaded of it's value: it's certainly important for any
> frontend that wants to keep the console.  This still provides access to a lot
> of funtionality in GDB that hasn't yet been properly implemented in MI.
>   
I think it would be a great disservice to GDB to disable the console.  
Even though the console in the IDE is pretty broken for the reasons you 
mention, a lot of our Eclipse users still value it greatly, so fixing 
it's interaction with MI would actually be very good for GDB.
> Currently commands like -break-insert gives synchronous MI output for the
> frontend to parse while the CLI command "break" issued under MI,
> "-interpreter-exec console break", say, just gives CLI output.  I think this is
> the wrong approach and that -break-insert shouldn't generate the output
> directly but instead MI should generate event notifications informing the
> frontend of changes:
>
> =breakpoints-changed,...
>
> This could be done with observers like Vladimir has done for threads and
> would produce the same notifications regardless of whether the breakpoint
> was created with a CLI or MI command.
>
> Likewise with the new commands for non-stop mode
Thank you for the explanation.  I agree.  Currently MI lacks out of band 
notifications for thread state changes (it has a running event only), 
thread lifecycle events, memory changed events, registers changed 
events, modules changed events, and as you mention above breakpoints 
changed events.  Like I said, I'm not familiar with GDB internals, but 
since CLI has notifications for most of these I can't imagine it would 
be that hard to add them to MI.

One note about the =breakpoints-changed proposed above.  I think it 
would be a mistake to just replace the ^done reply to 
-break-insert/-break-remove with an event alone, because a client would 
have no way of knowing whether an event was in response to that client's 
request or another client's request.  I.e. a client would not be able to 
positively determine the ID of the breakpoint he just created.  I see 
two equally good ways to address this:
1) Add the =breakpoints-changed event in addition to the ^done with 
breakpoint info, but the =breakpoints-changed would have to be sent 
AFTER the ^done reply.
2) Allow clients to specify their own arbitrary ID for a breakpoint, 
with an option such as -client-id="abc".  Then have the 
=breakpoints-changed event and other query commands echo the client-id 
along with the regular breakpoint ID.

Cheers,
Pawel

Re: MI non-stop mode spec

[Vladimir Prus]

On Friday 21 March 2008 21:12:34 Pawel Piech wrote:
> Hi Vladimir,
> Thank you for putting together this proposal for extending the MI 
> protocol to allow non-stop debugging.  In general I think the changes 
> you are proposing are positive I just have two criticisms when looking 
> at this proposal from the client's point of view.  BTW, I work with Marc 
> on the Eclipse DSF-GDB integration.  One disclaimer I should make is 
> that I'm not familiar with any of the GDB internals so I don't know what 
> is easier/more difficult.  I'm only speaking from a client's point of view.
> 
> 1) Changing the protocol to be (partially) stateless by adding -thread 
> and -global options to multitude of commands is not backwards compatible. 

Note that --global option is not longer proposed.

>  From the clients point of view, keeping the protocol backward 
> compatible is a great benefit.  I'm not a big fan of -thread-select, but 
> it's there and trying to remove it now I think will cause more problems 
> than it solves. 
> 
> You mentioned that when switching to non-stop mode there is an inherent 
> race condition in using -thread-select because GDB may switch current 
> threads due to an async stopped event.  A less jarring change to the 
> protocol than adding -thread/-global to all the commands would be to 
> change the behavior of *stopped not to switch the current thread.  To 
> remain backward compatible with older GDB versions, i.e. to work with 
> old *stopped event behavior, MI clients can make the assumption that 
> after a stopped event the current thread is not known and send a 
> -thread-select before sending new commands.  One objection to this 
> change may be that it's not forward compatible and older clients will 
> not work with the new MI protocol version, but I think older clients 
> will not work with GDB in non-stop mode anyhow with the protocol changes 
> that you propose, so that point should make no difference in your decision.

So, we have establishes that current GDB behaviour just is not suitable
for non-stop mode. It can be changed in two ways:

- Making GDB never change the current thread on its own
- Adding --thread option

I think both options require about same amount of work for frontend. In
the first case, the frontend will have to switch to the thread on *stopped.
In the second case, the frontend will have to add the --thread option to MI
commands -- and I suspect that such a change will be very local.

Both options, if implemented unconditionally, will break old frontends, so
we probably need an explicit way to enable new behaviour. In order
to allow new frontends to talk to both old and new GDB, the frontend should
be able to query for support of the new behaviour -- and we have the 
-list-features command for that.

My conclusion is that the effect on compatibility and the implementation
effort on the frontend side are basically the same, so we can judge the
alternatives based on how good they are from the protocol standpoint.

I think I still prefer --thread, as it allows the frontend to eventually
get rid of all the change-thread-do-operation-restore-thread logic. The
other alternative is less clean -- it means that there will be actually
three "current threads". One is the current thread from the frontend POV.
Another is the current thread variable that is implicitly used by most
of gdb codebase (which necessary changes as gdb works). The last is
the "current thread" for MI commands. Having those 3 current threads
and keeping those in sync sounds nasty.

> I don't fully understand why disabling CLI commands is desired, but I'm 
> guessing it's because the CLI commands would still rely on the current 
> thread.  If so, then keeping the MI protocol stateful would hopefully 
> address that concern and not force you to disable the CLI interface, 
> which would be unfortunate.

The reason I've proposed postponing CLI support is purely practical. Right now,
GDB explicitly decides which commands are allowed while the target is running.
In order to enable each specific command, one should:

  - decide what restrictions, if any, that command should have
  - actually try that command, and make sure GDB works fine, that is does
  not try to access frames in running threads, and so on.

This takes time. 

The problem of implicit thread switch is not so bad, because user typing
commands in GDB console probably waits for result of each one before
typing next. 

I guess I probably can enable the -interpreter-exec command for MI,
with the big warning that any given CLI command might not work. 

> 2) The proposal does not address multi-process debugging in any way.  I 
> understand that multi-process (or multi-core) debugging is not part of 
> this proposal, but I think keeping that use case in mind when making 
> this change would make it less painful for the clients when we actually 
> get to that point.  In context of multi-process debugging, options such 
> as -global and fields such as "all" are rather ambigeous.  Instead, I 
> have the following suggestion:
> 
> Assign an ID to a process in the same name space as threads.  Until 
> multi-process debugging is implemented you could just reserve a thread 
> id, such as "0" to be the process thread ID.  For completeness this ID 
> would be returned already in the thread-id field of the response to 
> -target-select command  In order to operate on a whole process context 
> use a -thread-select command with the process context id (followed by 
> the command).  -thread-list-ids would list threads within that process, 
> normally starting with "1".  In reporting stopped events, (in non-stop 
> debugging mode) if a thread has suspended, use the thread-id field to 
> indicate which thread has triggered the stop.  Use a separate field: 
> container-id, to indicate whether the whole process stopped, assuming 
> that some events still cause all threads to stop.  

Can we rename 'container-id' to 'process-id', as that is what it is?
I actually thought that the path forward for multi-process debugging
would be to add the --process option.

> In all-stop mode, the  
> container-id field would always be present in the stopped event.  I 
> think that making this change would allow clients to still work with 
> older versions of GDB in all-stop mode, since thread 0 in all-stop mode 
> would act like a container thread anyway, resuming and suspending all 
> threads.

I'm afraid I don't understand the last sentence. Can you clarify?

- Volodya

Re: MI non-stop mode spec

[Vladimir Prus]

Pawel Piech wrote:

>> Currently commands like -break-insert gives synchronous MI output for the
>> frontend to parse while the CLI command "break" issued under MI,
>> "-interpreter-exec console break", say, just gives CLI output.  I think this is
>> the wrong approach and that -break-insert shouldn't generate the output
>> directly but instead MI should generate event notifications informing the
>> frontend of changes:
>>
>> =breakpoints-changed,...
>>
>> This could be done with observers like Vladimir has done for threads and
>> would produce the same notifications regardless of whether the breakpoint
>> was created with a CLI or MI command.
>>
>> Likewise with the new commands for non-stop mode

> Thank you for the explanation.  I agree.  Currently MI lacks out of band
> notifications for thread state changes (it has a running event only),
> thread lifecycle events, memory changed events, 

Oh, I think that last one might be tricky to implement :-)

> registers changed 
> events, modules changed events, and as you mention above breakpoints
> changed events.  Like I said, I'm not familiar with GDB internals, but
> since CLI has notifications for most of these I can't imagine it would
> be that hard to add them to MI.
> 
> One note about the =breakpoints-changed proposed above.  I think it
> would be a mistake to just replace the ^done reply to
> -break-insert/-break-remove with an event alone, 

It's not being proposed. Per my spec, each MI command results in one of
^done, ^error, ^connected or ^running to be output.

> because a client would 
> have no way of knowing whether an event was in response to that client's
> request or another client's request.  I.e. a client would not be able to
> positively determine the ID of the breakpoint he just created.  I see
> two equally good ways to address this:
> 1) Add the =breakpoints-changed event in addition to the ^done with
> breakpoint info, but the =breakpoints-changed would have to be sent
> AFTER the ^done reply.

We probably can suppress =breakpoints-changed for the -break-insert command.

- Volodya

Re: MI non-stop mode spec

[Pawel Piech]

Hi Vladimir,
Thank you very much for the reply and for considering my input.  Below 
are my replies:

Vladimir Prus wrote:
> On Friday 21 March 2008 21:12:34 Pawel Piech wrote:
>   
>> Hi Vladimir,
>> Thank you for putting together this proposal for extending the MI 
>> protocol to allow non-stop debugging.  In general I think the changes 
>> you are proposing are positive I just have two criticisms when looking 
>> at this proposal from the client's point of view.  BTW, I work with Marc 
>> on the Eclipse DSF-GDB integration.  One disclaimer I should make is 
>> that I'm not familiar with any of the GDB internals so I don't know what 
>> is easier/more difficult.  I'm only speaking from a client's point of view.
>>
>> 1) Changing the protocol to be (partially) stateless by adding -thread 
>> and -global options to multitude of commands is not backwards compatible. 
>>     
>
> Note that --global option is not longer proposed.
>   
I saw this in the prior discussions, although I didn't see what was the 
the proposed replacement.  For a UI, it is still very useful to be able 
to resume or suspend all threads with a single command.
>   
>>  From the clients point of view, keeping the protocol backward 
>> compatible is a great benefit.  I'm not a big fan of -thread-select, but 
>> it's there and trying to remove it now I think will cause more problems 
>> than it solves. 
>>
>> You mentioned that when switching to non-stop mode there is an inherent 
>> race condition in using -thread-select because GDB may switch current 
>> threads due to an async stopped event.  A less jarring change to the 
>> protocol than adding -thread/-global to all the commands would be to 
>> change the behavior of *stopped not to switch the current thread.  To 
>> remain backward compatible with older GDB versions, i.e. to work with 
>> old *stopped event behavior, MI clients can make the assumption that 
>> after a stopped event the current thread is not known and send a 
>> -thread-select before sending new commands.  One objection to this 
>> change may be that it's not forward compatible and older clients will 
>> not work with the new MI protocol version, but I think older clients 
>> will not work with GDB in non-stop mode anyhow with the protocol changes 
>> that you propose, so that point should make no difference in your decision.
>>     
>
> So, we have establishes that current GDB behaviour just is not suitable
> for non-stop mode. It can be changed in two ways:
>
> - Making GDB never change the current thread on its own
> - Adding --thread option
>
> <snip>
>   
> My conclusion is that the effect on compatibility and the implementation
> effort on the frontend side are basically the same, so we can judge the
> alternatives based on how good they are from the protocol standpoint.
>   
I have to disagree with you on this point.  There is different impact on 
compatibility for the client between these two options.  The first 
change in protocol would allow the client to more easily remain backward 
compatible with old versions of GDB as I described above.  Adding the 
-thread option would force clients to query the GDB version explicitly 
and in order to know to use different a format of commands that now 
require the -thread parameter.  Just as you worry that the new version 
of GDB should work with older clients.  Clients worry that they won't 
work with old versions of GDB.  So if you would like quicker adoption of 
this new non-stop debugging feature by clients, I urge you not to 
introduce the -thread parameter. 
> I think I still prefer --thread, as it allows the frontend to eventually
> get rid of all the change-thread-do-operation-restore-thread logic. 
Getting rid of -thread-select is not going to make the MI protocol 
stateless, as there is still a current stack frame.  So the state 
handling logic would have to remain... at least in the Eclipse 
implementation.  Besides, every client that works with GDB already has 
this logic working and and time-tested, so besides pursuing an 
ideological goal I don't think you're doing the clients any favors by 
getting rid of -thread-select.  I would be very curious to hear what 
other clients think of this change though.
> The
> other alternative is less clean -- it means that there will be actually
> three "current threads". One is the current thread from the frontend POV.
> Another is the current thread variable that is implicitly used by most
> of gdb codebase (which necessary changes as gdb works). The last is
> the "current thread" for MI commands. Having those 3 current threads
> and keeping those in sync sounds nasty.
>   
Eclipse integrations with GDB already track the first two current 
threads efficiently.  I would even say that the logic to track it in 
DSF-GDB is rather elegant :-)  I can't speak for the GDB internal 
"current thread", but I assumed that it was the same as the MI protocol 
current thread.  Maybe I didn't make myself clear, but my suggestion was 
to keep the CLI current thread and MI current thread in sync, and avoid 
switching the current thread in the CLI interface upon suspended event 
also.  This actually makes a lot of sense even when the command line 
interface is used without a UI while in non-stop debug mode, because as 
a user I wouldn't want an asynchronous event to switch the current 
thread from under me as I'm entering a new command. 
>> I don't fully understand why disabling CLI commands is desired, but I'm 
>> guessing it's because the CLI commands would still rely on the current 
>> thread.  If so, then keeping the MI protocol stateful would hopefully 
>> address that concern and not force you to disable the CLI interface, 
>> which would be unfortunate.
>>     
>
> The reason I've proposed postponing CLI support is purely practical. Right now,
> GDB explicitly decides which commands are allowed while the target is running.
> In order to enable each specific command, one should:
>
>   - decide what restrictions, if any, that command should have
>   - actually try that command, and make sure GDB works fine, that is does
>   not try to access frames in running threads, and so on.
>
> This takes time. 
>
> The problem of implicit thread switch is not so bad, because user typing
> commands in GDB console probably waits for result of each one before
> typing next. 
>
> I guess I probably can enable the -interpreter-exec command for MI,
> with the big warning that any given CLI command might not work. 
>   
I think that's a fair compromise. 
>> 2) The proposal does not address multi-process debugging in any way.  I 
>> understand that multi-process (or multi-core) debugging is not part of 
>> this proposal, but I think keeping that use case in mind when making 
>> this change would make it less painful for the clients when we actually 
>> get to that point.  In context of multi-process debugging, options such 
>> as -global and fields such as "all" are rather ambigeous.  Instead, I 
>> have the following suggestion:
>>
>> Assign an ID to a process in the same name space as threads.  Until 
>> multi-process debugging is implemented you could just reserve a thread 
>> id, such as "0" to be the process thread ID.  For completeness this ID 
>> would be returned already in the thread-id field of the response to 
>> -target-select command  In order to operate on a whole process context 
>> use a -thread-select command with the process context id (followed by 
>> the command).  -thread-list-ids would list threads within that process, 
>> normally starting with "1".  In reporting stopped events, (in non-stop 
>> debugging mode) if a thread has suspended, use the thread-id field to 
>> indicate which thread has triggered the stop.  Use a separate field: 
>> container-id, to indicate whether the whole process stopped, assuming 
>> that some events still cause all threads to stop.  
>>     
>
> Can we rename 'container-id' to 'process-id', as that is what it is?
>   
I would guess is that there are more GDB extensions out there that 
connect to multiple cores rather than multiple processes.  But of course 
it's OK to change it, it's just a name.
> I actually thought that the path forward for multi-process debugging
> would be to add the --process option.
>   
I had an feeling you would say that :-)  The "-process" option would 
have to be added to just about every MI command, and just as -thread it 
would not be a backward compatible change.  I.e. more work for clients = 
slower adoption.
>> In all-stop mode, the  
>> container-id field would always be present in the stopped event.  I 
>> think that making this change would allow clients to still work with 
>> older versions of GDB in all-stop mode, since thread 0 in all-stop mode 
>> would act like a container thread anyway, resuming and suspending all 
>> threads.
>>     
>
> I'm afraid I don't understand the last sentence. Can you clarify?
>   
I was trying to explain the mechanics of how clients could remain 
backward compatible with older GDB versions with my proposed protocol 
changes.  It boils down to the following assumptions (for the client):
1) After a stopped event, the current selected thread is unknown.
2) If multi-container awareness is not yet implemented, container-id="0"
3) If a running/stopped event does not contain a container-id field, 
then container-id="0" is implied in the event.

One thing that was not clear to me from your proposal is how GDB would 
support all-stop targets with the new version of the protocol (such as 
on chip debugging).  If the answer is, by using the old protocol 
version, I must I admit I'll be rather disappointed.  For a clients, to 
support multiple versions of a protocol indefinitely into the future is 
going to be rather expensive.  I would imagine the same is true for GDB...
> - Volodya
>   
Thank you for your time.  I hope we can keep this conversation going so 
we can reach a conclusion that makes us all happy.  We have mostly the 
same goals, the only difference being whether the GDB or the clients 
will have to do more work to support these new features.  But I hope 
that it's also in GDB's interest to make it easy for clients to adopt 
this new functionality and reach a wider user base quickly.

Sincerely,
Pawel

Re: MI non-stop mode spec

[Vladimir Prus]

Pawel Piech wrote:

> Hi Vladimir,
> Thank you very much for the reply and for considering my input.  Below
> are my replies:
> 
> Vladimir Prus wrote:
>> On Friday 21 March 2008 21:12:34 Pawel Piech wrote:
>>   
>>> Hi Vladimir,
>>> Thank you for putting together this proposal for extending the MI
>>> protocol to allow non-stop debugging.  In general I think the changes
>>> you are proposing are positive I just have two criticisms when looking
>>> at this proposal from the client's point of view.  BTW, I work with Marc
>>> on the Eclipse DSF-GDB integration.  One disclaimer I should make is
>>> that I'm not familiar with any of the GDB internals so I don't know what
>>> is easier/more difficult.  I'm only speaking from a client's point of view.
>>>
>>> 1) Changing the protocol to be (partially) stateless by adding -thread
>>> and -global options to multitude of commands is not backwards compatible.
>>>     
>>
>> Note that --global option is not longer proposed.
>>   
> I saw this in the prior discussions, although I didn't see what was the
> the proposed replacement.  For a UI, it is still very useful to be able
> to resume or suspend all threads with a single command.

This is supposed to be done by -exec-continue or -exec-interrupt
without --thread option.

>>>  From the clients point of view, keeping the protocol backward
>>> compatible is a great benefit.  I'm not a big fan of -thread-select, but
>>> it's there and trying to remove it now I think will cause more problems
>>> than it solves.
>>>
>>> You mentioned that when switching to non-stop mode there is an inherent
>>> race condition in using -thread-select because GDB may switch current
>>> threads due to an async stopped event.  A less jarring change to the
>>> protocol than adding -thread/-global to all the commands would be to
>>> change the behavior of *stopped not to switch the current thread.  To
>>> remain backward compatible with older GDB versions, i.e. to work with
>>> old *stopped event behavior, MI clients can make the assumption that
>>> after a stopped event the current thread is not known and send a
>>> -thread-select before sending new commands.  One objection to this
>>> change may be that it's not forward compatible and older clients will
>>> not work with the new MI protocol version, but I think older clients
>>> will not work with GDB in non-stop mode anyhow with the protocol changes
>>> that you propose, so that point should make no difference in your decision.
>>>     
>>
>> So, we have establishes that current GDB behaviour just is not suitable
>> for non-stop mode. It can be changed in two ways:
>>
>> - Making GDB never change the current thread on its own
>> - Adding --thread option
>>
>> <snip>
>>   
>> My conclusion is that the effect on compatibility and the implementation
>> effort on the frontend side are basically the same, so we can judge the
>> alternatives based on how good they are from the protocol standpoint.
>>   

> I have to disagree with you on this point.  There is different impact on
> compatibility for the client between these two options.  The first
> change in protocol would allow the client to more easily remain backward
> compatible with old versions of GDB as I described above.  Adding the
> -thread option would force clients to query the GDB version explicitly
> and in order to know to use different a format of commands that now
> require the -thread parameter.  Just as you worry that the new version
> of GDB should work with older clients.  Clients worry that they won't
> work with old versions of GDB.  So if you would like quicker adoption of
> this new non-stop debugging feature by clients, I urge you not to
> introduce the -thread parameter.

I'm afraid I still don't see where the complexity lieth. There's exactly 
one command that a frontend has to emit at startup in order to query
gdb functionality. Then, it's just a matter of single "if" statement
to send --thread. 

Definitely, this can be done in KDevelop fairly easily. BTW, this is
not different from other new MI features. For example, I've recently
added support for pending breakpoints via MI -- which is the -f option
to -break-insert. KDevelop was also modified to query for presence of
this features, and use -break-insert -f if available, and fallback to
emulation otherwise. This works fine, and not particularly complex.


>> I think I still prefer --thread, as it allows the frontend to eventually
>> get rid of all the change-thread-do-operation-restore-thread logic.

> Getting rid of -thread-select is not going to make the MI protocol
> stateless, as there is still a current stack frame.  

Note that when using variable objects, which is the recommended way to 
get at  variables and registers, you can already request specific frame.

> So the state 
> handling logic would have to remain... at least in the Eclipse
> implementation.  Besides, every client that works with GDB already has
> this logic working and and time-tested, so besides pursuing an
> ideological goal I don't think you're doing the clients any favors by
> getting rid of -thread-select.  

You might not want to remove that logic in a existing frontend right away,
especially if you want to support older GDB for a while. But for newer
frontends, and for the times when a frontend is being rearchitected,
I think it will be a help.

>> The
>> other alternative is less clean -- it means that there will be actually
>> three "current threads". One is the current thread from the frontend POV.
>> Another is the current thread variable that is implicitly used by most
>> of gdb codebase (which necessary changes as gdb works). The last is
>> the "current thread" for MI commands. Having those 3 current threads
>> and keeping those in sync sounds nasty.
>>   
> Eclipse integrations with GDB already track the first two current
> threads efficiently.  I would even say that the logic to track it in
> DSF-GDB is rather elegant :-)  

Well, have a pointer to source code? (Names of files will do, I have
DSF-GDB source here).

> I can't speak for the GDB internal 
> "current thread", but I assumed that it was the same as the MI protocol
> current thread.  Maybe I didn't make myself clear, but my suggestion was
> to keep the CLI current thread and MI current thread in sync, and avoid
> switching the current thread in the CLI interface upon suspended event
> also.  This actually makes a lot of sense even when the command line
> interface is used without a UI while in non-stop debug mode, because as
> a user I wouldn't want an asynchronous event to switch the current
> thread from under me as I'm entering a new command.

That's an interesting issue. I feel like thread switches might not be as
big issue for CLI user, but probably not switching the thread is indeed
better. It still seems like unknown amount of work. As I mention, gdb
currently has one notion of current thread, which it switches often to perform
its operations. We can introduce 'user-level current thread', and then
set gdb's current thread to user's current thread before executing any CLI
operation. However, I suspect this might break things badly, if user 
types new command when gdb is busy doing some run operation.

>>> I don't fully understand why disabling CLI commands is desired, but I'm
>>> guessing it's because the CLI commands would still rely on the current
>>> thread.  If so, then keeping the MI protocol stateful would hopefully
>>> address that concern and not force you to disable the CLI interface,
>>> which would be unfortunate.
>>>     
>>
>> The reason I've proposed postponing CLI support is purely practical. Right now,
>> GDB explicitly decides which commands are allowed while the target is running.
>> In order to enable each specific command, one should:
>>
>>   - decide what restrictions, if any, that command should have
>>   - actually try that command, and make sure GDB works fine, that is does
>>   not try to access frames in running threads, and so on.
>>
>> This takes time.
>>
>> The problem of implicit thread switch is not so bad, because user typing
>> commands in GDB console probably waits for result of each one before
>> typing next.
>>
>> I guess I probably can enable the -interpreter-exec command for MI,
>> with the big warning that any given CLI command might not work.
>>   
> I think that's a fair compromise.

OK, I have updated the spec. Does DSF-GDB actually use -interpreter-exec,
as opposed to sending the raw CLI command?

>>> 2) The proposal does not address multi-process debugging in any way.  I
>>> understand that multi-process (or multi-core) debugging is not part of
>>> this proposal, but I think keeping that use case in mind when making
>>> this change would make it less painful for the clients when we actually
>>> get to that point.  In context of multi-process debugging, options such
>>> as -global and fields such as "all" are rather ambigeous.  Instead, I
>>> have the following suggestion:
>>>
>>> Assign an ID to a process in the same name space as threads.  Until
>>> multi-process debugging is implemented you could just reserve a thread
>>> id, such as "0" to be the process thread ID.  For completeness this ID
>>> would be returned already in the thread-id field of the response to
>>> -target-select command  In order to operate on a whole process context
>>> use a -thread-select command with the process context id (followed by
>>> the command).  -thread-list-ids would list threads within that process,
>>> normally starting with "1".  In reporting stopped events, (in non-stop
>>> debugging mode) if a thread has suspended, use the thread-id field to
>>> indicate which thread has triggered the stop.  Use a separate field:
>>> container-id, to indicate whether the whole process stopped, assuming
>>> that some events still cause all threads to stop.
>>>     
>>
>> Can we rename 'container-id' to 'process-id', as that is what it is?
>>   
> I would guess is that there are more GDB extensions out there that
> connect to multiple cores rather than multiple processes.  But of course
> it's OK to change it, it's just a name.

OK.

>> I actually thought that the path forward for multi-process debugging
>> would be to add the --process option.
>>   
> I had an feeling you would say that :-)  The "-process" option would
> have to be added to just about every MI command, and just as -thread it
> would not be a backward compatible change.  I.e. more work for clients =
> slower adoption.

Again, it would seem to be like making the GDB announce the present of --process,
and having frontend only send it if supported should be a fairly trivial change.

>>> In all-stop mode, the
>>> container-id field would always be present in the stopped event.  I
>>> think that making this change would allow clients to still work with
>>> older versions of GDB in all-stop mode, since thread 0 in all-stop mode
>>> would act like a container thread anyway, resuming and suspending all
>>> threads.
>>>     
>>
>> I'm afraid I don't understand the last sentence. Can you clarify?
>> 
  
> I was trying to explain the mechanics of how clients could remain
> backward compatible with older GDB versions with my proposed protocol
> changes.  It boils down to the following assumptions (for the client):
> 1) After a stopped event, the current selected thread is unknown.
> 2) If multi-container awareness is not yet implemented, container-id="0"
> 3) If a running/stopped event does not contain a container-id field,
> then container-id="0" is implied in the event.

What is the benefit of container-id="0" if it's already implied? BTW,
how does one select thread N in process M in your scheme?

> One thing that was not clear to me from your proposal is how GDB would
> support all-stop targets with the new version of the protocol (such as
> on chip debugging).  If the answer is, by using the old protocol
> version, I must I admit I'll be rather disappointed.  For a clients, to
> support multiple versions of a protocol indefinitely into the future is
> going to be rather expensive.  I would imagine the same is true for GDB...

A frontend that is capable of new MI, will use it both for all-stop and
non-stop targets. A frontend that is capable of only new MI presumably
won't be able to send the command that enables non-stop mode.


>> - Volodya
>>   
> Thank you for your time.  I hope we can keep this conversation going so
> we can reach a conclusion that makes us all happy.  We have mostly the
> same goals, the only difference being whether the GDB or the clients
> will have to do more work to support these new features.  But I hope
> that it's also in GDB's interest to make it easy for clients to adopt
> this new functionality and reach a wider user base quickly.

FWIW, I'm looking at MI not only as GDB developer, but also as a frontend
developer :-)

- Volodya

Re: MI non-stop mode spec

[Pawel Piech]

Thanks Vladimir,
My comments below.


Vladimir Prus wrote:
> Pawel Piech wrote:
>
>   
>> Hi Vladimir,
>> Thank you very much for the reply and for considering my input.  Below
>> are my replies:
>>
>> Vladimir Prus wrote:
>>     
>>> On Friday 21 March 2008 21:12:34 Pawel Piech wrote:
>>>   
>>>       
>>>> Hi Vladimir,
>>>> Thank you for putting together this proposal for extending the MI
>>>> protocol to allow non-stop debugging.  In general I think the changes
>>>> you are proposing are positive I just have two criticisms when looking
>>>> at this proposal from the client's point of view.  BTW, I work with Marc
>>>> on the Eclipse DSF-GDB integration.  One disclaimer I should make is
>>>> that I'm not familiar with any of the GDB internals so I don't know what
>>>> is easier/more difficult.  I'm only speaking from a client's point of view.
>>>>
>>>> 1) Changing the protocol to be (partially) stateless by adding -thread
>>>> and -global options to multitude of commands is not backwards compatible.
>>>>     
>>>>         
>>> Note that --global option is not longer proposed.
>>>   
>>>       
>> I saw this in the prior discussions, although I didn't see what was the
>> the proposed replacement.  For a UI, it is still very useful to be able
>> to resume or suspend all threads with a single command.
>>     
>
> This is supposed to be done by -exec-continue or -exec-interrupt
> without --thread option.
>
>   
>>>>  From the clients point of view, keeping the protocol backward
>>>> compatible is a great benefit.  I'm not a big fan of -thread-select, but
>>>> it's there and trying to remove it now I think will cause more problems
>>>> than it solves.
>>>>
>>>> You mentioned that when switching to non-stop mode there is an inherent
>>>> race condition in using -thread-select because GDB may switch current
>>>> threads due to an async stopped event.  A less jarring change to the
>>>> protocol than adding -thread/-global to all the commands would be to
>>>> change the behavior of *stopped not to switch the current thread.  To
>>>> remain backward compatible with older GDB versions, i.e. to work with
>>>> old *stopped event behavior, MI clients can make the assumption that
>>>> after a stopped event the current thread is not known and send a
>>>> -thread-select before sending new commands.  One objection to this
>>>> change may be that it's not forward compatible and older clients will
>>>> not work with the new MI protocol version, but I think older clients
>>>> will not work with GDB in non-stop mode anyhow with the protocol changes
>>>> that you propose, so that point should make no difference in your decision.
>>>>     
>>>>         
>>> So, we have establishes that current GDB behaviour just is not suitable
>>> for non-stop mode. It can be changed in two ways:
>>>
>>> - Making GDB never change the current thread on its own
>>> - Adding --thread option
>>>
>>> <snip>
>>>   
>>> My conclusion is that the effect on compatibility and the implementation
>>> effort on the frontend side are basically the same, so we can judge the
>>> alternatives based on how good they are from the protocol standpoint.
>>>   
>>>       
>
>   
>> I have to disagree with you on this point.  There is different impact on
>> compatibility for the client between these two options.  The first
>> change in protocol would allow the client to more easily remain backward
>> compatible with old versions of GDB as I described above.  Adding the
>> -thread option would force clients to query the GDB version explicitly
>> and in order to know to use different a format of commands that now
>> require the -thread parameter.  Just as you worry that the new version
>> of GDB should work with older clients.  Clients worry that they won't
>> work with old versions of GDB.  So if you would like quicker adoption of
>> this new non-stop debugging feature by clients, I urge you not to
>> introduce the -thread parameter.
>>     
>
> I'm afraid I still don't see where the complexity lieth. There's exactly 
> one command that a frontend has to emit at startup in order to query
> gdb functionality. Then, it's just a matter of single "if" statement
> to send --thread. 
>
> Definitely, this can be done in KDevelop fairly easily. BTW, this is
> not different from other new MI features. For example, I've recently
> added support for pending breakpoints via MI -- which is the -f option
> to -break-insert. KDevelop was also modified to query for presence of
> this features, and use -break-insert -f if available, and fallback to
> emulation otherwise. This works fine, and not particularly complex.
>
>   

Perhaps complexity is in the eye of the beholder :-)  In DSF-GDB, the 
implementation is divided into services along the lines of the 
functionality such as run control, stack, variables, breakpoints, etc.  
Depending on GDB version or variant, different services or extensions to 
those services can be put together to support a given debugger back 
end.  BTW, the goal of DSF-GDB is not just to support GDB, it's to 
create a set of components that can be re-used to integrate with Eclipse 
any debugger that uses the MI protocol.

If a new feature is added to a particular command, such as the deferred 
breakpoints, that single service can be replaced, updated, or extended 
as needed.  While adding an option across all commands forces changes in 
all services.  There is a command factory mechanism in CDI-GDB to 
abstract some protocol changes which deals with small changes to 
individual commands based on the protocol version, which DSF-GDB is 
probably going to have to adopt as well.  But this mechanism wouldn't be 
necessary if the evolution in the protocol was more concerned with 
backward compatibility, especially in a case like this where the 
protocol change is not really necessary.

I'm not going to argue this point any further though.  It seems that 
you've made up your mind on this point and I stated my case clearly 
enough.  I'm not a GDB committer so it's up to you to make a decision.

>>> I think I still prefer --thread, as it allows the frontend to eventually
>>> get rid of all the change-thread-do-operation-restore-thread logic.
>>>       
>
>   
>> Getting rid of -thread-select is not going to make the MI protocol
>> stateless, as there is still a current stack frame.  
>>     
>
> Note that when using variable objects, which is the recommended way to 
> get at  variables and registers, you can already request specific frame.
>
>   
>> So the state 
>> handling logic would have to remain... at least in the Eclipse
>> implementation.  Besides, every client that works with GDB already has
>> this logic working and and time-tested, so besides pursuing an
>> ideological goal I don't think you're doing the clients any favors by
>> getting rid of -thread-select.  
>>     
>
> You might not want to remove that logic in a existing frontend right away,
> especially if you want to support older GDB for a while. But for newer
> frontends, and for the times when a frontend is being rearchitected,
> I think it will be a help.
>
>   
>>> The
>>> other alternative is less clean -- it means that there will be actually
>>> three "current threads". One is the current thread from the frontend POV.
>>> Another is the current thread variable that is implicitly used by most
>>> of gdb codebase (which necessary changes as gdb works). The last is
>>> the "current thread" for MI commands. Having those 3 current threads
>>> and keeping those in sync sounds nasty.
>>>   
>>>       
>> Eclipse integrations with GDB already track the first two current
>> threads efficiently.  I would even say that the logic to track it in
>> DSF-GDB is rather elegant :-)  
>>     
>
> Well, have a pointer to source code? (Names of files will do, I have
> DSF-GDB source here).
>   
The current context in the UI is not MI-debugger specific.  And it's 
implementation is rather flexible allowing for different simultaneous 
current contexts in current windows.  The handling of this context is 
mostly implemented in the org.eclipse.debug.internal.ui.context package 
of the org.eclipse.debug.ui plugin. 

The AbstractMIControl module in org.eclipse.dd.mi plugin handles 
tracking of the MI protocol state.  It determines when to send 
-thread-select or -stack-frame-select based on the context contained 
within each command.  It's simple, contained, and reliable.
>> I can't speak for the GDB internal 
>> "current thread", but I assumed that it was the same as the MI protocol
>> current thread.  Maybe I didn't make myself clear, but my suggestion was
>> to keep the CLI current thread and MI current thread in sync, and avoid
>> switching the current thread in the CLI interface upon suspended event
>> also.  This actually makes a lot of sense even when the command line
>> interface is used without a UI while in non-stop debug mode, because as
>> a user I wouldn't want an asynchronous event to switch the current
>> thread from under me as I'm entering a new command.
>>     
>
> That's an interesting issue. I feel like thread switches might not be as
> big issue for CLI user, but probably not switching the thread is indeed
> better. It still seems like unknown amount of work. As I mention, gdb
> currently has one notion of current thread, which it switches often to perform
> its operations. We can introduce 'user-level current thread', and then
> set gdb's current thread to user's current thread before executing any CLI
> operation. However, I suspect this might break things badly, if user 
> types new command when gdb is busy doing some run operation.
>   

Thanks for agreeing :-)  Like I said, I have no idea of the complexity 
of such a change.  However, it seems to me that you should be able to 
implement a simple protcol-state-tracking logic for both CLI and MI 
protocols, and simply simulate the --thread option for commands that 
don't supply one.  This way you could have both the backward compatible 
stateful protocol, and have the -thread option to override it on the 
individual commands.

>>>> I don't fully understand why disabling CLI commands is desired, but I'm
>>>> guessing it's because the CLI commands would still rely on the current
>>>> thread.  If so, then keeping the MI protocol stateful would hopefully
>>>> address that concern and not force you to disable the CLI interface,
>>>> which would be unfortunate.
>>>>     
>>>>         
>>> The reason I've proposed postponing CLI support is purely practical. Right now,
>>> GDB explicitly decides which commands are allowed while the target is running.
>>> In order to enable each specific command, one should:
>>>
>>>   - decide what restrictions, if any, that command should have
>>>   - actually try that command, and make sure GDB works fine, that is does
>>>   not try to access frames in running threads, and so on.
>>>
>>> This takes time.
>>>
>>> The problem of implicit thread switch is not so bad, because user typing
>>> commands in GDB console probably waits for result of each one before
>>> typing next.
>>>
>>> I guess I probably can enable the -interpreter-exec command for MI,
>>> with the big warning that any given CLI command might not work.
>>>   
>>>       
>> I think that's a fair compromise.
>>     
>
> OK, I have updated the spec. Does DSF-GDB actually use -interpreter-exec,
> as opposed to sending the raw CLI command?
>
>   

Yes, both CDI and DSF integrations with GDB use the -interpreter-exec 
command to execute CLI commands. 

>>>> 2) The proposal does not address multi-process debugging in any way.  I
>>>> understand that multi-process (or multi-core) debugging is not part of
>>>> this proposal, but I think keeping that use case in mind when making
>>>> this change would make it less painful for the clients when we actually
>>>> get to that point.  In context of multi-process debugging, options such
>>>> as -global and fields such as "all" are rather ambigeous.  Instead, I
>>>> have the following suggestion:
>>>>
>>>> Assign an ID to a process in the same name space as threads.  Until
>>>> multi-process debugging is implemented you could just reserve a thread
>>>> id, such as "0" to be the process thread ID.  For completeness this ID
>>>> would be returned already in the thread-id field of the response to
>>>> -target-select command  In order to operate on a whole process context
>>>> use a -thread-select command with the process context id (followed by
>>>> the command).  -thread-list-ids would list threads within that process,
>>>> normally starting with "1".  In reporting stopped events, (in non-stop
>>>> debugging mode) if a thread has suspended, use the thread-id field to
>>>> indicate which thread has triggered the stop.  Use a separate field:
>>>> container-id, to indicate whether the whole process stopped, assuming
>>>> that some events still cause all threads to stop.
>>>>     
>>>>         
>>> Can we rename 'container-id' to 'process-id', as that is what it is?
>>>   
>>>       
>> I would guess is that there are more GDB extensions out there that
>> connect to multiple cores rather than multiple processes.  But of course
>> it's OK to change it, it's just a name.
>>     
>
> OK.
>
>   
>>> I actually thought that the path forward for multi-process debugging
>>> would be to add the --process option.
>>>   
>>>       
>> I had an feeling you would say that :-)  The "-process" option would
>> have to be added to just about every MI command, and just as -thread it
>> would not be a backward compatible change.  I.e. more work for clients =
>> slower adoption.
>>     
>
> Again, it would seem to be like making the GDB announce the present of --process,
> and having frontend only send it if supported should be a fairly trivial change.
>
>   
>>>> In all-stop mode, the
>>>> container-id field would always be present in the stopped event.  I
>>>> think that making this change would allow clients to still work with
>>>> older versions of GDB in all-stop mode, since thread 0 in all-stop mode
>>>> would act like a container thread anyway, resuming and suspending all
>>>> threads.
>>>>     
>>>>         
>>> I'm afraid I don't understand the last sentence. Can you clarify?
>>>
>>>       
>   
>   
>> I was trying to explain the mechanics of how clients could remain
>> backward compatible with older GDB versions with my proposed protocol
>> changes.  It boils down to the following assumptions (for the client):
>> 1) After a stopped event, the current selected thread is unknown.
>> 2) If multi-container awareness is not yet implemented, container-id="0"
>> 3) If a running/stopped event does not contain a container-id field,
>> then container-id="0" is implied in the event.
>>     
>
> What is the benefit of container-id="0" if it's already implied? BTW,
> how does one select thread N in process M in your scheme?
>
>   

In the newer version of the protocol, the container-id field in events 
should be explicit.  If debugging multiple processes/cores, the 
container-id field would indicate whether the whole process/core changed 
state, or whether only a given thread suspended.  Although, I think I 
got ahead of myself a little bit.  In the Wind River debugger, the 
container-id field actually contains an array container-ids="[...]", and 
the same is true for threads, because there are cases where multiple, 
but not all threads may suspend with a given event.  In order to 
indicate that only a thread and not a container changed state, we use 
the container-ids field with an empty array.

Each thread id is unique, even across multiple processes.  To switch to 
a thread in a different process, only a thread-select is needed.

>> One thing that was not clear to me from your proposal is how GDB would
>> support all-stop targets with the new version of the protocol (such as
>> on chip debugging).  If the answer is, by using the old protocol
>> version, I must I admit I'll be rather disappointed.  For a clients, to
>> support multiple versions of a protocol indefinitely into the future is
>> going to be rather expensive.  I would imagine the same is true for GDB...
>>     
>
> A frontend that is capable of new MI, will use it both for all-stop and
> non-stop targets. A frontend that is capable of only new MI presumably
> won't be able to send the command that enables non-stop mode.
>
>
>   
>>> - Volodya
>>>   
>>>       
>> Thank you for your time.  I hope we can keep this conversation going so
>> we can reach a conclusion that makes us all happy.  We have mostly the
>> same goals, the only difference being whether the GDB or the clients
>> will have to do more work to support these new features.  But I hope
>> that it's also in GDB's interest to make it easy for clients to adopt
>> this new functionality and reach a wider user base quickly.
>>     
>
> FWIW, I'm looking at MI not only as GDB developer, but also as a frontend
> developer :-)
>
> - Volodya
>   
Thanks good, hopefully it will help you understand my concerns.  Thank 
you for your time.
Pawel

Re: MI non-stop mode spec

[Daniel Jacobowitz]

On Fri, Mar 21, 2008 at 12:11:17PM +0300, Vladimir Prus wrote:
> It seems like I have over-engineered this one. The --global option
> was meant so that we can evaluate expressions while the target is 
> running, but:
> 
> - Generally, we cannot access target memory when all threads are
> running
> - GDB is full with assumptions that we have current thread and current
> frame, so even if target can access memory when all threads is running,
> gdb will still try to grab selected frame, and that will try to grab
> target's register, and that will result in error.
> 
> So, I'd like to remove the --global option. This means that the --thread
> option must be used to specify which thread to operate on, and that
> thread must be stopped.

On targets that can access memory without stopping, I think that's a
valuable feature that we ought to support if we can.  Do you think
adding --global is impractical?  Maybe we can create a special frame
for this - one which returns an error if you access its registers or
unwind it, just a placeholder frame really.

-- 
Daniel Jacobowitz
CodeSourcery

Re: MI non-stop mode spec

[Daniel Jacobowitz]

On Fri, Mar 21, 2008 at 11:12:34AM -0700, Pawel Piech wrote:
> The Wind River debugger which implements the MI protocol, supprots  
> multi-core/multi-process debugging, non-stop and all-stop debugging modes 
> simultaneously (for different targets), uses the above protocol  
> extensions for several years now rather successfully.  So I hope you  
> consider these suggestions seriously even if they are not easiest to  
> implement given the GDB architecture.

Hi Pawel,

Is there any documentation for Wind River's MI protocol which you
could share with us as a basis?  Either with the list, or privately
with CodeSourcery - we have some NDAs in place with Wind River, if
that's needed.  Obviously you have some implementation experience
that we'd love to benefit from here.

Multi-core, multi-process, and multi-thread debugging are all
different but they're very tightly related so thinking about them
all at once may be best.  GDB's only current support for multi-core
models each core as a thread; in some cases that's exactly right,
in some it isn't.

Though perhaps we should break that out into a separate conversation.
The current threading model is enough to make hopefully solid progress
on non-stop for a single multi-threaded program.

I think the most attractive option I've seen so far is to make
automatic context switching optional.  We could disable it by default
when non-stop debugging is enabled.  Or a smarter version:
automatically switch contexts when we stop if previously selected
thread was running.  So if all threads are running and one thread gets
an event, we will automatically switch to that thread; but if two
threads stop in quick succession we'll leave the first one selected.
That might be overly confusing for MI clients; I'm thinking about
a hypothetical CLI version here.

-- 
Daniel Jacobowitz
CodeSourcery

Re: MI non-stop mode spec

[Nick Roberts]

 > Hi Nick,
 > Thank you for the reply.  I'm still rather curious what you, Vladimir, 
 > and anyone else think of the rest of my suggestions. 

Hi Pawel,

I try to only have an opinion on matters that I have influence over which
primarily means use of Gdb in Emacs.  Currently Emacs still uses annotations
and only a subset of MI for things like variable objects for watch expressions,
display of locals etc.  This means that I don't have to worry so much about
backward compatibility.

 > One note about the =breakpoints-changed proposed above.  I think it 
 > would be a mistake to just replace the ^done reply to 
 > -break-insert/-break-remove with an event alone, because a client would 
 > have no way of knowing whether an event was in response to that client's 
 > request or another client's request.  I.e. a client would not be able to 
 > positively determine the ID of the breakpoint he just created.  I see 
 > two equally good ways to address this:
 > 1) Add the =breakpoints-changed event in addition to the ^done with 
 > breakpoint info, but the =breakpoints-changed would have to be sent 
 > AFTER the ^done reply.
 > 2) Allow clients to specify their own arbitrary ID for a breakpoint, 
 > with an option such as -client-id="abc".  Then have the 
 > =breakpoints-changed event and other query commands echo the client-id 
 > along with the regular breakpoint ID.

Sure.  Events prefixed with "=" are NOTIFY-ASYNC-OUTPUT records as described
in the Gdb manual and are issued independently of RESULT-RECORDs like ^done.

For CLI commands maybe something like 2) would be needed.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: Thread bound variable objects

[Nick Roberts]

 > > OK, it should be:
 > > 
 > >   +  if (old_cleanups != NULL)
 > >   +    do_cleanups (old_cleanups);
 > 
 > I think that's also wrong. In the event that no cleanups were installed
 > before calling this function, this code will fail to run the
 > cleanups installed by this function. In the event that a cleanup
 > should be really installed conditionally, the right code is:
 > 
 > 	struct cleanups *back_to  = make_cleanup (null_cleanup, NULL);
 > 
 > 	if (...)
 > 		make_cleanup ();
 > 
 > 	do_cleanups (back_to);

OK, I hadn't realised that.  I was looking at other files, e.g., dwarf2read.c
which uses this technique.  That appears to just be with xfree so maybe only
results in a memory leak.

I guess

  struct cleanups *old_cleanups = 0x1;
  ...
  if (old_cleanups != 0x1)
    do_cleanups (old_cleanups);

would work but, as you say, it's probably best to use null_cleanup.


--
Nick                                           http://www.inet.net.nz/~nickrob

Re: Thread bound variable objects

[Daniel Jacobowitz]

On Tue, Mar 25, 2008 at 12:59:25PM +1200, Nick Roberts wrote:
> OK, I hadn't realised that.  I was looking at other files, e.g., dwarf2read.c
> which uses this technique.  That appears to just be with xfree so maybe only
> results in a memory leak.
> 
> I guess
> 
>   struct cleanups *old_cleanups = 0x1;
>   ...
>   if (old_cleanups != 0x1)
>     do_cleanups (old_cleanups);
> 
> would work but, as you say, it's probably best to use null_cleanup.

Right, that's what it's for.  There are a lot of places in GDB that
use the NULL check, but they're bugs waiting to happen.

-- 
Daniel Jacobowitz
CodeSourcery

Re: MI non-stop mode spec

[Vladimir Prus]

Pawel Piech wrote:


>> I'm afraid I still don't see where the complexity lieth. There's exactly
>> one command that a frontend has to emit at startup in order to query
>> gdb functionality. Then, it's just a matter of single "if" statement
>> to send --thread.
>>
>> Definitely, this can be done in KDevelop fairly easily. BTW, this is
>> not different from other new MI features. For example, I've recently
>> added support for pending breakpoints via MI -- which is the -f option
>> to -break-insert. KDevelop was also modified to query for presence of
>> this features, and use -break-insert -f if available, and fallback to
>> emulation otherwise. This works fine, and not particularly complex.
>>
>>   
> 
> Perhaps complexity is in the eye of the beholder :-)  In DSF-GDB, the
> implementation is divided into services along the lines of the
> functionality such as run control, stack, variables, breakpoints, etc.
> Depending on GDB version or variant, different services or extensions to
> those services can be put together to support a given debugger back
> end.  BTW, the goal of DSF-GDB is not just to support GDB, it's to
> create a set of components that can be re-used to integrate with Eclipse
> any debugger that uses the MI protocol.
> 
> If a new feature is added to a particular command, such as the deferred
> breakpoints, that single service can be replaced, updated, or extended
> as needed.  While adding an option across all commands forces changes in
> all services.  There is a command factory mechanism in CDI-GDB to
> abstract some protocol changes which deals with small changes to
> individual commands based on the protocol version, which DSF-GDB is
> probably going to have to adopt as well.  But this mechanism wouldn't be
> necessary if the evolution in the protocol was more concerned with
> backward compatibility, especially in a case like this where the
> protocol change is not really necessary.

Yes, I was thinking that a centralised entity that actually sends or
creates command can easily add --thread option.

> I'm not going to argue this point any further though.  It seems that
> you've made up your mind on this point and I stated my case clearly
> enough.  I'm not a GDB committer so it's up to you to make a decision.

Thanks; there's more about this issue below.

>>
>> Well, have a pointer to source code? (Names of files will do, I have
>> DSF-GDB source here).
>>   
> The current context in the UI is not MI-debugger specific.  And it's
> implementation is rather flexible allowing for different simultaneous
> current contexts in current windows.  The handling of this context is
> mostly implemented in the org.eclipse.debug.internal.ui.context package
> of the org.eclipse.debug.ui plugin.
> 
> The AbstractMIControl module in org.eclipse.dd.mi plugin handles
> tracking of the MI protocol state.  It determines when to send
> -thread-select or -stack-frame-select based on the context contained
> within each command.  It's simple, contained, and reliable.

Thanks for the pointer, I shall take a look.

>>> I can't speak for the GDB internal
>>> "current thread", but I assumed that it was the same as the MI protocol
>>> current thread.  Maybe I didn't make myself clear, but my suggestion was
>>> to keep the CLI current thread and MI current thread in sync, and avoid
>>> switching the current thread in the CLI interface upon suspended event
>>> also.  This actually makes a lot of sense even when the command line
>>> interface is used without a UI while in non-stop debug mode, because as
>>> a user I wouldn't want an asynchronous event to switch the current
>>> thread from under me as I'm entering a new command.
>>>     
>>
>> That's an interesting issue. I feel like thread switches might not be as
>> big issue for CLI user, but probably not switching the thread is indeed
>> better. It still seems like unknown amount of work. As I mention, gdb
>> currently has one notion of current thread, which it switches often to perform
>> its operations. We can introduce 'user-level current thread', and then
>> set gdb's current thread to user's current thread before executing any CLI
>> operation. However, I suspect this might break things badly, if user
>> types new command when gdb is busy doing some run operation.
>>   
> 
> Thanks for agreeing :-)  Like I said, I have no idea of the complexity
> of such a change.  However, it seems to me that you should be able to
> implement a simple protcol-state-tracking logic for both CLI and MI
> protocols, and simply simulate the --thread option for commands that
> don't supply one.  This way you could have both the backward compatible
> stateful protocol, and have the -thread option to override it on the
> individual commands.

This might be actually a reasonable thing. On one thing, not switching
the current thread might be good for CLI. On the other hand, I still believe
that --thread is ultimately better solution for MI. Then, it might be best to
make GDB not to switch threads (benefiting CLI and stateful MI), and
also accept --thread for the benefit of newer MI clients.

Now there's the catch that --thread is already implemented and is rather
straightforward, whereas I expect some corner cases in implementing thread
that does not change. I think we'll first get the non-stop mode working
with --thread, and then look into making current thread not change. 
(For avoidance of doubt, "then" means "immediately after" not "someday").

>>>> I guess I probably can enable the -interpreter-exec command for MI,
>>>> with the big warning that any given CLI command might not work.
>>>>   
>>>>       
>>> I think that's a fair compromise.
>>>     
>>
>> OK, I have updated the spec. Does DSF-GDB actually use -interpreter-exec,
>> as opposed to sending the raw CLI command?
>>
>>   
> 
> Yes, both CDI and DSF integrations with GDB use the -interpreter-exec
> command to execute CLI commands.

Good.

>>> I was trying to explain the mechanics of how clients could remain
>>> backward compatible with older GDB versions with my proposed protocol
>>> changes.  It boils down to the following assumptions (for the client):
>>> 1) After a stopped event, the current selected thread is unknown.
>>> 2) If multi-container awareness is not yet implemented, container-id="0"
>>> 3) If a running/stopped event does not contain a container-id field,
>>> then container-id="0" is implied in the event.
>>>     
>>
>> What is the benefit of container-id="0" if it's already implied? BTW,
>> how does one select thread N in process M in your scheme?
>>
>>   
> 
> In the newer version of the protocol, the container-id field in events
> should be explicit.  If debugging multiple processes/cores, the
> container-id field would indicate whether the whole process/core changed
> state, or whether only a given thread suspended.  Although, I think I
> got ahead of myself a little bit.  In the Wind River debugger, the
> container-id field actually contains an array container-ids="[...]", and
> the same is true for threads, because there are cases where multiple,
> but not all threads may suspend with a given event.  In order to
> indicate that only a thread and not a container changed state, we use
> the container-ids field with an empty array.
> 
> Each thread id is unique, even across multiple processes.  To switch to
> a thread in a different process, only a thread-select is needed.

Ah, I see. I guess at this point, support for multiple processes looks
like an independent thing, though I hope we'll get back to that relatively
soon.

Thanks for your feedback, I think it's rather important that frontend
developers are getting involved in MI design.

- Volodya

Re: MI non-stop mode spec

[Vladimir Prus]

Daniel Jacobowitz wrote:

> On Fri, Mar 21, 2008 at 12:11:17PM +0300, Vladimir Prus wrote:
>> It seems like I have over-engineered this one. The --global option
>> was meant so that we can evaluate expressions while the target is
>> running, but:
>> 
>> - Generally, we cannot access target memory when all threads are
>> running
>> - GDB is full with assumptions that we have current thread and current
>> frame, so even if target can access memory when all threads is running,
>> gdb will still try to grab selected frame, and that will try to grab
>> target's register, and that will result in error.
>> 
>> So, I'd like to remove the --global option. This means that the --thread
>> option must be used to specify which thread to operate on, and that
>> thread must be stopped.
> 
> On targets that can access memory without stopping, I think that's a
> valuable feature that we ought to support if we can.  Do you think
> adding --global is impractical?  Maybe we can create a special frame
> for this - one which returns an error if you access its registers or
> unwind it, just a placeholder frame really.

For variable objects, the --global is not a complete solution. Instead,
we need some kind of --scope option, and --global in itself is not
necessary.

- Volodya

Re: MI non-stop mode spec

[Pawel Piech]

Hi Daniel,
I will check with my management to see if we can share our protocol 
specification.  I don't think there will be any blocking issues but we 
may need to include an open source license on it.  I'll get back to you 
with this in a couple of days.

Cheers,
Pawel

Daniel Jacobowitz wrote:
> On Fri, Mar 21, 2008 at 11:12:34AM -0700, Pawel Piech wrote:
>   
>> The Wind River debugger which implements the MI protocol, supprots  
>> multi-core/multi-process debugging, non-stop and all-stop debugging modes 
>> simultaneously (for different targets), uses the above protocol  
>> extensions for several years now rather successfully.  So I hope you  
>> consider these suggestions seriously even if they are not easiest to  
>> implement given the GDB architecture.
>>     
>
> Hi Pawel,
>
> Is there any documentation for Wind River's MI protocol which you
> could share with us as a basis?  Either with the list, or privately
> with CodeSourcery - we have some NDAs in place with Wind River, if
> that's needed.  Obviously you have some implementation experience
> that we'd love to benefit from here.
>
> Multi-core, multi-process, and multi-thread debugging are all
> different but they're very tightly related so thinking about them
> all at once may be best.  GDB's only current support for multi-core
> models each core as a thread; in some cases that's exactly right,
> in some it isn't.
>
> Though perhaps we should break that out into a separate conversation.
> The current threading model is enough to make hopefully solid progress
> on non-stop for a single multi-threaded program.
>
> I think the most attractive option I've seen so far is to make
> automatic context switching optional.  We could disable it by default
> when non-stop debugging is enabled.  Or a smarter version:
> automatically switch contexts when we stop if previously selected
> thread was running.  So if all threads are running and one thread gets
> an event, we will automatically switch to that thread; but if two
> threads stop in quick succession we'll leave the first one selected.
> That might be overly confusing for MI clients; I'm thinking about
> a hypothetical CLI version here.
>
>   

Re: MI non-stop mode spec

[Pawel Piech]

Vladimir Prus wrote:
>>
>> Thanks for agreeing :-)  Like I said, I have no idea of the complexity
>> of such a change.  However, it seems to me that you should be able to
>> implement a simple protcol-state-tracking logic for both CLI and MI
>> protocols, and simply simulate the --thread option for commands that
>> don't supply one.  This way you could have both the backward compatible
>> stateful protocol, and have the -thread option to override it on the
>> individual commands.
>>     
>
> This might be actually a reasonable thing. On one thing, not switching
> the current thread might be good for CLI. On the other hand, I still believe
> that --thread is ultimately better solution for MI. Then, it might be best to
> make GDB not to switch threads (benefiting CLI and stateful MI), and
> also accept --thread for the benefit of newer MI clients.
>
> Now there's the catch that --thread is already implemented and is rather
> straightforward, whereas I expect some corner cases in implementing thread
> that does not change. I think we'll first get the non-stop mode working
> with --thread, and then look into making current thread not change. 
> (For avoidance of doubt, "then" means "immediately after" not "someday").
>   

I think this would be an excellent compromise.  If there is any kind of 
a layering between the protocol interpreter and the underlying 
functions, it shouldn't be too much of a stretch to add the state 
tracking in the protocol interpreter layer.  BTW, for the clients that 
support both a UI and a command console it would be even even better if 
CLI and MI maintained their own current thread and stack frame.  This 
would prevent the two interfaces from interfering with each other.

Cheers,
Pawel

Re: MI non-stop mode spec

[Nick Roberts]

 >...
 > > If a new feature is added to a particular command, such as the deferred
 > > breakpoints, that single service can be replaced, updated, or extended
 > > as needed.  While adding an option across all commands forces changes in
 > > all services.  There is a command factory mechanism in CDI-GDB to
 > > abstract some protocol changes which deals with small changes to
 > > individual commands based on the protocol version, which DSF-GDB is
 > > probably going to have to adopt as well.  But this mechanism wouldn't be
 > > necessary if the evolution in the protocol was more concerned with
 > > backward compatibility, especially in a case like this where the
 > > protocol change is not really necessary.
 > 
 > Yes, I was thinking that a centralised entity that actually sends or
 > creates command can easily add --thread option.

I'm nnot sure exactly how it is proposed that non-stop mode should work but
rather than a --thread option, why not have a separate command that determines
which thread(s) the following commands would act on?  Perhaps only the
execution commands, e.g run, next etc, would recognise this set. Using CLI
commands ;-) it could work something like this:

(gdb) threadset 2-4

would mean that

(gdb) continue&

would resume threads 2, 3 and 4

Unlike operations like print, It doesn't seem necessary for these commands to
select the thread/frame first.

Presumably you will be interested in implementing barrier points so there
could be further commands to hold and release threads, e.g.,

(gdb) hold 3

would mean that in the example above only threads 2 and 4 resumed

(gdb) release 3

would allow it to run again.

The command "threadset" on its own could show all the threads in the current
set and the commands "hold" and "release" on their own could show which threads
are currently held/released.

For the current mode of operation, Gdb would give

(gdb) threadset
all
(gdb) hold
none
(gdb) release
all

This would then provide a backward compatible way to use all the MI
execution commands.

-- 
Nick                                           http://www.inet.net.nz/~nickrob

Re: MI non-stop mode spec

[Pawel Piech]

Defining thread sets for purpose of synchronized run control is an often 
requested feature in multi core debugging.  IMO it would be easier for 
the MI client, if the "threadset" command would itself create a new 
thread id, which would then be selected with -thread-select.

Cheers,
Pawel

Nick Roberts wrote:
>  >...
>  > > If a new feature is added to a particular command, such as the deferred
>  > > breakpoints, that single service can be replaced, updated, or extended
>  > > as needed.  While adding an option across all commands forces changes in
>  > > all services.  There is a command factory mechanism in CDI-GDB to
>  > > abstract some protocol changes which deals with small changes to
>  > > individual commands based on the protocol version, which DSF-GDB is
>  > > probably going to have to adopt as well.  But this mechanism wouldn't be
>  > > necessary if the evolution in the protocol was more concerned with
>  > > backward compatibility, especially in a case like this where the
>  > > protocol change is not really necessary.
>  > 
>  > Yes, I was thinking that a centralised entity that actually sends or
>  > creates command can easily add --thread option.
>
> I'm nnot sure exactly how it is proposed that non-stop mode should work but
> rather than a --thread option, why not have a separate command that determines
> which thread(s) the following commands would act on?  Perhaps only the
> execution commands, e.g run, next etc, would recognise this set. Using CLI
> commands ;-) it could work something like this:
>
> (gdb) threadset 2-4
>
> would mean that
>
> (gdb) continue&
>
> would resume threads 2, 3 and 4
>
> Unlike operations like print, It doesn't seem necessary for these commands to
> select the thread/frame first.
>
> Presumably you will be interested in implementing barrier points so there
> could be further commands to hold and release threads, e.g.,
>
> (gdb) hold 3
>
> would mean that in the example above only threads 2 and 4 resumed
>
> (gdb) release 3
>
> would allow it to run again.
>
> The command "threadset" on its own could show all the threads in the current
> set and the commands "hold" and "release" on their own could show which threads
> are currently held/released.
>
> For the current mode of operation, Gdb would give
>
> (gdb) threadset
> all
> (gdb) hold
> none
> (gdb) release
> all
>
> This would then provide a backward compatible way to use all the MI
> execution commands.
>
>   

Re: MI non-stop mode spec

[Vladimir Prus]

Pawel Piech wrote:

> Vladimir Prus wrote:
>>>
>>> Thanks for agreeing :-)  Like I said, I have no idea of the complexity
>>> of such a change.  However, it seems to me that you should be able to
>>> implement a simple protcol-state-tracking logic for both CLI and MI
>>> protocols, and simply simulate the --thread option for commands that
>>> don't supply one.  This way you could have both the backward compatible
>>> stateful protocol, and have the -thread option to override it on the
>>> individual commands.
>>>     
>>
>> This might be actually a reasonable thing. On one thing, not switching
>> the current thread might be good for CLI. On the other hand, I still believe
>> that --thread is ultimately better solution for MI. Then, it might be best to
>> make GDB not to switch threads (benefiting CLI and stateful MI), and
>> also accept --thread for the benefit of newer MI clients.
>>
>> Now there's the catch that --thread is already implemented and is rather
>> straightforward, whereas I expect some corner cases in implementing thread
>> that does not change. I think we'll first get the non-stop mode working
>> with --thread, and then look into making current thread not change.
>> (For avoidance of doubt, "then" means "immediately after" not "someday").
>>   
> 
> I think this would be an excellent compromise.  If there is any kind of
> a layering between the protocol interpreter and the underlying
> functions, it shouldn't be too much of a stretch to add the state
> tracking in the protocol interpreter layer.  

This is one implementation approach -- add current thread in MI, and switch
GDB's global current thread to MI's current thread when executing any command.
This is not very hard, but won't help CLI. Another approach is to do the
same trick for CLI. Yet another approach is to make sure GDB never changes 
its global thread except internally, when it's guaranteed not to talk to user.
We'll see which approach is best.

> BTW, for the clients that 
> support both a UI and a command console it would be even even better if
> CLI and MI maintained their own current thread and stack frame.  This
> would prevent the two interfaces from interfering with each other.

Oh, that's interesting idea. But, would it not be confusing if frontend
shows certain thread and frame as current in UI, whereas commands in GDB
console operate on some other thread?

- Volodya

Re: MI non-stop mode spec

[Pawel Piech]

Vladimir Prus wrote:
> Pawel Piech wrote:
>
>   
>> Vladimir Prus wrote:
>>     
>>>> Thanks for agreeing :-)  Like I said, I have no idea of the complexity
>>>> of such a change.  However, it seems to me that you should be able to
>>>> implement a simple protcol-state-tracking logic for both CLI and MI
>>>> protocols, and simply simulate the --thread option for commands that
>>>> don't supply one.  This way you could have both the backward compatible
>>>> stateful protocol, and have the -thread option to override it on the
>>>> individual commands.
>>>>     
>>>>         
>>> This might be actually a reasonable thing. On one thing, not switching
>>> the current thread might be good for CLI. On the other hand, I still believe
>>> that --thread is ultimately better solution for MI. Then, it might be best to
>>> make GDB not to switch threads (benefiting CLI and stateful MI), and
>>> also accept --thread for the benefit of newer MI clients.
>>>
>>> Now there's the catch that --thread is already implemented and is rather
>>> straightforward, whereas I expect some corner cases in implementing thread
>>> that does not change. I think we'll first get the non-stop mode working
>>> with --thread, and then look into making current thread not change.
>>> (For avoidance of doubt, "then" means "immediately after" not "someday").
>>>   
>>>       
>> I think this would be an excellent compromise.  If there is any kind of
>> a layering between the protocol interpreter and the underlying
>> functions, it shouldn't be too much of a stretch to add the state
>> tracking in the protocol interpreter layer.  
>>     
>
> This is one implementation approach -- add current thread in MI, and switch
> GDB's global current thread to MI's current thread when executing any command.
> This is not very hard, but won't help CLI. Another approach is to do the
> same trick for CLI. Yet another approach is to make sure GDB never changes 
> its global thread except internally, when it's guaranteed not to talk to user.
> We'll see which approach is best.
>
>   
Great!
>> BTW, for the clients that 
>> support both a UI and a command console it would be even even better if
>> CLI and MI maintained their own current thread and stack frame.  This
>> would prevent the two interfaces from interfering with each other.
>>     
>
> Oh, that's interesting idea. But, would it not be confusing if frontend
> shows certain thread and frame as current in UI, whereas commands in GDB
> console operate on some other thread?
>
>   
It certainly could be confusing.  Although in Eclipse DSF-GDB the UI 
selection is already decoupled from the MI protocol state.  One elegant 
solution to this would be to allow the user to electively link the 
console view to the UI selection, where the linking implementation would 
explicitly write "thread" and "frame" commands into the console.  But 
such a feature would require additional work on the client side.

Cheers,
Pawel
> - Volody
>   

Re: MI non-stop mode spec

[Pawel Piech]

FYI, I'm still waiting for approvals to GPL our debugger's MI protocol 
extensions.  I will let you know as soon as I have an answer.
-Pawel

Pawel Piech wrote:
> Hi Daniel,
> I will check with my management to see if we can share our protocol 
> specification.  I don't think there will be any blocking issues but we 
> may need to include an open source license on it.  I'll get back to 
> you with this in a couple of days.
>
> Cheers,
> Pawel
>
> Daniel Jacobowitz wrote:
>> On Fri, Mar 21, 2008 at 11:12:34AM -0700, Pawel Piech wrote:
>>  
>>> The Wind River debugger which implements the MI protocol, supprots  
>>> multi-core/multi-process debugging, non-stop and all-stop debugging 
>>> modes simultaneously (for different targets), uses the above 
>>> protocol  extensions for several years now rather successfully.  So 
>>> I hope you  consider these suggestions seriously even if they are 
>>> not easiest to  implement given the GDB architecture.
>>>     
>>
>> Hi Pawel,
>>
>> Is there any documentation for Wind River's MI protocol which you
>> could share with us as a basis?  Either with the list, or privately
>> with CodeSourcery - we have some NDAs in place with Wind River, if
>> that's needed.  Obviously you have some implementation experience
>> that we'd love to benefit from here.
>>
>> Multi-core, multi-process, and multi-thread debugging are all
>> different but they're very tightly related so thinking about them
>> all at once may be best.  GDB's only current support for multi-core
>> models each core as a thread; in some cases that's exactly right,
>> in some it isn't.
>>
>> Though perhaps we should break that out into a separate conversation.
>> The current threading model is enough to make hopefully solid progress
>> on non-stop for a single multi-threaded program.
>>
>> I think the most attractive option I've seen so far is to make
>> automatic context switching optional.  We could disable it by default
>> when non-stop debugging is enabled.  Or a smarter version:
>> automatically switch contexts when we stop if previously selected
>> thread was running.  So if all threads are running and one thread gets
>> an event, we will automatically switch to that thread; but if two
>> threads stop in quick succession we'll leave the first one selected.
>> That might be overly confusing for MI clients; I'm thinking about
>> a hypothetical CLI version here.
>>
>>   
>