diff mbox series

[v3,2/8] Windows gdb: Use allocated buffer for CONTEXT

Message ID 20260829145823.1034821-2-ssbssa@yahoo.de
State New
Headers
Series [v3,1/8] gdb/testsuite: Add Windows replacement for aligned_alloc |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Test passed

Commit Message

Hannes Domani Aug. 29, 2026, 2:48 p.m. UTC 
  This is done in preparation for the XState functions, because the
extended registers are stored directly after the CONTEXT, and its actual
size depends on the available XState features.
---
v3:
  - fixed missing newline
  - merged initialize_context into the windows_process_info constructor
---
 gdb/aarch64-windows-nat.c      | 16 ++++++++--------
 gdb/nat/windows-nat.c          | 22 ++++++++++++++++++++++
 gdb/nat/windows-nat.h          | 20 +++++++++-----------
 gdbserver/win32-aarch64-low.cc | 10 +++++-----
 4 files changed, 44 insertions(+), 24 deletions(-)

Comments

Tom Tromey Sept. 1, 2026, 5:22 p.m. UTC | #1 
>>>>> "Hannes" == Hannes Domani <ssbssa@yahoo.de> writes:

Hannes> This is done in preparation for the XState functions, because the
Hannes> extended registers are stored directly after the CONTEXT, and its actual
Hannes> size depends on the available XState features.

Looks good, thanks.  I have one note but it's not really super
important.

Approved-By: Tom Tromey <tom@tromey.com>

Hannes> +windows_thread_info::windows_thread_info (windows_process_info *proc_,
Hannes> +					  DWORD tid_, HANDLE h_, CORE_ADDR tlb)
Hannes> +  : proc (proc_),
Hannes> +    tid (tid_),
Hannes> +    h (h_),
Hannes> +    thread_local_base (tlb)
Hannes> +{
Hannes> +#ifdef __x86_64__
Hannes> +  if (proc->wow64_process)
Hannes> +    {
Hannes> +      context_buffer.reset (xmalloc (sizeof (WOW64_CONTEXT)));
Hannes> +      wow64_context = (WOW64_CONTEXT *) context_buffer.get ();
Hannes> +    }
Hannes> +  else
Hannes> +#endif
Hannes> +    {
Hannes> +      context_buffer.reset (xmalloc (sizeof (CONTEXT)));
Hannes> +      context = (CONTEXT *) context_buffer.get ();
Hannes> +    }
Hannes> +  *proc->context_flags_ptr (this) = 0;

I guess this is because the memory isn't initialized?
You could use XCNEW instead of xmalloc to fix this.

Tom
Hannes Domani Sept. 1, 2026, 5:30 p.m. UTC | #2 
Am Dienstag, 1. September 2026 um 19:22:59 MESZ hat Tom Tromey <tom@tromey.com> Folgendes geschrieben:

> >>>>> "Hannes" == Hannes Domani <ssbssa@yahoo.de> writes:
> 
> Hannes> This is done in preparation for the XState functions, because the
> Hannes> extended registers are stored directly after the CONTEXT, and its actual
> Hannes> size depends on the available XState features.
> 
> Looks good, thanks.  I have one note but it's not really super
> important.
> 
> Approved-By: Tom Tromey <tom@tromey.com>
> 
> Hannes> +windows_thread_info::windows_thread_info (windows_process_info *proc_,
> Hannes> +                      DWORD tid_, HANDLE h_, CORE_ADDR tlb)
> Hannes> +  : proc (proc_),
> Hannes> +    tid (tid_),
> Hannes> +    h (h_),
> Hannes> +    thread_local_base (tlb)
> Hannes> +{
> Hannes> +#ifdef __x86_64__
> Hannes> +  if (proc->wow64_process)
> Hannes> +    {
> Hannes> +      context_buffer.reset (xmalloc (sizeof (WOW64_CONTEXT)));
> Hannes> +      wow64_context = (WOW64_CONTEXT *) context_buffer.get ();
> Hannes> +    }
> Hannes> +  else
> Hannes> +#endif
> Hannes> +    {
> Hannes> +      context_buffer.reset (xmalloc (sizeof (CONTEXT)));
> Hannes> +      context = (CONTEXT *) context_buffer.get ();
> Hannes> +    }
> Hannes> +  *proc->context_flags_ptr (this) = 0;
> 
> I guess this is because the memory isn't initialized?
> You could use XCNEW instead of xmalloc to fix this.

It's not just because of uninitialized memory from malloc.
Patch 6 then introduces InitializeContext, which actually initializes
context_flags to CONTEXT_ALL | CONTEXT_XSTATE, so it has to be reset
to 0 anyways.


Thanks
Hannes
diff mbox series

Patch

diff --git a/gdb/aarch64-windows-nat.c b/gdb/aarch64-windows-nat.c
index ff2c9762467..630704a8caf 100644
--- a/gdb/aarch64-windows-nat.c
+++ b/gdb/aarch64-windows-nat.c
@@ -185,7 +185,7 @@  aarch64_windows_nat_target::cleanup_windows_arch ()
 void
 aarch64_windows_per_inferior::fill_thread_context (windows_thread_info *th)
 {
-  CONTEXT *context = &th->context;
+  CONTEXT *context = th->context;
 
   if (context->ContextFlags == 0)
     {
@@ -199,7 +199,7 @@  aarch64_windows_per_inferior::fill_thread_context (windows_thread_info *th)
 void
 aarch64_windows_per_inferior::invalidate_thread_context (windows_thread_info *th)
 {
-  CONTEXT *context = &th->context;
+  CONTEXT *context = th->context;
   context->ContextFlags = 0;
 }
 
@@ -209,7 +209,7 @@  void
 aarch64_windows_nat_target::thread_context_continue (windows_thread_info *th,
 						     int killed)
 {
-  CONTEXT *context = &th->context;
+  CONTEXT *context = th->context;
 
   if (th->debug_registers_changed)
     {
@@ -250,9 +250,9 @@  aarch64_windows_nat_target::thread_context_step (windows_thread_info *th,
 						 bool enable)
 {
   if (enable)
-    th->context.Cpsr |= 0x200000;
+    th->context->Cpsr |= 0x200000;
   else
-    th->context.Cpsr &= ~0x200000;
+    th->context->Cpsr &= ~0x200000;
 }
 
 /* See windows-nat.h.  */
@@ -263,7 +263,7 @@  aarch64_windows_nat_target::fetch_one_register (struct regcache *regcache,
 {
   gdb_assert (r >= 0);
 
-  char *context_ptr = (char *) &th->context;
+  char *context_ptr = (char *) th->context;
   char *context_offset = context_ptr + aarch64_windows_process.mappings[r];
   struct gdbarch *gdbarch = regcache->arch ();
 
@@ -292,9 +292,9 @@  aarch64_windows_nat_target::store_one_register (const struct regcache *regcache,
 						windows_thread_info *th, int r)
 {
   gdb_assert (r >= 0);
-  gdb_assert (th->context.ContextFlags != 0);
+  gdb_assert (th->context->ContextFlags != 0);
 
-  char *context_ptr = (char *) &th->context;
+  char *context_ptr = (char *) th->context;
 
   regcache->raw_collect (r, context_ptr + aarch64_windows_process.mappings[r]);
 }
diff --git a/gdb/nat/windows-nat.c b/gdb/nat/windows-nat.c
index e975892f487..974b1d3535b 100644
--- a/gdb/nat/windows-nat.c
+++ b/gdb/nat/windows-nat.c
@@ -79,6 +79,28 @@  DeleteProcThreadAttributeList_ftype *DeleteProcThreadAttributeList;
   debug_prefixed_printf_cond (debug_events, "windows events", fmt, \
 			      ## __VA_ARGS__)
 
+windows_thread_info::windows_thread_info (windows_process_info *proc_,
+					  DWORD tid_, HANDLE h_, CORE_ADDR tlb)
+  : proc (proc_),
+    tid (tid_),
+    h (h_),
+    thread_local_base (tlb)
+{
+#ifdef __x86_64__
+  if (proc->wow64_process)
+    {
+      context_buffer.reset (xmalloc (sizeof (WOW64_CONTEXT)));
+      wow64_context = (WOW64_CONTEXT *) context_buffer.get ();
+    }
+  else
+#endif
+    {
+      context_buffer.reset (xmalloc (sizeof (CONTEXT)));
+      context = (CONTEXT *) context_buffer.get ();
+    }
+  *proc->context_flags_ptr (this) = 0;
+}
+
 void
 windows_thread_info::suspend ()
 {
diff --git a/gdb/nat/windows-nat.h b/gdb/nat/windows-nat.h
index 44b628c9bd1..ee85cca2984 100644
--- a/gdb/nat/windows-nat.h
+++ b/gdb/nat/windows-nat.h
@@ -63,13 +63,7 @@  enum stopping_kind
 struct windows_thread_info
 {
   windows_thread_info (windows_process_info *proc_,
-		       DWORD tid_, HANDLE h_, CORE_ADDR tlb)
-    : proc (proc_),
-      tid (tid_),
-      h (h_),
-      thread_local_base (tlb)
-  {
-  }
+		       DWORD tid_, HANDLE h_, CORE_ADDR tlb);
 
   DISABLE_COPY_AND_ASSIGN (windows_thread_info);
 
@@ -183,9 +177,9 @@  struct windows_thread_info
   /* The context of the thread, including any manipulations.  */
   union
   {
-    CONTEXT context {};
+    CONTEXT *context = nullptr;
 #ifdef __x86_64__
-    WOW64_CONTEXT wow64_context;
+    WOW64_CONTEXT *wow64_context;
 #endif
   };
 
@@ -204,6 +198,10 @@  struct windows_thread_info
 
   /* The name of the thread.  */
   gdb::unique_xmalloc_ptr<char> name;
+
+  /* The buffer for the thread context, including any XState registers if
+     available.  */
+  gdb::unique_xmalloc_ptr<void> context_buffer;
 };
 
 enum handle_exception_result
@@ -318,10 +316,10 @@  struct windows_process_info
   {
 #ifdef __x86_64__
     if (wow64_process)
-      return function (th != nullptr ? &th->wow64_context : nullptr);
+      return function (th != nullptr ? th->wow64_context : nullptr);
     else
 #endif
-      return function (th != nullptr ? &th->context : nullptr);
+      return function (th != nullptr ? th->context : nullptr);
   }
 
   DWORD *context_flags_ptr (windows_thread_info *th)
diff --git a/gdbserver/win32-aarch64-low.cc b/gdbserver/win32-aarch64-low.cc
index 5a7b72155db..e94548b6fef 100644
--- a/gdbserver/win32-aarch64-low.cc
+++ b/gdbserver/win32-aarch64-low.cc
@@ -169,7 +169,7 @@  aarch64_initial_stuff (process_info *proc)
 static void
 aarch64_get_thread_context (windows_thread_info *th)
 {
-  CONTEXT *context = &th->context;
+  CONTEXT *context = th->context;
 
   context->ContextFlags = (WindowsContext<decltype(context)>::full
 			   | WindowsContext<decltype(context)>::floating
@@ -193,7 +193,7 @@  aarch64_prepare_to_resume (windows_thread_info *th)
     {
       win32_require_context (th);
 
-      CONTEXT *context = &th->context;
+      CONTEXT *context = th->context;
 
       for (int i = 0; i < aarch64_num_bp_regs; i++)
 	{
@@ -223,7 +223,7 @@  aarch64_thread_added (windows_thread_info *th)
 static void
 aarch64_single_step (windows_thread_info *th)
 {
-  th->context.Cpsr |= 0x200000;
+  th->context->Cpsr |= 0x200000;
 }
 
 /* An array of offset mappings into a Win32 Context structure.
@@ -322,7 +322,7 @@  aarch64_fetch_inferior_register (struct regcache *regcache,
   int mappings_count;
   get_mappings (mappings, mappings_count);
 
-  char *context_ptr = (char *) &th->context;
+  char *context_ptr = (char *) th->context;
   char *context_offset;
   if (r < mappings_count)
     context_offset = context_ptr + mappings[r];
@@ -341,7 +341,7 @@  aarch64_store_inferior_register (struct regcache *regcache,
   int mappings_count;
   get_mappings (mappings, mappings_count);
 
-  char *context_ptr = (char *) &th->context;
+  char *context_ptr = (char *) th->context;
   char *context_offset;
   if (r < mappings_count)
     context_offset = context_ptr + mappings[r];