[RFA] Allow Windows UNWIND_INFO version 2.

[RFA] Allow Windows UNWIND_INFO version 2.

[Joel Brobecker]

We've observed in Windows 2012 that ntdll.dll contains some unwind
records with the version field set to 2.  This patch adjusts the
decoder to accept records flagged with this version as well.

Version 2 appears to still be largely undocumented at this stage.
However, appart from a mysterious opcode 6, everything else still
seems to remain the same. So this patch also changes the decoder
to ignore those opcodes; before this change, the debugger would
silently stop the decoding, and let the frame unwinder make do
with what it the decoder managed to decode up to that point.

It's unclear at this point what we're losing by not being able to
decode that opcode. But the information does not appear to be critical,
at least as far as call unwinding is concerned.

gdb/ChangeLog:

	(from Tristan Gingold  <gingold@adacore.com>)
	(from Joel Brobecker  <brobecker@adacore.com>)
	* amd64-windows-tdep.c (amd64_windows_frame_decode_insns):
	Accept version 2.  Ignore operations using opcode 6.

Tested on all x64 versions of Windows available at AdaCore (from XP
to 2012).

OK to commit?

Thank you,
-- 
Joel

---
 gdb/amd64-windows-tdep.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/gdb/amd64-windows-tdep.c b/gdb/amd64-windows-tdep.c
index 359173a..6891e16 100644
--- a/gdb/amd64-windows-tdep.c
+++ b/gdb/amd64-windows-tdep.c
@@ -649,7 +649,8 @@ amd64_windows_frame_decode_insns (struct frame_info *this_frame,
 	   ex_ui.CountOfCodes, ex_ui.FrameRegisterOffset);
 
       /* Check version.  */
-      if (PEX64_UWI_VERSION (ex_ui.Version_Flags) != 1)
+      if (PEX64_UWI_VERSION (ex_ui.Version_Flags) != 1
+	  && PEX64_UWI_VERSION (ex_ui.Version_Flags) != 2)
 	return;
 
       if (j == 0
@@ -696,7 +697,17 @@ amd64_windows_frame_decode_insns (struct frame_info *this_frame,
 	return;
 
       end_insns = &insns[codes_count * 2];
-      for (p = insns; p < end_insns; p += 2)
+      p = insns;
+
+      /* Skip opcodes 6 of version 2.  This opcode is not documented.  */
+      if (PEX64_UWI_VERSION (ex_ui.Version_Flags) == 2)
+	{
+	  for (; p < end_insns; p += 2)
+	    if (PEX64_UNWCODE_CODE (p[1]) != 6)
+	      break;
+	}
+
+      for (; p < end_insns; p += 2)
 	{
 	  int reg;
 
-- 
1.8.1.2

Re: [RFA] Allow Windows UNWIND_INFO version 2.

[Pedro Alves]

On 12/03/2013 11:32 AM, Joel Brobecker wrote:
> @@ -696,7 +697,17 @@ amd64_windows_frame_decode_insns (struct frame_info *this_frame,
>  	return;
>  
>        end_insns = &insns[codes_count * 2];
> -      for (p = insns; p < end_insns; p += 2)
> +      p = insns;
> +
> +      /* Skip opcodes 6 of version 2.  This opcode is not documented.  */
> +      if (PEX64_UWI_VERSION (ex_ui.Version_Flags) == 2)
> +	{
> +	  for (; p < end_insns; p += 2)
> +	    if (PEX64_UNWCODE_CODE (p[1]) != 6)
> +	      break;
> +	}

I'd consider merging with the existing loop, so that
we print the opcodes when frame debug is enabled.

But anyway, this looks fine to me.

I clicked on "Did you find this helpful?  No" at:

  http://msdn.microsoft.com/en-us/library/ck9asaa9.aspx

and asked for info about v2 and opcode 6.  Not holding
my breath though.

-- 
Pedro Alves

Re: [RFA] Allow Windows UNWIND_INFO version 2.

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 2065 bytes --]

On Dec  3 18:47, Pedro Alves wrote:
> On 12/03/2013 11:32 AM, Joel Brobecker wrote:
> > @@ -696,7 +697,17 @@ amd64_windows_frame_decode_insns (struct frame_info *this_frame,
> >  	return;
> >  
> >        end_insns = &insns[codes_count * 2];
> > -      for (p = insns; p < end_insns; p += 2)
> > +      p = insns;
> > +
> > +      /* Skip opcodes 6 of version 2.  This opcode is not documented.  */
> > +      if (PEX64_UWI_VERSION (ex_ui.Version_Flags) == 2)
> > +	{
> > +	  for (; p < end_insns; p += 2)
> > +	    if (PEX64_UNWCODE_CODE (p[1]) != 6)
> > +	      break;
> > +	}
> 
> I'd consider merging with the existing loop, so that
> we print the opcodes when frame debug is enabled.
> 
> But anyway, this looks fine to me.
> 
> I clicked on "Did you find this helpful?  No" at:
> 
>   http://msdn.microsoft.com/en-us/library/ck9asaa9.aspx
> 
> and asked for info about v2 and opcode 6.  Not holding
> my breath though.

Calling strings(1) on the DLLs in the Windows system32 dir on a Windows
8.1 system reveals the following curious info:

$ strings /cygdrive/c/Windows/System32/dbghelp.dll | grep UWOP
  AMD64_UWOP_PUSH_NONVOL Register %x RSP %I64X
  AMD64_UWOP_ALLOC_LARGE FrameOffs %x %x RSP %I64X + %x
  AMD64_UWOP_ALLOC_LARGE FrameOffs %x RSP %I64X + %x
  AMD64_UWOP_ALLOC_SMALL Info %x RSP %I64X
  AMD64_UWOP_SET_FPREG FrameReg %x FrameOffs %x RSP %I64X
  AMD64_UWOP_SAVE_NONVOL Register %x FrameBase %I64X FrameOffs %x RSP %I64X
  AMD64_UWOP_SAVE_NONVOL_FAR Register %x FrameBase %I64X FrameOffs %x %x RSP %I64X
  AMD64_UWOP_EPILOG OpInfo: %x
  AMD64_UWOP_SPARE OpInfo: %x FrameOffs %x
  AMD64_UWOP_SAVE_XMM128 Register %x FrameBase %I64X FrameOffs %x RSP %I64X
  AMD64_UWOP_SAVE_XMM128_FAR Register %x FrameBase %I64X FrameOffs %x %x RSP %I64X
  AMD64_UWOP_PUSH_MACHFRAME Info %x RetAddr %I64X StkAddr %I64X RSP %I64X

So it looks like opcode 6 is AMD64_UWOP_EPILOG with a single 32 bit 
operation info.  What 32 bit info would that be?


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFA] Allow Windows UNWIND_INFO version 2.

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 2341 bytes --]

On Dec  3 20:58, Corinna Vinschen wrote:
> On Dec  3 18:47, Pedro Alves wrote:
> > On 12/03/2013 11:32 AM, Joel Brobecker wrote:
> > > @@ -696,7 +697,17 @@ amd64_windows_frame_decode_insns (struct frame_info *this_frame,
> > >  	return;
> > >  
> > >        end_insns = &insns[codes_count * 2];
> > > -      for (p = insns; p < end_insns; p += 2)
> > > +      p = insns;
> > > +
> > > +      /* Skip opcodes 6 of version 2.  This opcode is not documented.  */
> > > +      if (PEX64_UWI_VERSION (ex_ui.Version_Flags) == 2)
> > > +	{
> > > +	  for (; p < end_insns; p += 2)
> > > +	    if (PEX64_UNWCODE_CODE (p[1]) != 6)
> > > +	      break;
> > > +	}
> > 
> > I'd consider merging with the existing loop, so that
> > we print the opcodes when frame debug is enabled.
> > 
> > But anyway, this looks fine to me.
> > 
> > I clicked on "Did you find this helpful?  No" at:
> > 
> >   http://msdn.microsoft.com/en-us/library/ck9asaa9.aspx
> > 
> > and asked for info about v2 and opcode 6.  Not holding
> > my breath though.
> 
> Calling strings(1) on the DLLs in the Windows system32 dir on a Windows
> 8.1 system reveals the following curious info:
> 
> $ strings /cygdrive/c/Windows/System32/dbghelp.dll | grep UWOP
>   AMD64_UWOP_PUSH_NONVOL Register %x RSP %I64X
>   AMD64_UWOP_ALLOC_LARGE FrameOffs %x %x RSP %I64X + %x
>   AMD64_UWOP_ALLOC_LARGE FrameOffs %x RSP %I64X + %x
>   AMD64_UWOP_ALLOC_SMALL Info %x RSP %I64X
>   AMD64_UWOP_SET_FPREG FrameReg %x FrameOffs %x RSP %I64X
>   AMD64_UWOP_SAVE_NONVOL Register %x FrameBase %I64X FrameOffs %x RSP %I64X
>   AMD64_UWOP_SAVE_NONVOL_FAR Register %x FrameBase %I64X FrameOffs %x %x RSP %I64X
>   AMD64_UWOP_EPILOG OpInfo: %x
>   AMD64_UWOP_SPARE OpInfo: %x FrameOffs %x
>   AMD64_UWOP_SAVE_XMM128 Register %x FrameBase %I64X FrameOffs %x RSP %I64X
>   AMD64_UWOP_SAVE_XMM128_FAR Register %x FrameBase %I64X FrameOffs %x %x RSP %I64X
>   AMD64_UWOP_PUSH_MACHFRAME Info %x RetAddr %I64X StkAddr %I64X RSP %I64X
> 
> So it looks like opcode 6 is AMD64_UWOP_EPILOG with a single 32 bit 
> operation info.  What 32 bit info would that be?

Sorry about that.  the operation info is 4 bits, of course, I got
carried away by giving the size of a %x parameter too much meaning.


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFA] Allow Windows UNWIND_INFO version 2.

[Tristan Gingold]

On 03 Dec 2013, at 20:58, Corinna Vinschen <vinschen@redhat.com> wrote:

> On Dec  3 18:47, Pedro Alves wrote:
>> On 12/03/2013 11:32 AM, Joel Brobecker wrote:
>>> @@ -696,7 +697,17 @@ amd64_windows_frame_decode_insns (struct frame_info *this_frame,
>>> 	return;
>>> 
>>>       end_insns = &insns[codes_count * 2];
>>> -      for (p = insns; p < end_insns; p += 2)
>>> +      p = insns;
>>> +
>>> +      /* Skip opcodes 6 of version 2.  This opcode is not documented.  */
>>> +      if (PEX64_UWI_VERSION (ex_ui.Version_Flags) == 2)
>>> +	{
>>> +	  for (; p < end_insns; p += 2)
>>> +	    if (PEX64_UNWCODE_CODE (p[1]) != 6)
>>> +	      break;
>>> +	}
>> 
>> I'd consider merging with the existing loop, so that
>> we print the opcodes when frame debug is enabled.
>> 
>> But anyway, this looks fine to me.
>> 
>> I clicked on "Did you find this helpful?  No" at:
>> 
>>  http://msdn.microsoft.com/en-us/library/ck9asaa9.aspx
>> 
>> and asked for info about v2 and opcode 6.  Not holding
>> my breath though.
> 
> Calling strings(1) on the DLLs in the Windows system32 dir on a Windows
> 8.1 system reveals the following curious info:

Clever !

It also shows AMD64_UWOP_SPARE.

Tristan.

> 
> $ strings /cygdrive/c/Windows/System32/dbghelp.dll | grep UWOP
>  AMD64_UWOP_PUSH_NONVOL Register %x RSP %I64X
>  AMD64_UWOP_ALLOC_LARGE FrameOffs %x %x RSP %I64X + %x
>  AMD64_UWOP_ALLOC_LARGE FrameOffs %x RSP %I64X + %x
>  AMD64_UWOP_ALLOC_SMALL Info %x RSP %I64X
>  AMD64_UWOP_SET_FPREG FrameReg %x FrameOffs %x RSP %I64X
>  AMD64_UWOP_SAVE_NONVOL Register %x FrameBase %I64X FrameOffs %x RSP %I64X
>  AMD64_UWOP_SAVE_NONVOL_FAR Register %x FrameBase %I64X FrameOffs %x %x RSP %I64X
>  AMD64_UWOP_EPILOG OpInfo: %x
>  AMD64_UWOP_SPARE OpInfo: %x FrameOffs %x
>  AMD64_UWOP_SAVE_XMM128 Register %x FrameBase %I64X FrameOffs %x RSP %I64X
>  AMD64_UWOP_SAVE_XMM128_FAR Register %x FrameBase %I64X FrameOffs %x %x RSP %I64X
>  AMD64_UWOP_PUSH_MACHFRAME Info %x RetAddr %I64X StkAddr %I64X RSP %I64X
> 
> So it looks like opcode 6 is AMD64_UWOP_EPILOG with a single 32 bit 
> operation info.  What 32 bit info would that be?
> 
> 
> Corinna
> 
> -- 
> Corinna Vinschen
> Cygwin Maintainer
> Red Hat

Re: [RFA] Allow Windows UNWIND_INFO version 2.

[Tristan Gingold]

On 03 Dec 2013, at 19:47, Pedro Alves <palves@redhat.com> wrote:

> On 12/03/2013 11:32 AM, Joel Brobecker wrote:
>> @@ -696,7 +697,17 @@ amd64_windows_frame_decode_insns (struct frame_info *this_frame,
>> 	return;
>> 
>>       end_insns = &insns[codes_count * 2];
>> -      for (p = insns; p < end_insns; p += 2)
>> +      p = insns;
>> +
>> +      /* Skip opcodes 6 of version 2.  This opcode is not documented.  */
>> +      if (PEX64_UWI_VERSION (ex_ui.Version_Flags) == 2)
>> +	{
>> +	  for (; p < end_insns; p += 2)
>> +	    if (PEX64_UNWCODE_CODE (p[1]) != 6)
>> +	      break;
>> +	}
> 
> I'd consider merging with the existing loop, so that
> we print the opcodes when frame debug is enabled.

Not sure this is a good idea.  You can use objdump -p if you want to view the opcodes.

But this opcode was also used in version 1, and in version 2 appears only before all other
opcodes.

Tristan.

Re: [RFA] Allow Windows UNWIND_INFO version 2.

[Joel Brobecker]

> I'd consider merging with the existing loop, so that
> we print the opcodes when frame debug is enabled.
> 
> But anyway, this looks fine to me.

Thanks, Pedro. Given Tristan's feedback, I decided to push the patch
as is. But I remain available to make additional changes, should
the discussion lead us towards that.

> I clicked on "Did you find this helpful?  No" at:
> 
>   http://msdn.microsoft.com/en-us/library/ck9asaa9.aspx
> 
> and asked for info about v2 and opcode 6.  Not holding
> my breath though.

Ha! Good idea, and I will remember to do that too. It's not much
effort, and you never know.

-- 
Joel

Re: [RFA] Allow Windows UNWIND_INFO version 2.

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 1164 bytes --]

On Dec  5 07:44, Joel Brobecker wrote:
> > I'd consider merging with the existing loop, so that
> > we print the opcodes when frame debug is enabled.
> > 
> > But anyway, this looks fine to me.
> 
> Thanks, Pedro. Given Tristan's feedback, I decided to push the patch
> as is. But I remain available to make additional changes, should
> the discussion lead us towards that.
> 
> > I clicked on "Did you find this helpful?  No" at:
> > 
> >   http://msdn.microsoft.com/en-us/library/ck9asaa9.aspx
> > 
> > and asked for info about v2 and opcode 6.  Not holding
> > my breath though.
> 
> Ha! Good idea, and I will remember to do that too. It's not much
> effort, and you never know.

I'd suggest to utilize the Microsoft forums at
http://social.msdn.microsoft.com/Forums/en-US/home
My last bugreport got handled pretty quickly, see
http://social.msdn.microsoft.com/Forums/en-US/073d02a6-1181-4694-9e50-d6a05bd80663/

Out of curiosity, will there be a followup for this GDB patch
submission?  Handling the v2 opcodes is rather interesting for Cygwin
and Mingw...


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [RFA] Allow Windows UNWIND_INFO version 2.

[Tristan Gingold]

On 09 Dec 2013, at 11:43, Corinna Vinschen <vinschen@redhat.com> wrote:

> On Dec  5 07:44, Joel Brobecker wrote:
>>> I'd consider merging with the existing loop, so that
>>> we print the opcodes when frame debug is enabled.
>>> 
>>> But anyway, this looks fine to me.
>> 
>> Thanks, Pedro. Given Tristan's feedback, I decided to push the patch
>> as is. But I remain available to make additional changes, should
>> the discussion lead us towards that.
>> 
>>> I clicked on "Did you find this helpful?  No" at:
>>> 
>>>  http://msdn.microsoft.com/en-us/library/ck9asaa9.aspx
>>> 
>>> and asked for info about v2 and opcode 6.  Not holding
>>> my breath though.
>> 
>> Ha! Good idea, and I will remember to do that too. It's not much
>> effort, and you never know.
> 
> I'd suggest to utilize the Microsoft forums at
> http://social.msdn.microsoft.com/Forums/en-US/home
> My last bugreport got handled pretty quickly, see
> http://social.msdn.microsoft.com/Forums/en-US/073d02a6-1181-4694-9e50-d6a05bd80663/

Good idea!

> Out of curiosity, will there be a followup for this GDB patch
> submission?  Handling the v2 opcodes is rather interesting for Cygwin
> and Mingw...

This patch has been committed by Joel, so gdb should be able to unwind through frames with v2 unwind infos.

Tristan.

Re: [RFA] Allow Windows UNWIND_INFO version 2.

[Corinna Vinschen]

[-- Attachment #1: Type: text/plain, Size: 498 bytes --]

On Dec  9 12:47, Tristan Gingold wrote:
> On 09 Dec 2013, at 11:43, Corinna Vinschen <...> wrote:
> > Out of curiosity, will there be a followup for this GDB patch
> > submission?  Handling the v2 opcodes is rather interesting for Cygwin
> > and Mingw...
> 
> This patch has been committed by Joel, so gdb should be able to unwind through frames with v2 unwind infos.

Oh, I missed the checkin.  Sorry about that.


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]