binutils-gdb/gdb/nat/x86-linux-tdesc.c
Andrew Burgess c3466dee85 gdb/i386: fix tdesc rejection issue for targets without PTRACE_GETREGSET
After the x86 target description changes that I committed recently,
the first commit in the series being:

  commit 8a29222b85
  Date:   Sat Jan 27 10:40:35 2024 +0000

      gdb/gdbserver: share I386_LINUX_XSAVE_XCR0_OFFSET definition

and the last commit in the series being:

  commit 646d754d14
  Author: Andrew Burgess <aburgess@redhat.com>
  Date:   Tue Jan 30 15:37:23 2024 +0000

      gdb/gdbserver: share x86/linux tdesc caching

The sourceware buildbot highlighted a regression on i386.  On the GDB
side we'd see this:

  Remote debugging using :54321
  warning: Architecture rejected target-supplied description
  Remote connection closed
  (gdb)

while on the gdbserver side we'd see this:

  $ ./gdbserver/gdbserver --once :54321 ~/empty
  Process /srv/aburgess/empty created; pid = 31406
  Listening on port 54321
  Remote debugging from host ::1, port 39488
  ../../src/gdbserver/regcache.cc:272: A problem internal to GDBserver has been detected.
  Unknown register st0 requested
  Aborted (core dumped)

When I tried to reproduce this regression on my local i386 VM the
issue would not reproduce.

I eventually tracked the problem down to x86_linux_tdesc_for_tid in
gdb/nat/x86-linux-tdesc.c.  In this function we have this line:

  /* Check if PTRACE_GETREGSET works.  */
  if (ptrace (PTRACE_GETREGSET, tid,
              (unsigned int) NT_X86_XSTATE, &iov) < 0)
    {
      ... handle failure ...
    }
  else
    {
      ... handle success ...
    }

The problem is that on my VM the PTRACE_GETREGSET feature is
supported, while on sourceware's buildbot machine this feature is not
supported.

I did a quick search and it seems like the 'xsave' feature in
/proc/cpuinfo might be the indicator for whether PTRACE_GETREGSET is
supported or not, and indeed my machine has the 'xsave' feature while
the sourceware machine does not.

The point of divergence then is this ptrace call, on my machine the
call succeeds and we extract the xcr0 value from the iov vector, while
on the sourceware machine the ptrace call fails and we use a default
xcr0 value of 0.

This xcr0 value is then passed to i386_linux_read_description at the
end of x86_linux_tdesc_for_tid.

In gdb/arch/i386-linux-tdesc.c we find i386_linux_read_description
which does some caching but calls i386_create_target_description to
actually create the target descriptions when needed.  The xcr0 value
is masked to only the bits that are interesting, but given a value of
0 we'll just pass 0 through to i386_create_target_description.

In gdb/arch/i386.c we find i386_create_target_description which checks
the xcr0 bits and builds the target description.  What we can see is
that if no bits are set in the xcr0 value then no features will be
added to the created target description.  This featureless target
description is then transmitted back to GDB, which is then rejected
due to lack of essential core registers.

So, how did things work prior to the above commit series?  There are
three places of interest, on the GDB side there is
x86_linux_nat_target::read_description and
i386_linux_core_read_description.  Then on the gdbserver side there is
x86_linux_read_description.

All of these locations have a call to i386_linux_read_description
followed by a check if the return value was nullptr.  If we do get
back nullptr then we perform another call to
i386_linux_read_description with a default xcr0 value.

Looking in i386_linux_read_description we see a specific check for
xcr0 being 0 in which case we return nullptr.

And so, prior to the above series, if xcr0 was 0 due to
PTRACE_GETREGSET being unavailable we'd use a default xcr0 value.

After the above series this is no longer the case, the 'xcr0 == 0'
check has been removed from i386_linux_read_description and the
calling code is streamlined to remove the use of default xcr0 values.

The fix I propose here is to setup the default xcr0 value at the point
where we find that PTRACE_GETREGSET is unavailable.  The default value
used is X86_XSTATE_SSE_MASK.  This is the default used in
x86_linux_nat_target::read_description (for GDB) and in
x86_linux_read_description (for gdbserver).  The above commit series
already fixed i386_linux_core_read_description to ensure that the
correct default xcr0 value was used, this case is a little special in
that it uses different defaults depending on which sections are
present in the core file, so that case always needed to be handled
differently.

The choice of X86_XSTATE_SSE_MASK corresponds to the default used for
i386 before the above series was committed.  This mask includes the
X87 and SSE bits only, neither of these bits are checked for on amd64
or x32, so this default doesn't change the behaviour on these targets.

By setting the default xcr0 value at this early stage we ensure that
the cached xcr0 value on the gdbserver side is correct.  This is
critical as this cached xcr0 value is passed through to the in process
agent (IPA).  If we leave the cached xcr0 value as 0 and apply the
defaults later in the series we also have to encode the knowledge of
the default into the IPA, this just means we have the default encoded
in multiple locations, which seems like a bad idea.  The approach used
in this patch means the default is present in just one location.

This commit should fix the i386 regressions seen on the sourceware
buildbot.

In addition to the fix in nat/x86-linux-tdesc.c I've also fixed the
layout of the declaration of x86_linux_tdesc_for_tid in the header
file.

Approved-By: Felix Willgerodt <felix.willgerodt@intel.com>
2024-06-24 11:15:54 +01:00

132 lines
3.7 KiB
C

/* Target description related code for GNU/Linux x86 (i386 and x86-64).
Copyright (C) 2024 Free Software Foundation, Inc.
This file is part of GDB.
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 3 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, see <http://www.gnu.org/licenses/>. */
#include "nat/x86-linux-tdesc.h"
#ifdef __x86_64__
#include "arch/amd64.h"
#include "arch/amd64-linux-tdesc.h"
#endif
#include "arch/i386.h"
#include "arch/i386-linux-tdesc.h"
#include "nat/x86-linux.h"
#include "nat/gdb_ptrace.h"
#include "nat/x86-xstate.h"
#include "gdbsupport/x86-xstate.h"
#ifndef __x86_64__
#include <sys/procfs.h>
#include "nat/i386-linux.h"
#endif
#include <sys/uio.h>
#include <elf.h>
#ifndef IN_PROCESS_AGENT
/* See nat/x86-linux-tdesc.h. */
const target_desc *
x86_linux_tdesc_for_tid (int tid, uint64_t *xcr0_storage,
x86_xsave_layout *xsave_layout_storage)
{
#ifdef __x86_64__
x86_linux_arch_size arch_size = x86_linux_ptrace_get_arch_size (tid);
bool is_64bit = arch_size.is_64bit ();
bool is_x32 = arch_size.is_x32 ();
if (sizeof (void *) == 4 && is_64bit && !is_x32)
{
#ifdef GDBSERVER
error (_("Can't debug 64-bit process with 32-bit GDBserver"));
#else
error (_("Can't debug 64-bit process with 32-bit GDB"));
#endif
}
#elif HAVE_PTRACE_GETFPXREGS
if (have_ptrace_getfpxregs == TRIBOOL_UNKNOWN)
{
elf_fpxregset_t fpxregs;
if (ptrace (PTRACE_GETFPXREGS, tid, 0, (int) &fpxregs) < 0)
{
have_ptrace_getfpxregs = TRIBOOL_FALSE;
have_ptrace_getregset = TRIBOOL_FALSE;
}
else
have_ptrace_getfpxregs = TRIBOOL_TRUE;
}
if (have_ptrace_getfpxregs == TRIBOOL_FALSE)
return i386_linux_read_description (X86_XSTATE_X87_MASK);
#endif
if (have_ptrace_getregset == TRIBOOL_UNKNOWN)
{
uint64_t xstateregs[(X86_XSTATE_SSE_SIZE / sizeof (uint64_t))];
struct iovec iov;
iov.iov_base = xstateregs;
iov.iov_len = sizeof (xstateregs);
/* Check if PTRACE_GETREGSET works. */
if (ptrace (PTRACE_GETREGSET, tid,
(unsigned int) NT_X86_XSTATE, &iov) < 0)
{
/* Can't fetch the xcr0 value so pick a simple default. This
default has x87 and sse bits set. These bits are ignored for
amd64 and x32 targets, but are checked for on i386. Without
these bits being set we generate a completely empty tdesc for
i386 which will be rejected by GDB. */
have_ptrace_getregset = TRIBOOL_FALSE;
*xcr0_storage = X86_XSTATE_SSE_MASK;
}
else
{
have_ptrace_getregset = TRIBOOL_TRUE;
/* Get XCR0 from XSAVE extended state. */
*xcr0_storage = xstateregs[(I386_LINUX_XSAVE_XCR0_OFFSET
/ sizeof (uint64_t))];
#ifdef __x86_64__
/* No MPX on x32. */
if (is_64bit && is_x32)
*xcr0_storage &= ~X86_XSTATE_MPX;
#endif /* __x86_64__ */
*xsave_layout_storage
= x86_fetch_xsave_layout (*xcr0_storage, x86_xsave_length ());
}
}
/* Use cached xcr0 value. */
uint64_t xcr0_features_bits = *xcr0_storage & X86_XSTATE_ALL_MASK;
#ifdef __x86_64__
if (is_64bit)
return amd64_linux_read_description (xcr0_features_bits, is_x32);
else
#endif
return i386_linux_read_description (xcr0_features_bits);
}
#endif /* !IN_PROCESS_AGENT */