* [PATCH 0/3] Fix an Ada sign-extension problem
@ 2021-09-22 17:10 Tom Tromey
2021-09-22 17:10 ` [PATCH 1/3] Remove defaulted 'tid' parameter to ptid_t constructor Tom Tromey
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Tom Tromey @ 2021-09-22 17:10 UTC (permalink / raw)
To: gdb-patches
On some platforms, the Ada "info task" command can fail with a memory
error. I tracked this down to an unintended sign extension, caused
partly by the use of 'long' in get_ada_task_ptid, and partly by the
use of 'long' in ptid_t.
This series changes these to use ULONGEST instead, avoiding any
unintentional sign extension.
Regression tested on x86-64 Fedora 34.
Tom
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/3] Remove defaulted 'tid' parameter to ptid_t constructor
2021-09-22 17:10 [PATCH 0/3] Fix an Ada sign-extension problem Tom Tromey
@ 2021-09-22 17:10 ` Tom Tromey
2021-09-22 17:22 ` John Baldwin
2021-09-22 17:10 ` [PATCH 2/3] Change ptid_t::tid to ULONGEST Tom Tromey
` (2 subsequent siblings)
3 siblings, 1 reply; 11+ messages in thread
From: Tom Tromey @ 2021-09-22 17:10 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
I wanted to find, and potentially modify, all the spots where the
'tid' parameter to the ptid_t constructor was used. So, I temporarily
removed this parameter and then rebuilt.
In order to make it simpler to search through the "real" (nonzero)
uses of this parameter, something I knew I'd have to do multiple
times, I removed any ", 0" from constructor calls.
---
gdb/linux-fork.c | 2 +-
gdb/linux-nat.c | 15 +++++++--------
gdb/linux-thread-db.c | 4 ++--
gdb/nat/linux-osdata.c | 4 ++--
gdb/nat/linux-procfs.c | 2 +-
gdb/ravenscar-thread.c | 2 +-
gdb/remote.c | 10 +++++-----
gdbserver/linux-low.cc | 12 ++++++------
gdbserver/remote-utils.cc | 4 ++--
gdbserver/thread-db.cc | 2 +-
gdbsupport/agent.cc | 2 +-
11 files changed, 29 insertions(+), 30 deletions(-)
diff --git a/gdb/linux-fork.c b/gdb/linux-fork.c
index 1559ad9fd71..83a124b38ef 100644
--- a/gdb/linux-fork.c
+++ b/gdb/linux-fork.c
@@ -41,7 +41,7 @@
struct fork_info
{
explicit fork_info (pid_t pid)
- : ptid (pid, pid, 0)
+ : ptid (pid, pid)
{
}
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 29ca62d2bc1..0492cc02439 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -929,7 +929,7 @@ find_lwp_pid (ptid_t ptid)
else
lwp = ptid.pid ();
- dummy.ptid = ptid_t (0, lwp, 0);
+ dummy.ptid = ptid_t (0, lwp);
lp = (struct lwp_info *) htab_find (lwp_lwpid_htab, &dummy);
return lp;
}
@@ -1176,8 +1176,7 @@ linux_nat_target::attach (const char *args, int from_tty)
/* The ptrace base target adds the main thread with (pid,0,0)
format. Decorate it with lwp info. */
ptid = ptid_t (inferior_ptid.pid (),
- inferior_ptid.pid (),
- 0);
+ inferior_ptid.pid ());
thread_change_ptid (linux_target, inferior_ptid, ptid);
/* Add the initial process as the first LWP to the list. */
@@ -1901,7 +1900,7 @@ linux_handle_extended_wait (struct lwp_info *lp, int status)
_("wait returned unexpected status 0x%x"), status);
}
- ourstatus->value.related_pid = ptid_t (new_pid, new_pid, 0);
+ ourstatus->value.related_pid = ptid_t (new_pid, new_pid);
if (event == PTRACE_EVENT_FORK || event == PTRACE_EVENT_VFORK)
{
@@ -1924,7 +1923,7 @@ linux_handle_extended_wait (struct lwp_info *lp, int status)
/* This won't actually modify the breakpoint list, but will
physically remove the breakpoints from the child. */
- detach_breakpoints (ptid_t (new_pid, new_pid, 0));
+ detach_breakpoints (ptid_t (new_pid, new_pid));
/* Retain child fork in ptrace (stopped) state. */
if (!find_fork_pid (new_pid))
@@ -1952,7 +1951,7 @@ linux_handle_extended_wait (struct lwp_info *lp, int status)
linux_nat_debug_printf
("Got clone event from LWP %d, new child is LWP %ld", pid, new_pid);
- new_lp = add_lwp (ptid_t (lp->ptid.pid (), new_pid, 0));
+ new_lp = add_lwp (ptid_t (lp->ptid.pid (), new_pid));
new_lp->stopped = 1;
new_lp->resumed = 1;
@@ -2840,7 +2839,7 @@ linux_nat_filter_event (int lwpid, int status)
/* A multi-thread exec after we had seen the leader exiting. */
linux_nat_debug_printf ("Re-adding thread group leader LWP %d.", lwpid);
- lp = add_lwp (ptid_t (lwpid, lwpid, 0));
+ lp = add_lwp (ptid_t (lwpid, lwpid));
lp->stopped = 1;
lp->resumed = 1;
add_thread (linux_target, lp->ptid);
@@ -4101,7 +4100,7 @@ linux_nat_target::static_tracepoint_markers_by_strid (const char *strid)
int pid = inferior_ptid.pid ();
std::vector<static_tracepoint_marker> markers;
const char *p = s;
- ptid_t ptid = ptid_t (pid, 0, 0);
+ ptid_t ptid = ptid_t (pid, 0);
static_tracepoint_marker marker;
/* Pause all */
diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
index 245939b6400..3929589dbb3 100644
--- a/gdb/linux-thread-db.c
+++ b/gdb/linux-thread-db.c
@@ -687,7 +687,7 @@ check_thread_db_callback (const td_thrhandle_t *th, void *arg)
calls are made, we just assume they were; future changes
to how GDB accesses TLS could result in this passing
without exercising the calls it's supposed to. */
- ptid_t ptid = ptid_t (tdb_testinfo->info->pid, ti.ti_lid, 0);
+ ptid_t ptid = ptid_t (tdb_testinfo->info->pid, ti.ti_lid);
thread_info *thread_info = find_thread_ptid (linux_target, ptid);
if (thread_info != NULL && thread_info->priv != NULL)
{
@@ -1842,7 +1842,7 @@ ptid_t
thread_db_target::get_ada_task_ptid (long lwp, long thread)
{
/* NPTL uses a 1:1 model, so the LWP id suffices. */
- return ptid_t (inferior_ptid.pid (), lwp, 0);
+ return ptid_t (inferior_ptid.pid (), lwp);
}
void
diff --git a/gdb/nat/linux-osdata.c b/gdb/nat/linux-osdata.c
index 12f66d3c981..9746d1210fe 100644
--- a/gdb/nat/linux-osdata.c
+++ b/gdb/nat/linux-osdata.c
@@ -266,7 +266,7 @@ get_cores_used_by_process (PID_T pid, int *cores, const int num_cores)
sscanf (dp->d_name, "%lld", &tid);
core = linux_common_core_of_thread (ptid_t ((pid_t) pid,
- (pid_t) tid, 0));
+ (pid_t) tid));
if (core >= 0 && core < num_cores)
{
@@ -521,7 +521,7 @@ linux_xfer_osdata_threads (struct buffer *buffer)
continue;
tid = atoi (dp2->d_name);
- core = linux_common_core_of_thread (ptid_t (pid, tid, 0));
+ core = linux_common_core_of_thread (ptid_t (pid, tid));
buffer_xml_printf
(buffer,
diff --git a/gdb/nat/linux-procfs.c b/gdb/nat/linux-procfs.c
index 25d36c10efc..76e62fc9c44 100644
--- a/gdb/nat/linux-procfs.c
+++ b/gdb/nat/linux-procfs.c
@@ -308,7 +308,7 @@ linux_proc_attach_tgid_threads (pid_t pid,
lwp = strtoul (dp->d_name, NULL, 10);
if (lwp != 0)
{
- ptid_t ptid = ptid_t (pid, lwp, 0);
+ ptid_t ptid = ptid_t (pid, lwp);
if (attach_lwp (ptid))
new_threads_found = 1;
diff --git a/gdb/ravenscar-thread.c b/gdb/ravenscar-thread.c
index 490af3a0bb4..634af466790 100644
--- a/gdb/ravenscar-thread.c
+++ b/gdb/ravenscar-thread.c
@@ -251,7 +251,7 @@ ravenscar_thread_target::get_base_thread_from_ravenscar_task (ptid_t ptid)
return ptid;
base_cpu = get_thread_base_cpu (ptid);
- return ptid_t (ptid.pid (), base_cpu, 0);
+ return ptid_t (ptid.pid (), base_cpu);
}
/* Fetch the ravenscar running thread from target memory, make sure
diff --git a/gdb/remote.c b/gdb/remote.c
index b6da6b086a2..49ed2099211 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -3101,7 +3101,7 @@ read_ptid (const char *buf, const char **obuf)
pp = unpack_varlen_hex (p + 1, &tid);
if (obuf)
*obuf = pp;
- return ptid_t (pid, tid, 0);
+ return ptid_t (pid, tid);
}
/* No multi-process. Just a tid. */
@@ -3126,7 +3126,7 @@ read_ptid (const char *buf, const char **obuf)
if (obuf)
*obuf = pp;
- return ptid_t (pid, tid, 0);
+ return ptid_t (pid, tid);
}
static int
@@ -4138,7 +4138,7 @@ remote_target::static_tracepoint_markers_by_strid (const char *strid)
ptid_t
remote_target::get_ada_task_ptid (long lwp, long thread)
{
- return ptid_t (inferior_ptid.pid (), lwp, 0);
+ return ptid_t (inferior_ptid.pid (), lwp);
}
\f
@@ -6242,7 +6242,7 @@ remote_target::append_resumption (char *p, char *endp,
ptid_t nptid;
/* All (-1) threads of process. */
- nptid = ptid_t (ptid.pid (), -1, 0);
+ nptid = ptid_t (ptid.pid (), -1);
p += xsnprintf (p, endp - p, ":");
p = write_ptid (p, endp, nptid);
@@ -6959,7 +6959,7 @@ remote_target::remote_stop_ns (ptid_t ptid)
if (ptid.is_pid ())
/* All (-1) threads of process. */
- nptid = ptid_t (ptid.pid (), -1, 0);
+ nptid = ptid_t (ptid.pid (), -1);
else
{
/* Small optimization: if we already have a stop reply for
diff --git a/gdbserver/linux-low.cc b/gdbserver/linux-low.cc
index fc7a995351d..d27a2169029 100644
--- a/gdbserver/linux-low.cc
+++ b/gdbserver/linux-low.cc
@@ -491,7 +491,7 @@ linux_process_target::handle_extended_wait (lwp_info **orig_event_lwp,
struct lwp_info *child_lwp;
struct thread_info *child_thr;
- ptid = ptid_t (new_pid, new_pid, 0);
+ ptid = ptid_t (new_pid, new_pid);
if (debug_threads)
{
@@ -597,7 +597,7 @@ linux_process_target::handle_extended_wait (lwp_info **orig_event_lwp,
"from LWP %ld, new child is LWP %ld\n",
lwpid_of (event_thr), new_pid);
- ptid = ptid_t (pid_of (event_thr), new_pid, 0);
+ ptid = ptid_t (pid_of (event_thr), new_pid);
new_lwp = add_lwp (ptid);
/* Either we're going to immediately resume the new thread
@@ -974,7 +974,7 @@ linux_process_target::create_inferior (const char *program,
add_linux_process (pid, 0);
- ptid = ptid_t (pid, pid, 0);
+ ptid = ptid_t (pid, pid);
new_lwp = add_lwp (ptid);
new_lwp->must_set_ptrace_flags = 1;
@@ -1139,7 +1139,7 @@ linux_process_target::attach (unsigned long pid)
{
struct process_info *proc;
struct thread_info *initial_thread;
- ptid_t ptid = ptid_t (pid, pid, 0);
+ ptid_t ptid = ptid_t (pid, pid);
int err;
proc = add_linux_process (pid, 1);
@@ -1157,7 +1157,7 @@ linux_process_target::attach (unsigned long pid)
/* Don't ignore the initial SIGSTOP if we just attached to this
process. It will be collected by wait shortly. */
- initial_thread = find_thread_ptid (ptid_t (pid, pid, 0));
+ initial_thread = find_thread_ptid (ptid_t (pid, pid));
initial_thread->last_resume_kind = resume_stop;
/* We must attach to every LWP. If /proc is mounted, use that to
@@ -2272,7 +2272,7 @@ linux_process_target::filter_event (int lwpid, int wstat)
"after exec.\n", lwpid);
}
- child_ptid = ptid_t (lwpid, lwpid, 0);
+ child_ptid = ptid_t (lwpid, lwpid);
child = add_lwp (child_ptid);
child->stopped = 1;
current_thread = child->thread;
diff --git a/gdbserver/remote-utils.cc b/gdbserver/remote-utils.cc
index 198a75a4476..b79c2aae498 100644
--- a/gdbserver/remote-utils.cc
+++ b/gdbserver/remote-utils.cc
@@ -581,7 +581,7 @@ read_ptid (const char *buf, const char **obuf)
if (obuf)
*obuf = pp;
- return ptid_t (pid, tid, 0);
+ return ptid_t (pid, tid);
}
/* No multi-process. Just a tid. */
@@ -594,7 +594,7 @@ read_ptid (const char *buf, const char **obuf)
if (obuf)
*obuf = pp;
- return ptid_t (pid, tid, 0);
+ return ptid_t (pid, tid);
}
/* Write COUNT bytes in BUF to the client.
diff --git a/gdbserver/thread-db.cc b/gdbserver/thread-db.cc
index 055a0fa970f..9a70cdf4671 100644
--- a/gdbserver/thread-db.cc
+++ b/gdbserver/thread-db.cc
@@ -214,7 +214,7 @@ attach_thread (const td_thrhandle_t *th_p, td_thrinfo_t *ti_p)
{
struct process_info *proc = current_process ();
int pid = pid_of (proc);
- ptid_t ptid = ptid_t (pid, ti_p->ti_lid, 0);
+ ptid_t ptid = ptid_t (pid, ti_p->ti_lid);
struct lwp_info *lwp;
int err;
diff --git a/gdbsupport/agent.cc b/gdbsupport/agent.cc
index 086b08d9eaf..7a2c64684dd 100644
--- a/gdbsupport/agent.cc
+++ b/gdbsupport/agent.cc
@@ -190,7 +190,7 @@ agent_run_command (int pid, const char *cmd, int len)
{
int fd;
int tid = agent_get_helper_thread_id ();
- ptid_t ptid = ptid_t (pid, tid, 0);
+ ptid_t ptid = ptid_t (pid, tid);
int ret = target_write_memory (ipa_sym_addrs.addr_cmd_buf,
(gdb_byte *) cmd, len);
--
2.31.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/3] Change ptid_t::tid to ULONGEST
2021-09-22 17:10 [PATCH 0/3] Fix an Ada sign-extension problem Tom Tromey
2021-09-22 17:10 ` [PATCH 1/3] Remove defaulted 'tid' parameter to ptid_t constructor Tom Tromey
@ 2021-09-22 17:10 ` Tom Tromey
2021-09-22 17:24 ` John Baldwin
2021-09-23 10:31 ` Andrew Burgess
2021-09-22 17:10 ` [PATCH 3/3] Change get_ada_task_ptid parameter type Tom Tromey
2021-09-23 10:53 ` [PATCH 0/3] Fix an Ada sign-extension problem Andrew Burgess
3 siblings, 2 replies; 11+ messages in thread
From: Tom Tromey @ 2021-09-22 17:10 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
The ptid_t 'tid' member is normally used as an address in gdb -- both
bsd-uthread and ravenscar-thread use it this way. However, because
the type is 'long', this can cause problems with sign extension.
This patch changes the type to ULONGEST to ensure that sign extension
does not occur.
---
gdb/ada-tasks.c | 3 ++-
gdb/bsd-uthread.c | 5 +++--
gdb/infrun.c | 8 ++++----
gdb/python/py-infthread.c | 5 +++--
gdb/ravenscar-thread.c | 5 +++--
gdb/remote.c | 5 +++--
gdbserver/target.cc | 4 ++--
gdbsupport/ptid.cc | 3 ++-
gdbsupport/ptid.h | 7 ++++---
9 files changed, 26 insertions(+), 19 deletions(-)
diff --git a/gdb/ada-tasks.c b/gdb/ada-tasks.c
index 80a72216f96..bef26e8e43f 100644
--- a/gdb/ada-tasks.c
+++ b/gdb/ada-tasks.c
@@ -1233,7 +1233,8 @@ info_task (struct ui_out *uiout, const char *taskno_str, struct inferior *inf)
fprintf_styled (gdb_stdout, metadata_style.style (), _("<no name>\n"));
/* Print the TID and LWP. */
- printf_filtered (_("Thread: %#lx\n"), task_info->ptid.tid ());
+ printf_filtered (_("Thread: 0x%s\n"), phex_nz (task_info->ptid.tid (),
+ sizeof (ULONGEST)));
printf_filtered (_("LWP: %#lx\n"), task_info->ptid.lwp ());
/* If set, print the base CPU. */
diff --git a/gdb/bsd-uthread.c b/gdb/bsd-uthread.c
index f8353f07041..1e594983bcd 100644
--- a/gdb/bsd-uthread.c
+++ b/gdb/bsd-uthread.c
@@ -538,8 +538,9 @@ std::string
bsd_uthread_target::pid_to_str (ptid_t ptid)
{
if (ptid.tid () != 0)
- return string_printf ("process %d, thread 0x%lx",
- ptid.pid (), ptid.tid ());
+ return string_printf ("process %d, thread 0x%s",
+ ptid.pid (),
+ phex_nz (ptid.tid (), sizeof (ULONGEST)));
return normal_pid_to_str (ptid);
}
diff --git a/gdb/infrun.c b/gdb/infrun.c
index d1ac9b4cbbb..146d06e92b5 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -4646,11 +4646,11 @@ wait_one ()
static void
save_waitstatus (struct thread_info *tp, const target_waitstatus *ws)
{
- infrun_debug_printf ("saving status %s for %d.%ld.%ld",
+ infrun_debug_printf ("saving status %s for %d.%ld.%s",
target_waitstatus_to_string (ws).c_str (),
tp->ptid.pid (),
tp->ptid.lwp (),
- tp->ptid.tid ());
+ pulongest (tp->ptid.tid ()));
/* Record for later. */
tp->set_pending_waitstatus (*ws);
@@ -4845,9 +4845,9 @@ handle_one (const wait_one_event &event)
struct regcache *regcache;
infrun_debug_printf
- ("target_wait %s, saving status for %d.%ld.%ld",
+ ("target_wait %s, saving status for %d.%ld.%s",
target_waitstatus_to_string (&event.ws).c_str (),
- t->ptid.pid (), t->ptid.lwp (), t->ptid.tid ());
+ t->ptid.pid (), t->ptid.lwp (), pulongest (t->ptid.tid ()));
/* Record for later. */
save_waitstatus (t, &event.ws);
diff --git a/gdb/python/py-infthread.c b/gdb/python/py-infthread.c
index 5645442a426..74bbe9935f0 100644
--- a/gdb/python/py-infthread.c
+++ b/gdb/python/py-infthread.c
@@ -296,7 +296,8 @@ PyObject *
gdbpy_create_ptid_object (ptid_t ptid)
{
int pid;
- long tid, lwp;
+ long lwp;
+ ULONGEST tid;
PyObject *ret;
ret = PyTuple_New (3);
@@ -313,7 +314,7 @@ gdbpy_create_ptid_object (ptid_t ptid)
gdbpy_ref<> lwp_obj = gdb_py_object_from_longest (lwp);
if (lwp_obj == nullptr)
return nullptr;
- gdbpy_ref<> tid_obj = gdb_py_object_from_longest (tid);
+ gdbpy_ref<> tid_obj = gdb_py_object_from_ulongest (tid);
if (tid_obj == nullptr)
return nullptr;
diff --git a/gdb/ravenscar-thread.c b/gdb/ravenscar-thread.c
index 634af466790..6cc583ce348 100644
--- a/gdb/ravenscar-thread.c
+++ b/gdb/ravenscar-thread.c
@@ -164,7 +164,7 @@ struct ravenscar_thread_target final : public target_ops
needed because sometimes the runtime will report an active task
that hasn't yet been put on the list of tasks that is read by
ada-tasks.c. */
- std::unordered_map<long, int> m_cpu_map;
+ std::unordered_map<ULONGEST, int> m_cpu_map;
};
/* Return true iff PTID corresponds to a ravenscar task. */
@@ -469,7 +469,8 @@ ravenscar_thread_target::pid_to_str (ptid_t ptid)
if (!is_ravenscar_task (ptid))
return beneath ()->pid_to_str (ptid);
- return string_printf ("Ravenscar Thread %#x", (int) ptid.tid ());
+ return string_printf ("Ravenscar Thread 0x%s",
+ phex_nz (ptid.tid (), sizeof (ULONGEST)));
}
/* Temporarily set the ptid of a regcache to some other value. When
diff --git a/gdb/remote.c b/gdb/remote.c
index 49ed2099211..3ece443ad18 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -6907,8 +6907,9 @@ remote_target::remote_stop_ns (ptid_t ptid)
== resume_state::RESUMED_PENDING_VCONT)
{
remote_debug_printf ("Enqueueing phony stop reply for thread pending "
- "vCont-resume (%d, %ld, %ld)", tp->ptid.pid(),
- tp->ptid.lwp (), tp->ptid.tid ());
+ "vCont-resume (%d, %ld, %s)", tp->ptid.pid(),
+ tp->ptid.lwp (),
+ pulongest (tp->ptid.tid ()));
/* Check that the thread wasn't resumed with a signal.
Generating a phony stop would result in losing the
diff --git a/gdbserver/target.cc b/gdbserver/target.cc
index 6660cc04fd5..988c6d50c5c 100644
--- a/gdbserver/target.cc
+++ b/gdbserver/target.cc
@@ -286,8 +286,8 @@ target_pid_to_str (ptid_t ptid)
else if (ptid == null_ptid)
xsnprintf (buf, sizeof (buf), "<null thread>");
else if (ptid.tid () != 0)
- xsnprintf (buf, sizeof (buf), "Thread %d.0x%lx",
- ptid.pid (), ptid.tid ());
+ xsnprintf (buf, sizeof (buf), "Thread %d.0x%s",
+ ptid.pid (), phex_nz (ptid.tid (), sizeof (ULONGEST)));
else if (ptid.lwp () != 0)
xsnprintf (buf, sizeof (buf), "LWP %d.%ld",
ptid.pid (), ptid.lwp ());
diff --git a/gdbsupport/ptid.cc b/gdbsupport/ptid.cc
index 2417120b1ee..e5180662612 100644
--- a/gdbsupport/ptid.cc
+++ b/gdbsupport/ptid.cc
@@ -19,6 +19,7 @@
#include "common-defs.h"
#include "ptid.h"
+#include "print-utils.h"
/* See ptid.h for these. */
@@ -30,5 +31,5 @@ ptid_t const minus_one_ptid = ptid_t::make_minus_one ();
std::string
ptid_t::to_string () const
{
- return string_printf ("%d.%ld.%ld", m_pid, m_lwp, m_tid);
+ return string_printf ("%d.%ld.%s", m_pid, m_lwp, pulongest (m_tid));
}
diff --git a/gdbsupport/ptid.h b/gdbsupport/ptid.h
index a2553b29c1a..7cdf468589d 100644
--- a/gdbsupport/ptid.h
+++ b/gdbsupport/ptid.h
@@ -34,6 +34,7 @@
#include <functional>
#include <string>
+#include "gdbsupport/common-types.h"
class ptid_t
{
@@ -47,7 +48,7 @@ class ptid_t
A ptid with only a PID (LWP and TID equal to zero) is usually used to
represent a whole process, including all its lwps/threads. */
- explicit constexpr ptid_t (int pid, long lwp = 0, long tid = 0)
+ explicit constexpr ptid_t (int pid, long lwp = 0, ULONGEST tid = 0)
: m_pid (pid), m_lwp (lwp), m_tid (tid)
{}
@@ -73,7 +74,7 @@ class ptid_t
/* Fetch the tid (thread id) component from a ptid. */
- constexpr long tid () const
+ constexpr ULONGEST tid () const
{ return m_tid; }
/* Return true if the ptid represents a whole process, including all its
@@ -149,7 +150,7 @@ class ptid_t
long m_lwp;
/* Thread id. */
- long m_tid;
+ ULONGEST m_tid;
};
/* Functor to hash a ptid. */
--
2.31.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/3] Change get_ada_task_ptid parameter type
2021-09-22 17:10 [PATCH 0/3] Fix an Ada sign-extension problem Tom Tromey
2021-09-22 17:10 ` [PATCH 1/3] Remove defaulted 'tid' parameter to ptid_t constructor Tom Tromey
2021-09-22 17:10 ` [PATCH 2/3] Change ptid_t::tid to ULONGEST Tom Tromey
@ 2021-09-22 17:10 ` Tom Tromey
2021-09-23 10:53 ` [PATCH 0/3] Fix an Ada sign-extension problem Andrew Burgess
3 siblings, 0 replies; 11+ messages in thread
From: Tom Tromey @ 2021-09-22 17:10 UTC (permalink / raw)
To: gdb-patches; +Cc: Tom Tromey
get_ada_task_ptid currently takes a 'long' as its 'thread' parameter
type. However, on some platforms this is actually a pointer, and
using 'long' can sometimes end up with the value being sign-extended.
This sign extension can cause problems later, if the tid is then later
used as an address again.
This patch changes the parameter type to ULONGEST and updates all the
uses. This approach preserves sign extension on the targets where it
is apparently intended, while avoiding it on others.
---
gdb/ada-tasks.c | 2 +-
gdb/aix-thread.c | 4 ++--
gdb/darwin-nat.c | 2 +-
gdb/darwin-nat.h | 2 +-
gdb/linux-thread-db.c | 4 ++--
gdb/ravenscar-thread.c | 4 ++--
gdb/remote.c | 4 ++--
gdb/sol-thread.c | 6 +++---
gdb/target-delegates.c | 12 ++++++------
gdb/target.c | 6 +++---
gdb/target.h | 4 ++--
gdb/windows-nat.c | 4 ++--
12 files changed, 27 insertions(+), 27 deletions(-)
diff --git a/gdb/ada-tasks.c b/gdb/ada-tasks.c
index bef26e8e43f..9a5bcc6c91c 100644
--- a/gdb/ada-tasks.c
+++ b/gdb/ada-tasks.c
@@ -600,7 +600,7 @@ ada_get_tcb_types_info (void)
static ptid_t
ptid_from_atcb_common (struct value *common_value)
{
- long thread = 0;
+ ULONGEST thread;
CORE_ADDR lwp = 0;
struct value *ll_value;
ptid_t ptid;
diff --git a/gdb/aix-thread.c b/gdb/aix-thread.c
index 0ab4d7ba9b5..04649015d23 100644
--- a/gdb/aix-thread.c
+++ b/gdb/aix-thread.c
@@ -144,7 +144,7 @@ class aix_thread_target final : public target_ops
const char *extra_thread_info (struct thread_info *) override;
- ptid_t get_ada_task_ptid (long lwp, long thread) override;
+ ptid_t get_ada_task_ptid (long lwp, ULONGEST thread) override;
};
static aix_thread_target aix_thread_ops;
@@ -1825,7 +1825,7 @@ aix_thread_target::extra_thread_info (struct thread_info *thread)
}
ptid_t
-aix_thread_target::get_ada_task_ptid (long lwp, long thread)
+aix_thread_target::get_ada_task_ptid (long lwp, ULONGEST thread)
{
return ptid_t (inferior_ptid.pid (), 0, thread);
}
diff --git a/gdb/darwin-nat.c b/gdb/darwin-nat.c
index a6790792fb6..84558bfc132 100644
--- a/gdb/darwin-nat.c
+++ b/gdb/darwin-nat.c
@@ -2376,7 +2376,7 @@ darwin_nat_target::pid_to_exec_file (int pid)
}
ptid_t
-darwin_nat_target::get_ada_task_ptid (long lwp, long thread)
+darwin_nat_target::get_ada_task_ptid (long lwp, ULONGEST thread)
{
struct inferior *inf = current_inferior ();
darwin_inferior *priv = get_darwin_inferior (inf);
diff --git a/gdb/darwin-nat.h b/gdb/darwin-nat.h
index a288ed47458..0e5951d47c8 100644
--- a/gdb/darwin-nat.h
+++ b/gdb/darwin-nat.h
@@ -111,7 +111,7 @@ class darwin_nat_target : public inf_child_target
bool supports_multi_process () override;
- ptid_t get_ada_task_ptid (long lwp, long thread) override;
+ ptid_t get_ada_task_ptid (long lwp, ULONGEST thread) override;
private:
ptid_t wait_1 (ptid_t, struct target_waitstatus *);
diff --git a/gdb/linux-thread-db.c b/gdb/linux-thread-db.c
index 3929589dbb3..bfb3bd64323 100644
--- a/gdb/linux-thread-db.c
+++ b/gdb/linux-thread-db.c
@@ -102,7 +102,7 @@ class thread_db_target final : public target_ops
CORE_ADDR load_module_addr,
CORE_ADDR offset) override;
const char *extra_thread_info (struct thread_info *) override;
- ptid_t get_ada_task_ptid (long lwp, long thread) override;
+ ptid_t get_ada_task_ptid (long lwp, ULONGEST thread) override;
thread_info *thread_handle_to_thread_info (const gdb_byte *thread_handle,
int handle_len,
@@ -1839,7 +1839,7 @@ thread_db_target::get_thread_local_address (ptid_t ptid,
/* Implement the to_get_ada_task_ptid target method for this target. */
ptid_t
-thread_db_target::get_ada_task_ptid (long lwp, long thread)
+thread_db_target::get_ada_task_ptid (long lwp, ULONGEST thread)
{
/* NPTL uses a 1:1 model, so the LWP id suffices. */
return ptid_t (inferior_ptid.pid (), lwp);
diff --git a/gdb/ravenscar-thread.c b/gdb/ravenscar-thread.c
index 6cc583ce348..85f934be3af 100644
--- a/gdb/ravenscar-thread.c
+++ b/gdb/ravenscar-thread.c
@@ -118,7 +118,7 @@ struct ravenscar_thread_target final : public target_ops
std::string pid_to_str (ptid_t) override;
- ptid_t get_ada_task_ptid (long lwp, long thread) override;
+ ptid_t get_ada_task_ptid (long lwp, ULONGEST thread) override;
struct btrace_target_info *enable_btrace (ptid_t ptid,
const struct btrace_config *conf)
@@ -682,7 +682,7 @@ ravenscar_inferior_created (inferior *inf)
}
ptid_t
-ravenscar_thread_target::get_ada_task_ptid (long lwp, long thread)
+ravenscar_thread_target::get_ada_task_ptid (long lwp, ULONGEST thread)
{
return ptid_t (m_base_ptid.pid (), 0, thread);
}
diff --git a/gdb/remote.c b/gdb/remote.c
index 3ece443ad18..0dfe36acec7 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -493,7 +493,7 @@ class remote_target : public process_stratum_target
const char *extra_thread_info (struct thread_info *) override;
- ptid_t get_ada_task_ptid (long lwp, long thread) override;
+ ptid_t get_ada_task_ptid (long lwp, ULONGEST thread) override;
thread_info *thread_handle_to_thread_info (const gdb_byte *thread_handle,
int handle_len,
@@ -4136,7 +4136,7 @@ remote_target::static_tracepoint_markers_by_strid (const char *strid)
/* Implement the to_get_ada_task_ptid function for the remote targets. */
ptid_t
-remote_target::get_ada_task_ptid (long lwp, long thread)
+remote_target::get_ada_task_ptid (long lwp, ULONGEST thread)
{
return ptid_t (inferior_ptid.pid (), lwp);
}
diff --git a/gdb/sol-thread.c b/gdb/sol-thread.c
index 18eacf1062a..513d0309bc4 100644
--- a/gdb/sol-thread.c
+++ b/gdb/sol-thread.c
@@ -88,7 +88,7 @@ class sol_thread_target final : public target_ops
void resume (ptid_t, int, enum gdb_signal) override;
void mourn_inferior () override;
std::string pid_to_str (ptid_t) override;
- ptid_t get_ada_task_ptid (long lwp, long thread) override;
+ ptid_t get_ada_task_ptid (long lwp, ULONGEST thread) override;
void fetch_registers (struct regcache *, int) override;
void store_registers (struct regcache *, int) override;
@@ -1120,7 +1120,7 @@ info_solthreads (const char *args, int from_tty)
static int
thread_db_find_thread_from_tid (struct thread_info *thread, void *data)
{
- long *tid = (long *) data;
+ ULONGEST *tid = (ULONGEST *) data;
if (thread->ptid.tid () == *tid)
return 1;
@@ -1129,7 +1129,7 @@ thread_db_find_thread_from_tid (struct thread_info *thread, void *data)
}
ptid_t
-sol_thread_target::get_ada_task_ptid (long lwp, long thread)
+sol_thread_target::get_ada_task_ptid (long lwp, ULONGEST thread)
{
struct thread_info *thread_info =
iterate_over_threads (thread_db_find_thread_from_tid, &thread);
diff --git a/gdb/target-delegates.c b/gdb/target-delegates.c
index 3fd2854955f..fb9c78a5f79 100644
--- a/gdb/target-delegates.c
+++ b/gdb/target-delegates.c
@@ -99,7 +99,7 @@ struct dummy_target : public target_ops
void flash_erase (ULONGEST arg0, LONGEST arg1) override;
void flash_done () override;
const struct target_desc *read_description () override;
- ptid_t get_ada_task_ptid (long arg0, long arg1) override;
+ ptid_t get_ada_task_ptid (long arg0, ULONGEST arg1) override;
int auxv_parse (gdb_byte **arg0, gdb_byte *arg1, CORE_ADDR *arg2, CORE_ADDR *arg3) override;
int search_memory (CORE_ADDR arg0, ULONGEST arg1, const gdb_byte *arg2, ULONGEST arg3, CORE_ADDR *arg4) override;
bool can_execute_reverse () override;
@@ -274,7 +274,7 @@ struct debug_target : public target_ops
void flash_erase (ULONGEST arg0, LONGEST arg1) override;
void flash_done () override;
const struct target_desc *read_description () override;
- ptid_t get_ada_task_ptid (long arg0, long arg1) override;
+ ptid_t get_ada_task_ptid (long arg0, ULONGEST arg1) override;
int auxv_parse (gdb_byte **arg0, gdb_byte *arg1, CORE_ADDR *arg2, CORE_ADDR *arg3) override;
int search_memory (CORE_ADDR arg0, ULONGEST arg1, const gdb_byte *arg2, ULONGEST arg3, CORE_ADDR *arg4) override;
bool can_execute_reverse () override;
@@ -2598,19 +2598,19 @@ debug_target::read_description ()
}
ptid_t
-target_ops::get_ada_task_ptid (long arg0, long arg1)
+target_ops::get_ada_task_ptid (long arg0, ULONGEST arg1)
{
return this->beneath ()->get_ada_task_ptid (arg0, arg1);
}
ptid_t
-dummy_target::get_ada_task_ptid (long arg0, long arg1)
+dummy_target::get_ada_task_ptid (long arg0, ULONGEST arg1)
{
return default_get_ada_task_ptid (this, arg0, arg1);
}
ptid_t
-debug_target::get_ada_task_ptid (long arg0, long arg1)
+debug_target::get_ada_task_ptid (long arg0, ULONGEST arg1)
{
ptid_t result;
fprintf_unfiltered (gdb_stdlog, "-> %s->get_ada_task_ptid (...)\n", this->beneath ()->shortname ());
@@ -2618,7 +2618,7 @@ debug_target::get_ada_task_ptid (long arg0, long arg1)
fprintf_unfiltered (gdb_stdlog, "<- %s->get_ada_task_ptid (", this->beneath ()->shortname ());
target_debug_print_long (arg0);
fputs_unfiltered (", ", gdb_stdlog);
- target_debug_print_long (arg1);
+ target_debug_print_ULONGEST (arg1);
fputs_unfiltered (") = ", gdb_stdlog);
target_debug_print_ptid_t (result);
fputs_unfiltered ("\n", gdb_stdlog);
diff --git a/gdb/target.c b/gdb/target.c
index d1c1bf523ed..2cb587d9cee 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -67,7 +67,7 @@ static int default_region_ok_for_hw_watchpoint (struct target_ops *,
static void default_rcmd (struct target_ops *, const char *, struct ui_file *);
static ptid_t default_get_ada_task_ptid (struct target_ops *self,
- long lwp, long tid);
+ long lwp, ULONGEST tid);
static void default_mourn_inferior (struct target_ops *self);
@@ -595,7 +595,7 @@ target_can_execute_reverse ()
}
ptid_t
-target_get_ada_task_ptid (long lwp, long tid)
+target_get_ada_task_ptid (long lwp, ULONGEST tid)
{
return current_inferior ()->top_target ()->get_ada_task_ptid (lwp, tid);
}
@@ -1133,7 +1133,7 @@ default_terminal_info (struct target_ops *self, const char *args, int from_tty)
inferior_ptid. */
static ptid_t
-default_get_ada_task_ptid (struct target_ops *self, long lwp, long tid)
+default_get_ada_task_ptid (struct target_ops *self, long lwp, ULONGEST tid)
{
return ptid_t (inferior_ptid.pid (), lwp, tid);
}
diff --git a/gdb/target.h b/gdb/target.h
index 57afebef876..61febdb183a 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -856,7 +856,7 @@ struct target_ops
based on LWP and THREAD. These values are extracted from the
task Private_Data section of the Ada Task Control Block, and
their interpretation depends on the target. */
- virtual ptid_t get_ada_task_ptid (long lwp, long thread)
+ virtual ptid_t get_ada_task_ptid (long lwp, ULONGEST thread)
TARGET_DEFAULT_FUNC (default_get_ada_task_ptid);
/* Read one auxv entry from *READPTR, not reading locations >= ENDPTR.
@@ -2141,7 +2141,7 @@ extern bool target_can_execute_reverse ();
extern const struct target_desc *target_read_description (struct target_ops *);
-extern ptid_t target_get_ada_task_ptid (long lwp, long tid);
+extern ptid_t target_get_ada_task_ptid (long lwp, ULONGEST tid);
/* Main entry point for searching memory. */
extern int target_search_memory (CORE_ADDR start_addr,
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 0c2d55ef67b..a052efe654c 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -277,7 +277,7 @@ struct windows_nat_target final : public x86_nat_target<inf_child_target>
char *pid_to_exec_file (int pid) override;
- ptid_t get_ada_task_ptid (long lwp, long thread) override;
+ ptid_t get_ada_task_ptid (long lwp, ULONGEST thread) override;
bool get_tib_address (ptid_t ptid, CORE_ADDR *addr) override;
@@ -3082,7 +3082,7 @@ windows_nat_target::get_tib_address (ptid_t ptid, CORE_ADDR *addr)
}
ptid_t
-windows_nat_target::get_ada_task_ptid (long lwp, long thread)
+windows_nat_target::get_ada_task_ptid (long lwp, ULONGEST thread)
{
return ptid_t (inferior_ptid.pid (), lwp, 0);
}
--
2.31.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] Remove defaulted 'tid' parameter to ptid_t constructor
2021-09-22 17:10 ` [PATCH 1/3] Remove defaulted 'tid' parameter to ptid_t constructor Tom Tromey
@ 2021-09-22 17:22 ` John Baldwin
0 siblings, 0 replies; 11+ messages in thread
From: John Baldwin @ 2021-09-22 17:22 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 9/22/21 10:10 AM, Tom Tromey via Gdb-patches wrote:
> I wanted to find, and potentially modify, all the spots where the
> 'tid' parameter to the ptid_t constructor was used. So, I temporarily
> removed this parameter and then rebuilt.
>
> In order to make it simpler to search through the "real" (nonzero)
> uses of this parameter, something I knew I'd have to do multiple
> times, I removed any ", 0" from constructor calls.
There are some similar cases in fbsd-nat.c you can also fix:
diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c
index 234e74fcfd4..e2970e4d1ca 100644
--- a/gdb/fbsd-nat.c
+++ b/gdb/fbsd-nat.c
@@ -889,7 +889,7 @@ fbsd_add_threads (fbsd_nat_target *target, pid_t pid)
for (i = 0; i < nlwps; i++)
{
- ptid_t ptid = ptid_t (pid, lwps[i], 0);
+ ptid_t ptid = ptid_t (pid, lwps[i]);
if (!in_thread_list (target, ptid))
{
@@ -1190,7 +1190,7 @@ fbsd_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
if (ptrace (PT_LWPINFO, pid, (caddr_t) &pl, sizeof pl) == -1)
perror_with_name (("ptrace"));
- wptid = ptid_t (pid, pl.pl_lwpid, 0);
+ wptid = ptid_t (pid, pl.pl_lwpid);
if (debug_fbsd_nat)
{
@@ -1286,7 +1286,7 @@ fbsd_nat_target::wait (ptid_t ptid, struct target_waitstatus *ourstatus,
perror_with_name (("ptrace"));
gdb_assert (pl.pl_flags & PL_FLAG_CHILD);
- child_ptid = ptid_t (child, pl.pl_lwpid, 0);
+ child_ptid = ptid_t (child, pl.pl_lwpid);
}
/* Enable additional events on the child process. */
--
John Baldwin
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] Change ptid_t::tid to ULONGEST
2021-09-22 17:10 ` [PATCH 2/3] Change ptid_t::tid to ULONGEST Tom Tromey
@ 2021-09-22 17:24 ` John Baldwin
2021-09-22 20:44 ` Tom Tromey
2021-09-23 10:31 ` Andrew Burgess
1 sibling, 1 reply; 11+ messages in thread
From: John Baldwin @ 2021-09-22 17:24 UTC (permalink / raw)
To: Tom Tromey, gdb-patches
On 9/22/21 10:10 AM, Tom Tromey via Gdb-patches wrote:
> The ptid_t 'tid' member is normally used as an address in gdb -- both
> bsd-uthread and ravenscar-thread use it this way. However, because
> the type is 'long', this can cause problems with sign extension.
>
> This patch changes the type to ULONGEST to ensure that sign extension
> does not occur.
One more in fbsd-nat.c:
diff --git a/gdb/fbsd-nat.c b/gdb/fbsd-nat.c
index 234e74fcfd4..cf916a4794e 100644
--- a/gdb/fbsd-nat.c
+++ b/gdb/fbsd-nat.c
@@ -1044,8 +1044,8 @@ fbsd_nat_target::resume (ptid_t ptid, int step, enum gdb_signal signo)
return;
#endif
- fbsd_lwp_debug_printf ("ptid (%d, %ld, %ld)", ptid.pid (), ptid.lwp (),
- ptid.tid ());
+ fbsd_lwp_debug_printf ("ptid (%d, %ld, %s)", ptid.pid (), ptid.lwp (),
+ pulongest (ptid.tid ()));
if (ptid.lwp_p ())
{
/* If ptid is a specific LWP, suspend all other LWPs in the process. */
> ---
> gdb/ada-tasks.c | 3 ++-
> gdb/bsd-uthread.c | 5 +++--
> gdb/infrun.c | 8 ++++----
> gdb/python/py-infthread.c | 5 +++--
> gdb/ravenscar-thread.c | 5 +++--
> gdb/remote.c | 5 +++--
> gdbserver/target.cc | 4 ++--
> gdbsupport/ptid.cc | 3 ++-
> gdbsupport/ptid.h | 7 ++++---
> 9 files changed, 26 insertions(+), 19 deletions(-)
>
> diff --git a/gdb/ada-tasks.c b/gdb/ada-tasks.c
> index 80a72216f96..bef26e8e43f 100644
> --- a/gdb/ada-tasks.c
> +++ b/gdb/ada-tasks.c
> @@ -1233,7 +1233,8 @@ info_task (struct ui_out *uiout, const char *taskno_str, struct inferior *inf)
> fprintf_styled (gdb_stdout, metadata_style.style (), _("<no name>\n"));
>
> /* Print the TID and LWP. */
> - printf_filtered (_("Thread: %#lx\n"), task_info->ptid.tid ());
> + printf_filtered (_("Thread: 0x%s\n"), phex_nz (task_info->ptid.tid (),
> + sizeof (ULONGEST)));
> printf_filtered (_("LWP: %#lx\n"), task_info->ptid.lwp ());
>
> /* If set, print the base CPU. */
> diff --git a/gdb/bsd-uthread.c b/gdb/bsd-uthread.c
> index f8353f07041..1e594983bcd 100644
> --- a/gdb/bsd-uthread.c
> +++ b/gdb/bsd-uthread.c
> @@ -538,8 +538,9 @@ std::string
> bsd_uthread_target::pid_to_str (ptid_t ptid)
> {
> if (ptid.tid () != 0)
> - return string_printf ("process %d, thread 0x%lx",
> - ptid.pid (), ptid.tid ());
> + return string_printf ("process %d, thread 0x%s",
> + ptid.pid (),
> + phex_nz (ptid.tid (), sizeof (ULONGEST)));
>
> return normal_pid_to_str (ptid);
> }
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index d1ac9b4cbbb..146d06e92b5 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -4646,11 +4646,11 @@ wait_one ()
> static void
> save_waitstatus (struct thread_info *tp, const target_waitstatus *ws)
> {
> - infrun_debug_printf ("saving status %s for %d.%ld.%ld",
> + infrun_debug_printf ("saving status %s for %d.%ld.%s",
> target_waitstatus_to_string (ws).c_str (),
> tp->ptid.pid (),
> tp->ptid.lwp (),
> - tp->ptid.tid ());
> + pulongest (tp->ptid.tid ()));
>
> /* Record for later. */
> tp->set_pending_waitstatus (*ws);
> @@ -4845,9 +4845,9 @@ handle_one (const wait_one_event &event)
> struct regcache *regcache;
>
> infrun_debug_printf
> - ("target_wait %s, saving status for %d.%ld.%ld",
> + ("target_wait %s, saving status for %d.%ld.%s",
> target_waitstatus_to_string (&event.ws).c_str (),
> - t->ptid.pid (), t->ptid.lwp (), t->ptid.tid ());
> + t->ptid.pid (), t->ptid.lwp (), pulongest (t->ptid.tid ()));
>
> /* Record for later. */
> save_waitstatus (t, &event.ws);
> diff --git a/gdb/python/py-infthread.c b/gdb/python/py-infthread.c
> index 5645442a426..74bbe9935f0 100644
> --- a/gdb/python/py-infthread.c
> +++ b/gdb/python/py-infthread.c
> @@ -296,7 +296,8 @@ PyObject *
> gdbpy_create_ptid_object (ptid_t ptid)
> {
> int pid;
> - long tid, lwp;
> + long lwp;
> + ULONGEST tid;
> PyObject *ret;
>
> ret = PyTuple_New (3);
> @@ -313,7 +314,7 @@ gdbpy_create_ptid_object (ptid_t ptid)
> gdbpy_ref<> lwp_obj = gdb_py_object_from_longest (lwp);
> if (lwp_obj == nullptr)
> return nullptr;
> - gdbpy_ref<> tid_obj = gdb_py_object_from_longest (tid);
> + gdbpy_ref<> tid_obj = gdb_py_object_from_ulongest (tid);
> if (tid_obj == nullptr)
> return nullptr;
>
> diff --git a/gdb/ravenscar-thread.c b/gdb/ravenscar-thread.c
> index 634af466790..6cc583ce348 100644
> --- a/gdb/ravenscar-thread.c
> +++ b/gdb/ravenscar-thread.c
> @@ -164,7 +164,7 @@ struct ravenscar_thread_target final : public target_ops
> needed because sometimes the runtime will report an active task
> that hasn't yet been put on the list of tasks that is read by
> ada-tasks.c. */
> - std::unordered_map<long, int> m_cpu_map;
> + std::unordered_map<ULONGEST, int> m_cpu_map;
> };
>
> /* Return true iff PTID corresponds to a ravenscar task. */
> @@ -469,7 +469,8 @@ ravenscar_thread_target::pid_to_str (ptid_t ptid)
> if (!is_ravenscar_task (ptid))
> return beneath ()->pid_to_str (ptid);
>
> - return string_printf ("Ravenscar Thread %#x", (int) ptid.tid ());
> + return string_printf ("Ravenscar Thread 0x%s",
> + phex_nz (ptid.tid (), sizeof (ULONGEST)));
> }
>
> /* Temporarily set the ptid of a regcache to some other value. When
> diff --git a/gdb/remote.c b/gdb/remote.c
> index 49ed2099211..3ece443ad18 100644
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -6907,8 +6907,9 @@ remote_target::remote_stop_ns (ptid_t ptid)
> == resume_state::RESUMED_PENDING_VCONT)
> {
> remote_debug_printf ("Enqueueing phony stop reply for thread pending "
> - "vCont-resume (%d, %ld, %ld)", tp->ptid.pid(),
> - tp->ptid.lwp (), tp->ptid.tid ());
> + "vCont-resume (%d, %ld, %s)", tp->ptid.pid(),
> + tp->ptid.lwp (),
> + pulongest (tp->ptid.tid ()));
>
> /* Check that the thread wasn't resumed with a signal.
> Generating a phony stop would result in losing the
> diff --git a/gdbserver/target.cc b/gdbserver/target.cc
> index 6660cc04fd5..988c6d50c5c 100644
> --- a/gdbserver/target.cc
> +++ b/gdbserver/target.cc
> @@ -286,8 +286,8 @@ target_pid_to_str (ptid_t ptid)
> else if (ptid == null_ptid)
> xsnprintf (buf, sizeof (buf), "<null thread>");
> else if (ptid.tid () != 0)
> - xsnprintf (buf, sizeof (buf), "Thread %d.0x%lx",
> - ptid.pid (), ptid.tid ());
> + xsnprintf (buf, sizeof (buf), "Thread %d.0x%s",
> + ptid.pid (), phex_nz (ptid.tid (), sizeof (ULONGEST)));
> else if (ptid.lwp () != 0)
> xsnprintf (buf, sizeof (buf), "LWP %d.%ld",
> ptid.pid (), ptid.lwp ());
> diff --git a/gdbsupport/ptid.cc b/gdbsupport/ptid.cc
> index 2417120b1ee..e5180662612 100644
> --- a/gdbsupport/ptid.cc
> +++ b/gdbsupport/ptid.cc
> @@ -19,6 +19,7 @@
>
> #include "common-defs.h"
> #include "ptid.h"
> +#include "print-utils.h"
>
> /* See ptid.h for these. */
>
> @@ -30,5 +31,5 @@ ptid_t const minus_one_ptid = ptid_t::make_minus_one ();
> std::string
> ptid_t::to_string () const
> {
> - return string_printf ("%d.%ld.%ld", m_pid, m_lwp, m_tid);
> + return string_printf ("%d.%ld.%s", m_pid, m_lwp, pulongest (m_tid));
> }
> diff --git a/gdbsupport/ptid.h b/gdbsupport/ptid.h
> index a2553b29c1a..7cdf468589d 100644
> --- a/gdbsupport/ptid.h
> +++ b/gdbsupport/ptid.h
> @@ -34,6 +34,7 @@
>
> #include <functional>
> #include <string>
> +#include "gdbsupport/common-types.h"
>
> class ptid_t
> {
> @@ -47,7 +48,7 @@ class ptid_t
> A ptid with only a PID (LWP and TID equal to zero) is usually used to
> represent a whole process, including all its lwps/threads. */
>
> - explicit constexpr ptid_t (int pid, long lwp = 0, long tid = 0)
> + explicit constexpr ptid_t (int pid, long lwp = 0, ULONGEST tid = 0)
> : m_pid (pid), m_lwp (lwp), m_tid (tid)
> {}
>
> @@ -73,7 +74,7 @@ class ptid_t
>
> /* Fetch the tid (thread id) component from a ptid. */
>
> - constexpr long tid () const
> + constexpr ULONGEST tid () const
> { return m_tid; }
>
> /* Return true if the ptid represents a whole process, including all its
> @@ -149,7 +150,7 @@ class ptid_t
> long m_lwp;
>
> /* Thread id. */
> - long m_tid;
> + ULONGEST m_tid;
> };
>
> /* Functor to hash a ptid. */
>
--
John Baldwin
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] Change ptid_t::tid to ULONGEST
2021-09-22 17:24 ` John Baldwin
@ 2021-09-22 20:44 ` Tom Tromey
0 siblings, 0 replies; 11+ messages in thread
From: Tom Tromey @ 2021-09-22 20:44 UTC (permalink / raw)
To: John Baldwin; +Cc: Tom Tromey, gdb-patches
>>>>> "John" == John Baldwin <jhb@FreeBSD.org> writes:
John> One more in fbsd-nat.c:
[...]
Thanks, I've applied both of these changes to my series.
Tom
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] Change ptid_t::tid to ULONGEST
2021-09-22 17:10 ` [PATCH 2/3] Change ptid_t::tid to ULONGEST Tom Tromey
2021-09-22 17:24 ` John Baldwin
@ 2021-09-23 10:31 ` Andrew Burgess
2021-09-23 14:10 ` Tom Tromey
1 sibling, 1 reply; 11+ messages in thread
From: Andrew Burgess @ 2021-09-23 10:31 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
* Tom Tromey via Gdb-patches <gdb-patches@sourceware.org> [2021-09-22 11:10:54 -0600]:
> The ptid_t 'tid' member is normally used as an address in gdb -- both
> bsd-uthread and ravenscar-thread use it this way. However, because
> the type is 'long', this can cause problems with sign extension.
>
> This patch changes the type to ULONGEST to ensure that sign extension
> does not occur.
> ---
> gdb/ada-tasks.c | 3 ++-
> gdb/bsd-uthread.c | 5 +++--
> gdb/infrun.c | 8 ++++----
> gdb/python/py-infthread.c | 5 +++--
> gdb/ravenscar-thread.c | 5 +++--
> gdb/remote.c | 5 +++--
> gdbserver/target.cc | 4 ++--
> gdbsupport/ptid.cc | 3 ++-
> gdbsupport/ptid.h | 7 ++++---
> 9 files changed, 26 insertions(+), 19 deletions(-)
>
> diff --git a/gdb/ada-tasks.c b/gdb/ada-tasks.c
> index 80a72216f96..bef26e8e43f 100644
> --- a/gdb/ada-tasks.c
> +++ b/gdb/ada-tasks.c
> @@ -1233,7 +1233,8 @@ info_task (struct ui_out *uiout, const char *taskno_str, struct inferior *inf)
> fprintf_styled (gdb_stdout, metadata_style.style (), _("<no name>\n"));
>
> /* Print the TID and LWP. */
> - printf_filtered (_("Thread: %#lx\n"), task_info->ptid.tid ());
> + printf_filtered (_("Thread: 0x%s\n"), phex_nz (task_info->ptid.tid (),
> + sizeof (ULONGEST)));
> printf_filtered (_("LWP: %#lx\n"), task_info->ptid.lwp ());
>
> /* If set, print the base CPU. */
> diff --git a/gdb/bsd-uthread.c b/gdb/bsd-uthread.c
> index f8353f07041..1e594983bcd 100644
> --- a/gdb/bsd-uthread.c
> +++ b/gdb/bsd-uthread.c
> @@ -538,8 +538,9 @@ std::string
> bsd_uthread_target::pid_to_str (ptid_t ptid)
> {
> if (ptid.tid () != 0)
> - return string_printf ("process %d, thread 0x%lx",
> - ptid.pid (), ptid.tid ());
> + return string_printf ("process %d, thread 0x%s",
> + ptid.pid (),
> + phex_nz (ptid.tid (), sizeof (ULONGEST)));
>
> return normal_pid_to_str (ptid);
> }
> diff --git a/gdb/infrun.c b/gdb/infrun.c
> index d1ac9b4cbbb..146d06e92b5 100644
> --- a/gdb/infrun.c
> +++ b/gdb/infrun.c
> @@ -4646,11 +4646,11 @@ wait_one ()
> static void
> save_waitstatus (struct thread_info *tp, const target_waitstatus *ws)
> {
> - infrun_debug_printf ("saving status %s for %d.%ld.%ld",
> + infrun_debug_printf ("saving status %s for %d.%ld.%s",
> target_waitstatus_to_string (ws).c_str (),
> tp->ptid.pid (),
> tp->ptid.lwp (),
> - tp->ptid.tid ());
> + pulongest (tp->ptid.tid ()));
As you're changing this anyway, I think it would be better to write:
infrun_debug_printf ("saving status %s for %s",
target_waitstatus_to_string (ws).c_str (),
tp->ptid.to_string ().c_str ());
>
> /* Record for later. */
> tp->set_pending_waitstatus (*ws);
> @@ -4845,9 +4845,9 @@ handle_one (const wait_one_event &event)
> struct regcache *regcache;
>
> infrun_debug_printf
> - ("target_wait %s, saving status for %d.%ld.%ld",
> + ("target_wait %s, saving status for %d.%ld.%s",
> target_waitstatus_to_string (&event.ws).c_str (),
> - t->ptid.pid (), t->ptid.lwp (), t->ptid.tid ());
> + t->ptid.pid (), t->ptid.lwp (), pulongest (t->ptid.tid ()));
And here.
Thanks,
Andrew
>
> /* Record for later. */
> save_waitstatus (t, &event.ws);
> diff --git a/gdb/python/py-infthread.c b/gdb/python/py-infthread.c
> index 5645442a426..74bbe9935f0 100644
> --- a/gdb/python/py-infthread.c
> +++ b/gdb/python/py-infthread.c
> @@ -296,7 +296,8 @@ PyObject *
> gdbpy_create_ptid_object (ptid_t ptid)
> {
> int pid;
> - long tid, lwp;
> + long lwp;
> + ULONGEST tid;
> PyObject *ret;
>
> ret = PyTuple_New (3);
> @@ -313,7 +314,7 @@ gdbpy_create_ptid_object (ptid_t ptid)
> gdbpy_ref<> lwp_obj = gdb_py_object_from_longest (lwp);
> if (lwp_obj == nullptr)
> return nullptr;
> - gdbpy_ref<> tid_obj = gdb_py_object_from_longest (tid);
> + gdbpy_ref<> tid_obj = gdb_py_object_from_ulongest (tid);
> if (tid_obj == nullptr)
> return nullptr;
>
> diff --git a/gdb/ravenscar-thread.c b/gdb/ravenscar-thread.c
> index 634af466790..6cc583ce348 100644
> --- a/gdb/ravenscar-thread.c
> +++ b/gdb/ravenscar-thread.c
> @@ -164,7 +164,7 @@ struct ravenscar_thread_target final : public target_ops
> needed because sometimes the runtime will report an active task
> that hasn't yet been put on the list of tasks that is read by
> ada-tasks.c. */
> - std::unordered_map<long, int> m_cpu_map;
> + std::unordered_map<ULONGEST, int> m_cpu_map;
> };
>
> /* Return true iff PTID corresponds to a ravenscar task. */
> @@ -469,7 +469,8 @@ ravenscar_thread_target::pid_to_str (ptid_t ptid)
> if (!is_ravenscar_task (ptid))
> return beneath ()->pid_to_str (ptid);
>
> - return string_printf ("Ravenscar Thread %#x", (int) ptid.tid ());
> + return string_printf ("Ravenscar Thread 0x%s",
> + phex_nz (ptid.tid (), sizeof (ULONGEST)));
> }
>
> /* Temporarily set the ptid of a regcache to some other value. When
> diff --git a/gdb/remote.c b/gdb/remote.c
> index 49ed2099211..3ece443ad18 100644
> --- a/gdb/remote.c
> +++ b/gdb/remote.c
> @@ -6907,8 +6907,9 @@ remote_target::remote_stop_ns (ptid_t ptid)
> == resume_state::RESUMED_PENDING_VCONT)
> {
> remote_debug_printf ("Enqueueing phony stop reply for thread pending "
> - "vCont-resume (%d, %ld, %ld)", tp->ptid.pid(),
> - tp->ptid.lwp (), tp->ptid.tid ());
> + "vCont-resume (%d, %ld, %s)", tp->ptid.pid(),
> + tp->ptid.lwp (),
> + pulongest (tp->ptid.tid ()));
>
> /* Check that the thread wasn't resumed with a signal.
> Generating a phony stop would result in losing the
> diff --git a/gdbserver/target.cc b/gdbserver/target.cc
> index 6660cc04fd5..988c6d50c5c 100644
> --- a/gdbserver/target.cc
> +++ b/gdbserver/target.cc
> @@ -286,8 +286,8 @@ target_pid_to_str (ptid_t ptid)
> else if (ptid == null_ptid)
> xsnprintf (buf, sizeof (buf), "<null thread>");
> else if (ptid.tid () != 0)
> - xsnprintf (buf, sizeof (buf), "Thread %d.0x%lx",
> - ptid.pid (), ptid.tid ());
> + xsnprintf (buf, sizeof (buf), "Thread %d.0x%s",
> + ptid.pid (), phex_nz (ptid.tid (), sizeof (ULONGEST)));
> else if (ptid.lwp () != 0)
> xsnprintf (buf, sizeof (buf), "LWP %d.%ld",
> ptid.pid (), ptid.lwp ());
> diff --git a/gdbsupport/ptid.cc b/gdbsupport/ptid.cc
> index 2417120b1ee..e5180662612 100644
> --- a/gdbsupport/ptid.cc
> +++ b/gdbsupport/ptid.cc
> @@ -19,6 +19,7 @@
>
> #include "common-defs.h"
> #include "ptid.h"
> +#include "print-utils.h"
>
> /* See ptid.h for these. */
>
> @@ -30,5 +31,5 @@ ptid_t const minus_one_ptid = ptid_t::make_minus_one ();
> std::string
> ptid_t::to_string () const
> {
> - return string_printf ("%d.%ld.%ld", m_pid, m_lwp, m_tid);
> + return string_printf ("%d.%ld.%s", m_pid, m_lwp, pulongest (m_tid));
> }
> diff --git a/gdbsupport/ptid.h b/gdbsupport/ptid.h
> index a2553b29c1a..7cdf468589d 100644
> --- a/gdbsupport/ptid.h
> +++ b/gdbsupport/ptid.h
> @@ -34,6 +34,7 @@
>
> #include <functional>
> #include <string>
> +#include "gdbsupport/common-types.h"
>
> class ptid_t
> {
> @@ -47,7 +48,7 @@ class ptid_t
> A ptid with only a PID (LWP and TID equal to zero) is usually used to
> represent a whole process, including all its lwps/threads. */
>
> - explicit constexpr ptid_t (int pid, long lwp = 0, long tid = 0)
> + explicit constexpr ptid_t (int pid, long lwp = 0, ULONGEST tid = 0)
> : m_pid (pid), m_lwp (lwp), m_tid (tid)
> {}
>
> @@ -73,7 +74,7 @@ class ptid_t
>
> /* Fetch the tid (thread id) component from a ptid. */
>
> - constexpr long tid () const
> + constexpr ULONGEST tid () const
> { return m_tid; }
>
> /* Return true if the ptid represents a whole process, including all its
> @@ -149,7 +150,7 @@ class ptid_t
> long m_lwp;
>
> /* Thread id. */
> - long m_tid;
> + ULONGEST m_tid;
> };
>
> /* Functor to hash a ptid. */
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/3] Fix an Ada sign-extension problem
2021-09-22 17:10 [PATCH 0/3] Fix an Ada sign-extension problem Tom Tromey
` (2 preceding siblings ...)
2021-09-22 17:10 ` [PATCH 3/3] Change get_ada_task_ptid parameter type Tom Tromey
@ 2021-09-23 10:53 ` Andrew Burgess
2021-09-23 15:30 ` Tom Tromey
3 siblings, 1 reply; 11+ messages in thread
From: Andrew Burgess @ 2021-09-23 10:53 UTC (permalink / raw)
To: Tom Tromey; +Cc: gdb-patches
* Tom Tromey via Gdb-patches <gdb-patches@sourceware.org> [2021-09-22 11:10:52 -0600]:
> On some platforms, the Ada "info task" command can fail with a memory
> error. I tracked this down to an unintended sign extension, caused
> partly by the use of 'long' in get_ada_task_ptid, and partly by the
> use of 'long' in ptid_t.
>
> This series changes these to use ULONGEST instead, avoiding any
> unintentional sign extension.
>
> Regression tested on x86-64 Fedora 34.
I looked through the series, all looks good with my one suggestion on
patch #2.
Thanks,
Andrew
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] Change ptid_t::tid to ULONGEST
2021-09-23 10:31 ` Andrew Burgess
@ 2021-09-23 14:10 ` Tom Tromey
0 siblings, 0 replies; 11+ messages in thread
From: Tom Tromey @ 2021-09-23 14:10 UTC (permalink / raw)
To: Andrew Burgess; +Cc: Tom Tromey, gdb-patches
Andrew> As you're changing this anyway, I think it would be better to write:
Andrew> infrun_debug_printf ("saving status %s for %s",
Andrew> target_waitstatus_to_string (ws).c_str (),
Andrew> tp->ptid.to_string ().c_str ());
Make sense, I made these changes.
Tom
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/3] Fix an Ada sign-extension problem
2021-09-23 10:53 ` [PATCH 0/3] Fix an Ada sign-extension problem Andrew Burgess
@ 2021-09-23 15:30 ` Tom Tromey
0 siblings, 0 replies; 11+ messages in thread
From: Tom Tromey @ 2021-09-23 15:30 UTC (permalink / raw)
To: Andrew Burgess; +Cc: Tom Tromey, gdb-patches
Andrew> I looked through the series, all looks good with my one suggestion on
Andrew> patch #2.
Thanks. I'm checking this in with the various updates.
Tom
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-09-23 15:30 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-22 17:10 [PATCH 0/3] Fix an Ada sign-extension problem Tom Tromey
2021-09-22 17:10 ` [PATCH 1/3] Remove defaulted 'tid' parameter to ptid_t constructor Tom Tromey
2021-09-22 17:22 ` John Baldwin
2021-09-22 17:10 ` [PATCH 2/3] Change ptid_t::tid to ULONGEST Tom Tromey
2021-09-22 17:24 ` John Baldwin
2021-09-22 20:44 ` Tom Tromey
2021-09-23 10:31 ` Andrew Burgess
2021-09-23 14:10 ` Tom Tromey
2021-09-22 17:10 ` [PATCH 3/3] Change get_ada_task_ptid parameter type Tom Tromey
2021-09-23 10:53 ` [PATCH 0/3] Fix an Ada sign-extension problem Andrew Burgess
2021-09-23 15:30 ` Tom Tromey
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).