[PATCHv6,3/9] gdb/x86: move have_ptrace_getfpxregs global into gdb/nat directory
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gdb_build--master-arm |
success
|
Testing passed
|
| linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 |
success
|
Testing passed
|
Commit Message
The have_ptrace_getfpxregs global tracks whether GDB or gdbserver is
running on a kernel that supports the GETFPXREGS ptrace request.
Currently this global is declared twice (once in GDB and once in
gdbserver), but it makes sense to move this global into the nat/
directory, and have a single declaration and definition.
While moving this variable I have converted it to a tribool, as that
was what it really was, if even used the same numbering as the tribool
enum (-1, 0, 1). Where have_ptrace_getfpxregs was used I have updated
in the obvious way.
However, while making this change I noticed what I think is a bug in
x86_linux_nat_target::read_description and x86_linux_read_description,
both of these functions can be called multiple times, but in both
cases we only end up calling i386_linux_read_description the first
time through in the event that PTRACE_GETFPXREGS is not supported.
This is because initially have_ptrace_getfpxregs will be
TRIBOOL_UNKNOWN, but after the ptrace call fails we set
have_ptrace_getfpxregs to TRIBOOL_FALSE. The next time we attempt to
read the target description we'll skip the ptrace call, and so skip
the call to i386_linux_read_description.
I've not tried to address this preexisting bug in this commit, this is
purely a refactor, there should be no user visible changes after this
commit. In later commits I'll merge the gdbserver and GDB code
together into the nat/ directory, and after that I'll try to address
this bug.
Reviewed-By: Felix Willgerodt <felix.willgerodt@intel.com>
---
gdb/Makefile.in | 1 -
gdb/i386-linux-nat.c | 26 +++++---------------------
gdb/i386-linux-nat.h | 26 --------------------------
gdb/nat/x86-linux.c | 9 +++++++++
gdb/nat/x86-linux.h | 14 ++++++++++++++
gdb/x86-linux-nat.c | 7 ++-----
gdbserver/linux-x86-low.cc | 17 +++--------------
7 files changed, 33 insertions(+), 67 deletions(-)
delete mode 100644 gdb/i386-linux-nat.h
Comments
On 5/8/24 9:46 AM, Andrew Burgess wrote:
> The have_ptrace_getfpxregs global tracks whether GDB or gdbserver is
> running on a kernel that supports the GETFPXREGS ptrace request.
>
> Currently this global is declared twice (once in GDB and once in
> gdbserver), but it makes sense to move this global into the nat/
> directory, and have a single declaration and definition.
>
> While moving this variable I have converted it to a tribool, as that
> was what it really was, if even used the same numbering as the tribool
> enum (-1, 0, 1). Where have_ptrace_getfpxregs was used I have updated
> in the obvious way.
>
> However, while making this change I noticed what I think is a bug in
> x86_linux_nat_target::read_description and x86_linux_read_description,
> both of these functions can be called multiple times, but in both
> cases we only end up calling i386_linux_read_description the first
> time through in the event that PTRACE_GETFPXREGS is not supported.
> This is because initially have_ptrace_getfpxregs will be
> TRIBOOL_UNKNOWN, but after the ptrace call fails we set
> have_ptrace_getfpxregs to TRIBOOL_FALSE. The next time we attempt to
> read the target description we'll skip the ptrace call, and so skip
> the call to i386_linux_read_description.
>
> I've not tried to address this preexisting bug in this commit, this is
> purely a refactor, there should be no user visible changes after this
> commit. In later commits I'll merge the gdbserver and GDB code
> together into the nat/ directory, and after that I'll try to address
> this bug.
>
> Reviewed-By: Felix Willgerodt <felix.willgerodt@intel.com>
> ---
> gdb/Makefile.in | 1 -
> gdb/i386-linux-nat.c | 26 +++++---------------------
> gdb/i386-linux-nat.h | 26 --------------------------
> gdb/nat/x86-linux.c | 9 +++++++++
> gdb/nat/x86-linux.h | 14 ++++++++++++++
> gdb/x86-linux-nat.c | 7 ++-----
> gdbserver/linux-x86-low.cc | 17 +++--------------
> 7 files changed, 33 insertions(+), 67 deletions(-)
> delete mode 100644 gdb/i386-linux-nat.h
>
> diff --git a/gdb/nat/x86-linux.c b/gdb/nat/x86-linux.c
> index 7a21c8f2c26..f394dc5d5e1 100644
> --- a/gdb/nat/x86-linux.c
> +++ b/gdb/nat/x86-linux.c
> @@ -20,6 +20,15 @@
> #include "x86-linux.h"
> #include "x86-linux-dregs.h"
>
> +/* See nat/x86-linux.h. */
> +tribool have_ptrace_getfpxregs =
> +#ifdef HAVE_PTRACE_GETFPXREGS
> + TRIBOOL_UNKNOWN
> +#else
> + TRIBOOL_FALSE
> +#endif
> +;
> +
Should this be under #ifdef __i386__ or the like? My understanding is that
this request isn't supported for Linux x86-64, just for i386?
@@ -1397,7 +1397,6 @@ HFILES_NO_SRCDIR = \
hppa-tdep.h \
i386-bsd-nat.h \
i386-darwin-tdep.h \
- i386-linux-nat.h \
i386-linux-tdep.h \
i386-tdep.h \
i387-tdep.h \
@@ -26,7 +26,7 @@
#include "gregset.h"
#include "gdb_proc_service.h"
-#include "i386-linux-nat.h"
+#include "nat/x86-linux.h"
#include "i387-tdep.h"
#include "i386-tdep.h"
#include "i386-linux-tdep.h"
@@ -80,22 +80,6 @@ int have_ptrace_getregs =
0
#endif
;
-
-/* Does the current host support the GETFPXREGS request? The header
- file may or may not define it, and even if it is defined, the
- kernel will return EIO if it's running on a pre-SSE processor.
-
- My instinct is to attach this to some architecture- or
- target-specific data structure, but really, a particular GDB
- process can only run on top of one kernel at a time. So it's okay
- for this to be a simple variable. */
-int have_ptrace_getfpxregs =
-#ifdef HAVE_PTRACE_GETFPXREGS
- -1
-#else
- 0
-#endif
-;
/* Accessing registers through the U area, one at a time. */
@@ -388,14 +372,14 @@ fetch_fpxregs (struct regcache *regcache, int tid)
{
elf_fpxregset_t fpxregs;
- if (! have_ptrace_getfpxregs)
+ if (have_ptrace_getfpxregs == TRIBOOL_FALSE)
return 0;
if (ptrace (PTRACE_GETFPXREGS, tid, 0, (int) &fpxregs) < 0)
{
if (errno == EIO)
{
- have_ptrace_getfpxregs = 0;
+ have_ptrace_getfpxregs = TRIBOOL_FALSE;
return 0;
}
@@ -415,14 +399,14 @@ store_fpxregs (const struct regcache *regcache, int tid, int regno)
{
elf_fpxregset_t fpxregs;
- if (! have_ptrace_getfpxregs)
+ if (have_ptrace_getfpxregs == TRIBOOL_FALSE)
return 0;
if (ptrace (PTRACE_GETFPXREGS, tid, 0, &fpxregs) == -1)
{
if (errno == EIO)
{
- have_ptrace_getfpxregs = 0;
+ have_ptrace_getfpxregs = TRIBOOL_FALSE;
return 0;
}
deleted file mode 100644
@@ -1,26 +0,0 @@
-/* Native-dependent code for GNU/Linux i386.
-
- Copyright (C) 1999-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 I386_LINUX_NAT_H
-#define I386_LINUX_NAT_H 1
-
-/* Does the current host support the GETFPXREGS request? */
-extern int have_ptrace_getfpxregs;
-
-#endif
@@ -20,6 +20,15 @@
#include "x86-linux.h"
#include "x86-linux-dregs.h"
+/* See nat/x86-linux.h. */
+tribool have_ptrace_getfpxregs =
+#ifdef HAVE_PTRACE_GETFPXREGS
+ TRIBOOL_UNKNOWN
+#else
+ TRIBOOL_FALSE
+#endif
+;
+
/* Per-thread arch-specific data we want to keep. */
struct arch_lwp_info
@@ -22,6 +22,20 @@
#include "nat/linux-nat.h"
+/* Does the current host support the GETFPXREGS request? The system header
+ file may or may not define it, but even if it is defined, the kernel
+ will return EIO if it's running on a pre-SSE processor.
+
+ Initially this will be TRIBOOL_UNKNOWN and should be changed to
+ TRIBOOL_FALSE if the ptrace call is attempted and fails or changed to
+ TRIBOOL_TRUE if the ptrace call is attempted and succeeds.
+
+ My instinct is to attach this to some architecture- or target-specific
+ data structure, but really, a particular GDB process can only run on top
+ of one kernel at a time. So it's okay - for this to be a global
+ variable. */
+extern tribool have_ptrace_getfpxregs;
+
/* Set whether our local mirror of LWP's debug registers has been
changed since the values were last written to the thread. Nonzero
indicates that a change has been made, zero indicates no change. */
@@ -26,9 +26,6 @@
#include <sys/uio.h>
#include "x86-nat.h"
-#ifndef __x86_64__
-#include "i386-linux-nat.h"
-#endif
#include "x86-linux-nat.h"
#include "i386-linux-tdep.h"
#ifdef __x86_64__
@@ -147,13 +144,13 @@ x86_linux_nat_target::read_description ()
error (_("Can't debug 64-bit process with 32-bit GDB"));
}
#elif HAVE_PTRACE_GETFPXREGS
- if (have_ptrace_getfpxregs == -1)
+ if (have_ptrace_getfpxregs == TRIBOOL_UNKNOWN)
{
elf_fpxregset_t fpxregs;
if (ptrace (PTRACE_GETFPXREGS, tid, 0, (int) &fpxregs) < 0)
{
- have_ptrace_getfpxregs = 0;
+ have_ptrace_getfpxregs = TRIBOOL_FALSE;
have_ptrace_getregset = TRIBOOL_FALSE;
return i386_linux_read_description (X86_XSTATE_X87_MASK);
}
@@ -831,17 +831,6 @@ x86_target::low_siginfo_fixup (siginfo_t *ptrace, gdb_byte *inf, int direction)
static int use_xml;
-/* Does the current host support the GETFPXREGS request? The header
- file may or may not define it, and even if it is defined, the
- kernel will return EIO if it's running on a pre-SSE processor. */
-int have_ptrace_getfpxregs =
-#ifdef HAVE_PTRACE_GETFPXREGS
- -1
-#else
- 0
-#endif
-;
-
/* Get Linux/x86 target description from running target. */
static const struct target_desc *
@@ -886,18 +875,18 @@ x86_linux_read_description (void)
}
#if !defined __x86_64__ && defined HAVE_PTRACE_GETFPXREGS
- if (machine == EM_386 && have_ptrace_getfpxregs == -1)
+ if (machine == EM_386 && have_ptrace_getfpxregs == TRIBOOL_UNKNOWN)
{
elf_fpxregset_t fpxregs;
if (ptrace (PTRACE_GETFPXREGS, tid, 0, (long) &fpxregs) < 0)
{
- have_ptrace_getfpxregs = 0;
+ have_ptrace_getfpxregs = TRIBOOL_FALSE;
have_ptrace_getregset = TRIBOOL_FALSE;
return i386_linux_read_description (X86_XSTATE_X87);
}
else
- have_ptrace_getfpxregs = 1;
+ have_ptrace_getfpxregs = TRIBOOL_TRUE;
}
#endif