From: Simon Marchi <simon.marchi@ericsson.com>
Here's what I pushed. I had to make a slight change, in make_null and
make_minus_one, the initializer list didn't play well with the explicit
constructor, so I used the constructor form directly.
I grew a bit tired of using ptid_get_{lwp,pid,tid} and friends, so I decided to
make it a bit easier to use by making it a proper class. The fields are now
private, so it's not possible to change a ptid_t field by mistake.
The new methods of ptid_t map to existing functions/practice like this:
ptid_t (pid, lwp, tid) -> ptid_build (pid, lwp, tid)
ptid_t (pid) -> pid_to_ptid (pid)
ptid.is_pid () -> ptid_is_pid (ptid)
ptid == other -> ptid_equal (ptid, other)
ptid != other -> !ptid_equal (ptid, other)
ptid.pid () -> ptid_get_pid (ptid)
ptid.lwp_p () -> ptid_lwp_p (ptid)
ptid.lwp () -> ptid_get_lwp (ptid)
ptid.tid_p () -> ptid_tid_p (ptid)
ptid.tid () -> ptid_get_tid (ptid)
ptid.matches (filter) -> ptid_match (ptid, filter)
I've replaced the implementation of the existing functions with calls to
the new methods. People are encouraged to gradually switch to using the
ptid_t methods instead of the functions (or we can change them all in
one pass eventually).
Also, I'm not sure if it's worth it (because of ptid_t's relatively
small size), but I have made the functions and methods take ptid_t
arguments by const reference instead of by value.
gdb/ChangeLog:
* common/ptid.h (struct ptid): Change to...
(class ptid_t): ... this.
<ptid_t>: New constructors.
<pid, lwp_p, lwp, tid_p, tid, is_pid, operator==, operator!=,
matches>: New methods.
<make_null, make_minus_one>: New static methods.
<pid>: Rename to...
<m_pid>: ...this.
<lwp>: Rename to...
<m_lwp>: ...this.
<tid>: Rename to...
<m_tid>: ...this.
(ptid_build, ptid_get_pid, ptid_get_lwp, ptid_get_tid, ptid_equal,
ptid_is_pid, ptid_lwp_p, ptid_tid_p, ptid_match): Take ptid arguments
as references, move comment to class ptid_t.
* common/ptid.c (null_ptid, minus_one_ptid): Initialize with
ptid_t static methods.
(ptid_build, pid_to_ptid, ptid_get_pid, ptid_get_tid,
ptid_equal, ptid_is_pid, ptid_lwp_p, ptid_tid_p, ptid_match):
Take ptid arguments as references, implement using ptid_t methods.
* unittests/ptid-selftests.c: New file.
* Makefile.in (SUBDIR_UNITTESTS_SRCS): Add
unittests/ptid-selftests.c.
(SUBDIR_UNITTESTS_OBS): Add unittests/ptid-selftests.o.
gdb/gdbserver/ChangeLog:
* server.c (handle_v_cont): Initialize thread_resume::thread
with null_ptid.
---
gdb/ChangeLog | 27 +++++++
gdb/Makefile.in | 6 +-
gdb/common/ptid.c | 61 ++++++---------
gdb/common/ptid.h | 173 ++++++++++++++++++++++++++++++++---------
gdb/gdbserver/ChangeLog | 5 ++
gdb/gdbserver/server.c | 2 +-
gdb/unittests/ptid-selftests.c | 153 ++++++++++++++++++++++++++++++++++++
7 files changed, 350 insertions(+), 77 deletions(-)
create mode 100644 gdb/unittests/ptid-selftests.c
@@ -1,3 +1,30 @@
+2017-04-06 Simon Marchi <simon.marchi@ericsson.com>
+
+ * common/ptid.h (struct ptid): Change to...
+ (class ptid_t): ... this.
+ <ptid_t>: New constructors.
+ <pid, lwp_p, lwp, tid_p, tid, is_pid, operator==, operator!=,
+ matches>: New methods.
+ <make_null, make_minus_one>: New static methods.
+ <pid>: Rename to...
+ <m_pid>: ...this.
+ <lwp>: Rename to...
+ <m_lwp>: ...this.
+ <tid>: Rename to...
+ <m_tid>: ...this.
+ (ptid_build, ptid_get_pid, ptid_get_lwp, ptid_get_tid, ptid_equal,
+ ptid_is_pid, ptid_lwp_p, ptid_tid_p, ptid_match): Take ptid arguments
+ as references, move comment to class ptid_t.
+ * common/ptid.c (null_ptid, minus_one_ptid): Initialize with
+ ptid_t static methods.
+ (ptid_build, pid_to_ptid, ptid_get_pid, ptid_get_tid,
+ ptid_equal, ptid_is_pid, ptid_lwp_p, ptid_tid_p, ptid_match):
+ Take ptid arguments as references, implement using ptid_t methods.
+ * unittests/ptid-selftests.c: New file.
+ * Makefile.in (SUBDIR_UNITTESTS_SRCS): Add
+ unittests/ptid-selftests.c.
+ (SUBDIR_UNITTESTS_OBS): Add unittests/ptid-selftests.o.
+
2017-04-06 Thomas Preud'homme <thomas.preudhomme@arm.com>
* python/python.c (python_run_simple_file): Cast mode literal to
@@ -525,11 +525,13 @@ SUBDIR_PYTHON_CFLAGS =
SUBDIR_UNITTESTS_SRCS = \
unittests/function-view-selftests.c \
- unittests/offset-type-selftests.c
+ unittests/offset-type-selftests.c \
+ unittests/ptid-selftests.c
SUBDIR_UNITTESTS_OBS = \
function-view-selftests.o \
- offset-type-selftests.o
+ offset-type-selftests.o \
+ ptid-selftests.o
# Opcodes currently live in one of two places. Either they are in the
# opcode library, typically ../opcodes, or they are in a header file
@@ -22,20 +22,15 @@
/* See ptid.h for these. */
-ptid_t null_ptid = { 0, 0, 0 };
-ptid_t minus_one_ptid = { -1, 0, 0 };
+ptid_t null_ptid = ptid_t::make_null ();
+ptid_t minus_one_ptid = ptid_t::make_minus_one ();
/* See ptid.h. */
ptid_t
ptid_build (int pid, long lwp, long tid)
{
- ptid_t ptid;
-
- ptid.pid = pid;
- ptid.lwp = lwp;
- ptid.tid = tid;
- return ptid;
+ return ptid_t (pid, lwp, tid);
}
/* See ptid.h. */
@@ -43,81 +38,69 @@ ptid_build (int pid, long lwp, long tid)
ptid_t
pid_to_ptid (int pid)
{
- return ptid_build (pid, 0, 0);
+ return ptid_t (pid);
}
/* See ptid.h. */
int
-ptid_get_pid (ptid_t ptid)
+ptid_get_pid (const ptid_t &ptid)
{
- return ptid.pid;
+ return ptid.pid ();
}
/* See ptid.h. */
long
-ptid_get_lwp (ptid_t ptid)
+ptid_get_lwp (const ptid_t &ptid)
{
- return ptid.lwp;
+ return ptid.lwp ();
}
/* See ptid.h. */
long
-ptid_get_tid (ptid_t ptid)
+ptid_get_tid (const ptid_t &ptid)
{
- return ptid.tid;
+ return ptid.tid ();
}
/* See ptid.h. */
int
-ptid_equal (ptid_t ptid1, ptid_t ptid2)
+ptid_equal (const ptid_t &ptid1, const ptid_t &ptid2)
{
- return (ptid1.pid == ptid2.pid
- && ptid1.lwp == ptid2.lwp
- && ptid1.tid == ptid2.tid);
+ return ptid1 == ptid2;
}
/* See ptid.h. */
int
-ptid_is_pid (ptid_t ptid)
+ptid_is_pid (const ptid_t &ptid)
{
- if (ptid_equal (minus_one_ptid, ptid)
- || ptid_equal (null_ptid, ptid))
- return 0;
-
- return (ptid_get_lwp (ptid) == 0 && ptid_get_tid (ptid) == 0);
+ return ptid.is_pid ();
}
/* See ptid.h. */
int
-ptid_lwp_p (ptid_t ptid)
+ptid_lwp_p (const ptid_t &ptid)
{
- return (ptid_get_lwp (ptid) != 0);
+ return ptid.lwp_p ();
}
/* See ptid.h. */
int
-ptid_tid_p (ptid_t ptid)
+ptid_tid_p (const ptid_t &ptid)
{
- return (ptid_get_tid (ptid) != 0);
+ return ptid.tid_p ();
}
+/* See ptid.h. */
+
int
-ptid_match (ptid_t ptid, ptid_t filter)
+ptid_match (const ptid_t &ptid, const ptid_t &filter)
{
- if (ptid_equal (filter, minus_one_ptid))
- return 1;
- if (ptid_is_pid (filter)
- && ptid_get_pid (ptid) == ptid_get_pid (filter))
- return 1;
- else if (ptid_equal (ptid, filter))
- return 1;
-
- return 0;
+ return ptid.matches (filter);
}
@@ -32,65 +32,168 @@
thread_stratum target that might want to sit on top.
*/
-struct ptid
+class ptid_t
{
+public:
+ /* Must have a trivial defaulted default constructor so that the
+ type remains POD. */
+ ptid_t () noexcept = default;
+
+ /* Make a ptid given the necessary PID, LWP, and TID components.
+
+ 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)
+ : m_pid (pid), m_lwp (lwp), m_tid (tid)
+ {}
+
+ /* Fetch the pid (process id) component from the ptid. */
+
+ constexpr int pid () const
+ { return m_pid; }
+
+ /* Return true if the ptid's lwp member is non-zero. */
+
+ constexpr bool lwp_p () const
+ { return m_lwp != 0; }
+
+ /* Fetch the lwp (lightweight process) component from the ptid. */
+
+ constexpr long lwp () const
+ { return m_lwp; }
+
+ /* Return true if the ptid's tid member is non-zero. */
+
+ constexpr bool tid_p () const
+ { return m_tid != 0; }
+
+ /* Fetch the tid (thread id) component from a ptid. */
+
+ constexpr long tid () const
+ { return m_tid; }
+
+ /* Return true if the ptid represents a whole process, including all its
+ lwps/threads. Such ptids have the form of (pid, 0, 0), with
+ pid != -1. */
+
+ constexpr bool is_pid () const
+ {
+ return (*this != make_null ()
+ && *this != make_minus_one ()
+ && m_lwp == 0
+ && m_tid == 0);
+ }
+
+ /* Compare two ptids to see if they are equal. */
+
+ constexpr bool operator== (const ptid_t &other) const
+ {
+ return (m_pid == other.m_pid
+ && m_lwp == other.m_lwp
+ && m_tid == other.m_tid);
+ }
+
+ /* Compare two ptids to see if they are different. */
+
+ constexpr bool operator!= (const ptid_t &other) const
+ {
+ return !(*this == other);
+ }
+
+ /* Return true if the ptid matches FILTER. FILTER can be the wild
+ card MINUS_ONE_PTID (all ptids match it); can be a ptid representing
+ a process (ptid.is_pid () returns true), in which case, all lwps and
+ threads of that given process match, lwps and threads of other
+ processes do not; or, it can represent a specific thread, in which
+ case, only that thread will match true. The ptid must represent a
+ specific LWP or THREAD, it can never be a wild card. */
+
+ constexpr bool matches (const ptid_t &filter) const
+ {
+ return (/* If filter represents any ptid, it's always a match. */
+ filter == make_minus_one ()
+ /* If filter is only a pid, any ptid with that pid
+ matches. */
+ || (filter.is_pid () && m_pid == filter.pid ())
+
+ /* Otherwise, this ptid only matches if it's exactly equal
+ to filter. */
+ || *this == filter);
+ }
+
+ /* Make a null ptid. */
+
+ static constexpr ptid_t make_null ()
+ { return ptid_t (0, 0, 0); }
+
+ /* Make a minus one ptid. */
+
+ static constexpr ptid_t make_minus_one ()
+ { return ptid_t (-1, 0, 0); }
+
+private:
/* Process id. */
- int pid;
+ int m_pid;
/* Lightweight process id. */
- long lwp;
+ long m_lwp;
/* Thread id. */
- long tid;
+ long m_tid;
};
-typedef struct ptid ptid_t;
-
/* The null or zero ptid, often used to indicate no process. */
+
extern ptid_t null_ptid;
/* The (-1,0,0) ptid, often used to indicate either an error condition
or a "don't care" condition, i.e, "run all threads." */
+
extern ptid_t minus_one_ptid;
-/* Make a ptid given the necessary PID, LWP, and TID components. */
-ptid_t ptid_build (int pid, long lwp, long tid);
-/* Make a new ptid from just a pid. This ptid is usually used to
- represent a whole process, including all its lwps/threads. */
-ptid_t pid_to_ptid (int pid);
+/* The following functions are kept for backwards compatibility. The use of
+ the ptid_t methods is preferred. */
+
+/* See ptid_t::ptid_t. */
+
+extern ptid_t ptid_build (int pid, long lwp, long tid);
+
+/* See ptid_t::ptid_t. */
+
+extern ptid_t pid_to_ptid (int pid);
+
+/* See ptid_t::pid. */
+
+extern int ptid_get_pid (const ptid_t &ptid);
+
+/* See ptid_t::lwp. */
+
+extern long ptid_get_lwp (const ptid_t &ptid);
+
+/* See ptid_t::tid. */
+
+extern long ptid_get_tid (const ptid_t &ptid);
+
+/* See ptid_t::operator== and ptid_t::operator!=. */
-/* Fetch the pid (process id) component from a ptid. */
-int ptid_get_pid (ptid_t ptid);
+extern int ptid_equal (const ptid_t &ptid1, const ptid_t &ptid2);
-/* Fetch the lwp (lightweight process) component from a ptid. */
-long ptid_get_lwp (ptid_t ptid);
+/* See ptid_t::is_pid. */
-/* Fetch the tid (thread id) component from a ptid. */
-long ptid_get_tid (ptid_t ptid);
+extern int ptid_is_pid (const ptid_t &ptid);
-/* Compare two ptids to see if they are equal. */
-int ptid_equal (ptid_t ptid1, ptid_t ptid2);
+/* See ptid_t::lwp_p. */
-/* Returns true if PTID represents a whole process, including all its
- lwps/threads. Such ptids have the form of (pid,0,0), with pid !=
- -1. */
-int ptid_is_pid (ptid_t ptid);
+extern int ptid_lwp_p (const ptid_t &ptid);
-/* Return true if PTID's lwp member is non-zero. */
-int ptid_lwp_p (ptid_t ptid);
+/* See ptid_t::tid_p. */
-/* Return true if PTID's tid member is non-zero. */
-int ptid_tid_p (ptid_t ptid);
+extern int ptid_tid_p (const ptid_t &ptid);
-/* Returns true if PTID matches filter FILTER. FILTER can be the wild
- card MINUS_ONE_PTID (all ptid match it); can be a ptid representing
- a process (ptid_is_pid returns true), in which case, all lwps and
- threads of that given process match, lwps and threads of other
- processes do not; or, it can represent a specific thread, in which
- case, only that thread will match true. PTID must represent a
- specific LWP or THREAD, it can never be a wild card. */
+/* See ptid_t::matches. */
-extern int ptid_match (ptid_t ptid, ptid_t filter);
+extern int ptid_match (const ptid_t &ptid, const ptid_t &filter);
#endif
@@ -1,3 +1,8 @@
+2017-04-06 Simon Marchi <simon.marchi@ericsson.com>
+
+ * server.c (handle_v_cont): Initialize thread_resume::thread
+ with null_ptid.
+
2017-04-05 Pedro Alves <palves@redhat.com>
* configure: Regenerate.
@@ -2654,7 +2654,7 @@ handle_v_cont (char *own_buf)
char *p, *q;
int n = 0, i = 0;
struct thread_resume *resume_info;
- struct thread_resume default_action = {{0}};
+ struct thread_resume default_action { null_ptid };
/* Count the number of semicolons in the packet. There should be one
for every action. */
new file mode 100644
@@ -0,0 +1,153 @@
+/* Self tests for ptid_t for GDB, the GNU debugger.
+
+ Copyright (C) 2017 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#include "defs.h"
+#include "common/ptid.h"
+#include <type_traits>
+
+namespace selftests {
+namespace ptid {
+
+/* Check that the ptid_t class is POD.
+
+ This is a requirement for as long as we have ptids embedded in
+ structures allocated with malloc. */
+
+static_assert (std::is_pod<ptid_t>::value, "ptid_t is POD");
+
+/* We want to avoid implicit conversion from int to ptid_t. */
+
+static_assert (!std::is_convertible<int, ptid_t>::value,
+ "constructor is explicit");
+
+/* Build some useful ptids. */
+
+static constexpr ptid_t pid = ptid_t (1);
+static constexpr ptid_t lwp = ptid_t (1, 2, 0);
+static constexpr ptid_t tid = ptid_t (1, 0, 2);
+static constexpr ptid_t both = ptid_t (1, 2, 2);
+
+/* Build some constexpr version of null_ptid and minus_one_ptid to use in
+ static_assert. Once the real ones are made constexpr, we can get rid of
+ these. */
+
+static constexpr ptid_t null = ptid_t::make_null ();
+static constexpr ptid_t minus_one = ptid_t::make_minus_one ();
+
+/* Verify pid. */
+
+static_assert (pid.pid () == 1, "pid's pid is right");
+static_assert (lwp.pid () == 1, "lwp's pid is right");
+static_assert (tid.pid () == 1, "tid's pid is right");
+static_assert (both.pid () == 1, "both's pid is right");
+
+/* Verify lwp_p. */
+
+static_assert (!pid.lwp_p (), "pid's lwp_p is right");
+static_assert (lwp.lwp_p (), "lwp's lwp_p is right");
+static_assert (!tid.lwp_p (), "tid's lwp_p is right");
+static_assert (both.lwp_p (), "both's lwp_p is right");
+
+/* Verify lwp. */
+
+static_assert (pid.lwp () == 0, "pid's lwp is right");
+static_assert (lwp.lwp () == 2, "lwp's lwp is right");
+static_assert (tid.lwp () == 0, "tid's lwp is right");
+static_assert (both.lwp () == 2, "both's lwp is right");
+
+/* Verify tid_p. */
+
+static_assert (!pid.tid_p (), "pid's tid_p is right");
+static_assert (!lwp.tid_p (), "lwp's tid_p is right");
+static_assert (tid.tid_p (), "tid's tid_p is right");
+static_assert (both.tid_p (), "both's tid_p is right");
+
+/* Verify tid. */
+
+static_assert (pid.tid () == 0, "pid's tid is right");
+static_assert (lwp.tid () == 0, "lwp's tid is right");
+static_assert (tid.tid () == 2, "tid's tid is right");
+static_assert (both.tid () == 2, "both's tid is right");
+
+/* Verify is_pid. */
+
+static_assert (pid.is_pid (), "pid is a pid");
+static_assert (!lwp.is_pid (), "lwp isn't a pid");
+static_assert (!tid.is_pid (), "tid isn't a pid");
+static_assert (!both.is_pid (), "both isn't a pid");
+static_assert (!null.is_pid (), "null ptid isn't a pid");
+static_assert (!minus_one.is_pid (), "minus one ptid isn't a pid");
+
+/* Verify operator ==. */
+
+static_assert (pid == ptid_t (1, 0, 0), "pid operator== is right");
+static_assert (lwp == ptid_t (1, 2, 0), "lwp operator== is right");
+static_assert (tid == ptid_t (1, 0, 2), "tid operator== is right");
+static_assert (both == ptid_t (1, 2, 2), "both operator== is right");
+
+/* Verify operator !=. */
+
+static_assert (pid != ptid_t (2, 0, 0), "pid isn't equal to a different pid");
+static_assert (pid != lwp, "pid isn't equal to one of its thread");
+static_assert (lwp != tid, "lwp isn't equal to tid");
+static_assert (both != lwp, "both isn't equal to lwp");
+static_assert (both != tid, "both isn't equal to tid");
+
+/* Verify matches against minus_one. */
+
+static_assert (pid.matches (minus_one), "pid matches minus one");
+static_assert (lwp.matches (minus_one), "lwp matches minus one");
+static_assert (tid.matches (minus_one), "tid matches minus one");
+static_assert (both.matches (minus_one), "both matches minus one");
+
+/* Verify matches against pid. */
+
+static_assert (pid.matches (pid), "pid matches pid");
+static_assert (lwp.matches (pid), "lwp matches pid");
+static_assert (tid.matches (pid), "tid matches pid");
+static_assert (both.matches (pid), "both matches pid");
+static_assert (!ptid_t (2, 0, 0).matches (pid), "other pid doesn't match pid");
+static_assert (!ptid_t (2, 2, 0).matches (pid), "other lwp doesn't match pid");
+static_assert (!ptid_t (2, 0, 2).matches (pid), "other tid doesn't match pid");
+static_assert (!ptid_t (2, 2, 2).matches (pid), "other both doesn't match pid");
+
+/* Verify matches against exact matches. */
+
+static_assert (!pid.matches (lwp), "pid matches lwp");
+static_assert (lwp.matches (lwp), "lwp matches lwp");
+static_assert (!tid.matches (lwp), "tid matches lwp");
+static_assert (!both.matches (lwp), "both matches lwp");
+static_assert (!ptid_t (2, 2, 0).matches (lwp), "other lwp doesn't match lwp");
+
+static_assert (!pid.matches (tid), "pid matches tid");
+static_assert (!lwp.matches (tid), "lwp matches tid");
+static_assert (tid.matches (tid), "tid matches tid");
+static_assert (!both.matches (tid), "both matches tid");
+static_assert (!ptid_t (2, 0, 2).matches (tid), "other tid doesn't match tid");
+
+static_assert (!pid.matches (both), "pid matches both");
+static_assert (!lwp.matches (both), "lwp matches both");
+static_assert (!tid.matches (both), "tid matches both");
+static_assert (both.matches (both), "both matches both");
+static_assert (!ptid_t (2, 2, 2).matches (both),
+ "other both doesn't match both");
+
+
+} /* namespace ptid */
+} /* namespace selftests */