Re: [PATCH 2/5] gdb/amd-dbgapi: add assert in require_forward_progress

[Lancelot SIX <lancelot.six@amd.com>]

On Thu, Jun 05, 2025 at 04:16:25PM -0400, Simon Marchi wrote:
> I didn't have a problem in this area, but it seems to me that this
> pre-condition should always hold.  We should only disable forward
> progress requirement if the target says it's ok to do so.  Otherwise, we
> could get in a situation where we wait for events from amd-dbgapi, which
> will never arrive, because amd-dbgapi didn't actually resume things.
> 

This is OK.

Approved-by: Lancelot Six <lancelot.six@amd.com> (amdgpu)

Best,
Lancelot

> Change-Id: Ifc49f55c7874924b7c47888b8391a07a01d960fc
> ---
>  gdb/amd-dbgapi-target.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/gdb/amd-dbgapi-target.c b/gdb/amd-dbgapi-target.c
> index c11e834aa408..8fd8bcf5ae49 100644
> --- a/gdb/amd-dbgapi-target.c
> +++ b/gdb/amd-dbgapi-target.c
> @@ -450,6 +450,12 @@ static void
>  require_forward_progress (ptid_t ptid, process_stratum_target *proc_target,
>  			  bool require)
>  {
> +  /* If we try to disable forward progress requirement but the target expects
> +     resumed threads to be committed to the target, we could wait for events
> +     that will never arrive.  */
> +  if (!require)
> +    gdb_assert (!proc_target->commit_resumed_state);
> +
>    for (inferior *inf : all_inferiors (proc_target))
>      {
>        if (ptid != minus_one_ptid && inf->pid != ptid.pid ())
> -- 
> 2.49.0