Re: [PATCH] Windows gdb: Fix resetting of the debug-registers bit in ContextFlags

[Hannes Domani <ssbssa@yahoo.de>]

 It's just great that all your mails are blocked by yahoo...


Am Mittwoch, 1. Juli 2026 um 21:03:09 MESZ hat Pedro Alves <pedro () palves ! net> Folgendes geschrieben:

> On 2026-06-27 15:27, Hannes Domani wrote:
> > The CONTEXT_DEBUG_REGISTERS also includes the arch-specific bit
> > (CONTEXT_i386 or CONTEXT_AMD64) which is included in all CONTEXT_*
> > defines.
> >
> > So this basically just checks if any CONTEXT_* define is set:
> >  if ((context->ContextFlags & CONTEXT_DEBUG_REGISTERS) != 0)
> >
> > And similarily, unsetting CONTEXT_DEBUG_REGISTERS removes the
> 
> similarily => similarly

Right.


> > arch-specific bit as well.
> >
> > So this creates a CONTEXT_DEBUG_REG_FLAG define with just the
> > debug-registers bit, and uses it in these problematic locations.
> 
> How did you notice this?  Like, GDB was misbehaving and you found the
> issue, was it by inspection?  I'd be good to have that info in the commit log.

I noticed because I was doing some changes in that function, and
CONTEXT_DEBUG_REGISTERS stood out to me very quickly, because for WOW64 I
would expect WindowsContext<decltype(context)>::debug to be used instead.


> > ---
> >  gdb/x86-windows-nat.c | 10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/gdb/x86-windows-nat.c b/gdb/x86-windows-nat.c
> > index 27adeb1f154..3368814ed96 100644
> > --- a/gdb/x86-windows-nat.c
> > +++ b/gdb/x86-windows-nat.c
> > @@ -42,6 +42,10 @@ enum
> > 
> >  #define DR6_CLEAR_VALUE 0xffff0ff0
> 
> > 
> > +/* The CONTEXT_DEBUG_REGISTERS define without the arch-specific bit
> > +  (CONTEXT_i386 or CONTEXT_AMD64).  */
> > +#define CONTEXT_DEBUG_REG_FLAG 0x10
> > +
> 
> Did you consider avoiding harcoding numbers, like:
> 
> #ifdef __x86_64__
> # define CONTEXT_ARCH_BIT CONTEXT_AMD64
> #else
> # define CONTEXT_ARCH_BIT CONTEXT_i386
> #endif
> 
> #define CONTEXT_DEBUG_REG_FLAG (CONTEXT_DEBUG_REGISTERS & ~CONTEXT_ARCH_BIT)

I did consider this:

#define CONTEXT_DEBUG_REG_FLAG (CONTEXT_DEBUG_REGISTERS & ~CONTEXT_CONTROL)


> >  struct x86_windows_per_inferior : public windows_per_inferior
> >  {
> >    /* The function to use in order to determine whether a register is
> > @@ -142,7 +146,7 @@ x86_windows_nat_target::thread_context_continue (windows_thread_info *th,
> >      {
> >        windows_process->fill_thread_context (th);
> > 
> > -      gdb_assert ((context->ContextFlags & CONTEXT_DEBUG_REGISTERS) != 0);
> > +      gdb_assert ((context->ContextFlags & CONTEXT_DEBUG_REG_FLAG) != 0);
> > 
> >        /* Check whether the thread has Dr6 set indicating a
> >          watchpoint hit, and we haven't seen the watchpoint event
> > @@ -173,13 +177,13 @@ x86_windows_nat_target::thread_context_continue (windows_thread_info *th,
> >              update the debug registers later when the thread
> >              is re-resumed by the core after the watchpoint
> >              event.  */
> > -          context->ContextFlags &= ~CONTEXT_DEBUG_REGISTERS;
> > +          context->ContextFlags &= ~CONTEXT_DEBUG_REG_FLAG;
> 
> My bad, I suppose...
> 
> Does clearing the arch bit make the context be basically as if it was fully zeroed?
> 
> So if we notice we had a pending watchpoint hit, we were not writing _any_ register?
> 
> That was not the original intention, for sure.
> 
> But OTOH, looking back at this, I'm wondering whether that wasn't really the right thing to do.
> If we e.g., change the PC to point elsewhere, and then process the pending watchpoint, we'd want
> to see the PC as it was when the watchpoint triggered, not what it was modified to.  Same for
> other registers, as the watchpoint's value will very likely depend on the state of registers.
> 
> Right?
> 
> OTOH, if e.g., the user did an infcall on a thread that has a pending watchpoint, and we don't
> modify registers, and then the watchpoint doesn't cause a stop, not writing registers means
> we'll not really do the infcall, and the inferior proceeds as if we had done a "continue"...
> Not great either.
> 
> OK, let's just do what you're suggesting until we come up with a better way, as it was the
> original intention.  I'm just curious for more details.

I did some experiments, and it looks like SetThreadContext doesn't care at
all about the arch bit, so it is working like your original intention.
I thought it would fail in the arch bit is missing, but I was wrong about that.


Hannes