uefi-ref/binutils-gdb
1
0
Fork 0
mirror of https://sourceware.org/git/binutils-gdb.git synced 2026-08-27 00:26:02 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions

Do not write negative PID or TID in remote protocol

Browse source 
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.

v1 is here

    https://inbox.sourceware.org/gdb-patches/20260311202152.2704410-1-tromey@adacore.com/

Regression tested on x86-64 Fedora 43.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=25111
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33979
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33983
This commit is contained in:
Tom Tromey 2026-03-11 14:17:50 -06:00
parent 54e80f9747
commit c2d4b5e113
5 changed files with 201 additions and 172 deletions
Download patch file Download diff file Expand all files Collapse all files

100
gdb/remote.c
View file

@ -3684,31 +3684,17 @@ static char *pack_threadlist_request (char *pkt, int startflag,
static int remote_newthread_step (threadref *ref, void *context);
/* Write a PTID to BUF. ENDBUF points to one-passed-the-end of the
/* Write a PTID to BUF. ENDBUF points to one-past-the-end of the
buffer we're allowed to write to. Returns
BUF+CHARACTERS_WRITTEN. */
char *
remote_target::write_ptid (char *buf, const char *endbuf, ptid_t ptid)
{
ptid_t::pid_type pid;
ptid_t::lwp_type lwp;
if (m_features.remote_multi_process_p ())
{
pid = ptid.pid ();
if (pid < 0)
buf += xsnprintf (buf, endbuf - buf, "p-%x.", -pid);
else
buf += xsnprintf (buf, endbuf - buf, "p%x.", pid);
}
lwp = ptid.lwp ();
if (lwp < 0)
buf += xsnprintf (buf, endbuf - buf, "-%lx", -lwp);
else
buf += xsnprintf (buf, endbuf - buf, "%lx", lwp);
return buf;
std::string repr = ptid.to_rsp_string (m_features.remote_multi_process_p ());
gdb_assert (repr.length () < endbuf - buf);
strcpy (buf, repr.c_str ());
return buf + repr.length ();
}
/* Extract a PTID from BUF. If non-null, OBUF is set to one past the
@ -3718,67 +3704,21 @@ remote_target::write_ptid (char *buf, const char *endbuf, ptid_t ptid)
static ptid_t
read_ptid (const char *buf, const char **obuf)
{
const char *p = buf;
const char *pp;
ptid_t::pid_type pid = 0;
ptid_t::lwp_type lwp = 0;
ULONGEST hex;
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) (LONGEST) hex;
if (hex != ((ULONGEST) pid))
error (_("invalid remote ptid: %s"), buf);
p = pp + 1;
pp = unpack_varlen_hex (p, &hex);
if (pp == p)
error (_("invalid remote ptid: %s"), buf);
lwp = (ptid_t::lwp_type) (LONGEST) hex;
if (hex != ((ULONGEST) lwp))
error (_("invalid remote ptid: %s"), buf);
if (obuf)
*obuf = pp;
return ptid_t (pid, lwp);
}
/* No multi-process. Just a thread id. */
pp = unpack_varlen_hex (p, &hex);
/* Return null_ptid when no thread id is found. */
if (p == pp)
{
if (obuf)
*obuf = pp;
return null_ptid;
}
lwp = (ptid_t::lwp_type) (LONGEST) hex;
if (hex != ((ULONGEST) lwp))
error (_("invalid remote ptid: %s"), buf);
/* Since the stub is not sending a process id, default to what's
current_inferior, unless it doesn't have a PID yet. If so,
then since there's no way to know the pid of the reported
threads, use the magic number. */
inferior *inf = current_inferior ();
if (inf->pid == 0)
pid = magic_null_ptid.pid ();
else
pid = inf->pid;
if (obuf)
*obuf = pp;
return ptid_t (pid, lwp);
return ptid_t::parse (buf, obuf, false,
[] ()
{
/* Since the stub is not sending a process id, default to
what's current_inferior, unless it doesn't have a PID yet.
If so, then since there's no way to know the pid of the
reported threads, use the magic number. */
inferior *inf = current_inferior ();
ptid_t::pid_type pid;
if (inf->pid == 0)
pid = magic_null_ptid.pid ();
else
pid = inf->pid;
return pid;
});
}
static int

46
gdb/unittests/ptid-selftests.c
View file

@ -18,6 +18,7 @@
along with this program. If not, see <http://www.gnu.org/licenses/>. */
#include "gdbsupport/ptid.h"
#include "gdbsupport/selftest.h"
#include <type_traits>
namespace selftests {
@ -149,6 +150,51 @@ static_assert (both.matches (both), "both matches both");
static_assert (!ptid_t (2, 2, 2).matches (both),
"other both doesn't match both");
/* Helper function to parse and return a single PTID. */
static ptid_t
parse_one (const char *str, bool for_remote)
{
const char *out;
ptid_t result = ptid_t::parse (str, &out, for_remote,
[] ()
{
return ptid_t::pid_type (23);
});
SELF_CHECK (*out == '\0');
return result;
}
/* Test ptid_t reading and writing. */
static void
test ()
{
SELF_CHECK (minus_one_ptid.to_rsp_string (false) == "0");
SELF_CHECK (minus_one_ptid.to_rsp_string (true) == "p-1.0");
SELF_CHECK (lwp.to_rsp_string (false) == "2");
SELF_CHECK (lwp.to_rsp_string (true) == "p1.2");
ptid_t negative (-57, -32);
/* This is kind of lame but currently the types are host-dependent
so we have to adapt to those here. */
std::string the_pid = string_printf ("%x", (unsigned) -57);
std::string the_lwp = string_printf ("%lx", (unsigned long) -32);
std::string full = "p" + the_pid + "." + the_lwp;
SELF_CHECK (negative.to_rsp_string (false) == the_lwp);
SELF_CHECK (negative.to_rsp_string (true) == full);
SELF_CHECK (parse_one ("p1.2", false) == lwp);
SELF_CHECK (parse_one ("p1.-1", true) == ptid_t (1, -1));
SELF_CHECK (parse_one ("-1", true) == minus_one_ptid);
SELF_CHECK (parse_one ("0", false) == null_ptid);
SELF_CHECK (parse_one (full.c_str (), false) == negative);
SELF_CHECK (parse_one (full.c_str (), true) == negative);
}
} /* namespace ptid */
} /* namespace selftests */
INIT_GDB_FILE (ptid_selftests)
{
selftests::register_test ("ptid_t", selftests::ptid::test);
}

103
gdbserver/remote-utils.cc
View file

@ -519,43 +519,9 @@ char *
write_ptid (char *buf, ptid_t ptid)
{
client_state &cs = get_client_state ();
ptid_t::pid_type pid;
ptid_t::lwp_type lwp;
if (cs.multi_process)
{
pid = ptid.pid ();
if (pid < 0)
buf += sprintf (buf, "p-%x.", -pid);
else
buf += sprintf (buf, "p%x.", pid);
}
lwp = ptid.lwp ();
if (lwp < 0)
buf += sprintf (buf, "-%lx", -lwp);
else
buf += sprintf (buf, "%lx", lwp);
return buf;
}
static ULONGEST
hex_or_minus_one (const char *buf, const char **obuf)
{
ULONGEST ret;
if (startswith (buf, "-1"))
{
ret = (ULONGEST) -1;
buf += 2;
}
else
buf = unpack_varlen_hex (buf, &ret);
if (obuf)
*obuf = buf;
return ret;
std::string repr = ptid.to_rsp_string (cs.multi_process);
strcpy (buf, repr.c_str ());
return buf + repr.length ();
}
/* Extract a PTID from BUF. If non-null, OBUF is set to one past the last
@ -563,61 +529,14 @@ hex_or_minus_one (const char *buf, const char **obuf)
ptid_t
read_ptid (const char *buf, const char **obuf)
{
const char *p = buf;
const char *pp;
ptid_t::pid_type pid = 0;
ptid_t::lwp_type lwp = 0;
ULONGEST hex;
if (*p == 'p')
{
/* Multi-process ptid. */
pp = unpack_varlen_hex (p + 1, &hex);
if (pp == (p + 1) || *pp != '.')
error (_("invalid remote ptid: %s\n"), buf);
pid = (ptid_t::pid_type) (LONGEST) hex;
if (hex != ((ULONGEST) pid))
error (_("invalid remote ptid: %s"), buf);
p = pp + 1;
hex = hex_or_minus_one (p, &pp);
if (pp == p)
error (_("invalid remote ptid: %s\n"), buf);
lwp = (ptid_t::lwp_type) (LONGEST) hex;
if (hex != ((ULONGEST) lwp))
error (_("invalid remote ptid: %s"), buf);
if (obuf)
*obuf = pp;
return ptid_t (pid, lwp);
}
/* No multi-process. Just a thread id. */
hex = hex_or_minus_one (p, &pp);
/* Handle special thread ids. */
if (hex == (ULONGEST) -1)
return minus_one_ptid;
if (hex == 0)
return null_ptid;
lwp = (ptid_t::lwp_type) (LONGEST) hex;
if (hex != ((ULONGEST) lwp))
error (_("invalid remote ptid: %s"), buf);
/* Since GDB is not sending a process id (multi-process extensions
are off), then there's only one process. Default to the first in
the list. */
pid = get_first_process ()->pid;
if (obuf)
*obuf = pp;
return ptid_t (pid, lwp);
return ptid_t::parse (buf, obuf, true,
[] ()
{
/* Since GDB is not sending a process id (multi-process
extensions are off), then there's only one process.
Default to the first in the list. */
return get_first_process ()->pid;
});
}
/* Write COUNT bytes in BUF to the client. Returns true if all bytes

107
gdbsupport/ptid.cc
View file

@ -19,6 +19,7 @@
#include "ptid.h"
#include "print-utils.h"
#include "rsp-low.h"
/* See ptid.h for these. */
@ -27,8 +28,114 @@ ptid_t const minus_one_ptid = ptid_t::make_minus_one ();
/* See ptid.h. */
std::string
ptid_t::to_rsp_string (bool multi) const
{
char result[50];
char *buf = result;
char *endbuf = &result[sizeof (result)];
if (multi)
{
if (m_pid == -1)
buf += xsnprintf (buf, endbuf - buf, "p-1.");
else
buf += xsnprintf (buf, endbuf - buf, "p%x.", (unsigned) m_pid);
}
if (m_lwp == -1)
xsnprintf (buf, endbuf - buf, "-1");
else
xsnprintf (buf, endbuf - buf, "%lx", (unsigned long) m_lwp);
return result;
}
/* See ptid.h. */
std::string
ptid_t::to_string () const
{
return string_printf ("%d.%ld.%s", m_pid, m_lwp, pulongest (m_tid));
}
/* See ptid.h. */
static ULONGEST
hex_or_minus_one (const char *buf, const char **obuf, bool for_remote)
{
ULONGEST ret;
if (for_remote && startswith (buf, "-1"))
{
ret = (ULONGEST) -1;
buf += 2;
}
else
buf = unpack_varlen_hex (buf, &ret);
if (obuf != nullptr)
*obuf = buf;
return ret;
}
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);
if (obuf != nullptr)
*obuf = pp;
return ptid_t (pid, lwp);
}
/* No multi-process. Just a thread id. */
hex = hex_or_minus_one (p, &pp, for_remote);
/* Handle special thread ids. */
if (hex == (ULONGEST) -1)
return minus_one_ptid;
if (hex == 0)
return null_ptid;
lwp = (ptid_t::lwp_type) hex;
if (hex != ((unsigned_lwp_type) lwp))
error (_("invalid remote ptid: %s"), buf);
pid = default_pid ();
if (obuf != nullptr)
*obuf = pp;
return ptid_t (pid, lwp);
}

17
gdbsupport/ptid.h
View file

@ -35,6 +35,7 @@
#include <functional>
#include <string>
#include "gdbsupport/common-types.h"
#include "gdbsupport/function-view.h"
class ptid_t
{
@ -130,6 +131,12 @@ public:
|| *this == filter);
}
/* Return a remote-protocol-compatible string representation of this
ptid. The MULTI argument controls whether the multi-process
representation is used. */
std::string to_rsp_string (bool multi) const;
/* Return a string representation of the ptid.
This is only meant to be used in debug messages. */
@ -146,6 +153,16 @@ public:
static constexpr ptid_t make_minus_one ()
{ return ptid_t (-1, 0, 0); }
/* Extract a PTID from BUF. If non-null, OBUF is set to one past
the last parsed char. FOR_REMOTE is true for "gdbserver" style
parsing, that is, applying a special meaning to "-1" (see RSP
docs for details). DEFAULT_PID is called when the parsed string
does not provide a process ID; it should return the default PID
to use. This function throws on error. */
static ptid_t parse (const char *buf, const char **obuf, bool for_remote,
gdb::function_view<pid_type ()> default_pid);
private:
/* Process id. */
pid_type m_pid;