Re: [PATCH 2/2] gdb/remote: return early from remote_check_symbols if waiting for stop reply

[Pedro Alves <pedro@palves.net>]

On 2022-04-29 15:53, Simon Marchi wrote:
> On 2022-04-29 08:50, Pedro Alves wrote:

> Since inferiors can share a program space, perhaps the call to
> target_has_execution here is just wrong, since it only checks one (the
> current) inferior.  What if you have two inferiors sharing a program
> space, one with execution and one without.  If remote_new_objfile gets
> called with the one without execution as the current inferior, we'll
> conclude we have no execution and won't send qSymbol.  But maybe we
> should have sent it, for the benefit of the inferior that has execution.
> 
> So maybe we need to change the question from "does the current inferior
> have execution" to "does any inferior in the affected program space have
> execution", since the current inferior is not always reliable.  And
> anyway, an objfile change is pspace-specific, not inferior-specific.

I think so, yes.

> 
> Something like that:
> 
> diff --git a/gdb/remote.c b/gdb/remote.c
> index ff98024cd78..9888aab88bd 100644
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -5116,14 +5116,6 @@ remote_target::remote_check_symbols ()
>    char *tmp;
>    int end;
> 
> -  /* The remote side has no concept of inferiors that aren't running
> -     yet, it only knows about running processes.  If we're connected
> -     but our current inferior is not running, we should not invite the
> -     remote target to request symbol lookups related to its
> -     (unrelated) current process.  */
> -  if (!target_has_execution ())
> -    return;
> -
>    if (packet_support (PACKET_qSymbol) == PACKET_DISABLE)
>      return;
> 
> @@ -14595,6 +14587,22 @@ remote_new_objfile (struct objfile *objfile)
>    if (current_inferior ()->in_initial_library_scan)
>      return;
> 
> +  bool has_execution = false;
> +  program_space *pspace = current_program_space;
> +  for (inferior *inf : all_inferiors (remote))
> +    {
> +      if (inf->pspace != pspace)
> +       continue;
> +
> +      has_execution = remote->has_execution (inf);
> +
> +      if (has_execution)
> +       break;
> +    }
> +
> +  if (!has_execution)
> +    return;
> +
>    remote->remote_check_symbols ();
>  }
> 

remote_new_objfile has another reference to current_inferior():

 /* Function to be called whenever a new objfile (shlib) is detected.  */
 static void
 remote_new_objfile (struct objfile *objfile)
 {
 ...
   if (current_inferior ()->in_initial_library_scan)
     return;

   remote->remote_check_symbols ();
 }

so seems to me that it should be here that we'd loop over inferiors.