[PATCH v2] gdb: print thread names in thread apply command output

Simon Marchi simark@simark.ca
Wed Feb 19 05:25:00 GMT 2020


On 2020-02-18 11:24 p.m., Jérémie Galarneau wrote:
>> IIUC this is fixing a latent bug, is that correct?
> 
> Hi Tom,
> 
> This is not a bug fix. AFAIK, this information has never been provided
> as part of that command's output.

I think that Tom is talking about the fact that you moved computing the thread
header before calling the command.  Despite not causing an assertion, it was
technically not correct.  Prior to this patch, we compute the thread label using:

  std::string
  target_pid_to_str (ptid_t ptid)
  {
    return current_top_target ()->pid_to_str (ptid);
  }

Where current_top_target is implemented as:

  target_ops *
  current_top_target ()
  {
    return current_inferior ()->top_target ();
  }

So as you can see, calling target_pid_to_str implicitly relies on the current
inferior (for historical and multi-target reasons that would be too long to
explain this late) to obtain the target to ask to format the ptid.

If the executed command switches the current inferior, it could switch to an
inferior of a different target.  So we would format the ptid of a thread of
target A using the pid_to_str method of target B.

Changing the code to using thread_target_id_str caused us to call
target_thread_name, which luckily has this assertion:

  gdb_assert (info->inf == current_inferior ());

I suppose this assertion should not be necessary normally, as we should be
able to call the thread_name method using a target inferred from the thread:

  info->inf->top_target ()->thread_name (info)

... and not care about the current inferior/target.  However, I presume the
assertion was put there as part of the multi-target work because some
implementations of the thread_name target method might implicitly rely in the
current inferior being set.  So until we audit all these implementations, it
has to stay true.

Simon



More information about the Gdb-patches mailing list