Philippe Waroquiers 348775f34b 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.
2022-10-16 00:44:40 +02:00

198 lines
5.1 KiB
C

/* Register support routines for the remote server for GDB.
Copyright (C) 2001, 2002, 2004, 2005, 2011
Free Software Foundation, Inc.
This file is part of GDB.
It has been modified to integrate it in valgrind
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 2 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, write to the Free Software
Foundation, Inc., 51 Franklin Street, Fifth Floor,
Boston, MA 02110-1301, USA. */
#include "server.h"
#include "regdef.h"
/* The private data for the register cache. Note that we have one
per inferior; this is primarily for simplicity, as the performance
benefit is minimal. */
struct inferior_regcache_data
{
unsigned char *registers;
};
static int register_bytes;
static struct reg *reg_defs;
static int num_registers;
const char **gdbserver_expedite_regs;
static
struct inferior_regcache_data * get_regcache (struct thread_info *inf)
{
struct inferior_regcache_data *regcache;
regcache = (struct inferior_regcache_data *) inferior_regcache_data (inf);
if (regcache == NULL)
fatal ("no register cache\n");
return regcache;
}
int registers_length (void)
{
return 2 * register_bytes;
}
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,
in case there are registers the target never fetches. This way they'll
read as zero instead of garbage. */
regcache->registers = calloc (1, register_bytes);
if (regcache->registers == NULL)
fatal ("Could not allocate register cache.\n");
return regcache;
}
void free_register_cache (void *regcache_p)
{
struct inferior_regcache_data *regcache
= (struct inferior_regcache_data *) regcache_p;
free (regcache->registers);
free (regcache);
}
/* if a regcache exists for entry, reallocate it.
This is needed if the shadow registers are added.
In such a case, a 2nd call to set_register_cache is done
which will cause the reallocation of already created caches. */
static
void regcache_realloc_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) {
free_register_cache (regcache);
set_inferior_regcache_data (thread, new_register_cache ());
}
}
void set_register_cache (struct reg *regs, int n)
{
int offset, i;
reg_defs = regs;
num_registers = n;
offset = 0;
for (i = 0; i < n; i++) {
regs[i].offset = offset;
offset += regs[i].size;
}
register_bytes = offset / 8;
for_each_inferior (&all_threads, regcache_realloc_one);
}
void registers_to_string (char *buf)
{
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)->registers;
if (len != register_bytes * 2) {
warning ("Wrong sized register packet (expected %d bytes, got %d)\n",
2*register_bytes, len);
if (len > register_bytes * 2)
len = register_bytes * 2;
}
convert_ascii_to_int (buf, registers, len / 2);
}
int find_regno (const char *name)
{
int i;
for (i = 0; i < num_registers; i++)
if (!strcmp (name, reg_defs[i].name))
return i;
fatal ("Unknown register %s requested\n", name);
return -1;
}
struct reg *find_register_by_number (int n)
{
return &reg_defs[n];
}
int register_size (int n)
{
return reg_defs[n].size / 8;
}
void supply_register (int n, const void *buf)
{
valgrind_store_register (n, buf);
}
void supply_register_from_string (int n, const char *buf)
{
unsigned char bytes_register[register_size (n)];
convert_ascii_to_int (buf, bytes_register, register_size (n));
valgrind_store_register (n, bytes_register);
}
void supply_register_by_name (const char *name, const void *buf)
{
supply_register (find_regno (name), buf);
}
void collect_register (int n, void *buf)
{
valgrind_fetch_register (n, buf);
}
void collect_register_as_string (int n, char *buf)
{
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)
{
collect_register (find_regno (name), buf);
}