Re: [PATCH] Fix assert failure in invocation of "[kill|detach] inferiors 1"

[Patrick Palka <patrick@parcs.ath.cx>]

On Tue, Jul 28, 2015 at 7:42 PM, Pedro Alves <palves@redhat.com> wrote:
> On 07/28/2015 09:29 PM, Patrick Palka wrote:
>> When only the dummy PID 0 inferior exists, invoking either of the above
>> commands triggers the following assert failure:
>>
>>   .../binutils-gdb/gdb/thread.c:514: internal-error: any_thread_of_process: Assertion `pid != 0' failed.
>>
>> The fix is straightforward.  Tested on x86_64 Debian Stretch.
>
> Thanks.
>
>>
>> gdb/ChangeLog:
>>
>>       * inferior.c (detach_inferior_command): Don't call
>>       any_thread_of_process when pid is 0.
>>       (kill_inferior_command): Likewise.
>>
>> gdb/testsuite/ChangeLog:
>>
>>       * gdb.base/no-inferiors.exp: New test file.
>> ---
>>  gdb/inferior.c                          | 12 ++++++++++--
>>  gdb/testsuite/gdb.base/no-inferiors.exp | 24 ++++++++++++++++++++++++
>>  2 files changed, 34 insertions(+), 2 deletions(-)
>>  create mode 100644 gdb/testsuite/gdb.base/no-inferiors.exp
>>
>> diff --git a/gdb/inferior.c b/gdb/inferior.c
>> index 5e98df5..ff85a1f 100644
>> --- a/gdb/inferior.c
>> +++ b/gdb/inferior.c
>> @@ -626,7 +626,11 @@ detach_inferior_command (char *args, int from_tty)
>>
>>        pid = gdb_inferior_id_to_pid (num);
>>
>> -      tp = any_thread_of_process (pid);
>> +      if (pid != 0)
>> +     tp = any_thread_of_process (pid);
>> +      else
>> +     tp = 0;
>
> Should be tp = NULL.

Oops, ok.

>
> This is old code that predates the multi-executable support
> (6c95b8df7fef), and was never really updated since.  Guess
> not many people use it...

I see.  Though I think the regression may have been introduced more
recently, by commit 32990ad, when the gdb_assert in question was
added.

>
> Doesn't make that much sense to say "has no threads"
> nowadays.  I think this should give out a warning similar
> to the error "detach_command" throws, like:
>
> +      if (pid == 0)
> +        {
> +          warning (_("Inferior ID %s is not being run."), num);
> +          continue;
> +        }
>
>       tp = any_thread_of_process (pid);
>       if (!tp)
>         {
>           warning (_("Inferior ID %d has no threads."), num);
>           continue;
>         }
>
> The "has no threads" case is then more of an internal error,
> but there's no need to be so drastic, so leave it as is.
>
> (Likewise for kill, of course.)
>
> OK with that change.

Ah, so the internal error may also be triggered by passing to these
commands an inferior that is not being run, not just by passing the
initial dummy inferior.  I will change the test file to cover these
bases.  I notice that "[kill|attach] inferiors" has no test coverage
at all, so this will be a good time to add some.