Re: [patch/cygwin] Remove dependency on __COPY_CONTEXT_SIZE

[Pedro Alves <palves@redhat.com>]

On 03/31/2015 04:42 PM, Corinna Vinschen wrote:
> 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,

I didn't say it was.  The GetThreadContext function takes
an IN+OUT size parameter in _addition to the ContextFlags flag.
Both can be used for versioning.

>> 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.

I'm almost sure in the old days, the CONTEXT structure didn't have
the ExtendedRegisters field at all.  I think it's bad to hard code
the size of the CONTEXT structure, but won't argue further.  Patch
is OK if you'd really like to apply it as is.


@@ -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)
+		   && n == sizeof (CONTEXT))
 	    have_saved_context = 1;
 	  current_event.dwThreadId = retval;
 	}

> 
> 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.

Where can the debugger find the signal's xstate state then?

Thanks,
Pedro Alves