This is the mail archive of the mailing list for the GDB project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch/cygwin] Remove dependency on __COPY_CONTEXT_SIZE

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:
>> 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,
-					 __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

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

Pedro Alves

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]