Re: [PATCH v1] [gdb/python]: Add StepEndedEvent (simplifies DAP)

[Andrew Burgess <aburgess@redhat.com>]

Simon Farre <simon.farre.cx@gmail.com> writes:

> Adds the StepEndedEvent which signals that one of the thread finite state machines
> finished. Matches the behavior of what is generated by MI; the "end stepping range".
>
> This should simplify some of the DAP code, where "expected stop reason" is being tracked.
> This logic should be handled by the Python interpreter to begin with, instead.
> ---
>  gdb/NEWS                      |  3 +++
>  gdb/doc/python.texi           |  3 +++
>  gdb/python/py-event-types.def |  5 +++++
>  gdb/python/py-stopevent.c     | 17 +++++++++++++++++

This will definitely need some tests writing for it.  It might be
possible to just add some additional cases to gdb.python/py-events.exp
to cover this new functionality.

>  4 files changed, 28 insertions(+)
>
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 81264c0cfb3..b2abaa6a2ce 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -11,6 +11,9 @@
>    ** New function gdb.notify_mi(NAME, DATA), that emits custom
>       GDB/MI async notification.
>  
> +  ** Added StepEndedEvent which is emitted during stops where it could be determined
> +     that what triggered the stop was that the stepping state machine finished.
> +
>  *** Changes in GDB 14
>  
>  * GDB now supports the AArch64 Scalable Matrix Extension 2 (SME2), which
> diff --git a/gdb/doc/python.texi b/gdb/doc/python.texi
> index 546b4d4b962..c6968791eb1 100644
> --- a/gdb/doc/python.texi
> +++ b/gdb/doc/python.texi
> @@ -3733,6 +3733,9 @@ registry extend @code{gdb.StopEvent}.  As a child of
>  thread when @value{GDBN} is running in non-stop mode.  Refer to
>  @code{gdb.ThreadEvent} above for more details.
>  
> +Emits @code{gdb.StepEndedEvent} that signals that this stop event was generated
> +because one of the step-like commands finished.
> +
>  Emits @code{gdb.SignalEvent}, which extends @code{gdb.StopEvent}.
>  
>  This event indicates that the inferior or one of its threads has
> diff --git a/gdb/python/py-event-types.def b/gdb/python/py-event-types.def
> index c6225115027..36146b181c5 100644
> --- a/gdb/python/py-event-types.def
> +++ b/gdb/python/py-event-types.def
> @@ -106,6 +106,11 @@ GDB_PY_DEFINE_EVENT_TYPE (signal,
>  			  "GDB signal event object",
>  			  stop_event_object_type);
>  
> +GDB_PY_DEFINE_EVENT_TYPE (step_ended,
> +			  "StepEndedEvent",
> +			  "GDB step ended event object",
> +			  stop_event_object_type);
> +
>  GDB_PY_DEFINE_EVENT_TYPE (stop,
>  			  "StopEvent",
>  			  "GDB stop event object",
> diff --git a/gdb/python/py-stopevent.c b/gdb/python/py-stopevent.c
> index 0aa9d5381f8..13e1b12d33a 100644
> --- a/gdb/python/py-stopevent.c
> +++ b/gdb/python/py-stopevent.c
> @@ -18,6 +18,7 @@
>     along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
>  
>  #include "defs.h"
> +#include "mi/mi-common.h"
>  #include "py-stopevent.h"
>  
>  gdbpy_ref<>
> @@ -27,6 +28,21 @@ create_stop_event_object (PyTypeObject *py_type)
>    return create_thread_event_object (py_type, thread.get ());
>  }
>  
> +/** If thread finite state machine reports that it has finished, inform
> +    the Python-interpreter of this. */

Start comments with '/*' and end with two spaces after the '.' before '*/'.

> +
> +static gdbpy_ref<> maybe_create_step_ended () 

Newline before the function name.  Plus should have a comment above the
function explaining what it does.

> +{
> +  const auto tp = inferior_thread();
> +  const auto fsm = tp != nullptr ? tp->thread_fsm() : nullptr;

2x missing space before ().

> +  if (fsm != nullptr && fsm->finished_p () && 
> +	fsm->async_reply_reason () == EXEC_ASYNC_END_STEPPING_RANGE)

The '&&' should be at the start of the new line.

> +    {
> +      return create_stop_event_object (&step_ended_event_object_type);
> +    }

Single statement blocks don't require { ... } around them.

Thanks,
Andrew

> +   return nullptr;
> +}
> +
>  /* Callback observers when a stop event occurs.  This function will create a
>     new Python stop event object.  If only a specific thread is stopped the
>     thread object of the event will be set to that thread.  Otherwise, if all
> @@ -86,6 +102,7 @@ emit_stop_event (struct bpstat *bs, enum gdb_signal stop_signal)
>  	return -1;
>      }
>  
> +  stop_event_obj = maybe_create_step_ended();
>    /* If all fails emit an unknown stop event.  All event types should
>       be known and this should eventually be unused.  */
>    if (stop_event_obj == NULL)
> -- 
> 2.41.0