gdb-15-branch: Clear the X86_XSTATE_MPX bit in XCRO for x32

Message ID 20240904230150.2248673-1-hjl.tools@gmail.com
State New
Headers
Series gdb-15-branch: Clear the X86_XSTATE_MPX bit in XCRO for x32 |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 fail Patch failed to apply
linaro-tcwg-bot/tcwg_gdb_build--master-arm fail Patch failed to apply

Commit Message

H.J. Lu Sept. 4, 2024, 11:01 p.m. UTC
  commit 868883583e7520ff1bd99fcb224d2b33a990edff
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Sat Mar 23 16:17:36 2024 +0000

    gdb/arch: assert that X86_XSTATE_MPX is not set for x32

added

  if (xcr0 & X86_XSTATE_MPX)
    {
      /* MPX is not available on x32.  */
      gdb_assert (!is_x32);
      regnum = create_feature_i386_64bit_mpx (tdesc.get (), regnum);
    }

But x32 is a software convention.  There is no x32 mode in hardware and
CPU always returns the 64-bit mode XCR0 value for x32 processes.  This
regression was fixed on master branch by

commit bf616be99153b43c1077be9dbb7b081b4c080031 (HEAD)
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Thu Jan 25 14:25:57 2024 +0000

    gdb/gdbserver: share some code relating to target description creation

which used the gdbserver code to clear the X86_XSTATE_MPX bit in XCR0 for
x32.  Fix this regression on gdb-15-branch by clearing the X86_XSTATE_MPX
bit in XCR0 for x32 in gdb.

	PR gdb/32143
	* x86-linux-nat.c (x86_linux_nat_target::read_description): Clear
	the X86_XSTATE_MPX bit in XCR0 for x32.

Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
---
 gdb/x86-linux-nat.c | 6 ++++++
 1 file changed, 6 insertions(+)
  

Comments

H.J. Lu Sept. 5, 2024, 3:35 p.m. UTC | #1
On Wed, Sep 4, 2024 at 4:01 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> commit 868883583e7520ff1bd99fcb224d2b33a990edff
> Author: Andrew Burgess <aburgess@redhat.com>
> Date:   Sat Mar 23 16:17:36 2024 +0000
>
>     gdb/arch: assert that X86_XSTATE_MPX is not set for x32
>
> added
>
>   if (xcr0 & X86_XSTATE_MPX)
>     {
>       /* MPX is not available on x32.  */
>       gdb_assert (!is_x32);
>       regnum = create_feature_i386_64bit_mpx (tdesc.get (), regnum);
>     }
>
> But x32 is a software convention.  There is no x32 mode in hardware and
> CPU always returns the 64-bit mode XCR0 value for x32 processes.  This
> regression was fixed on master branch by
>
> commit bf616be99153b43c1077be9dbb7b081b4c080031 (HEAD)
> Author: Andrew Burgess <aburgess@redhat.com>
> Date:   Thu Jan 25 14:25:57 2024 +0000
>
>     gdb/gdbserver: share some code relating to target description creation
>
> which used the gdbserver code to clear the X86_XSTATE_MPX bit in XCR0 for
> x32.  Fix this regression on gdb-15-branch by clearing the X86_XSTATE_MPX
> bit in XCR0 for x32 in gdb.
>
>         PR gdb/32143
>         * x86-linux-nat.c (x86_linux_nat_target::read_description): Clear
>         the X86_XSTATE_MPX bit in XCR0 for x32.
>
> Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
> ---
>  gdb/x86-linux-nat.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/gdb/x86-linux-nat.c b/gdb/x86-linux-nat.c
> index a3d8ffb60f1..1214aa4be96 100644
> --- a/gdb/x86-linux-nat.c
> +++ b/gdb/x86-linux-nat.c
> @@ -180,6 +180,12 @@ x86_linux_nat_target::read_description ()
>           xcr0 = xstateregs[(I386_LINUX_XSAVE_XCR0_OFFSET
>                              / sizeof (uint64_t))];
>
> +#ifdef __x86_64__
> +         /* No MPX on x32.  */
> +         if (is_x32)
> +           xcr0 &= ~X86_XSTATE_MPX;
> +#endif
> +
>           m_xsave_layout = x86_fetch_xsave_layout (xcr0, x86_xsave_length ());
>         }
>      }
> --
> 2.46.0
>

Is this OK for gdb-15-branch?  Another option is to revert

commit 868883583e7520ff1bd99fcb224d2b33a990edff
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Sat Mar 23 16:17:36 2024 +0000

    gdb/arch: assert that X86_XSTATE_MPX is not set for x32

Either will work.

Thanks.
  
Joel Brobecker Sept. 8, 2024, 2:15 p.m. UTC | #2
On Wed, Sep 04, 2024 at 04:01:50PM -0700, H.J. Lu wrote:
> commit 868883583e7520ff1bd99fcb224d2b33a990edff
> Author: Andrew Burgess <aburgess@redhat.com>
> Date:   Sat Mar 23 16:17:36 2024 +0000
> 
>     gdb/arch: assert that X86_XSTATE_MPX is not set for x32
> 
> added
> 
>   if (xcr0 & X86_XSTATE_MPX)
>     {
>       /* MPX is not available on x32.  */
>       gdb_assert (!is_x32);
>       regnum = create_feature_i386_64bit_mpx (tdesc.get (), regnum);
>     }
> 
> But x32 is a software convention.  There is no x32 mode in hardware and
> CPU always returns the 64-bit mode XCR0 value for x32 processes.  This
> regression was fixed on master branch by
> 
> commit bf616be99153b43c1077be9dbb7b081b4c080031 (HEAD)
> Author: Andrew Burgess <aburgess@redhat.com>
> Date:   Thu Jan 25 14:25:57 2024 +0000
> 
>     gdb/gdbserver: share some code relating to target description creation
> 
> which used the gdbserver code to clear the X86_XSTATE_MPX bit in XCR0 for
> x32.  Fix this regression on gdb-15-branch by clearing the X86_XSTATE_MPX
> bit in XCR0 for x32 in gdb.
> 
> 	PR gdb/32143
> 	* x86-linux-nat.c (x86_linux_nat_target::read_description): Clear
> 	the X86_XSTATE_MPX bit in XCR0 for x32.
> 
> Signed-off-by: H.J. Lu <hjl.tools@gmail.com>

Thank you. Approved for the gdb-15-branch.

> ---
>  gdb/x86-linux-nat.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/gdb/x86-linux-nat.c b/gdb/x86-linux-nat.c
> index a3d8ffb60f1..1214aa4be96 100644
> --- a/gdb/x86-linux-nat.c
> +++ b/gdb/x86-linux-nat.c
> @@ -180,6 +180,12 @@ x86_linux_nat_target::read_description ()
>  	  xcr0 = xstateregs[(I386_LINUX_XSAVE_XCR0_OFFSET
>  			     / sizeof (uint64_t))];
>  
> +#ifdef __x86_64__
> +	  /* No MPX on x32.  */
> +	  if (is_x32)
> +	    xcr0 &= ~X86_XSTATE_MPX;
> +#endif
> +
>  	  m_xsave_layout = x86_fetch_xsave_layout (xcr0, x86_xsave_length ());
>  	}
>      }
> -- 
> 2.46.0
>
  

Patch

diff --git a/gdb/x86-linux-nat.c b/gdb/x86-linux-nat.c
index a3d8ffb60f1..1214aa4be96 100644
--- a/gdb/x86-linux-nat.c
+++ b/gdb/x86-linux-nat.c
@@ -180,6 +180,12 @@  x86_linux_nat_target::read_description ()
 	  xcr0 = xstateregs[(I386_LINUX_XSAVE_XCR0_OFFSET
 			     / sizeof (uint64_t))];
 
+#ifdef __x86_64__
+	  /* No MPX on x32.  */
+	  if (is_x32)
+	    xcr0 &= ~X86_XSTATE_MPX;
+#endif
+
 	  m_xsave_layout = x86_fetch_xsave_layout (xcr0, x86_xsave_length ());
 	}
     }