mirror of
https://sourceware.org/git/binutils-gdb.git
synced 2024-11-21 01:12:32 +08:00
c3466dee85
After the x86 target description changes that I committed recently, the first commit in the series being: commit8a29222b85Date: 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: commit646d754d14Author: 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>
51 lines
2.0 KiB
C
51 lines
2.0 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/>. */
|
|
|
|
#ifndef NAT_X86_LINUX_TDESC_H
|
|
#define NAT_X86_LINUX_TDESC_H
|
|
|
|
#include "gdbsupport/function-view.h"
|
|
|
|
struct target_desc;
|
|
struct x86_xsave_layout;
|
|
|
|
/* Return the target description for Linux thread TID.
|
|
|
|
The storage pointed to by XCR0_STORAGE and XSAVE_LAYOUT_STORAGE must
|
|
exist until the program (GDB or gdbserver) terminates, this storage is
|
|
used to cache the xcr0 and xsave layout values. The values pointed to
|
|
by these arguments are only updated at most once, the first time this
|
|
function is called if the have_ptrace_getregset global is set to
|
|
TRIBOOL_UNKNOWN.
|
|
|
|
This function returns a target description based on the extracted xcr0
|
|
value along with other characteristics of the thread identified by TID.
|
|
|
|
This function can return nullptr if we encounter a machine configuration
|
|
for which a target_desc cannot be created. Ideally this would not be
|
|
the case, we should be able to create a target description for every
|
|
possible machine configuration. See amd64_linux_read_description and
|
|
i386_linux_read_description for cases when nullptr might be
|
|
returned. */
|
|
|
|
extern const target_desc *x86_linux_tdesc_for_tid
|
|
(int tid, uint64_t *xcr0_storage, x86_xsave_layout *xsave_layout_storage);
|
|
|
|
#endif /* NAT_X86_LINUX_TDESC_H */
|