Re: [PATCH 00/28] Decouple inferior_ptid/inferior_thread(); dup ptids in thread list (PR/25412)

[Pedro Alves <palves@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1395 bytes --]

On 4/17/20 9:20 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves via Gdb-patches <gdb-patches@sourceware.org> writes:
> 
> I meant to mention, I skimmed through this series and didn't see
> anything objectionable.  It's hard to know about this kind of thing
> without trying it (which I didn't do); but IMO it's fine to land this
> and then iron out anything that pops up.
> 
> Pedro> After this, inferior_ptid still exists, but it is mostly read-only and
> Pedro> mainly used by target backend code.
> 
> Could it be made completely read-only?

We could, with e.g. a hack like in the attached patch.

That patch is sufficient to make GDB build on x86-64 GNU/Linux, but
certainly it would require more changes across all the native backends.
I'm not sure it really buys us much, since the remaining writes are just
as easily found by grepping for "inferior_ptid =".

> 
> Pedro> It is also relied on by a number
> Pedro> of target methods as a global input argument.  E.g., the target_resume
> Pedro> interface and the memory reading routines -- we still need it there
> Pedro> because we need to be able to access memory off of processes for which
> Pedro> we don't have a corresponding inferior/thread object, like when
> Pedro> handling forks.  Maybe we could pass down a context explicitly to
> Pedro> target_read_memory, etc.
> 
> This sounds like a good direction.

Thanks,
Pedro Alves

[-- Attachment #2: 0001-inferior_ptid_t.patch --]
[-- Type: text/x-patch, Size: 8081 bytes --]

From dd45a29433d527344f0c89bed01dfa645d845351 Mon Sep 17 00:00:00 2001
From: Pedro Alves <palves@redhat.com>
Date: Thu, 18 Jun 2020 20:35:10 +0100
Subject: [PATCH] inferior_ptid_t

---
 gdb/breakpoint.c   |  2 +-
 gdb/infcmd.c       |  2 +-
 gdb/inferior.h     | 33 ++++++++++++++++++++++++++++++++-
 gdb/infrun.c       |  5 ++---
 gdb/linux-nat.c    |  2 +-
 gdb/proc-service.c |  2 +-
 gdb/regcache.c     |  5 +++--
 gdb/remote.c       |  2 +-
 gdb/thread.c       |  4 ++--
 9 files changed, 44 insertions(+), 13 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index aead882acd..dc080122bf 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -3584,7 +3584,7 @@ detach_breakpoints (ptid_t ptid)
     error (_("Cannot detach breakpoints of inferior_ptid"));
 
   /* Set inferior_ptid; remove_breakpoint_1 uses this global.  */
-  inferior_ptid = ptid;
+  inferior_ptid.set (ptid);
   ALL_BP_LOCATIONS (bl, blp_tmp)
   {
     if (bl->pspace != inf->pspace)
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 891da91c80..9b06f03433 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -88,7 +88,7 @@ static char *inferior_io_terminal_scratch;
    being debugged it should be nonzero (currently 3 is used) for remote
    debugging.  */
 
-ptid_t inferior_ptid;
+inferior_ptid_t inferior_ptid;
 
 /* Nonzero if stopped due to completion of a stack dummy routine.  */
 
diff --git a/gdb/inferior.h b/gdb/inferior.h
index 95af474eed..79bd9cb351 100644
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -122,10 +122,41 @@ extern void clear_sigint_trap (void);
 extern void set_inferior_io_terminal (const char *terminal_name);
 extern const char *get_inferior_io_terminal (void);
 
+class inferior_ptid_t
+{
+public:
+  int pid () const { return m_ptid.pid (); }
+  bool lwp_p () const { return m_ptid.lwp_p (); }
+  long lwp () const { return m_ptid.lwp (); }
+  bool tid_p () const { return m_ptid.tid_p (); }
+  long tid () const { return m_ptid.tid (); }
+  long is_tid () const { return m_ptid.tid (); }
+  bool is_pid () const { return m_ptid.is_pid (); }
+
+  bool matches (const ptid_t &filter) const
+  { return m_ptid.matches (filter); }
+
+  void set (ptid_t ptid) { m_ptid = ptid; }
+
+  operator ptid_t () { return m_ptid; }
+
+  friend bool operator== (const inferior_ptid_t &lhs, const ptid_t &rhs)
+  { return lhs.m_ptid == rhs; }
+  friend bool operator== (const ptid_t &lhs, const inferior_ptid_t &rhs)
+  { return lhs == rhs.m_ptid; }
+  friend bool operator!= (const inferior_ptid_t &lhs, const ptid_t &rhs)
+  { return lhs.m_ptid != rhs; }
+  friend bool operator!= (const ptid_t &lhs, const inferior_ptid_t &rhs)
+  { return lhs != rhs.m_ptid; }
+
+private:
+  ptid_t m_ptid = null_ptid;
+};
+
 /* Collected pid, tid, etc. of the debugged inferior.  When there's
    no inferior, inferior_ptid.pid () will be 0.  */
 
-extern ptid_t inferior_ptid;
+extern inferior_ptid_t inferior_ptid;
 
 extern void generic_mourn_inferior (void);
 
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 7bc405f103..0f38485536 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -1835,7 +1835,7 @@ write_memory_ptid (ptid_t ptid, CORE_ADDR memaddr,
 {
   scoped_restore save_inferior_ptid = make_scoped_restore (&inferior_ptid);
 
-  inferior_ptid = ptid;
+  inferior_ptid.set (ptid);
   write_memory (memaddr, myaddr, len);
 }
 
@@ -2071,7 +2071,7 @@ static void
 infrun_thread_ptid_changed (ptid_t old_ptid, ptid_t new_ptid)
 {
   if (inferior_ptid == old_ptid)
-    inferior_ptid = new_ptid;
+    inferior_ptid.set (new_ptid);
 }
 
 \f
@@ -9700,7 +9700,6 @@ enabled by default on some platforms."),
 			   &setlist, &showlist);
 
   /* ptid initializations */
-  inferior_ptid = null_ptid;
   target_last_wait_ptid = minus_one_ptid;
 
   gdb::observers::thread_ptid_changed.attach (infrun_thread_ptid_changed);
diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c
index 0a2bfdc57d..65438743d1 100644
--- a/gdb/linux-nat.c
+++ b/gdb/linux-nat.c
@@ -2444,7 +2444,7 @@ static int
 check_stopped_by_watchpoint (struct lwp_info *lp)
 {
   scoped_restore save_inferior_ptid = make_scoped_restore (&inferior_ptid);
-  inferior_ptid = lp->ptid;
+  inferior_ptid.set (lp->ptid);
 
   if (linux_target->low_stopped_by_watchpoint ())
     {
diff --git a/gdb/proc-service.c b/gdb/proc-service.c
index e0383700a1..2d4749bd21 100644
--- a/gdb/proc-service.c
+++ b/gdb/proc-service.c
@@ -77,7 +77,7 @@ ps_xfer_memory (const struct ps_prochandle *ph, psaddr_t addr,
   set_current_program_space (ph->thread->inf->pspace);
 
   scoped_restore save_inferior_ptid = make_scoped_restore (&inferior_ptid);
-  inferior_ptid = ph->thread->ptid;
+  inferior_ptid.set (ph->thread->ptid);
 
   CORE_ADDR core_addr = ps_addr_to_core_addr (addr);
 
diff --git a/gdb/regcache.c b/gdb/regcache.c
index 6a4359d0f3..756ea16c03 100644
--- a/gdb/regcache.c
+++ b/gdb/regcache.c
@@ -1637,8 +1637,9 @@ cooked_read_test (struct gdbarch *gdbarch)
   } pop_targets;
 
   /* Switch to the mock thread.  */
-  scoped_restore restore_inferior_ptid
-    = make_scoped_restore (&inferior_ptid, mock_ptid);
+  ptid_t save_ptid = inferior_ptid;
+  inferior_ptid.set (mock_ptid);
+  SCOPE_EXIT { inferior_ptid.set (save_ptid); };
 
   /* Test that read one raw register from regcache_no_target will go
      to the target layer.  */
diff --git a/gdb/remote.c b/gdb/remote.c
index fd89f2c084..8259831662 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -5910,7 +5910,7 @@ extended_remote_target::attach (const char *args, int from_tty)
 
   switch_to_inferior_no_thread (remote_add_inferior (false, pid, 1, 0));
 
-  inferior_ptid = ptid_t (pid);
+  inferior_ptid.set (ptid_t (pid));
 
   if (target_is_non_stop_p ())
     {
diff --git a/gdb/thread.c b/gdb/thread.c
index f0722d3588..73d8f23b5d 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -1285,7 +1285,7 @@ switch_to_thread_no_regs (struct thread_info *thread)
   set_current_inferior (inf);
 
   current_thread_ = thread;
-  inferior_ptid = current_thread_->ptid;
+  inferior_ptid.set (current_thread_->ptid);
 }
 
 /* See gdbthread.h.  */
@@ -1297,7 +1297,7 @@ switch_to_no_thread ()
     return;
 
   current_thread_ = nullptr;
-  inferior_ptid = null_ptid;
+  inferior_ptid.set (null_ptid);
   reinit_frame_cache ();
 }
 

base-commit: 2c074f49026acbe0597e0d2d2f7385195dcac565
prerequisite-patch-id: cf75804d57b7783ba470ada8e827d46c73c0d9e1
prerequisite-patch-id: 2f376c3e9de1bb5426723bc3081d27dc0221340b
prerequisite-patch-id: 7532dda900933a23405793d23231b7201b13c9e5
prerequisite-patch-id: b8e62093defdd58b9320d2d0cabef5797e962be8
prerequisite-patch-id: 060947c675effc4648dfc5f9929b084ee2b644a5
prerequisite-patch-id: e7c21d96301996d2e937aab3642fcda95c64c2c6
prerequisite-patch-id: 87412331bbd9689c4c93fc21fc70bc886230f57d
prerequisite-patch-id: 89d26483bf1c3300911e98f8a969fe97114dc671
prerequisite-patch-id: 59e116edd554952f722a8a741bf29d2080fdad3c
prerequisite-patch-id: 963bb9047825cf80900913b7c0ca4d975a00eb9d
prerequisite-patch-id: dbee563bfde550440a76eb4efa9441a84c19d366
prerequisite-patch-id: 1699f61662e53f0cb5d907d52d21480fdbab912c
prerequisite-patch-id: 7b0edf3f61a48e06ff1468be1234fa307e978629
prerequisite-patch-id: 259ed6608d652dc47d90539bce36c3002d3a75e4
prerequisite-patch-id: 6fb426c105967a6dcc0778c15a9881520537bb17
prerequisite-patch-id: c50eef1048b695c5d2305c38aaa510ccd30c671d
prerequisite-patch-id: 807aa4cdc04dac6c477d415115cf43283708fcb6
prerequisite-patch-id: 47b63abf5b4378c3d7c86774c302ccf29eda8182
prerequisite-patch-id: 00f4830b5d5dcea8d12ec42615ebf46df55909cf
prerequisite-patch-id: c06aebbe76e78f013baffeb2fcd3d6858b696b8a
prerequisite-patch-id: 84773b2ba6ae77d03da98678e2cf4ad966ad84fb
prerequisite-patch-id: 7a79ad19e2848b4b8ddc0acc7462955c327afe55
prerequisite-patch-id: b119df65172817eaccc0032178bb4173348d4ea4
prerequisite-patch-id: 0611a6654ffd0287a490debede469daea32c656b
prerequisite-patch-id: 729699cf50d1057fa11919434fcac6590a32e2eb
prerequisite-patch-id: 082610a38a64e718d229eb4484ef87dcba547bda
prerequisite-patch-id: 3e123af7bab734e29247f210a29a3d7c8946b533
prerequisite-patch-id: da39a3ee5e6b4b0d3255bfef95601890afd80709
-- 
2.14.5