Re: [PATCHv6 7/9] gdb/gdbserver: share some code relating to target description creation

[John Baldwin <jhb@FreeBSD.org>]

On 5/8/24 9:46 AM, Andrew Burgess wrote:
> This commit is part of a series to share more of the x86 target
> description creation code between GDB and gdbserver.
> 
> Unlike previous commits which were mostly refactoring, this commit is
> the first that makes a real change, though that change should mostly
> be for gdbserver; I've largely adopted the "GDB" way of doing things
> for gdbserver, and this fixes a real gdbserver bug.
> 
> On a x86-64 Linux target, running the test:
> 
>    gdb.server/connect-with-no-symbol-file.exp
> 
> results in two core files being created.  Both of these core files are
> from the inferior process, created after gdbserver has detached.
> 
> In this test a gdbserver process is started and then, after gdbserver
> has started, but before GDB attaches, we either delete the inferior
> executable, or change its permissions so it can't be read.  Only after
> doing this do we attempt to connect with GDB.
> 
> As GDB connects to gdbserver, gdbserver attempts to figure out the
> target description so that it can send the description to GDB, this
> involves a call to x86_linux_read_description.
> 
> In x86_linux_read_description one of the first things we do is try to
> figure out if the process is 32-bit or 64-bit.  To do this we look up
> the executable via the thread-id, and then attempt to read the
> architecture size from the executable.  This isn't going to work if
> the executable has been deleted, or is no longer readable.
> 
> And so, as we can't read the executable, we default to an i386 target
> and use an i386 target description.
> 
> A consequence of using an i386 target description is that addresses
> are assumed to be 32-bits.  Here's an example session that shows the
> problems this causes.  This is run on an x86-64 machine, and the test
> binary (xx.x) is a standard 64-bit x86-64 binary:
> 
>    shell_1$ gdbserver --once localhost :54321 /tmp/xx.x
> 
>    shell_2$ gdb -q
>    (gdb) set sysroot
>    (gdb) shell chmod 000 /tmp/xx.x
>    (gdb) target remote :54321
>    Remote debugging using :54321
>    warning: /tmp/xx.x: Permission denied.
>    0xf7fd3110 in ?? ()
>    (gdb) show architecture
>    The target architecture is set to "auto" (currently "i386").
>    (gdb) p/x $pc
>    $1 = 0xf7fd3110
>    (gdb) info proc mappings
>    process 2412639
>    Mapped address spaces:
> 
>    	Start Addr   End Addr       Size     Offset  Perms   objfile
>    	  0x400000   0x401000     0x1000        0x0  r--p   /tmp/xx.x
>    	  0x401000   0x402000     0x1000     0x1000  r-xp   /tmp/xx.x
>    	  0x402000   0x403000     0x1000     0x2000  r--p   /tmp/xx.x
>    	  0x403000   0x405000     0x2000     0x2000  rw-p   /tmp/xx.x
>    	0xf7fcb000 0xf7fcf000     0x4000        0x0  r--p   [vvar]
>    	0xf7fcf000 0xf7fd1000     0x2000        0x0  r-xp   [vdso]
>    	0xf7fd1000 0xf7fd3000     0x2000        0x0  r--p   /usr/lib64/ld-2.30.so
>    	0xf7fd3000 0xf7ff3000    0x20000     0x2000  r-xp   /usr/lib64/ld-2.30.so
>    	0xf7ff3000 0xf7ffb000     0x8000    0x22000  r--p   /usr/lib64/ld-2.30.so
>    	0xf7ffc000 0xf7ffe000     0x2000    0x2a000  rw-p   /usr/lib64/ld-2.30.so
>    	0xf7ffe000 0xf7fff000     0x1000        0x0  rw-p
>    	0xfffda000 0xfffff000    0x25000        0x0  rw-p   [stack]
>    	0xff600000 0xff601000     0x1000        0x0  r-xp   [vsyscall]
>    (gdb) info inferiors
>      Num  Description       Connection           Executable
>    * 1    process 2412639   1 (remote :54321)
>    (gdb) shell cat /proc/2412639/maps
>    00400000-00401000 r--p 00000000 fd:03 45907133           /tmp/xx.x
>    00401000-00402000 r-xp 00001000 fd:03 45907133           /tmp/xx.x
>    00402000-00403000 r--p 00002000 fd:03 45907133           /tmp/xx.x
>    00403000-00405000 rw-p 00002000 fd:03 45907133           /tmp/xx.x
>    7ffff7fcb000-7ffff7fcf000 r--p 00000000 00:00 0          [vvar]
>    7ffff7fcf000-7ffff7fd1000 r-xp 00000000 00:00 0          [vdso]
>    7ffff7fd1000-7ffff7fd3000 r--p 00000000 fd:00 143904     /usr/lib64/ld-2.30.so
>    7ffff7fd3000-7ffff7ff3000 r-xp 00002000 fd:00 143904     /usr/lib64/ld-2.30.so
>    7ffff7ff3000-7ffff7ffb000 r--p 00022000 fd:00 143904     /usr/lib64/ld-2.30.so
>    7ffff7ffc000-7ffff7ffe000 rw-p 0002a000 fd:00 143904     /usr/lib64/ld-2.30.so
>    7ffff7ffe000-7ffff7fff000 rw-p 00000000 00:00 0
>    7ffffffda000-7ffffffff000 rw-p 00000000 00:00 0          [stack]
>    ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0  [vsyscall]
>    (gdb)
> 
> Notice the difference between the mappings reported via GDB and those
> reported directly from the kernel via /proc/PID/maps, the addresses of
> every mapping is clamped to 32-bits for GDB, while the kernel reports
> real 64-bit addresses.
> 
> Notice also that the $pc value is a 32-bit value.  It appears to be
> within one of the mappings reported by GDB, but is outside any of the
> mappings reported from the kernel.
> 
> And this is where the problem arises.  When gdbserver detaches from
> the inferior we pass the inferior the address from which it should
> resume.  Due to the 32/64 bit confusion we tell the inferior to resume
> from the 32-bit $pc value, which is not within any valid mapping, and
> so, as soon as the inferior resumes, it segfaults.
> 
> If we look at how GDB (not gdbserver) figures out its target
> description then we see an interesting difference.  GDB doesn't try to
> read the executable.  Instead GDB uses ptrace to query the thread's
> state, and uses this to figure out the if the thread is 32 or 64 bit.
> 
> If we update gdbserver to do it the "GDB" way then the above problem
> is resolved, gdbserver now sees the process as 64-bit, and when we
> detach from the inferior we give it the correct 64-bit address, and
> the inferior no longer segfaults.
> 
> Now, I could just update the gdbserver code, but better, I think, to
> share one copy of the code between GDB and gdbserver in gdb/nat/.
> That is what this commit does.
> 
> The cores of x86_linux_read_description from gdbserver and
> x86_linux_nat_target::read_description from GDB are moved into a new
> file gdb/nat/x86-linux-tdesc.c and combined into a single function
> x86_linux_tdesc_for_tid which is called from each location.
> 
> This new function does things mostly the GDB way, some changes are
> needed to allow for the sharing; we now take some pointers for where
> the shared code can cache the xcr0 and xsave layout values.
> 
> Another thing to note about this commit is how the functions
> i386_linux_read_description and amd64_linux_read_description are
> handled.  For now I've left these function as implemented separately
> in GDB and gdbserver.  I've moved the declarations of these functions
> into gdb/arch/{i386,amd64}-linux-tdesc.h, but the implementations are
> left where they are.
> 
> A later commit in this series will make these functions shared too,
> but doing this is not trivial, so I've left that for a separate
> commit.  Merging the declarations as I've done here ensures that
> everyone implements the function to the same API, and once these
> functions are shared (in a later commit) we'll want a shared
> declaration anyway.
> 
> Reviewed-By: Felix Willgerodt <felix.willgerodt@intel.com>

Acked-By: John Baldwin <jhb@FreeBSD.org>

-- 
John Baldwin