Re: [RFA/commit+doco 2/2] Windows x64 SEH unwinder.

[Tristan Gingold <gingold@adacore.com>]

On Jan 10, 2013, at 5:24 PM, Pedro Alves wrote:

> On 01/09/2013 08:07 PM, Tristan Gingold wrote:
> 
>>>> This is in fact an optimization. If we found a pop, followed by
>>>> an epilog marker, there is not need to decode unwind info.
>>> 
>>> I don't understand.  pops will always be followed by a marker.
>>> How can that be an optimization?
>> 
>> If pop weren't in this list, then if pc points to a pop the unwinder
>> will consider that the pc is in the body and need to decode unwind
>> infos.  Now, if pop is in the list, the unwinder will continue to decode
>> until a ret and if a ret is found, it will consider that the pc is in
>> the epilogue, avoiding decoding unwind infos.
> 
> (and again for quotability:)
> 
>> If pop weren't in this list,
> 
> I think you meant, that if pops were not handled in the
> while loop.  While I'm saying that the comment implies
> that pop is an epilog marker, and claiming it's not,
> therefore the comment is wrong and misleading.  It should
> be clarified/extended.

Ok.

> But let me see if I understood, by trying to rewrite
> and extend your reply into something that I'd find clearer:
> 
> We want to detect if the PC points to an epilogue (entry or midway).
> If so, the following epilogue detection+decoding below is
> sufficient.  Otherwise, the unwinder will consider that the PC
> is in the body of the function and will need to decode unwind info.
> According to MSDN, an epilogue "must consist of either an
> add RSP,constant or lea RSP,constant[FPReg], followed by a
> series of zero or more 8-byte register pops and a return
> or a jmp".
> 
> It confuses me to call this an optimization though,
> because MSDN says:
> 
> "This reduces the amount of unwind data required, because no extra
> data is needed to describe each epilog. Instead, the unwind code
> can determine that an epilog is being executed by scanning
> forward through a code stream to identify an epilog."
> 
> That reads to me that prologue decoding is not really
> something we can forgo, but something that is _required_,
> because the unwind data describing the epilog will simply
> not be there.

Yes, I now think that this is correct. The unwind data are only for the
prolog, the body don't need any additional data, while the epilog must
be specially handled.

>>> The jump to an epilogue would not be part of the epilogue.
>> 
>> Doesn't really matter. The point is that if the current instruction
>> is a jump to the epilogue, there is no need to decode unwind infs.
> 
> Okay, that one makes sense to me.  Please expand the comments
> in that direction.
> 
>> 
>>>> But I may miss your point.
>>> 
>>> My point is that the docs say the epilogue has this rigid
>>> format that always ends in a marker, and that a marker is
>>> a ret or a jmp (therefore calling "pop" a marker as in the
>>> "I would add" comment seems to me misleading).  The code
>>> continues following the jmp, so it makes me believe the code
>>> is erroneously decoding something after the jmp that is
>>> not an epilogue (the caller or perhaps a tailcall).
>> 
>> No, it doesn't say that it ends in a marker.  The epilogue
>> ends by a ret.
> 
> Not by my reading:
> 
> http://msdn.microsoft.com/en-us/library/tawsa7cb.aspx
> 
> "It must consist of either an add RSP,constant or
> lea RSP,constant[FPReg], followed by a series of zero or
> more 8-byte register pops and a return or a jmp"
>                               ^^^^^^^^^^^^^^^^^
> 
> IOW, I read that as (in pseudo-bnf):
> 
> add|lea
> (pop)*
> ret|jmp
> 
> This http://blogs.msdn.com/b/freik/archive/2006/01/04/509372.aspx
> confirms my reading.
> 
> Hence all this confusion and the asking for an example
> (a disassembly) where jmp appears in the middle
> of an epilogue, and my fear that this prologue detection
> might be following jmps when it should not, and
> therefore not detecting prologue ends correctly.
> 
> "All function epilogues must look like this:
> 
> (optional) lea rsp, [frame ptr + frame size] or add rsp, frame size
> pop reg (zero or more)
> ret (or jmp)
> 
> (...)
> No other instructions may occur betwen the first lea/add and the
> final jmp or ret.
> (...)
> One other note: if the final jmp isn't an ip-relative jmp,
> but an indirect jmp, it must be preceded by the REX prefix,
> to indicate to the OS unwind routines that the jump is headed
> outside of the function, otherwise, the OS assumes it's a jump
> to a different location inside the same function."

Ah, it looks like I missed this note!  That really make sense.

> So it looks to me that even if you follow
> jumps to the epilogue as an optimization, the current
> "rex jmp reg" handling is wrong - it should not follow the PC,
> but instead by handling like ret and return 1.

That correct.

> Reading the patch in more detail, I now see that all the other
> jmps handling in the patch's epilogue decoding are relative
> jumps, and those are _not_ considered epilog markers (so it's
> your optimization applying), per RtlVirtualUnwind's docs.  But,
> as can be seen in the comment in the patch, immediate jmps
> _are_ epilogue markers, so I do believe they should be handled,
> and terminate the epilog decoding, just like ret.

Yes, I think that's right.

Thanks for digging deeply!

Tristan.