RE: [PATCH 7/8] Windows gdb: Implement XState (Intel AVX) support
"Schimpe, Christina" <[email protected]>
| Newsgroups | gmane.comp.gdb.patches |
|---|---|
| Message-ID | <SN7PR11MB763800041C7ECC74D3345F61F9C62@SN7PR11MB7638.namprd11.prod.outlook.com> |
> -----Original Message----- > From: Hannes Domani <[email protected]> > Sent: Freitag, 17. Juli 2026 16:50 > To: [email protected]; Schimpe, Christina > <[email protected]> > Subject: Re: [PATCH 7/8] Windows gdb: Implement XState (Intel AVX) support > > Am Freitag, 17. Juli 2026 um 16:16:31 MESZ hat Schimpe, Christina > <[email protected]> Folgendes geschrieben: > > > > -----Original Message----- > > > From: Hannes Domani <[email protected]> > > > Sent: Freitag, 17. Juli 2026 15:47 > > > To: [email protected]; Schimpe, Christina > > > <[email protected]> > > > Subject: Re: [PATCH 7/8] Windows gdb: Implement XState (Intel AVX) > > > support > > > > > > Am Freitag, 17. Juli 2026 um 14:54:59 MESZ hat Schimpe, Christina > > ><[email protected]> Folgendes geschrieben: > > > > > > > Hi Hannes, > > > > > > > > Thank you for working on this. > > > > It appears that this patch uses the same commit message header as > > > > patch #8. Is it intentional that these are separate commits? > > > > > > This one is the gdb part, and #8 is the gdbserver part, that is > > > stated in the title. > > > > Oups, I wonder how I came to that conclusion. Sorry for that. > > > > > > > > > In any case, IMO, commit messages should not have identical headers. > > > > Also, this patch seems big enough for a commit message which is > > > > not header only. :) > > > > This part still stands for the commit message. Since it's not only avx > > registers, I believe it makes sense to share more details here. > > Yes, I will do that. > > > > > > Do any AVX-* specific tests pass on Windows now? If so, it would > > > > be helpful to mention that in the commit message as well. > > > > > > I actually planned to add that info, but forgot. > > > > > > These tests then pass on Windows: > > > gdb.arch/i386-avx.exp > > > gdb.arch/i386-avx512.exp > > > > What's the state for SSE (gdb.arch/i386-sse.exp) ? > > i386-sse.exp works since patch #1, it was only a compile issue of the test > itself. > I will mention this in the patch as well. Thanks. > > > I also saw you introduced some code for PKRU and the shadow stack > pointer. > > We have GDB tests for those registers, too: > > - gdb.arch/i386-pkru.exp > > - gdb.arch/amd64-shadow-stack*.exp > > For these I just added the equivalent code as is done on Linux, without really > knowing what they are for. > > i386-pkru.exp tells me: > > (gdb) print have_pkru() > $1 = 0 > (gdb) PASS: gdb.arch/i386-pkru.exp: probe PKRU support > UNSUPPORTED: gdb.arch/i386-pkru.exp: processor does not support > protection key feature. > > Do only certain CPU's have this register? Most recent CPUs should have it. To be sure you can check if the corresponding bit is configured in xcr0. I believe for windows this should be the mask returned by get_xstate_features_mask. If your cpu does have it, I think you must look at the test in more detail to find out why it's unsupported. It might need some adaptions for windows. > > And amd64-shadow-stack.exp: > > (gdb) print $pl3_ssp > $1 = (void *) 0x0 > (gdb) FAIL: gdb.arch/amd64-shadow-stack.exp: test shadow stack support > > No idea if that should work, since Windows is supplying data for the $pl3_ssp > register. > > > Hannes Without knowing any of the details in windows I believe adding full support for CET shadow stack might need some more changes. At least for linux, this was the case: https://inbox.sourceware.org/gdb-patches/[email protected]/ I believe it would be better to handle this in a separate patch-(series). This also applies for any other register that you add and whose dedicated test does not pass. Does that make sense to you? Christina Intel Deutschland GmbH Registered Address: Dornacher Strasse 1, 85622 Feldkirchen, Germany Tel: +49 89 991 430, www.intel.de Managing Directors: Harry Demas, Jeffrey Schneiderman, Yin Chong Sorrell Chairperson of the Supervisory Board: Nicole Lau Registered Seat: Munich Commercial Register: Amtsgericht Muenchen HRB 186928