[v3,7/8] Windows gdb: Implement AVX register support

  This adds support for the Intel AVX registers on Windows.
It enables accessing registers $ymm0 - $ymm15 where they are available.

After this patch gdb.arch/i386-avx.exp passes on windows.
---
v3:
  - merged gdb+gdbserver parts, and split again AVX/AVX-512 parts
---
 gdb/NEWS                    |  3 ++
 gdb/nat/windows-nat.c       |  2 +-
 gdb/x86-windows-nat.c       | 67 +++++++++++++++++++++++++++++++++++--
 gdbserver/win32-i386-low.cc | 61 +++++++++++++++++++++++++++++----
 gdbserver/win32-low.cc      | 15 ++++++---
 5 files changed, 133 insertions(+), 15 deletions(-)
  

>>>>> "Hannes" == Hannes Domani <ssbssa@yahoo.de> writes:

Hannes> This adds support for the Intel AVX registers on Windows.
Hannes> It enables accessing registers $ymm0 - $ymm15 where they are available.

Hannes> After this patch gdb.arch/i386-avx.exp passes on windows.

I'd like it if Christina would review this.

Hannes> @@ -1339,7 +1339,7 @@ initialize_loadable ()
Hannes>      {
Hannes>        /* Available XState features masked with implemented features.  */
Hannes>        xstate_features = (GetEnabledXStateFeatures ()
Hannes> -			 & X86_XSTATE_SSE_MASK);
Hannes> +			 & X86_XSTATE_AVX_MASK);
Hannes>        /* The extended XState functions are only needed if the available
Hannes>  	 features exceed SSE.  */
Hannes>        if ((xstate_features & ~X86_XSTATE_SSE_MASK) == 0)

Ok, ignore my earlier message for patch 6.  I see what's going on now.

I don't have any problem with this patch.  I'm not certain I totally
understand it, but that's ok.

One thing I did see while doing the background reading is that
supposedly the xstate feature flags are vendor-specific.  So does some
check of the vendor ID need to be done?  I do not know.

thanks,
Tom
  

Am Dienstag, 1. September 2026 um 19:48:28 MESZ hat Tom Tromey <tom@tromey.com> Folgendes geschrieben:

> >>>>> "Hannes" == Hannes Domani <ssbssa@yahoo.de> writes:
> 
> Hannes> This adds support for the Intel AVX registers on Windows.
> Hannes> It enables accessing registers $ymm0 - $ymm15 where they are available.
> 
> Hannes> After this patch gdb.arch/i386-avx.exp passes on windows.
> 
> I'd like it if Christina would review this.
> 
> Hannes> @@ -1339,7 +1339,7 @@ initialize_loadable ()
> Hannes>      {
> Hannes>        /* Available XState features masked with implemented features.  */
> Hannes>        xstate_features = (GetEnabledXStateFeatures ()
> Hannes> -            & X86_XSTATE_SSE_MASK);
> Hannes> +            & X86_XSTATE_AVX_MASK);
> Hannes>        /* The extended XState functions are only needed if the available
> Hannes>      features exceed SSE.  */
> Hannes>        if ((xstate_features & ~X86_XSTATE_SSE_MASK) == 0)
> 
> Ok, ignore my earlier message for patch 6.  I see what's going on now.
> 
> I don't have any problem with this patch.  I'm not certain I totally
> understand it, but that's ok.
> 
> One thing I did see while doing the background reading is that
> supposedly the xstate feature flags are vendor-specific.  So does some
> check of the vendor ID need to be done?  I do not know.

As I understand it (and I might be wrong about that), there are some
vendor-specific XState-bits, but stuff regarding AVX and AVX-512 are the
same everywhere.


Hannes
  

Hi Hannes,

please see some feedback inlined below.

Let me know if you have any questions.

Thanks
Stephan

> -----Original Message-----
> From: Hannes Domani <ssbssa@yahoo.de>
> Sent: Saturday, 29 August 2026 16:49
> To: gdb-patches@sourceware.org
> Subject: [PATCH v3 7/8] Windows gdb: Implement AVX register support
> 
> This adds support for the Intel AVX registers on Windows.
> It enables accessing registers $ymm0 - $ymm15 where they are available.
> 
> After this patch gdb.arch/i386-avx.exp passes on windows.
> ---
> v3:
>   - merged gdb+gdbserver parts, and split again AVX/AVX-512 parts
> ---
>  gdb/NEWS                    |  3 ++
>  gdb/nat/windows-nat.c       |  2 +-
>  gdb/x86-windows-nat.c       | 67
> +++++++++++++++++++++++++++++++++++--
>  gdbserver/win32-i386-low.cc | 61 +++++++++++++++++++++++++++++----
>  gdbserver/win32-low.cc      | 15 ++++++---
>  5 files changed, 133 insertions(+), 15 deletions(-)
> 
> diff --git a/gdb/NEWS b/gdb/NEWS
> index 10c182067f9..f7effc822e9 100644
> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -118,6 +118,9 @@
>    intent to remove it in a future release.
>    The s390 64-bit target (s390x-*) remains supported.
> 
> +* Support for Intel AVX registers on Windows.
> +  Support displaying and modifying Intel AVX registers $ymm0 - $ymm31.
> +

I think this should be registers $ymm0 - $ymm15 ?

>  * Configure changes
> 
>  ** --with-babeltrace has been removed.  The babeltrace library was
> diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c
> index 8930536f3ba..c9a21d7c41f 100644
> --- a/gdb/nat/windows-nat.c
> +++ b/gdb/nat/windows-nat.c
> @@ -1339,7 +1339,7 @@ initialize_loadable ()
>      {
>        /* Available XState features masked with implemented features.  */
>        xstate_features = (GetEnabledXStateFeatures ()
> -			 & X86_XSTATE_SSE_MASK);
> +			 & X86_XSTATE_AVX_MASK);
>        /* The extended XState functions are only needed if the available
>  	 features exceed SSE.  */
>        if ((xstate_features & ~X86_XSTATE_SSE_MASK) == 0)
> diff --git a/gdb/x86-windows-nat.c b/gdb/x86-windows-nat.c
> index 3af5ef4dae0..1cefe6171be 100644
> --- a/gdb/x86-windows-nat.c
> +++ b/gdb/x86-windows-nat.c
> @@ -27,6 +27,9 @@
> 
>  #include "i386-tdep.h"
>  #include "i387-tdep.h"
> +#ifdef __x86_64__
> +#include "amd64-tdep.h"
> +#endif
> 
>  using namespace windows_nat;
> 
> @@ -70,6 +73,8 @@ struct x86_windows_nat_target final : public
> x86_nat_target<windows_nat_target>
>  			   windows_thread_info *th, int r) override;
> 
>    bool is_sw_breakpoint (const EXCEPTION_RECORD *er) const override;
> +
> +  const struct target_desc *read_description () override;
>  };
> 
>  /* The current process.  */
> @@ -109,7 +114,31 @@ x86_windows_per_inferior::fill_thread_context
> (windows_thread_info *th)
>        if (context->ContextFlags == 0)
>  	{
>  	  context->ContextFlags = WindowsContext<decltype(context)>::all;
> +	  if (xstate_features != 0)
> +	    {
> +	      context->ContextFlags |= CONTEXT_XSTATE_FLAG;
> +	      set_xstate_features_mask (context, xstate_features);
> +	    }

We have the same code in "i386_get_thread_context" in "win32-i386-low.cc".
Make a shared function in gdb/nat/windows-nat.h?

>  	  CHECK (get_thread_context (th->h, context));
> +
> +	  if (xstate_features != 0)
> +	    {
> +	      DWORD64 features = 0;
> +	      CHECK (get_xstate_features_mask (context, &features));

Should this be changed to sth. like 

  if (!get_xstate_features_mask (context, &features))
    {
      warning (..)
      return;
    }

The call of "CHECK" only prints a message but doesn't error out.  If this
call fails we may still have features == 0.  This implies
"zeroed_features == xstate_features".  With this, the loop clears all features.
IIUC, this would clear the AVX registers on the next call of
"SetThreadContext".

Also refer to the implementation in gdbserver/win32-i386-low.cc:

      DWORD64 features = 0;
      if (xstate_features != 0
	  && get_xstate_features_mask (context, &features))
	{

I think it makes sense to unify those as the rest of the code is basically identical.  Put 
shared function into gdb/nat/windows-nat.h?  This keeps the code consistent.

> +	      DWORD64 zeroed_features = xstate_features & ~features;
> +
> +	      for (int f = X86_XSTATE_AVX_ID; f <= X86_XSTATE_CET_U_ID; f++)
> +		{
> +		  DWORD64 flag = 1ULL << f;
> +		  if ((zeroed_features & flag) != 0)
> +		    {
> +		      DWORD size = 0;
> +		      void *loc = locate_xstate_feature (context, f, &size);
> +		      if (loc != nullptr && size > 0)
> +			memset (loc, 0, size);
> +		    }
> +		}
> +	    }
>  	}
>      });
>  }
> @@ -198,6 +227,14 @@
> x86_windows_nat_target::thread_context_continue (windows_thread_info
> *th,
>  	  if (GetExitCodeThread (th->h, &ec)
>  	      && ec == STILL_ACTIVE)
>  	    {
> +	      DWORD debug_registers =
> WindowsContext<decltype(context)>::debug;
> +	      if (xstate_features != 0
> +		  && (context->ContextFlags & ~debug_registers) != 0)
> +		{
> +		  context->ContextFlags |= CONTEXT_XSTATE_FLAG;
> +		  set_xstate_features_mask (context, xstate_features);
> +		}
> +
>  	      BOOL status = set_thread_context (th->h, context);
> 
>  	      if (!killed)
> @@ -227,7 +264,7 @@ x86_windows_nat_target::thread_context_step
> (windows_thread_info *th,
> 
>  template<typename Context>
>  static char *
> -get_context_reg_ptr (Context *context, int r)
> +get_context_reg_ptr (Context *context, int r, i386_gdbarch_tdep *tdep)
>  {
>    const int *mappings;
>    int mappings_count;
> @@ -247,6 +284,13 @@ get_context_reg_ptr (Context *context, int r)
>    char *context_offset;
>    if (r < mappings_count)
>      context_offset = (char *) context + mappings[r];
> +  else if (I387_YMM0H_REGNUM (tdep) > 0 && r >= I387_YMM0H_REGNUM
> (tdep)
> +	   && r < I387_YMMENDH_REGNUM (tdep))

The implementation on gdbserver side guards against 

  xstate_features & X86_XSTATE_AVX) != 0

I wonder if the same guard would be helpful here, too.  I understand the register
number is initialized to -1, so this should not fire.  I'm not sure if it is possible to
have $ymm0 register number > 0 w/o xstate support, e.g., if the target description
is read from file, see "target_find_description"?

> +    {
> +      context_offset = (char *) locate_xstate_feature
> +	(context, X86_XSTATE_AVX_ID, NULL);
> +      context_offset += 16 * (r - I387_YMM0H_REGNUM (tdep));
> +    }
>    else
>      gdb_assert_not_reached ("invalid register number %d", r);
> 
> @@ -267,7 +311,7 @@ x86_windows_nat_target::fetch_one_register (struct
> regcache *regcache,
>    char *context_offset
>      = x86_windows_process.with_context (th, [&] (auto *context)
>      {
> -      return get_context_reg_ptr (context, r);
> +      return get_context_reg_ptr (context, r, tdep);
>      });
> 
>    gdb_assert (!gdbarch_read_pc_p (gdbarch));
> @@ -333,7 +377,7 @@ x86_windows_nat_target::store_one_register (const
> struct regcache *regcache,
>      = x86_windows_process.with_context (th, [&] (auto *context)
>      {
>        gdb_assert (context->ContextFlags != 0);
> -      return get_context_reg_ptr (context, r);
> +      return get_context_reg_ptr (context, r, tdep);
>      });
> 
>    /* GDB treats some registers as 32-bit, where they are in fact only
> @@ -368,6 +412,23 @@ x86_windows_nat_target::is_sw_breakpoint (const
> EXCEPTION_RECORD *er) const
>  	  || er->ExceptionCode == STATUS_WX86_BREAKPOINT);
>  }
> 
> +const struct target_desc *
> +x86_windows_nat_target::read_description ()
> +{
> +  if (inferior_ptid == null_ptid)
> +    return this->beneath ()->read_description ();
> +
> +  if (xstate_features == 0)
> +    return nullptr;
> +
> +#ifdef __x86_64__
> +  if (!x86_windows_process.wow64_process)
> +    return amd64_target_description (xstate_features, false);
> +  else
> +#endif
> +    return i386_target_description (xstate_features, false);
> +}
> +
>  /* Hardware watchpoint support, adapted from go32-nat.c code.  */
> 
>  /* Pass the address ADDR to the inferior in the I'th debug register.
> diff --git a/gdbserver/win32-i386-low.cc b/gdbserver/win32-i386-low.cc
> index b77f6adc6ed..a7e83c0239c 100644
> --- a/gdbserver/win32-i386-low.cc
> +++ b/gdbserver/win32-i386-low.cc
> @@ -253,6 +253,11 @@ i386_get_thread_context (windows_thread_info
> *th)
>  			       | WindowsContext<decltype(context)>::floating
>  			       | WindowsContext<decltype(context)>::debug
>  			       | extended_registers);
> +      if (xstate_features != 0)
> +	{
> +	  context->ContextFlags |= CONTEXT_XSTATE_FLAG;
> +	  set_xstate_features_mask (context, xstate_features);
> +	}
> 
>        BOOL ret = get_thread_context (th->h, context);
>        if (!ret)
> @@ -267,6 +272,24 @@ i386_get_thread_context (windows_thread_info
> *th)
> 
>  	  error (_("GetThreadContext failure %ld\n"), (long) e);
>  	}
> +
> +      DWORD64 features = 0;
> +      if (xstate_features != 0
> +	  && get_xstate_features_mask (context, &features))
> +	{
> +	  DWORD64 zeroed_features = xstate_features & ~features;
> +	  for (int f = X86_XSTATE_AVX_ID; f <= X86_XSTATE_CET_U_ID; f++)
> +	    {
> +	      DWORD64 flag = 1ULL << f;
> +	      if ((zeroed_features & flag) != 0)
> +		{
> +		  DWORD size = 0;
> +		  void *loc = locate_xstate_feature (context, f, &size);
> +		  if (loc != nullptr && size > 0)
> +		    memset (loc, 0, size);
> +		}
> +	    }
> +	}
>      });
>  }
> 
> @@ -292,6 +315,17 @@ i386_prepare_to_resume (windows_thread_info
> *th)
> 
>        th->debug_registers_changed = false;
>      }
> +
> +  windows_process.with_context (th, [&] (auto *context)
> +    {
> +      DWORD debug_registers = WindowsContext<decltype(context)>::debug;
> +      if (xstate_features != 0
> +	  && (context->ContextFlags & ~debug_registers) != 0)
> +	{
> +	  context->ContextFlags |= CONTEXT_XSTATE_FLAG;
> +	  set_xstate_features_mask (context, xstate_features);
> +	}
> +    });
>  }
> 
>  static void
> @@ -477,7 +511,7 @@ is_segment_register (int r)
> 
>  template<typename Context>
>  static char *
> -get_context_reg_ptr (Context *context, int r)
> +get_context_reg_ptr (Context *context, int r, const target_desc *tdesc)
>  {
>    const int *mappings;
>    int mappings_count;
> @@ -494,9 +528,21 @@ get_context_reg_ptr (Context *context, int r)
>        mappings_count = sizeof (i386_mappings) / sizeof (i386_mappings[0]);
>      }
> 
> +  bool amd64 = register_size (tdesc, 0) == 8;

There is already an " if (!windows_process.wow64_process)" a few lines above.
Wouldn't it make sense to move the "bool amd64" in the ifdef blocks and
assign accordingly?

> +  int ymm0h_regnum;
> +  const int num_xmm_registers = amd64 ? 16 : 8;
> +
>    char *context_offset;
>    if (r < mappings_count)
>      context_offset = (char *) context + mappings[r];
> +  else if ((xstate_features & X86_XSTATE_AVX) != 0
> +	   && r >= (ymm0h_regnum = find_regno (tdesc, "ymm0h"))
> +	   && r < ymm0h_regnum + num_xmm_registers)
> +    {
> +      context_offset = (char *) locate_xstate_feature
> +	(context, X86_XSTATE_AVX_ID, NULL);
> +      context_offset += 16 * (r - ymm0h_regnum);
> +    }
>    else
>      gdb_assert_not_reached ("invalid register number %d", r);
> 
> @@ -510,7 +556,7 @@ i386_fetch_inferior_register (struct regcache
> *regcache,
>  {
>    char *context_offset = windows_process.with_context (th, [&] (auto
> *context)
>      {
> -      return get_context_reg_ptr (context, r);
> +      return get_context_reg_ptr (context, r, regcache->tdesc);
>      });
> 
>    /* GDB treats some registers as 32-bit, where they are in fact only
> @@ -538,7 +584,7 @@ i386_store_inferior_register (struct regcache
> *regcache,
>  {
>    char *context_offset = windows_process.with_context (th, [&] (auto
> *context)
>      {
> -      return get_context_reg_ptr (context, r);
> +      return get_context_reg_ptr (context, r, regcache->tdesc);
>      });
> 
>    /* GDB treats some registers as 32-bit, where they are in fact only
> @@ -571,14 +617,17 @@ i386_arch_setup (void)
>  {
>   target_desc_up tdesc;
> 
> +  DWORD64 xcr0 = xstate_features;
> +  if (xcr0 == 0)
> +    xcr0 = X86_XSTATE_SSE_MASK;
> +
>  #ifdef __x86_64__
> -  tdesc = amd64_create_target_description (X86_XSTATE_SSE_MASK, false,
> -					   false, false);
> +  tdesc = amd64_create_target_description (xcr0, false, false, false);
>    init_target_desc (tdesc.get (), amd64_expedite_regs, WINDOWS_OSABI);
>    win32_tdesc = std::move (tdesc);
>  #endif
> 
> -  tdesc = i386_create_target_description (X86_XSTATE_SSE_MASK, false,
> false);
> +  tdesc = i386_create_target_description (xcr0, false, false);
>    init_target_desc (tdesc.get (), i386_expedite_regs, WINDOWS_OSABI);
>  #ifdef __x86_64__
>    wow64_win32_tdesc = std::move (tdesc);
> diff --git a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc
> index 7629beca213..5ccdc89a7ef 100644
> --- a/gdbserver/win32-low.cc
> +++ b/gdbserver/win32-low.cc
> @@ -33,6 +33,7 @@
>  #include <process.h>
>  #include "gdbsupport/gdb_tilde_expand.h"
>  #include "gdbsupport/common-inferior.h"
> +#include "tdesc.h"
> 
>  using namespace windows_nat;
> 
> @@ -426,8 +427,9 @@ child_fetch_inferior_registers (struct regcache
> *regcache, int r)
>    int regno;
>    windows_thread_info *th = windows_process.find_thread (current_thread-
> >id);
>    win32_require_context (th);
> -  if (r == -1 || r > NUM_REGS)
> -    child_fetch_inferior_registers (regcache, NUM_REGS);

IIUC this was the only use of the NUM_REGS define.  We can remove it.
Same for " i386_win32_num_regs (void)" and "aarch64_win32_num_regs ()".
This allows removing the num_regs hook in win32_target_ops.

> +  if (r == -1)
> +    child_fetch_inferior_registers (regcache,
> +				    regcache->tdesc->reg_defs.size ());
>    else
>      for (regno = 0; regno < r; regno++)
>        (*the_low_target.fetch_inferior_register) (regcache, th, regno);
> @@ -441,8 +443,9 @@ child_store_inferior_registers (struct regcache
> *regcache, int r)
>    int regno;
>    windows_thread_info *th = windows_process.find_thread (current_thread-
> >id);
>    win32_require_context (th);
> -  if (r == -1 || r == 0 || r > NUM_REGS)
> -    child_store_inferior_registers (regcache, NUM_REGS);
> +  if (r == -1)
> +    child_store_inferior_registers (regcache,
> +				    regcache->tdesc->reg_defs.size ());
>    else
>      for (regno = 0; regno < r; regno++)
>        (*the_low_target.store_inferior_register) (regcache, th, regno);
> @@ -1349,7 +1352,9 @@ void
>  initialize_low (void)
>  {
>    set_target_ops (&the_win32_target);
> -  the_low_target.arch_setup ();
> 
>    initialize_loadable ();
> +  /* Has to be done after initialize_loadable, because it uses the xstate
> +     functions if available.  */
> +  the_low_target.arch_setup ();
>  }
> --
> 2.54.0

________________________________________
Intel Deutschland GmbH 

Registered Address: Dornacher Strasse 1, 85622 Feldkirchen, Germany 

Tel: +49 (89) 99143-0 

www.intel.de 

Managing Directors: Candice Moore, Jeffrey Schneiderman, Ramachandran Sitaraman

Chairperson of the Supervisory Board: Sonja Pierer

Registered Seat: Munich Commercial Register B: Amtsgericht Munich HRB 186928

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
  

> -----Original Message-----
> From: Hannes Domani <ssbssa@yahoo.de>
> Sent: Dienstag, 1. September 2026 20:01
> To: Tom Tromey <tom@tromey.com>
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH v3 7/8] Windows gdb: Implement AVX register support
> 
>  Am Dienstag, 1. September 2026 um 19:48:28 MESZ hat Tom Tromey
> <tom@tromey.com> Folgendes geschrieben:
> 
> > >>>>> "Hannes" == Hannes Domani <ssbssa@yahoo.de> writes:
> >
> > Hannes> This adds support for the Intel AVX registers on Windows.
> > Hannes> It enables accessing registers $ymm0 - $ymm15 where they are
> available.
> >
> > Hannes> After this patch gdb.arch/i386-avx.exp passes on windows.
> >
> > I'd like it if Christina would review this.
> >
> > Hannes> @@ -1339,7 +1339,7 @@ initialize_loadable ()
> > Hannes>      {
> > Hannes>        /* Available XState features masked with implemented
> > Hannes>features.  */
> > Hannes>        xstate_features = (GetEnabledXStateFeatures ()
> > Hannes> -            & X86_XSTATE_SSE_MASK);
> > Hannes> +            & X86_XSTATE_AVX_MASK);
> > Hannes>        /* The extended XState functions are only needed if the
> > Hannes>available
> > Hannes>      features exceed SSE.  */
> > Hannes>        if ((xstate_features & ~X86_XSTATE_SSE_MASK) == 0)
> >
> > Ok, ignore my earlier message for patch 6.  I see what's going on now.
> >
> > I don't have any problem with this patch.  I'm not certain I totally
> > understand it, but that's ok.
> >
> > One thing I did see while doing the background reading is that
> > supposedly the xstate feature flags are vendor-specific.  So does some
> > check of the vendor ID need to be done?  I do not know.
> 
> As I understand it (and I might be wrong about that), there are some vendor-
> specific XState-bits, but stuff regarding AVX and AVX-512 are the same
> everywhere.

The MPX state, for instance, is not implemented by AMD (and has been removed
from GDB in the meantime, since the feature is deprecated).
For AVX and AVX-512 I can confirm that they are both implemented for AMD and Intel.
For CET I'd need to double check, but we decided to not add it anyways for now.
So using the masks as you do in the 2 patches should be fine.

Due to AMD not implementing MPX, there are some differences in the xsave
offsets for AMD and Intel CPUs, see the dedicated handling here:
- gdb/i387-tdep.c:i387_guess_xsave_layout: This is relevant for corefiles only.
- gdb/i386-tdep.c: i387_fallback_xsave_layout

Both functions are called at some point from i386_gdbarch_init.

I believe for live targets, since you don't hard-code the offsets in your patch series
but get them using LocateXStateFeature in the end, it should be safe.

However, we still call i386_gdbarch_init for windows, too.
So tdep->xsave_layout should still be available but is not consumed or configured.
This it at least what I observed when doing some basic debugging in windows, which I just started today.
At my current level of understanding, I don't think it makes sense to use it for Windows.

Reading 
"The LocateXStateFeature function must be used to find an individual XState feature within an extensible CONTEXT structure. Features are not necessarily contiguous in memory and applications should not assume the offset between two consecutive features will remain constant in the future."
https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-locatexstatefeature confirms that I think.

Corefiles:
I started some basis testing in windows and checked out my own patch series
https://sourceware.org/pipermail/gdb-patches/2026-September/230051.html
which extends avx and avx512 testing for corefiles together with your patches.

I see UNSUPPORTED for those new corefile tests and other existing corefile tests in gdb return similar results. So I believe this is ok for windows.

If you have any concerns with those test extensions (or my reasoning above), I'd be glad if you'd let me now.

I will provide some feedback on the entire patch in a separate email.

Christina



________________________________________
Intel Deutschland GmbH 

Registered Address: Dornacher Strasse 1, 85622 Feldkirchen, Germany 

Tel: +49 (89) 99143-0 

www.intel.de 

Managing Directors: Candice Moore, Jeffrey Schneiderman, Ramachandran Sitaraman

Chairperson of the Supervisory Board: Sonja Pierer

Registered Seat: Munich Commercial Register B: Amtsgericht Munich HRB 186928

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
  

Hi Hannes, 

I saw that Stephan already reviewed this (thanks!).
I added my remarks on top, see below.

> -----Original Message-----
> From: Rohr, Stephan <stephan.rohr@intel.com>
> Sent: Dienstag, 8. September 2026 15:05
> To: Hannes Domani <ssbssa@yahoo.de>; gdb-patches@sourceware.org; gdb-
> patches@sourceware.org
> Cc: Joos, Christina <christina.joos@intel.com>; Tom Tromey
> <tom@tromey.com>
> Subject: RE: [PATCH v3 7/8] Windows gdb: Implement AVX register support
> 
> Hi Hannes,
> 
> please see some feedback inlined below.
> 
> Let me know if you have any questions.
> 
> Thanks
> Stephan
> 
> > -----Original Message-----
> > From: Hannes Domani <ssbssa@yahoo.de>
> > Sent: Saturday, 29 August 2026 16:49
> > To: gdb-patches@sourceware.org
> > Subject: [PATCH v3 7/8] Windows gdb: Implement AVX register support
> >
> > This adds support for the Intel AVX registers on Windows.
> > It enables accessing registers $ymm0 - $ymm15 where they are available.
> >
> > After this patch gdb.arch/i386-avx.exp passes on windows.
> > ---
> > v3:
> >   - merged gdb+gdbserver parts, and split again AVX/AVX-512 parts
> > ---
> >  gdb/NEWS                    |  3 ++
> >  gdb/nat/windows-nat.c       |  2 +-
> >  gdb/x86-windows-nat.c       | 67
> > +++++++++++++++++++++++++++++++++++--
> >  gdbserver/win32-i386-low.cc | 61 +++++++++++++++++++++++++++++----
> >  gdbserver/win32-low.cc      | 15 ++++++---
> >  5 files changed, 133 insertions(+), 15 deletions(-)
> >
> > diff --git a/gdb/NEWS b/gdb/NEWS
> > index 10c182067f9..f7effc822e9 100644
> > --- a/gdb/NEWS
> > +++ b/gdb/NEWS
> > @@ -118,6 +118,9 @@
> >    intent to remove it in a future release.
> >    The s390 64-bit target (s390x-*) remains supported.
> >
> > +* Support for Intel AVX registers on Windows.
> > +  Support displaying and modifying Intel AVX registers $ymm0 - $ymm31.
> > +
> 
> I think this should be registers $ymm0 - $ymm15 ?

Yes, I agree with Stephan's feedback here.
The AVX state only comprises only YMM0–YMM15 for 64 bit. 
For 32-bit mode, it's YMM0- YMM7 only. 
See the docs added in commit "Add org.gnu.gdb.i386.avx."

> >  * Configure changes
> >
> >  ** --with-babeltrace has been removed.  The babeltrace library was
> > diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c index
> > 8930536f3ba..c9a21d7c41f 100644
> > --- a/gdb/nat/windows-nat.c
> > +++ b/gdb/nat/windows-nat.c
> > @@ -1339,7 +1339,7 @@ initialize_loadable ()
> >      {
> >        /* Available XState features masked with implemented features.  */
> >        xstate_features = (GetEnabledXStateFeatures ()
> > -			 & X86_XSTATE_SSE_MASK);
> > +			 & X86_XSTATE_AVX_MASK);
> >        /* The extended XState functions are only needed if the available
> >  	 features exceed SSE.  */
> >        if ((xstate_features & ~X86_XSTATE_SSE_MASK) == 0) diff --git
> > a/gdb/x86-windows-nat.c b/gdb/x86-windows-nat.c index
> > 3af5ef4dae0..1cefe6171be 100644
> > --- a/gdb/x86-windows-nat.c
> > +++ b/gdb/x86-windows-nat.c
> > @@ -27,6 +27,9 @@
> >
> >  #include "i386-tdep.h"
> >  #include "i387-tdep.h"
> > +#ifdef __x86_64__
> > +#include "amd64-tdep.h"
> > +#endif
> >
> >  using namespace windows_nat;
> >
> > @@ -70,6 +73,8 @@ struct x86_windows_nat_target final : public
> > x86_nat_target<windows_nat_target>
> >  			   windows_thread_info *th, int r) override;
> >
> >    bool is_sw_breakpoint (const EXCEPTION_RECORD *er) const override;
> > +
> > +  const struct target_desc *read_description () override;
> >  };
> >
> >  /* The current process.  */
> > @@ -109,7 +114,31 @@ x86_windows_per_inferior::fill_thread_context
> > (windows_thread_info *th)
> >        if (context->ContextFlags == 0)
> >  	{
> >  	  context->ContextFlags = WindowsContext<decltype(context)>::all;
> > +	  if (xstate_features != 0)
> > +	    {
> > +	      context->ContextFlags |= CONTEXT_XSTATE_FLAG;
> > +	      set_xstate_features_mask (context, xstate_features);
> > +	    }
> 
> We have the same code in "i386_get_thread_context" in "win32-i386-low.cc".
> Make a shared function in gdb/nat/windows-nat.h?
> 
> >  	  CHECK (get_thread_context (th->h, context));
> > +
> > +	  if (xstate_features != 0)
> > +	    {
> > +	      DWORD64 features = 0;
> > +	      CHECK (get_xstate_features_mask (context, &features));
> 
> Should this be changed to sth. like
> 
>   if (!get_xstate_features_mask (context, &features))
>     {
>       warning (..)
>       return;
>     }
> 
> The call of "CHECK" only prints a message but doesn't error out.  If this call fails
> we may still have features == 0.  This implies "zeroed_features ==
> xstate_features".  With this, the loop clears all features.
> IIUC, this would clear the AVX registers on the next call of "SetThreadContext".
> 
> Also refer to the implementation in gdbserver/win32-i386-low.cc:
> 
>       DWORD64 features = 0;
>       if (xstate_features != 0
> 	  && get_xstate_features_mask (context, &features))
> 	{
> 
> I think it makes sense to unify those as the rest of the code is basically identical.
> Put shared function into gdb/nat/windows-nat.h?  This keeps the code
> consistent.
> 
> > +	      DWORD64 zeroed_features = xstate_features & ~features;
> > +
> > +	      for (int f = X86_XSTATE_AVX_ID; f <= X86_XSTATE_CET_U_ID; f++)

In addition to Stephan's feedback for this code area:
Since we decided to not add CET, and this patch is even for AVX only for now only, do we need a loop at this point already?

For the follow up AVX-512 patch I think we can stop at the highest supported feature (AVX-512) in windows, can't we?

> > +		  DWORD64 flag = 1ULL << f;
> > +		  if ((zeroed_features & flag) != 0)
> > +		    {
> > +		      DWORD size = 0;
> > +		      void *loc = locate_xstate_feature (context, f, &size);
> > +		      if (loc != nullptr && size > 0)
> > +			memset (loc, 0, size);
> > +		    }
> > +		}
> > +	    }
> >  	}
> >      });
> >  }
> > @@ -198,6 +227,14 @@
> > x86_windows_nat_target::thread_context_continue (windows_thread_info
> > *th,
> >  	  if (GetExitCodeThread (th->h, &ec)
> >  	      && ec == STILL_ACTIVE)
> >  	    {
> > +	      DWORD debug_registers =
> > WindowsContext<decltype(context)>::debug;
> > +	      if (xstate_features != 0
> > +		  && (context->ContextFlags & ~debug_registers) != 0)
> > +		{
> > +		  context->ContextFlags |= CONTEXT_XSTATE_FLAG;
> > +		  set_xstate_features_mask (context, xstate_features);
> > +		}
> > +
> >  	      BOOL status = set_thread_context (th->h, context);
> >
> >  	      if (!killed)
> > @@ -227,7 +264,7 @@ x86_windows_nat_target::thread_context_step
> > (windows_thread_info *th,
> >
> >  template<typename Context>
> >  static char *
> > -get_context_reg_ptr (Context *context, int r)
> > +get_context_reg_ptr (Context *context, int r, i386_gdbarch_tdep
> > +*tdep)
> >  {
> >    const int *mappings;
> >    int mappings_count;
> > @@ -247,6 +284,13 @@ get_context_reg_ptr (Context *context, int r)
> >    char *context_offset;
> >    if (r < mappings_count)
> >      context_offset = (char *) context + mappings[r];
> > +  else if (I387_YMM0H_REGNUM (tdep) > 0 && r >=
> I387_YMM0H_REGNUM
> > (tdep)
> > +	   && r < I387_YMMENDH_REGNUM (tdep))
> 
> The implementation on gdbserver side guards against
> 
>   xstate_features & X86_XSTATE_AVX) != 0
> 
> I wonder if the same guard would be helpful here, too.  I understand the
> register number is initialized to -1, so this should not fire.  I'm not sure if it is
> possible to have $ymm0 register number > 0 w/o xstate support, e.g., if the
> target description is read from file, see "target_find_description"?
> 
> > +    {
> > +      context_offset = (char *) locate_xstate_feature
> > +	(context, X86_XSTATE_AVX_ID, NULL);

We prefer to use nullptr.

> > +      context_offset += 16 * (r - I387_YMM0H_REGNUM (tdep));
> > +    }
> >    else
> >      gdb_assert_not_reached ("invalid register number %d", r);
> >
> > @@ -267,7 +311,7 @@ x86_windows_nat_target::fetch_one_register (struct
> > regcache *regcache,
> >    char *context_offset
> >      = x86_windows_process.with_context (th, [&] (auto *context)
> >      {
> > -      return get_context_reg_ptr (context, r);
> > +      return get_context_reg_ptr (context, r, tdep);
> >      });
> >
> >    gdb_assert (!gdbarch_read_pc_p (gdbarch)); @@ -333,7 +377,7 @@
> > x86_windows_nat_target::store_one_register (const struct regcache
> > *regcache,
> >      = x86_windows_process.with_context (th, [&] (auto *context)
> >      {
> >        gdb_assert (context->ContextFlags != 0);
> > -      return get_context_reg_ptr (context, r);
> > +      return get_context_reg_ptr (context, r, tdep);
> >      });
> >
> >    /* GDB treats some registers as 32-bit, where they are in fact only
> > @@ -368,6 +412,23 @@ x86_windows_nat_target::is_sw_breakpoint (const
> > EXCEPTION_RECORD *er) const
> >  	  || er->ExceptionCode == STATUS_WX86_BREAKPOINT);  }
> >
> > +const struct target_desc *
> > +x86_windows_nat_target::read_description () {
> > +  if (inferior_ptid == null_ptid)
> > +    return this->beneath ()->read_description ();
> > +
> > +  if (xstate_features == 0)
> > +    return nullptr;
> > +
> > +#ifdef __x86_64__
> > +  if (!x86_windows_process.wow64_process)
> > +    return amd64_target_description (xstate_features, false);
> > +  else
> > +#endif
> > +    return i386_target_description (xstate_features, false); }
> > +
> >  /* Hardware watchpoint support, adapted from go32-nat.c code.  */
> >
> >  /* Pass the address ADDR to the inferior in the I'th debug register.
> > diff --git a/gdbserver/win32-i386-low.cc b/gdbserver/win32-i386-low.cc
> > index b77f6adc6ed..a7e83c0239c 100644
> > --- a/gdbserver/win32-i386-low.cc
> > +++ b/gdbserver/win32-i386-low.cc
> > @@ -253,6 +253,11 @@ i386_get_thread_context (windows_thread_info
> > *th)
> >  			       | WindowsContext<decltype(context)>::floating
> >  			       | WindowsContext<decltype(context)>::debug
> >  			       | extended_registers);
> > +      if (xstate_features != 0)
> > +	{
> > +	  context->ContextFlags |= CONTEXT_XSTATE_FLAG;
> > +	  set_xstate_features_mask (context, xstate_features);
> > +	}
> >
> >        BOOL ret = get_thread_context (th->h, context);
> >        if (!ret)
> > @@ -267,6 +272,24 @@ i386_get_thread_context (windows_thread_info
> > *th)
> >
> >  	  error (_("GetThreadContext failure %ld\n"), (long) e);
> >  	}
> > +
> > +      DWORD64 features = 0;
> > +      if (xstate_features != 0
> > +	  && get_xstate_features_mask (context, &features))
> > +	{
> > +	  DWORD64 zeroed_features = xstate_features & ~features;
> > +	  for (int f = X86_XSTATE_AVX_ID; f <= X86_XSTATE_CET_U_ID; f++)
> > +	    {
> > +	      DWORD64 flag = 1ULL << f;
> > +	      if ((zeroed_features & flag) != 0)
> > +		{
> > +		  DWORD size = 0;
> > +		  void *loc = locate_xstate_feature (context, f, &size);
> > +		  if (loc != nullptr && size > 0)
> > +		    memset (loc, 0, size);
> > +		}
> > +	    }
> > +	}
> >      });
> >  }
> >
> > @@ -292,6 +315,17 @@ i386_prepare_to_resume (windows_thread_info
> > *th)
> >
> >        th->debug_registers_changed = false;
> >      }
> > +
> > +  windows_process.with_context (th, [&] (auto *context)
> > +    {
> > +      DWORD debug_registers = WindowsContext<decltype(context)>::debug;
> > +      if (xstate_features != 0
> > +	  && (context->ContextFlags & ~debug_registers) != 0)
> > +	{
> > +	  context->ContextFlags |= CONTEXT_XSTATE_FLAG;
> > +	  set_xstate_features_mask (context, xstate_features);
> > +	}
> > +    });
> >  }
> >
> >  static void
> > @@ -477,7 +511,7 @@ is_segment_register (int r)
> >
> >  template<typename Context>
> >  static char *
> > -get_context_reg_ptr (Context *context, int r)
> > +get_context_reg_ptr (Context *context, int r, const target_desc
> > +*tdesc)
> >  {
> >    const int *mappings;
> >    int mappings_count;
> > @@ -494,9 +528,21 @@ get_context_reg_ptr (Context *context, int r)
> >        mappings_count = sizeof (i386_mappings) / sizeof (i386_mappings[0]);
> >      }
> >
> > +  bool amd64 = register_size (tdesc, 0) == 8;
> 
> There is already an " if (!windows_process.wow64_process)" a few lines above.
> Wouldn't it make sense to move the "bool amd64" in the ifdef blocks and assign
> accordingly?
> 
> > +  int ymm0h_regnum;
> > +  const int num_xmm_registers = amd64 ? 16 : 8;
> > +
> >    char *context_offset;
> >    if (r < mappings_count)
> >      context_offset = (char *) context + mappings[r];
> > +  else if ((xstate_features & X86_XSTATE_AVX) != 0
> > +	   && r >= (ymm0h_regnum = find_regno (tdesc, "ymm0h"))
> > +	   && r < ymm0h_regnum + num_xmm_registers)
> > +    {
> > +      context_offset = (char *) locate_xstate_feature
> > +	(context, X86_XSTATE_AVX_ID, NULL);
> > +      context_offset += 16 * (r - ymm0h_regnum);
> > +    }
> >    else
> >      gdb_assert_not_reached ("invalid register number %d", r);
> >
> > @@ -510,7 +556,7 @@ i386_fetch_inferior_register (struct regcache
> > *regcache,  {
> >    char *context_offset = windows_process.with_context (th, [&] (auto
> > *context)
> >      {
> > -      return get_context_reg_ptr (context, r);
> > +      return get_context_reg_ptr (context, r, regcache->tdesc);
> >      });
> >
> >    /* GDB treats some registers as 32-bit, where they are in fact only
> > @@ -538,7 +584,7 @@ i386_store_inferior_register (struct regcache
> > *regcache,  {
> >    char *context_offset = windows_process.with_context (th, [&] (auto
> > *context)
> >      {
> > -      return get_context_reg_ptr (context, r);
> > +      return get_context_reg_ptr (context, r, regcache->tdesc);
> >      });
> >
> >    /* GDB treats some registers as 32-bit, where they are in fact only
> > @@ -571,14 +617,17 @@ i386_arch_setup (void)  {
> >   target_desc_up tdesc;
> >
> > +  DWORD64 xcr0 = xstate_features;
> > +  if (xcr0 == 0)
> > +    xcr0 = X86_XSTATE_SSE_MASK;
> > +
> >  #ifdef __x86_64__
> > -  tdesc = amd64_create_target_description (X86_XSTATE_SSE_MASK, false,
> > -					   false, false);
> > +  tdesc = amd64_create_target_description (xcr0, false, false,
> > + false);
> >    init_target_desc (tdesc.get (), amd64_expedite_regs, WINDOWS_OSABI);
> >    win32_tdesc = std::move (tdesc);
> >  #endif
> >
> > -  tdesc = i386_create_target_description (X86_XSTATE_SSE_MASK, false,
> > false);
> > +  tdesc = i386_create_target_description (xcr0, false, false);
> >    init_target_desc (tdesc.get (), i386_expedite_regs, WINDOWS_OSABI);
> > #ifdef __x86_64__
> >    wow64_win32_tdesc = std::move (tdesc); diff --git
> > a/gdbserver/win32-low.cc b/gdbserver/win32-low.cc index
> > 7629beca213..5ccdc89a7ef 100644
> > --- a/gdbserver/win32-low.cc
> > +++ b/gdbserver/win32-low.cc
> > @@ -33,6 +33,7 @@
> >  #include <process.h>
> >  #include "gdbsupport/gdb_tilde_expand.h"
> >  #include "gdbsupport/common-inferior.h"
> > +#include "tdesc.h"
> >
> >  using namespace windows_nat;
> >
> > @@ -426,8 +427,9 @@ child_fetch_inferior_registers (struct regcache
> > *regcache, int r)
> >    int regno;
> >    windows_thread_info *th = windows_process.find_thread
> > (current_thread-
> > >id);
> >    win32_require_context (th);
> > -  if (r == -1 || r > NUM_REGS)
> > -    child_fetch_inferior_registers (regcache, NUM_REGS);
> 
> IIUC this was the only use of the NUM_REGS define.  We can remove it.

Yes, I agree. Would you mind explaining why we don't need this check anymore, too ? :)
I don't understand it yet, unfortunately. Was it necessary before or is this due to AVX?

> Same for " i386_win32_num_regs (void)" and "aarch64_win32_num_regs ()".
> This allows removing the num_regs hook in win32_target_ops.

> > +  if (r == -1)
> > +    child_fetch_inferior_registers (regcache,
> > +				    regcache->tdesc->reg_defs.size ());
> >    else
> >      for (regno = 0; regno < r; regno++)
> >        (*the_low_target.fetch_inferior_register) (regcache, th,
> > regno); @@ -441,8 +443,9 @@ child_store_inferior_registers (struct
> > regcache *regcache, int r)
> >    int regno;
> >    windows_thread_info *th = windows_process.find_thread
> > (current_thread-
> > >id);
> >    win32_require_context (th);
> > -  if (r == -1 || r == 0 || r > NUM_REGS)
> > -    child_store_inferior_registers (regcache, NUM_REGS);
> > +  if (r == -1)
> > +    child_store_inferior_registers (regcache,
> > +				    regcache->tdesc->reg_defs.size ());

I assume removing r == 0 is not related to AVX register support and just a cleanup, right?
Could we make it a separate cleanup patch (with reason)?

I know it seems like a super tiny nit, but it would help to understand why that kind of
refactoring/cleanup is necessary (e.g. due to AVX or not).


> >    else
> >      for (regno = 0; regno < r; regno++)
> >        (*the_low_target.store_inferior_register) (regcache, th,
> > regno); @@ -1349,7 +1352,9 @@ void  initialize_low (void)  {
> >    set_target_ops (&the_win32_target);
> > -  the_low_target.arch_setup ();
> >
> >    initialize_loadable ();
> > +  /* Has to be done after initialize_loadable, because it uses the xstate
> > +     functions if available.  */
> > +  the_low_target.arch_setup ();
> >  }
> > --
> > 2.54.0

Christina

________________________________________
Intel Deutschland GmbH 

Registered Address: Dornacher Strasse 1, 85622 Feldkirchen, Germany 

Tel: +49 (89) 99143-0 

www.intel.de 

Managing Directors: Candice Moore, Jeffrey Schneiderman, Ramachandran Sitaraman

Chairperson of the Supervisory Board: Sonja Pierer

Registered Seat: Munich Commercial Register B: Amtsgericht Munich HRB 186928

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
  

> -----Original Message-----
> From: Joos, Christina
> Sent: Dienstag, 8. September 2026 17:28
> To: 'Hannes Domani' <ssbssa@yahoo.de>; Tom Tromey <tom@tromey.com>
> Cc: gdb-patches@sourceware.org
> Subject: RE: [PATCH v3 7/8] Windows gdb: Implement AVX register support
> 
> > -----Original Message-----
> > From: Hannes Domani <ssbssa@yahoo.de>
> > Sent: Dienstag, 1. September 2026 20:01
> > To: Tom Tromey <tom@tromey.com>
> > Cc: gdb-patches@sourceware.org
> > Subject: Re: [PATCH v3 7/8] Windows gdb: Implement AVX register
> > support
> >
> >  Am Dienstag, 1. September 2026 um 19:48:28 MESZ hat Tom Tromey
> > <tom@tromey.com> Folgendes geschrieben:
> >
> > > >>>>> "Hannes" == Hannes Domani <ssbssa@yahoo.de> writes:
> > >
> > > Hannes> This adds support for the Intel AVX registers on Windows.
> > > Hannes> It enables accessing registers $ymm0 - $ymm15 where they are
> > available.
> > >
> > > Hannes> After this patch gdb.arch/i386-avx.exp passes on windows.
> > >
> > > I'd like it if Christina would review this.
> > >
> > > Hannes> @@ -1339,7 +1339,7 @@ initialize_loadable ()
> > > Hannes>      {
> > > Hannes>        /* Available XState features masked with implemented
> > > Hannes>features.  */
> > > Hannes>        xstate_features = (GetEnabledXStateFeatures ()
> > > Hannes> -            & X86_XSTATE_SSE_MASK);
> > > Hannes> +            & X86_XSTATE_AVX_MASK);
> > > Hannes>        /* The extended XState functions are only needed if
> > > Hannes>the available
> > > Hannes>      features exceed SSE.  */
> > > Hannes>        if ((xstate_features & ~X86_XSTATE_SSE_MASK) == 0)
> > >
> > > Ok, ignore my earlier message for patch 6.  I see what's going on now.
> > >
> > > I don't have any problem with this patch.  I'm not certain I totally
> > > understand it, but that's ok.
> > >
> > > One thing I did see while doing the background reading is that
> > > supposedly the xstate feature flags are vendor-specific.  So does
> > > some check of the vendor ID need to be done?  I do not know.
> >
> > As I understand it (and I might be wrong about that), there are some
> > vendor- specific XState-bits, but stuff regarding AVX and AVX-512 are
> > the same everywhere.
> 
> The MPX state, for instance, is not implemented by AMD (and has been
> removed from GDB in the meantime, since the feature is deprecated).
> For AVX and AVX-512 I can confirm that they are both implemented for AMD
> and Intel.
> For CET I'd need to double check, but we decided to not add it anyways for
> now.
> So using the masks as you do in the 2 patches should be fine.
> 
> Due to AMD not implementing MPX, there are some differences in the xsave
> offsets for AMD and Intel CPUs, see the dedicated handling here:
> - gdb/i387-tdep.c:i387_guess_xsave_layout: This is relevant for corefiles only.
> - gdb/i386-tdep.c: i387_fallback_xsave_layout
> 
> Both functions are called at some point from i386_gdbarch_init.
> 
> I believe for live targets, since you don't hard-code the offsets in your patch
> series but get them using LocateXStateFeature in the end, it should be safe.
> 
> However, we still call i386_gdbarch_init for windows, too.
> So tdep->xsave_layout should still be available but is not consumed or
> configured.
> This it at least what I observed when doing some basic debugging in windows,
> which I just started today.
> At my current level of understanding, I don't think it makes sense to use it for
> Windows.
> 
> Reading
> "The LocateXStateFeature function must be used to find an individual XState
> feature within an extensible CONTEXT structure. Features are not necessarily
> contiguous in memory and applications should not assume the offset between
> two consecutive features will remain constant in the future."
> https://learn.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-
> locatexstatefeature confirms that I think.
> 

Hi Hannes, 

I believe that this thread might be the right starting point to discuss the per-register locate_xstate_feature calls in get_context_reg_ptr, since it describes the background and documentation of this topic a bit.

You said
"I was also thinking that these look-ups are not ideal.
As I understand it, the offsets for each feature are not fixed values on windows, but they should be the same for all threads on a system.
In that case, maybe we could calculate these offsets for all registers once, put them in a vector, and use that in get_context_reg_ptr?
I'm also assuming that the register numbers always stay the same.
I'm just not sure at which point the offset calculation should be done.
Any suggestions?"

I believe you are right for the fixed offset for all threads on a system, but this is not clear from the official documentation in windows, or did I miss anything here ? Due to that I am not sure if we can assume that.

On linux, we don't have those per-register function calls to fetch the registers, so we also don't have the per-register find_regno calls on gdbserver side. If we would have one function call for the xsave registers, it would shrink the number of locate_xstate_feature calls, too. But this is probably a bigger refactoring.

So, from my perspective, the safest approach would be either to do the refactoring as described above, or to accept those repeated locate_xstate_feature calls.

What do you think?

Christina
________________________________________
Intel Deutschland GmbH 

Registered Address: Dornacher Strasse 1, 85622 Feldkirchen, Germany 

Tel: +49 (89) 99143-0 

www.intel.de 

Managing Directors: Candice Moore, Jeffrey Schneiderman, Ramachandran Sitaraman

Chairperson of the Supervisory Board: Sonja Pierer

Registered Seat: Munich Commercial Register B: Amtsgericht Munich HRB 186928

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.