From: Simon Marchi Date: Fri, 7 Apr 2017 03:29:53 +0000 (-0400) Subject: Class-ify ptid_t X-Git-Url: https://git.libre-soc.org/?a=commitdiff_plain;h=436252de3e9de546001c4312d0863ce7e10aa200;p=binutils-gdb.git Class-ify ptid_t 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. : New constructors. : New methods. : New static methods. : Rename to... : ...this. : Rename to... : ...this. : Rename to... : ...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. --- diff --git a/gdb/ChangeLog b/gdb/ChangeLog index e906b870c5c..0f36e84e541 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,30 @@ +2017-04-06 Simon Marchi + + * common/ptid.h (struct ptid): Change to... + (class ptid_t): ... this. + : New constructors. + : New methods. + : New static methods. + : Rename to... + : ...this. + : Rename to... + : ...this. + : Rename to... + : ...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 * python/python.c (python_run_simple_file): Cast mode literal to diff --git a/gdb/Makefile.in b/gdb/Makefile.in index 479d27344b0..23e4bed3aa6 100644 --- a/gdb/Makefile.in +++ b/gdb/Makefile.in @@ -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 diff --git a/gdb/common/ptid.c b/gdb/common/ptid.c index b56971b9af2..81f16d047e8 100644 --- a/gdb/common/ptid.c +++ b/gdb/common/ptid.c @@ -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); } diff --git a/gdb/common/ptid.h b/gdb/common/ptid.h index 337bfb0899d..70f5199b62f 100644 --- a/gdb/common/ptid.h +++ b/gdb/common/ptid.h @@ -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 diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog index 9b0c8870fff..8b1adfe2709 100644 --- a/gdb/gdbserver/ChangeLog +++ b/gdb/gdbserver/ChangeLog @@ -1,3 +1,8 @@ +2017-04-06 Simon Marchi + + * server.c (handle_v_cont): Initialize thread_resume::thread + with null_ptid. + 2017-04-05 Pedro Alves * configure: Regenerate. diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c index 51f6a28419d..9cc6145b051 100644 --- a/gdb/gdbserver/server.c +++ b/gdb/gdbserver/server.c @@ -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. */ diff --git a/gdb/unittests/ptid-selftests.c b/gdb/unittests/ptid-selftests.c new file mode 100644 index 00000000000..5fc2ca6c18d --- /dev/null +++ b/gdb/unittests/ptid-selftests.c @@ -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 . */ + +#include "defs.h" +#include "common/ptid.h" +#include + +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::value, "ptid_t is POD"); + +/* We want to avoid implicit conversion from int to ptid_t. */ + +static_assert (!std::is_convertible::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 */