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

 Am Mittwoch, 22. Juli 2026 um 14:24:36 MESZ hat Pedro Alves <pedro () palves ! net> Folgendes geschrieben:

> On 2026-07-21 17:51, Tom Tromey wrote:
> >>>>>> "Hannes" == Hannes Domani <[email protected]> writes:
> >
> > Hannes>  Ping.
> >
> > I'd prefer Pedro reply, but FWIW I think the patch looks reasonable.
> > You can have my approval but please wait a bit to see if Pedro has some
> > other comments.
> 
> I'm a little confused, since AFAICT, there has been no patch update to address the
> comments I made earlier.
> 
> We've established that the patch isn't really "fixing" the resetting of registers, as the
> missing context flag is ignored anyway.  So at the very least I was expecting that the subject and commit
> log would be updated to match reality.

I wasn't waiting for an Approved-By, but i thought you would have some
comments to my last mail.


> > Am Montag, 6. Juli 2026 um 17:02:35 MESZ hat Hannes Domani <[email protected]> Folgendes geschrieben:
> >
> >> It's just great that all your mails are blocked by yahoo...
> 
> Sorry, but I don't know what I can do about that.  My hosting provider, including email is dreamhost,
> which is quite popular and I believe used by others in the community too.  I don't have anything special
> going on with my email AFAIK.

I know, it's just very frustrating.
Even more so because you recently submitted more windows-related stuff.


> >> 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.
> 
> Can you fix?

Yes.


> >>>> 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.
> 
> OK.  Can you please add this to the commit log?  That does seem like something
> we need to fix, and should be rationale for the change, right?

Will do.


> >>>> ---
> >>>>  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)
> 
> OK, so why did you decide against it?

I didn't really have a good reason, so I'll do it like this in v2.


> >> 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.
> >> 
> 
> Seeing this, FYI, I didn't know if you planed on dropping the patch, or sending an
> updated one with a commit log that reflects the finding.  But I didn't think the
> current one as it was, was ready.

This was also a point where I thought you would give some comment, I
was wrong about that too.

So, when in doubt, I should send an updated patch?


Hannes