Re: [patch/cygwin] Remove dependency on __COPY_CONTEXT_SIZE

[Corinna Vinschen <vinschen at redhat dot com>]

On Mar 31 15:58, Pedro Alves wrote:
> On 03/31/2015 03:36 PM, Corinna Vinschen wrote:
> > On Mar 31 13:34, Pedro Alves wrote:
> >> On 03/30/2015 11:04 AM, Corinna Vinschen wrote:
> >>
> >>> @@ -820,7 +819,7 @@ handle_output_debug_string (struct target_waitstatus *ourstatus)
> >>>  #endif
> >>>  	warning (("%s"), s);
> >>>      }
> >>> -#ifdef __COPY_CONTEXT_SIZE
> >>> +#ifdef __CYGWIN__
> >>>    else
> >>>      {
> >>>        /* Got a cygwin signal marker.  A cygwin signal is followed by
> >>> @@ -847,8 +846,8 @@ handle_output_debug_string (struct target_waitstatus *ourstatus)
> >>>  	  else if ((x = (LPCVOID) (uintptr_t) strtoull (p, NULL, 0))
> >>>  		   && ReadProcessMemory (current_process_handle, x,
> >>>  					 &saved_context,
> >>> -					 __COPY_CONTEXT_SIZE, &n)
> >>> -		   && n == __COPY_CONTEXT_SIZE)
> >>> +					 sizeof (CONTEXT), &n)
> >>
> >> Is that really wise?  AFAIK, the size of the CONTEXT structure can
> >> grow as MSFT adds more registers to support newer machines.
> > 
> > No, that's not possible.  The CONTEXT structure matches the platform.
> > It doesn't even contain a version number.  Consider that the structure
> > is available in user space.  If Microsoft changes the size on a given
> > platform, applications built for this platform might crash due to
> > overwritten memory.  They wouldn't do that.
> 
> That's not true.   GetThreadContext takes a size parameter,
> and only writes to the bits that the caller requests with
> context.ContextFlags.

The ContextFlags member is not a size parameter, it's a bit flag
parameter.  It tells the OS which values are required, but the flags are
responsible for different aspects of the CONTEXT structure and this does
not change the size of the CONTEXT datatype.  The code in question
always copies the exact size of CONTEXT, and that doesn't change per
platform.

> A size parameter is common in Windows API land
> to permit later versions.  If the structure grows, evidently the new
> fields will need to be requested with a new context.ContextFlags flag.  Old
> applications will never request that extra flag, and will be passing
> a smaller SIZE to GetThreadContext, so it won't ever overwrite memory.
> 
> See the description of InitializeContext's parameters:
> 
>  https://msdn.microsoft.com/en-us/library/windows/desktop/hh134237%28v=vs.85%29.aspx
> 
> And the remarks section:
> 
> "InitializeContext can be used to initialize a CONTEXT structure within a buffer
> with the required size and alignment characteristics. This routine is required if
> the CONTEXT_XSTATE ContextFlag is specified since the required context
> size and alignment may change depending on which processor features are
> enabled on the system.

Right, but this does not change the size of the CONTEXT datatype.  The
additional AVX values require more space than available in the CONTEXT
struct.  That's why using CONTEXT_XSTATE and the AVX functions require
to use InitializeContext; the size required to get these values is
larger than CONTEXT, thus the function returns ERROR_INSUFFICIENT_BUFFER
if ContextLength is == sizeof (CONTEXT) only.

And, we're not using this.  The local variable filled with the data is
of type CONTEXT and the data transmitted from Cygwin to GDB is of type
CONTEXT.  It's still the same size, independent of the availablity of
CONTEXT_XSTATE.


Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat

Attachment: pgpUCxBnjJ6cS.pgp
Description: PGP signature