Re: [PATCH v2] gdbserver: Clear X86_XSTATE_MPX bits in xcr0 on x32

[Andrew Burgess <aburgess@redhat.com>]

"H.J. Lu" <hjl.tools@gmail.com> writes:

> Since MPX isn't available for x32, we should clear X86_XSTATE_MPX bits
> on x32.
>
> 	PR server/31511
> 	* linux-x86-low.cc (x86_linux_read_description): Clear
> 	X86_XSTATE_MPX bits in xcr0 on x32.
> ---
>  gdbserver/linux-x86-low.cc | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/gdbserver/linux-x86-low.cc b/gdbserver/linux-x86-low.cc
> index 3af0a009052..933d1fb012a 100644
> --- a/gdbserver/linux-x86-low.cc
> +++ b/gdbserver/linux-x86-low.cc
> @@ -938,6 +938,10 @@ x86_linux_read_description (void)
>  	  xcr0 = xstateregs[(I386_LINUX_XSAVE_XCR0_OFFSET
>  			     / sizeof (uint64_t))];
>  
> +	  /* No MPX on x32.  */
> +	  if (machine == EM_X86_64 && !is_elf64)
> +	    xcr0 &= ~X86_XSTATE_MPX;

Hi,

I have a series in flight that conflicts with this change:

  https://inbox.sourceware.org/gdb-patches/cover.1706801009.git.aburgess@redhat.com

And so I'm trying to resolve the conflicts.

Initially I thought I could test this by compiling something with the
gcc -m32 option, but I now believe that is wrong, and I'd actually need
to compile using -mx32, is this correct?

Gcc seems to understand this option, but I appear to be missing the
required development libraries, and I'm currently struggling to figure
out what I'd need to install.  Can you suggest a particular
distro/version that should have the required development libraries
available?  Or maybe I'm just not understanding something here and you
can point out what I'd doing wrong.

The series of mine above came about for one reason: the `machine` type
checks in the gdbserver are not a good idea, they rely on access to an
executable, which isn't always available (e.g. if gdbserver attaches to
a running process for which the executable is not available).  When I
started looking at this code I then decided to merge the whole target
description logic between GDB and gdbserver for x86/linux.

Which leads onto my second question; you changed the gdbserver code, but
not the GDB code.  Was this just an oversight?  Or is there some reason
why this fix doesn't need making for GDB too?

Now worryingly, on the GDB side, in
x86_linux_nat_target::read_description (gdb/x86-linux-nat.c) we do have
a variable `is_x32` which I sounds like it should be true when we are
debugging an x32 process.... but.... this variable is true for a binary
compiled with the -m32 flag, which I don't think is right.  If my
understanding is correct then a -m32 binary is one compiled on an x86-64
machine, but which is uses 32-bit int/pointer types, and which only uses
i386 instructions, and can use MPX, right?

To summarise:

  1. How might I go about compiling an x32 binary?

  2. How can we detect an x32 process without checking the machine type?

  3. Should your fix above be applied on the GDB side too?

I've included below my initial attempt at a test for this change,
currently it compiles with -m32 as (like I said) -mx32 doesn't work on
my machine.  I've been testing this:

  make check-gdb TESTS="gdb.arch/amd64-x32-no-mpx-in-tdesc.exp" \
      RUNTESTFLAGS="--target_board=unix"
  make check-gdb TESTS="gdb.arch/amd64-x32-no-mpx-in-tdesc.exp" \
      RUNTESTFLAGS="--target_board=native-gdbserver"
  make check-gdb TESTS="gdb.arch/amd64-x32-no-mpx-in-tdesc.exp" \
      RUNTESTFLAGS="--target_board=native-extended-gdbserver"

Currently it will fail for all as -m32 != -mx32 and the .mpx feature
shows up (as expected).

I guess you have a target which supports -mx32, so on that target you
should be able to change the '-m32' in the test to '-mx32' and I would
expect you to see the test pass for the gdbserver boards, but fail on
the unix board.

Thanks,
Andrew

---

commit 22167bc5c2f2770e1b1a3f0eb0569df3140921e8
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Sat Mar 23 10:45:22 2024 +0000

    WIP: New test

diff --git a/gdb/testsuite/gdb.arch/amd64-x32-no-mpx-in-tdesc.c b/gdb/testsuite/gdb.arch/amd64-x32-no-mpx-in-tdesc.c
new file mode 100644
index 00000000000..e2119ba7d92
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-x32-no-mpx-in-tdesc.c
@@ -0,0 +1,22 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2024 Free Software Foundation, Inc.
+
+   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/>.  */
+
+int
+main (void)
+{
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.arch/amd64-x32-no-mpx-in-tdesc.exp b/gdb/testsuite/gdb.arch/amd64-x32-no-mpx-in-tdesc.exp
new file mode 100644
index 00000000000..78e0d948348
--- /dev/null
+++ b/gdb/testsuite/gdb.arch/amd64-x32-no-mpx-in-tdesc.exp
@@ -0,0 +1,52 @@
+# Copyright 2024 Free Software Foundation, Inc.
+#
+# 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/>.
+#
+# This file is part of the gdb testsuite.
+
+# Check that the mpx feature is not present in the target description
+# when debugging x32 binaries on an x86-64 target.
+
+require {istarget x86_64-*-*}
+
+standard_testfile
+if { [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} \
+	  {debug additional_flags=-m32}] } {
+    return -1
+}
+
+if ![runto_main] {
+    return -1
+}
+
+set found_mpx false
+gdb_test_multiple "maint print xml-tdesc" "" {
+    -re "^maint print xml-tdesc\r\n" {
+	exp_continue
+    }
+
+    -re "^\\s+<feature name=\"org.gnu.gdb.i386.mpx\">\r\n" {
+	set found_mpx true
+	exp_continue
+    }
+
+    -re "^$gdb_prompt $" {
+	gdb_assert { !$found_mpx } \
+	    "mpx feature not in target description"
+    }
+
+    -re "^\[^\r\n\]+\r\n" {
+	exp_continue
+    }
+}