Fix gdb.Inferior.read_memory without execution (PR dap/30644)

Andrew reported that the previous change to gdb.Inferior.read_memory &
friends introducing scoped_restore_current_inferior_for_memory broke
gdb.dap/stop-at-main.exp.  This is also reported as PR dap/30644.

The root of the problem is that all the methods that now use
scoped_restore_current_inferior_for_memory cause GDB to crash with a
failed assert if they are run on an inferior that is not yet started.

E.g.:

 (gdb) python i = gdb.selected_inferior ()
 (gdb) python i.read_memory (4,4)
 gdb/thread.c:626: internal-error: any_thread_of_inferior: Assertion `inf->pid != 0' failed.

This patch fixes the problem by removing
scoped_restore_current_inferior_for_memory's ctor ptid parameter and
the any_thread_of_inferior calls completely, and making
scoped_restore_current_inferior_for_memory switch inferior_ptid to a
pid ptid.

I was a little worried that some port might be assuming inferior_ptid
points at a thread in the xfer_partial memory access routines.  We
know that anything that supports forks must not assume that, due to
how detach_breakpoints works.  I looked at a number of xfer_partial
implementations, and didn't see anything that is looking at
inferior_ptid in a way that would misbehave.  I'm thinking that we
could go forward with this and just fix ports if they break.

While on some ports like on AMD GPU we have thread-specific address
spaces, and so when accessing memory for those address spaces, we must
have the right thread context (via inferior_ptid) selected, in
Inferior.read_memory, we only have the inferior to work with, so this
API as is can't be used to access thread-specific address spaces.
IOW, it can only be used to access the global address space that is
visible to both the CPU and the GPUs.

In proc-service.c:ps_xfer_memory, the other spot using
scoped_restore_current_inferior_for_memory, we're always accessing
per-inferior memory.

If we end up using scoped_restore_current_inferior_for_memory later to
set up the context to read memory from a specific thread, then we can
add an alternative ctor that takes a thread_info pointer, and make
inferior_ptid point to the thread, for example.

New test added to gdb.python/py-inferior.exp, exercising
Inferior.read_memory without execution.

No regressions on native and extended-gdbserver x86_64 GNU/Linux.

Reviewed-By: Tom Tromey <tom@tromey.com>
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30644
Change-Id: I11309c5ddbbb51a4594cf63c21b3858bfd9aed19

diff --git a/gdb/inferior.h b/gdb/inferior.h
index 98501652a276482f0d2ca099def67cfefc396f86..712f9b3f330ad7ab0c939449230ea4944a6c6419 100644 (file)
--- a/gdb/inferior.h
+++ b/gdb/inferior.h
@@ -769,14 +769,14 @@ class scoped_restore_current_inferior_for_memory
 public:
 
   /* Save the current globals and switch to the given inferior and the
-     inferior's program space.  PTID must name a thread in INF, it is
-     used as the new inferior_ptid.  */
-  scoped_restore_current_inferior_for_memory (inferior *inf, ptid_t ptid)
+     inferior's program space.  inferior_ptid is set to point to the
+     inferior's process id (and not to any particular thread).  */
+  explicit scoped_restore_current_inferior_for_memory (inferior *inf)
     : m_save_ptid (&inferior_ptid)
   {
     set_current_inferior (inf);
     set_current_program_space (inf->pspace);
-    inferior_ptid = ptid;
+    inferior_ptid = ptid_t (inf->pid);
   }
 
   DISABLE_COPY_AND_ASSIGN (scoped_restore_current_inferior_for_memory);

diff --git a/gdb/proc-service.c b/gdb/proc-service.c
index 366e0510070fa4629c76fb55aa8b510c13f511ea..f735eb0ac812d5896c6a5ab846a7f7601447e894 100644 (file)
--- a/gdb/proc-service.c
+++ b/gdb/proc-service.c
@@ -72,8 +72,7 @@ static ps_err_e
 ps_xfer_memory (const struct ps_prochandle *ph, psaddr_t addr,
                gdb_byte *buf, size_t len, int write)
 {
-  scoped_restore_current_inferior_for_memory save_inferior (ph->thread->inf,
-                                                           ph->thread->ptid);
+  scoped_restore_current_inferior_for_memory save_inferior (ph->thread->inf);
 
   CORE_ADDR core_addr = ps_addr_to_core_addr (addr);
 

diff --git a/gdb/python/py-inferior.c b/gdb/python/py-inferior.c
index f6000b944da3c7e995518bf959840d8515710e13..792f05b118e46c011179aef8d0f510a48e92659d 100644 (file)
--- a/gdb/python/py-inferior.c
+++ b/gdb/python/py-inferior.c
@@ -550,7 +550,7 @@ infpy_read_memory (PyObject *self, PyObject *args, PyObject *kw)
       /* Use this scoped-restore because we want to be able to read
         memory from an unwinder.  */
       scoped_restore_current_inferior_for_memory restore_inferior
-       (inf->inferior, any_thread_of_inferior (inf->inferior)->ptid);
+       (inf->inferior);
 
       buffer.reset ((gdb_byte *) xmalloc (length));
 
@@ -608,7 +608,7 @@ infpy_write_memory (PyObject *self, PyObject *args, PyObject *kw)
         still used here, just to keep the code similar to other code
         in this file.  */
       scoped_restore_current_inferior_for_memory restore_inferior
-       (inf->inferior, any_thread_of_inferior (inf->inferior)->ptid);
+       (inf->inferior);
 
       write_memory_with_notification (addr, buffer, length);
     }
@@ -683,7 +683,7 @@ infpy_search_memory (PyObject *self, PyObject *args, PyObject *kw)
         still used here, just to keep the code similar to other code
         in this file.  */
       scoped_restore_current_inferior_for_memory restore_inferior
-       (inf->inferior, any_thread_of_inferior (inf->inferior)->ptid);
+       (inf->inferior);
 
       found = target_search_memory (start_addr, length,
                                    buffer, pattern_size,
@@ -944,7 +944,7 @@ infpy_get_main_name (PyObject *self, void *closure)
         still used, just to keep the code similar to other code in
         this file.  */
       scoped_restore_current_inferior_for_memory restore_inferior
-       (inf->inferior, any_thread_of_inferior (inf->inferior)->ptid);
+       (inf->inferior);
 
       name = main_name ();
     }

diff --git a/gdb/testsuite/gdb.python/py-inferior.c b/gdb/testsuite/gdb.python/py-inferior.c
index 3ee9a4620604ecd0f7676b70dc4cb189275b186d..870f7196444eede5b895b35cc1ce75a57c935f81 100644 (file)
--- a/gdb/testsuite/gdb.python/py-inferior.c
+++ b/gdb/testsuite/gdb.python/py-inferior.c
@@ -16,6 +16,7 @@ int64_t int64_search_buf[100];
 static char *search_buf;
 static int search_buf_size;
 
+int8_t int8_global = 42;
 
 int f2 (int a)
 {

diff --git a/gdb/testsuite/gdb.python/py-inferior.exp b/gdb/testsuite/gdb.python/py-inferior.exp
index 13beebd08cc09878efa6ebda82e32b85d3e50ef0..6fbcdd6822f02fd050cf3cc6347be310b656f8bb 100644 (file)
--- a/gdb/testsuite/gdb.python/py-inferior.exp
+++ b/gdb/testsuite/gdb.python/py-inferior.exp
@@ -34,6 +34,16 @@ switch [get_endianness] {
     big { set python_pack_char ">" }
 }
 
+# Test memory read operations without execution.
+
+gdb_py_test_silent_cmd "python addr = gdb.lookup_global_symbol ('int8_global').value().address" \
+  "get global variable address" 0
+gdb_test "python \
+           int8_global_mv = gdb.selected_inferior().read_memory (addr, 1); \
+           print(int.from_bytes(int8_global_mv\[0\], byteorder='little'))" \
+    "\r\n42" \
+    "read memory without execution"
+
 # The following tests require execution.
 
 if {![runto_main]} {