From: Philippe Waroquiers <philippe.waroquiers@skynet.be>
Date: Sat, 15 Oct 2022 20:59:08 +0000 (+0200)
Subject: Remove register cache to fix 458915 gdbserver causes wrong syscall return
X-Git-Tag: VALGRIND_3_20_0~10
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=348775f34ba073af3307489f0689f961f6de8854;p=thirdparty%2Fvalgrind.git

Remove register cache to fix 458915 gdbserver causes wrong syscall return

The valgrind gdbserver inheritated a register cache from the original
GDBserver implementation.
The objective of this register cache was to improve the performance
of GDB-> gdbserver -> inferior by avoiding the gdbserver having to
do ptrace system calls each time GDB wants to read or write a register
when the inferior is stopped.

This register cache is however not useful for the valgrind gdbserver:
As the valgrind gdbserver being co-located with the inferior, it
can directly and efficiently read and write registers from/to the VEX
state.

This commit ensures the valgrind GDBserver directly reads from
VEX state instead of fetching the registers from the VEX state and
copying them to the gdbserver regcache.

Similarly, when GDB wants to modify a register, the valgrind GDB server now
directly writes into the VEX state instead of writing the registers
in the regcache and having the regcache flushed to the VEX state
when execution is resumed.

The files regcache.h and regcache.c are still useful as they provide
a translation between a register number, a register name on one side
and the offset in an array of bytes in the format expected by GDB.
The regcache now is only used to create this array of bytes, which is
itself only used temporarily when GDB reads or writes the complete
set of registers instead of reading/writing one register at a time.

Removing the usage of this regcache avoids the bug 458915.
The regcache was causing the bug in the following circumstances:
We have a thread executing code, while we have a bunch of threads
that are blocked in a syscall.
When a thread is blocked in a syscall, the VEX rax register is set to the
syscall nr.
A thread executing code will check from time to time if GDB tries to
attach.
When GDB attaches to the valgrind gdbserver , the thread executing code
will copy the registers from all the threads to the thread gdbserver regcache.
However, the threads blocked in a system call can be unblocked e.g.
because the epoll_wait timeout expires. In such a case, the thread will
still execute the few instructions that follow the syscall instructions
till the thread is blocked trying to acquire the scheduler lock.
These instructions are extracting the syscall return code from the host
register and copies it to the valgrind VEX state.
However, this assembly code is not aware that there is a gdbserver cache.
When the unblocked thread is on the acquire lock statement,
the GDB server regcache is now inconsistent (i.e. different from) the
real VEX state.
When finally GDB tells GDB server to continue execution, the GDB server
wrongly detected that its regcache was modified compared to the VEX state:
the regcache still contains e.g. for the rax register the syscall number
while the unblocked thread has put the syscall return code in the VEX
rax register. GDBserver then flushed the regcache rax (containing the
syscall number) to the VEX rax.
And that led to the detected bug that the syscall return code seen by
the guest application was the syscall number.

Removing the regcache ensures that GDB directly reads the values
from VEX and directly writes to VEX state.

Note that we could still have GDB reading from VEX a register value
that will be changed a few instructions later.
GDB will then show some (slightly) old/obsolete values
for some registers to the user.
This should have no consequence as long as GDB does not try to modify
the registers to execute an inferior call.

The bug did not happen systematically as most of the time, when threads are
blocked in syscalls, vgdb attaches using ptrace to the valgrind process.
When vgdb attaches with ptrace, it stops all the threads using linux syscall.
When vgdb stops the threads, the threads blocked in a syscall will not
execute the instructions between the syscall instruction and the lock
acquire, and so the problem of desynchronisation between the VEX state
and the register cache could not happen.

This commit touches architecture specific files of the gdbserver,
it has been tested on amd64/debian, on pcc64/centos and on arm64/ubuntu.
Possibly, some untested arch might not compile but the fix should be trivial.
---

diff --git a/NEWS b/NEWS
index e6094b5bf2..a5c0a2d017 100644
--- a/NEWS
+++ b/NEWS
@@ -51,6 +51,7 @@ are not entered into bugzilla tend to get forgotten about or ignored.
 n-i-bz  Implement vgdb invoker on FreeBSD
 458845  PowerPC: The L field for the dcbf and sync instruction should be
         3 bits in ISA 3.1.
+458915  Remove register cache to fix 458915 gdbserver causes wrong syscall return
 459031  Documentation on --error-exitcode incomplete
 459477  XERROR messages lacks ending '\n' in vgdb
 
diff --git a/coregrind/m_gdbserver/regcache.c b/coregrind/m_gdbserver/regcache.c
index 6b3d54bd89..7357967d2c 100644
--- a/coregrind/m_gdbserver/regcache.c
+++ b/coregrind/m_gdbserver/regcache.c
@@ -29,9 +29,7 @@
 
 struct inferior_regcache_data
 {
-   int registers_valid;
    unsigned char *registers;
-   Bool *register_supplied; /* set to True once it has been supplied */
 };
 
 static int register_bytes;
@@ -42,8 +40,7 @@ static int num_registers;
 const char **gdbserver_expedite_regs;
 
 static
-struct inferior_regcache_data * get_regcache (struct thread_info *inf,
-                                              int fetch)
+struct inferior_regcache_data * get_regcache (struct thread_info *inf)
 {
    struct inferior_regcache_data *regcache;
 
@@ -52,38 +49,9 @@ struct inferior_regcache_data * get_regcache (struct thread_info *inf,
    if (regcache == NULL)
       fatal ("no register cache\n");
 
-   /* FIXME - fetch registers for INF */
-   if (fetch && regcache->registers_valid == 0) {
-      valgrind_fetch_registers (0);
-      regcache->registers_valid = 1;
-   }
-
    return regcache;
 }
 
-void regcache_invalidate_one (struct inferior_list_entry *entry)
-{
-   struct thread_info *thread = (struct thread_info *) entry;
-   struct inferior_regcache_data *regcache;
-
-   regcache = (struct inferior_regcache_data *) inferior_regcache_data (thread);
-
-   if (regcache->registers_valid) {
-      struct thread_info *saved_inferior = current_inferior;
-
-      current_inferior = thread;
-      valgrind_store_registers (-1);
-      current_inferior = saved_inferior;
-   }
-
-   regcache->registers_valid = 0;
-}
-
-void regcache_invalidate ()
-{
-   for_each_inferior (&all_threads, regcache_invalidate_one);
-}
-
 int registers_length (void)
 {
    return 2 * register_bytes;
@@ -92,7 +60,7 @@ int registers_length (void)
 void *new_register_cache (void)
 {
    struct inferior_regcache_data *regcache;
-   
+
    regcache = malloc (sizeof (*regcache));
 
    /* Make sure to zero-initialize the register cache when it is created,
@@ -102,12 +70,6 @@ void *new_register_cache (void)
    if (regcache->registers == NULL)
       fatal ("Could not allocate register cache.\n");
 
-   regcache->register_supplied = calloc (1, num_registers);
-   if (regcache->register_supplied == NULL)
-      fatal ("Could not allocate register_supplied cache.\n");
-
-   regcache->registers_valid = 0;
-
    return regcache;
 }
 
@@ -117,7 +79,6 @@ void free_register_cache (void *regcache_p)
       = (struct inferior_regcache_data *) regcache_p;
 
    free (regcache->registers);
-   free (regcache->register_supplied);
    free (regcache);
 }
 
@@ -142,7 +103,7 @@ void regcache_realloc_one (struct inferior_list_entry *entry)
 void set_register_cache (struct reg *regs, int n)
 {
    int offset, i;
-  
+
    reg_defs = regs;
    num_registers = n;
 
@@ -159,15 +120,17 @@ void set_register_cache (struct reg *regs, int n)
 
 void registers_to_string (char *buf)
 {
-   unsigned char *registers = get_regcache (current_inferior, 1)->registers;
+   unsigned char *registers = get_regcache (current_inferior)->registers;
 
+   for (int i = 0; i < num_registers; i++)
+      valgrind_fetch_register (i, registers + (reg_defs[i].offset / 8));
    convert_int_to_ascii (registers, buf, register_bytes);
 }
 
 void registers_from_string (const char *buf)
 {
    int len = strlen (buf);
-   unsigned char *registers = get_regcache (current_inferior, 1)->registers;
+   unsigned char *registers = get_regcache (current_inferior)->registers;
 
    if (len != register_bytes * 2) {
       warning ("Wrong sized register packet (expected %d bytes, got %d)\n",
@@ -199,62 +162,33 @@ int register_size (int n)
    return reg_defs[n].size / 8;
 }
 
-static
-unsigned char *register_data (int n, int fetch)
-{
-   unsigned char *registers
-      = get_regcache (current_inferior, fetch)->registers;
-
-   return registers + (reg_defs[n].offset / 8);
-}
-static
-unsigned char *register_data_for_supply (int n, int fetch, Bool *mod)
-{
-   struct inferior_regcache_data * cache 
-      = get_regcache (current_inferior, fetch);
-   unsigned char *registers = cache->registers;
-
-   if (cache->register_supplied[n])
-      *mod = False;
-   else
-      *mod = True;
-   cache->register_supplied[n] = True;
-   return registers + (reg_defs[n].offset / 8);
-}
-
-void supply_register (int n, const void *buf, Bool *mod)
+void supply_register (int n, const void *buf)
 {
-   Bool new;
-   VG_(dmemcpy) (register_data_for_supply (n, 0, &new), 
-                 buf, register_size (n), mod);
-   if (new)
-      *mod = True;
+   valgrind_store_register (n, buf);
 }
 
-void supply_register_from_string (int n, const char *buf, Bool *mod)
+void supply_register_from_string (int n, const char *buf)
 {
-   Bool new;
    unsigned char bytes_register[register_size (n)];
    convert_ascii_to_int (buf, bytes_register, register_size (n));
-   VG_(dmemcpy) (register_data_for_supply (n, 0, &new), 
-                 bytes_register, register_size (n), mod);
-   if (new)
-      *mod = True;
+   valgrind_store_register (n, bytes_register);
 }
 
-void supply_register_by_name (const char *name, const void *buf, Bool *mod)
+void supply_register_by_name (const char *name, const void *buf)
 {
-   supply_register (find_regno (name), buf, mod);
+   supply_register (find_regno (name), buf);
 }
 
 void collect_register (int n, void *buf)
 {
-   VG_(memcpy) (buf, register_data (n, 1), register_size (n));
+   valgrind_fetch_register (n, buf);
 }
 
 void collect_register_as_string (int n, char *buf)
 {
-   convert_int_to_ascii (register_data (n, 1), buf, register_size (n));
+   unsigned char local_buf [register_size (n)];
+   valgrind_fetch_register (n, local_buf);
+   convert_int_to_ascii (local_buf, buf, register_size (n));
 }
 
 void collect_register_by_name (const char *name, void *buf)
diff --git a/coregrind/m_gdbserver/regcache.h b/coregrind/m_gdbserver/regcache.h
index cb6fef7990..881dcc37b1 100644
--- a/coregrind/m_gdbserver/regcache.h
+++ b/coregrind/m_gdbserver/regcache.h
@@ -22,7 +22,15 @@
 #ifndef REGCACHE_H
 #define REGCACHE_H
 
-#include "pub_core_basics.h"    // Bool
+/* Defines support routines to get/set registers for the valgrind
+   remote GDB server.
+   This file used to provide a real register cache, where the register
+   values were written to by GDB without directly reaching the valgrind VEX
+   state.  In the real GDB gdbserver, this cache was used to avoid a ptrace
+   system call each time a register has to be re-read. In valgrind, registers
+   are directly accessible by the embedded gdbserver. So, read/write registers
+   operations by GDB are directly executed from/to the valgrind VEX registers. */
+
 
 struct inferior_list_entry;
 
@@ -34,11 +42,6 @@ void *new_register_cache (void);
 
 void free_register_cache (void *regcache);
 
-/* Invalidate cached registers for one or all threads.  */
-
-void regcache_invalidate_one (struct inferior_list_entry *);
-void regcache_invalidate (void);
-
 /* Convert all registers to a string in the currently specified remote
    format.  */
 
@@ -62,16 +65,15 @@ int find_regno (const char *name);
 
 extern const char **gdbserver_expedite_regs;
 
-/* *mod set to True if *buf provides a new value. */
-void supply_register (int n, const void *buf, Bool *mod);
+/* Sets the value of register N to buf content. */
+void supply_register (int n, const void *buf);
 
 /* Reads register data from buf (hex string in target byte order)
-   and stores it in the register cache.
-   *mod set to True if *buf provides a new value. */
-void supply_register_from_string (int n, const char *buf, Bool *mod);
+   and stores it in the register cache.  */
+void supply_register_from_string (int n, const char *buf);
 
-/* *mod set to True if *buf provides a new value. */
-void supply_register_by_name (const char *name, const void *buf, Bool *mod);
+/* Sets the value of register identified by NAME to buf content. */
+void supply_register_by_name (const char *name, const void *buf);
 
 void collect_register (int n, void *buf);
 
diff --git a/coregrind/m_gdbserver/remote-utils.c b/coregrind/m_gdbserver/remote-utils.c
index 559d8dd8e9..2ec8e6fe59 100644
--- a/coregrind/m_gdbserver/remote-utils.c
+++ b/coregrind/m_gdbserver/remote-utils.c
@@ -688,7 +688,7 @@ int hexify (char *hex, const char *bin, int count)
 
 /* builds an image of bin according to byte order of the architecture 
    Useful for register and int image */
-char* heximage (char *buf, char *bin, int count)
+char* heximage (char *buf, const char *bin, int count)
 {
 #if (VKI_LITTLE_ENDIAN)
    char rev[count]; 
diff --git a/coregrind/m_gdbserver/server.c b/coregrind/m_gdbserver/server.c
index 8e4bae8567..0f639b2743 100644
--- a/coregrind/m_gdbserver/server.c
+++ b/coregrind/m_gdbserver/server.c
@@ -1306,7 +1306,6 @@ void server_main (void)
       case 'P': {
          int regno;
          char *regbytes;
-         Bool mod;
          ThreadState *tst;
          regno = strtol(&own_buf[1], NULL, 16);
          regbytes = strchr(&own_buf[0], '=') + 1;
@@ -1318,7 +1317,7 @@ void server_main (void)
             We assume we do not need to very specific here, and that we
             can just refuse all of these. */
          if (tst->status == VgTs_Runnable || tst->status == VgTs_Yielding) {
-            supply_register_from_string (regno, regbytes, &mod);
+            supply_register_from_string (regno, regbytes);
             write_ok (own_buf);
          } else {
             /* at least from gdb 6.6 onwards, an E. error
diff --git a/coregrind/m_gdbserver/server.h b/coregrind/m_gdbserver/server.h
index 1c2d8612e3..953abbd61d 100644
--- a/coregrind/m_gdbserver/server.h
+++ b/coregrind/m_gdbserver/server.h
@@ -315,7 +315,7 @@ int unhexify (char *bin, const char *hex, int count);
 int hexify (char *hex, const char *bin, int count);
 /* heximage builds an image of bin according to byte order of the architecture 
    Useful for register and int image */
-char* heximage (char *buf, char *bin, int count);
+char* heximage (char *buf, const char *bin, int count);
 
 /* convert from CORE_ADDR to void* */
 void* C2v(CORE_ADDR addr);
diff --git a/coregrind/m_gdbserver/target.c b/coregrind/m_gdbserver/target.c
index b49d210779..490276b6c4 100644
--- a/coregrind/m_gdbserver/target.c
+++ b/coregrind/m_gdbserver/target.c
@@ -290,7 +290,6 @@ void valgrind_resume (struct thread_resume *resume_info)
            "stop_pc %p changed to be resume_pc %s\n",
            C2v(stop_pc), sym(resume_pc));
    }
-   regcache_invalidate();
 }
 
 unsigned char valgrind_wait (char *ourstatus)
@@ -304,7 +303,6 @@ unsigned char valgrind_wait (char *ourstatus)
    pid = VG_(getpid) ();
    dlog(1, "enter valgrind_wait pid %d\n", pid);
 
-   regcache_invalidate();
    valgrind_update_threads(pid);
 
    /* First see if we are done with this process. */
@@ -355,14 +353,13 @@ unsigned char valgrind_wait (char *ourstatus)
 }
 
 /* Fetch one register from valgrind VEX guest state.  */
-static
-void fetch_register (int regno)
+void valgrind_fetch_register (int regno, unsigned char *buf)
 {
    int size;
    ThreadState *tst = (ThreadState *) inferior_target_data (current_inferior);
    ThreadId tid = tst->tid;
 
-   if (regno >= the_low_target.num_regs) {
+   if (regno < 0 || regno >= the_low_target.num_regs) {
       dlog(0, "error fetch_register regno %d max %d\n",
            regno, the_low_target.num_regs);
       return;
@@ -370,16 +367,13 @@ void fetch_register (int regno)
    size = register_size (regno);
    if (size > 0) {
       Bool mod;
-      char buf [size];
       VG_(memset) (buf, 0, size); // registers not fetched will be seen as 0.
       (*the_low_target.transfer_register) (tid, regno, buf,
                                            valgrind_to_gdbserver, size, &mod);
       // Note: the *mod received from transfer_register is not interesting.
-      // We are interested to see if the register data in the register cache is modified.
-      supply_register (regno, buf, &mod);
       if (mod && VG_(debugLog_getLevel)() > 1) {
          char bufimage [2*size + 1];
-         heximage (bufimage, buf, size);
+         heximage (bufimage, (char*) buf, size);
          dlog(3, "fetched register %d size %d name %s value %s tid %u status %s\n", 
               regno, size, the_low_target.reg_defs[regno].name, bufimage, 
               tid, VG_(name_of_ThreadStatus) (tst->status));
@@ -387,102 +381,74 @@ void fetch_register (int regno)
    }
 }
 
-/* Fetch all registers, or just one, from the child process.  */
-static
-void usr_fetch_inferior_registers (int regno)
-{
-   if (regno == -1 || regno == 0)
-      for (regno = 0; regno < the_low_target.num_regs; regno++)
-         fetch_register (regno);
-   else
-      fetch_register (regno);
-}
-
-/* Store our register values back into the inferior.
-   If REGNO is -1, do this for all registers.
-   Otherwise, REGNO specifies which register (so we can save time).  */
-static
-void usr_store_inferior_registers (int regno)
+/* Store register REGNO value back into the inferior VEX state.  */
+void valgrind_store_register (int regno, const unsigned char *buf)
 {
    int size;
    ThreadState *tst = (ThreadState *) inferior_target_data (current_inferior);
    ThreadId tid = tst->tid;
-   
-   if (regno >= 0) {
 
-      if (regno >= the_low_target.num_regs) {
-         dlog(0, "error store_register regno %d max %d\n",
-              regno, the_low_target.num_regs);
-         return;
+   if (regno < 0 || regno >= the_low_target.num_regs) {
+      dlog(0, "error store_register regno %d max %d\n",
+           regno, the_low_target.num_regs);
+      return;
+   }
+
+   size = register_size (regno);
+   if (size > 0) {
+      Bool mod;
+      Addr old_SP, new_SP;
+
+      if (regno == the_low_target.stack_pointer_regno) {
+         /* When the stack pointer register is changed such that
+            the stack is extended, we better inform the tool of the
+            stack increase.  This is needed in particular to avoid
+            spurious Memcheck errors during Inferior calls. So, we
+            save in old_SP the SP before the change. A change of
+            stack pointer is also assumed to have initialised this
+            new stack space. For the typical example of an inferior
+            call, gdb writes arguments on the stack, and then
+            changes the stack pointer. As the stack increase tool
+            function might mark it as undefined, we have to call it
+            at the good moment. */
+         VG_(memset) ((void *) &old_SP, 0, size);
+         (*the_low_target.transfer_register) (tid, regno, (void *) &old_SP,
+                                              valgrind_to_gdbserver, size, &mod);
       }
-      
-      size = register_size (regno);
-      if (size > 0) {
-         Bool mod;
-         Addr old_SP, new_SP;
-         char buf[size];
-
-         if (regno == the_low_target.stack_pointer_regno) {
-            /* When the stack pointer register is changed such that
-               the stack is extended, we better inform the tool of the
-               stack increase.  This is needed in particular to avoid
-               spurious Memcheck errors during Inferior calls. So, we
-               save in old_SP the SP before the change. A change of
-               stack pointer is also assumed to have initialised this
-               new stack space. For the typical example of an inferior
-               call, gdb writes arguments on the stack, and then
-               changes the stack pointer. As the stack increase tool
-               function might mark it as undefined, we have to call it
-               at the good moment. */
-            VG_(memset) ((void *) &old_SP, 0, size);
-            (*the_low_target.transfer_register) (tid, regno, (void *) &old_SP, 
-                                                 valgrind_to_gdbserver, size, &mod);
-         }
 
-         VG_(memset) (buf, 0, size);
-         collect_register (regno, buf);
-         (*the_low_target.transfer_register) (tid, regno, buf, 
-                                              gdbserver_to_valgrind, size, &mod);
-         if (mod && VG_(debugLog_getLevel)() > 1) {
-            char bufimage [2*size + 1];
-            heximage (bufimage, buf, size);
-            dlog(2, 
-                 "stored register %d size %d name %s value %s "
-                 "tid %u status %s\n", 
-                 regno, size, the_low_target.reg_defs[regno].name, bufimage, 
-                 tid, VG_(name_of_ThreadStatus) (tst->status));
-         }
-         if (regno == the_low_target.stack_pointer_regno) {
-            VG_(memcpy) (&new_SP, buf, size);
-            if (old_SP > new_SP) {
-               Word delta  = (Word)new_SP - (Word)old_SP;
-               dlog(1, 
-                    "   stack increase by stack pointer changed from %p to %p "
-                    "delta %ld\n",
-                    (void*) old_SP, (void *) new_SP,
-                    delta);
-               VG_TRACK( new_mem_stack_w_ECU, new_SP, -delta, 0 );
-               VG_TRACK( new_mem_stack,       new_SP, -delta );
-               VG_TRACK( post_mem_write, Vg_CoreClientReq, tid,
-                         new_SP, -delta);
-            }
+      char buf_copy[size]; 
+      /* copy buf to buf_copy to avoid warnings passing a const to transfer_register.
+         This is ok as transfer_register called with gdbserver_to_valgrind will read from
+         buf and write to VEX state. */
+      VG_(memcpy) (buf_copy, buf, size);
+
+      (*the_low_target.transfer_register) (tid, regno, buf_copy,
+                                           gdbserver_to_valgrind, size, &mod);
+      if (mod && VG_(debugLog_getLevel)() > 1) {
+         char bufimage [2*size + 1];
+         heximage (bufimage, buf_copy, size);
+         dlog(2, 
+              "stored register %d size %d name %s value %s "
+              "tid %u status %s\n", 
+              regno, size, the_low_target.reg_defs[regno].name, bufimage, 
+              tid, VG_(name_of_ThreadStatus) (tst->status));
+      }
+      if (regno == the_low_target.stack_pointer_regno) {
+         VG_(memcpy) (&new_SP, buf, size);
+         if (old_SP > new_SP) {
+            Word delta  = (Word)new_SP - (Word)old_SP;
+            dlog(1, 
+                 "   stack increase by stack pointer changed from %p to %p "
+                 "delta %ld\n",
+                 (void*) old_SP, (void *) new_SP,
+                 delta);
+            VG_TRACK( new_mem_stack_w_ECU, new_SP, -delta, 0 );
+            VG_TRACK( new_mem_stack,       new_SP, -delta );
+            VG_TRACK( post_mem_write, Vg_CoreClientReq, tid,
+                      new_SP, -delta);
          }
       }
    }
-   else {
-      for (regno = 0; regno < the_low_target.num_regs; regno++)
-         usr_store_inferior_registers (regno);
-   }
-}
-
-void valgrind_fetch_registers (int regno)
-{
-   usr_fetch_inferior_registers (regno);
-}
-
-void valgrind_store_registers (int regno)
-{
-   usr_store_inferior_registers (regno);
 }
 
 Bool hostvisibility = False;
@@ -806,7 +772,6 @@ void initialize_shadow_low(Bool shadow_mode)
     non_shadow_num_regs = the_low_target.num_regs;
   }
 
-  regcache_invalidate();
   if (the_low_target.reg_defs != non_shadow_reg_defs) {
      free (the_low_target.reg_defs);
   }
diff --git a/coregrind/m_gdbserver/target.h b/coregrind/m_gdbserver/target.h
index 9a8b5f2b4e..1309bba7a5 100644
--- a/coregrind/m_gdbserver/target.h
+++ b/coregrind/m_gdbserver/target.h
@@ -179,13 +179,11 @@ extern int valgrind_thread_alive (unsigned long tid);
    set to the first valid thread. */
 extern void set_desired_inferior (int use_general);
 
-/* Fetch registers from the current_inferior thread.
-   If REGNO is -1, fetch all registers; otherwise, fetch at least REGNO.  */
-extern void valgrind_fetch_registers (int regno);
+/* Fetch register regno from the current_inferior thread and put its value in buf.  */
+extern void valgrind_fetch_register (int regno, unsigned char *buf);
 
-/* Store registers to the current_inferior thread.
-   If REGNO is -1, store all registers; otherwise, store at least REGNO.  */
-extern void valgrind_store_registers (int regno);
+/* Store register REGNO value from BUF to the VEX valgrind state.  */
+extern void valgrind_store_register (int regno, const unsigned char *buf);
 
 
 
diff --git a/coregrind/m_gdbserver/valgrind-low-amd64.c b/coregrind/m_gdbserver/valgrind-low-amd64.c
index c0d85bf3d3..0f83de1e1e 100644
--- a/coregrind/m_gdbserver/valgrind-low-amd64.c
+++ b/coregrind/m_gdbserver/valgrind-low-amd64.c
@@ -129,7 +129,7 @@ CORE_ADDR get_pc (void)
    unsigned long pc;
 
    collect_register_by_name ("rip", &pc);
-   
+
    dlog(1, "stop pc is %p\n", (void *) pc);
    return pc;
 }
@@ -137,12 +137,7 @@ CORE_ADDR get_pc (void)
 static
 void set_pc (CORE_ADDR newpc)
 {
-   Bool mod;
-   supply_register_by_name ("rip", &newpc, &mod);
-   if (mod)
-      dlog(1, "set pc to %p\n", C2v (newpc));
-   else
-      dlog(1, "set pc not changed %p\n", C2v (newpc));
+   supply_register_by_name ("rip", &newpc);
 }
 
 /* store registers in the guest state (gdbserver_to_valgrind)
diff --git a/coregrind/m_gdbserver/valgrind-low-arm.c b/coregrind/m_gdbserver/valgrind-low-arm.c
index fecbb071c9..5a2348732b 100644
--- a/coregrind/m_gdbserver/valgrind-low-arm.c
+++ b/coregrind/m_gdbserver/valgrind-low-arm.c
@@ -114,12 +114,7 @@ CORE_ADDR get_pc (void)
 static
 void set_pc (CORE_ADDR newpc)
 {
-   Bool mod;
-   supply_register_by_name ("pc", &newpc, &mod);
-   if (mod)
-      dlog(1, "set pc to %p\n", C2v (newpc));
-   else
-      dlog(1, "set pc not changed %p\n", C2v (newpc));
+   supply_register_by_name ("pc", &newpc);
 }
 
 Addr thumb_pc (Addr pc)
diff --git a/coregrind/m_gdbserver/valgrind-low-arm64.c b/coregrind/m_gdbserver/valgrind-low-arm64.c
index 17aa2068bf..292650dc1a 100644
--- a/coregrind/m_gdbserver/valgrind-low-arm64.c
+++ b/coregrind/m_gdbserver/valgrind-low-arm64.c
@@ -125,12 +125,7 @@ CORE_ADDR get_pc (void)
 static
 void set_pc (CORE_ADDR newpc)
 {
-   Bool mod;
-   supply_register_by_name ("pc", &newpc, &mod);
-   if (mod)
-      dlog(1, "set pc to %p\n", C2v (newpc));
-   else
-      dlog(1, "set pc not changed %p\n", C2v (newpc));
+   supply_register_by_name ("pc", &newpc);
 }
 
 /* store registers in the guest state (gdbserver_to_valgrind)
diff --git a/coregrind/m_gdbserver/valgrind-low-mips32.c b/coregrind/m_gdbserver/valgrind-low-mips32.c
index 5f965f54a3..a132a43505 100644
--- a/coregrind/m_gdbserver/valgrind-low-mips32.c
+++ b/coregrind/m_gdbserver/valgrind-low-mips32.c
@@ -130,12 +130,7 @@ CORE_ADDR get_pc (void)
 static
 void set_pc (CORE_ADDR newpc)
 {
-   Bool mod;
-   supply_register_by_name ("pc", &newpc, &mod);
-   if (mod)
-      dlog(1, "set pc to %p\n", C2v (newpc));
-   else
-      dlog(1, "set pc not changed %p\n", C2v (newpc));
+   supply_register_by_name ("pc", &newpc);
 }
 
 /* These are the fields of 32 bit mips instructions. */
diff --git a/coregrind/m_gdbserver/valgrind-low-mips64.c b/coregrind/m_gdbserver/valgrind-low-mips64.c
index 20323a3b69..026dbe7638 100644
--- a/coregrind/m_gdbserver/valgrind-low-mips64.c
+++ b/coregrind/m_gdbserver/valgrind-low-mips64.c
@@ -131,12 +131,7 @@ CORE_ADDR get_pc (void)
 static
 void set_pc (CORE_ADDR newpc)
 {
-   Bool mod;
-   supply_register_by_name ("pc", &newpc, &mod);
-   if (mod)
-      dlog(1, "set pc to %p\n", C2v (newpc));
-   else
-      dlog(1, "set pc not changed %p\n", C2v (newpc));
+   supply_register_by_name ("pc", &newpc);
 }
 
 /* These are the fields of 32 bit mips instructions. */
diff --git a/coregrind/m_gdbserver/valgrind-low-nanomips.c b/coregrind/m_gdbserver/valgrind-low-nanomips.c
index 77a4190fe0..d2c4f0332f 100644
--- a/coregrind/m_gdbserver/valgrind-low-nanomips.c
+++ b/coregrind/m_gdbserver/valgrind-low-nanomips.c
@@ -93,12 +93,7 @@ CORE_ADDR get_pc (void)
 static
 void set_pc (CORE_ADDR newpc)
 {
-   Bool mod;
-   supply_register_by_name ("pc", &newpc, &mod);
-   if (mod)
-      dlog(1, "set pc to %p\n", C2v (newpc));
-   else
-      dlog(1, "set pc not changed %p\n", C2v (newpc));
+   supply_register_by_name ("pc", &newpc);
 }
 
 /* These are the fields of 32 bit mips instructions. */
diff --git a/coregrind/m_gdbserver/valgrind-low-ppc32.c b/coregrind/m_gdbserver/valgrind-low-ppc32.c
index db5d8ced16..bdb0896b9b 100644
--- a/coregrind/m_gdbserver/valgrind-low-ppc32.c
+++ b/coregrind/m_gdbserver/valgrind-low-ppc32.c
@@ -164,12 +164,7 @@ CORE_ADDR get_pc (void)
 static
 void set_pc (CORE_ADDR newpc)
 {
-   Bool mod;
-   supply_register_by_name ("pc", &newpc, &mod);
-   if (mod)
-      dlog(1, "set pc to %p\n", C2v (newpc));
-   else
-      dlog(1, "set pc not changed %p\n", C2v (newpc));
+   supply_register_by_name ("pc", &newpc);
 }
 
 /* store registers in the guest state (gdbserver_to_valgrind)
diff --git a/coregrind/m_gdbserver/valgrind-low-ppc64.c b/coregrind/m_gdbserver/valgrind-low-ppc64.c
index 3c6146c59a..1e2a159442 100644
--- a/coregrind/m_gdbserver/valgrind-low-ppc64.c
+++ b/coregrind/m_gdbserver/valgrind-low-ppc64.c
@@ -239,12 +239,7 @@ CORE_ADDR get_pc (void)
 static
 void set_pc (CORE_ADDR newpc)
 {
-   Bool mod;
-   supply_register_by_name ("pc", &newpc, &mod);
-   if (mod)
-      dlog(1, "set pc to %p\n", C2v (newpc));
-   else
-      dlog(1, "set pc not changed %p\n", C2v (newpc));
+   supply_register_by_name ("pc", &newpc);
 }
 
 /* store registers in the guest state (gdbserver_to_valgrind)
diff --git a/coregrind/m_gdbserver/valgrind-low-s390x.c b/coregrind/m_gdbserver/valgrind-low-s390x.c
index a667f4b40b..74898e471a 100644
--- a/coregrind/m_gdbserver/valgrind-low-s390x.c
+++ b/coregrind/m_gdbserver/valgrind-low-s390x.c
@@ -139,12 +139,7 @@ CORE_ADDR get_pc (void)
 static
 void set_pc (CORE_ADDR newpc)
 {
-   Bool mod;
-   supply_register_by_name ("pswa", &newpc, &mod);
-   if (mod)
-      dlog(1, "set pc to %p\n", C2v (newpc));
-   else
-      dlog(1, "set pc not changed %p\n", C2v (newpc));
+   supply_register_by_name ("pswa", &newpc);
 }
 
 /* store registers in the guest state (gdbserver_to_valgrind)
diff --git a/coregrind/m_gdbserver/valgrind-low-x86.c b/coregrind/m_gdbserver/valgrind-low-x86.c
index 410c4257fd..e7adac700c 100644
--- a/coregrind/m_gdbserver/valgrind-low-x86.c
+++ b/coregrind/m_gdbserver/valgrind-low-x86.c
@@ -104,12 +104,7 @@ CORE_ADDR get_pc (void)
 static
 void set_pc (CORE_ADDR newpc)
 {
-   Bool mod;
-   supply_register_by_name ("eip", &newpc, &mod);
-   if (mod)
-      dlog(1, "set pc to %p\n", C2v (newpc));
-   else
-      dlog(1, "set pc not changed %p\n", C2v (newpc));
+   supply_register_by_name ("eip", &newpc);
 }
 
 /* store registers in the guest state (gdbserver_to_valgrind)