Re: [PATCH] Do not write negative PID or TID in remote protocol

[Mark Wielaard <mark@klomp.org>]

Hi Tom,

On Thu, Apr 30, 2026 at 11:59:04AM -0600, Tom Tromey wrote:
> Currently both gdb and gdbserver can write a negative number for the
> PID or TID.  However, the only negative value that really makes sense
> is the special case of "-1" -- in other cases if the PID or TID has
> the high bit set, it should still be written as a positive number.
> 
> This patch attempts to fix the bug.
> 
> v2 of this patch combines the implementations and moves them to
> gdbsupport.  I tried making these standalone functions in rsp-low.cc,
> but that runs afoul of libipa.  So, I made them methods of ptid_t.
> 
> The new unit tests pointed out that round-tripping a sign-extended
> number didn't really work, because the checks were done via ULONGEST.
> It's kind of unfortunate that the PID and LWP are host-dependent
> types.  This should probably be fixed, but I haven't done so here.
> Meanwhile I changed the checks to use the corresponding unsigned type.

It looks like there is still something wrong on 32bit arches.
At least after this patch the i386 debian buildbot fails:
https://builder.sourceware.org/buildbot/#/builders/105/builds/12989

> +ptid_t
> +ptid_t::parse (const char *buf, const char **obuf, bool for_remote,
> +	       gdb::function_view<pid_type ()> default_pid)
> +{
> +  const char *p = buf;
> +  const char *pp;
> +  ptid_t::pid_type pid = 0;
> +  ptid_t::lwp_type lwp = 0;
> +  ULONGEST hex;
> +
> +  using unsigned_pid_type = std::make_unsigned<pid_type>::type;
> +  using unsigned_lwp_type = std::make_unsigned<lwp_type>::type;
> +
> +  if (*p == 'p')
> +    {
> +      /* Multi-process ptid.  */
> +      pp = unpack_varlen_hex (p + 1, &hex);
> +      if (pp == (p + 1) || *pp != '.')
> +	error (_("invalid remote ptid: %s"), buf);
> +
> +      pid = (ptid_t::pid_type) hex;
> +      if (hex != ((unsigned_pid_type) pid))
> +	error (_("invalid remote ptid: %s"), buf);
> +
> +      p = pp + 1;
> +      hex = hex_or_minus_one (p, &pp, for_remote);
> +      if (pp == p)
> +	error (_("invalid remote ptid: %s"), buf);
> +
> +      lwp = (ptid_t::lwp_type) hex;
> +      if (hex != ((unsigned_lwp_type) lwp))
> +	error (_("invalid remote ptid: %s"), buf);

Are the types/castes here correct? hex is an ULONGEST that is being
cast to an ptid_t::lwp_type and then an unsigned_lwp_type. Then this
sanity check seems to fail (on i386 at least). I think this is because
ULONGEST on i386 is unsigned long long, while unsigned_lwp_type is
unsigned long.

Cheers,

Mark