Re: Program-assigned thread names on Windows

[Jon Turney <jon.turney@dronecode.org.uk>]

On 25/07/2016 14:34, LRN wrote:
> On 25.07.2016 15:17, Jon Turney wrote:
>> On 23/07/2016 18:01, LRN wrote:
>>> On 23.07.2016 19:39, John Baldwin wrote:
>>>>> On Saturday, July 23, 2016 12:25:15 PM LRN wrote:
>>>>>>>
>>>>>>> This works as documented[1] on MSDN - by catching a specific
>>>>>>> exception that the program throws.
>>>>>>>
>>>>>>> Setting thread name this way is supported by glib[2] and winpthreads[3]
>>>>>>> at least, as well as any program developed with MS toolchain (because
>>>>>>> WinDbg supported this for a long time).
>>>>>>>
>>>>>>> [1] https://msdn.microsoft.com/en-us/library/xcb2z8hs.aspx
>>>>>>> [2] https://git.gnome.org/browse/glib/commit/glib
>>>>>>> /gthread-win32.c?id=e118856430a798bbc529691ad235fd0b0684439d
>>>>>>> [3] https://sourceforge.net/p/mingw-w64/mingw-w64/ci
>>>>>>> /0d95c795b44b76e1b60dfc119fd93cfd0cb35816/
>>>>>
>>>
>>> This is done by catching an exception number 0x406D1388
>>> (it has no documented name), which is thrown by the program.
>>
>> The name used in the MSDN article [1] is 'MS_VC_EXCEPTION', so why not
>> use that?
>
> No reason. If you want, run sed -e
> 's/WINDOWS_THREADNAME_EXCEPTION/MS_VC_EXCEPTION' over the patch file prior
> to committing it.
>
> That said, i think "MS_VC_EXCEPTION" does not offer a good enough
> description (doesn't mention threads, does mention VisualC).

There might be current or future uses of this exception with type other 
than 0x1000 which aren't related to threads at all.

>>> +    case WINDOWS_THREADNAME_EXCEPTION:
>>> +      DEBUG_EXCEPTION_SIMPLE (WINDOWS_THREADNAME_EXCEPTION_S);
>>> +      ourstatus->value.sig = GDB_SIGNAL_TRAP;
>>> +      if (current_event.u.Exception.ExceptionRecord.NumberParameters == 4)
>>> +	{
>>> +	  DWORD named_thread_id;
>>> +	  ptid_t named_thread_ptid;
>>> +	  struct thread_info *named_thread;
>>> +	  uintptr_t thread_name_target;
>>> +	  char *thread_name;
>>> +
>>
>> Shouldn't this check for ExceptionInformation[0] = 0x1000, and treat
>> this as an unknown exception otherwise?
>
> Yes, it should.
>
>>
>>> +	  named_thread_id = (DWORD) current_event.u.Exception.ExceptionRecord.ExceptionInformation[2];
>>> +	  thread_name_target = (uintptr_t) current_event.u.Exception.ExceptionRecord.ExceptionInformation[1];
>>
>> Is this going to be correct for 64-bit builds?
>
> I've only tested this on i686.
>
> Which variable are you concerned about - named_thread_id or thread_name_target?

Both.  The ExceptionInformation isn't actually array of DWORDs, it's a 
THREADNAME_INFO structure, which contains a LPCSTR pointer (which has a 
different size on x86 and x86_64) *before* the thread id.

So, I think this should check that NumbersParameters * sizeof(DWORD) is 
equal to or greater than sizeof(THREADNAME_INFO), then cast 
ExceptionInformation to a THREADNAME_INFO.

> Tough this is a good point. MSDN says that i686 and x86_64 EXCEPTION_RECORD
> structures have different layout (well, to-be-pointer struct fields are
> DWORD64 on x86_64).

I don't think gdb currently supports 32/64 bit interworking on Windows, 
so perhaps that is all moot (although if that is the case, perhaps it 
should diagnose attempts to do that)