Re: Program-assigned thread names on Windows

[LRN <lrn1986 at gmail dot com>]

On 26.07.2016 0:32, LRN wrote:
> On 25.07.2016 17:23, LRN wrote:
>> On 25.07.2016 17:06, Jon Turney wrote:
>>> 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:
>>>>>> +	  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)
>>>
>>
>> Yep, just tried to attach to a 64-bit process from a 32-bit gdb, and gdb
>> failed to attach.
>>
>> I'll try to come up with a way to build 64-bit gdb... it might take a while
>> though.
>>
> 
> 1) 64-bit gdb can attach to 32-bit debugees.
> 64-bit gdb sure throws a number of warnings when attaching to a 32-bit
> debugee, but still attaches. However, it quickly gets into a tailspin, if i
> do anything other than "run" (set breakpoints, step through functions).
> 
> 2) EXCEPTION_RECORD does not need to be casted into EXCEPTION_RECORD32 or
> EXCEPTION_RECORD64 for native processes, as it's correctly aligned in
> either way ("2x4, 2 pointers, 4, pointer" - for 32-bit case everything is
> tightly packed and 4-byte aligned, for 64-bit case the last pointer moves 4
> bytes further to be self-aligned to 8 bytes, while everything else remains
> the same), so we can keep accessing stuff via EXCEPTION_RECORD natively.
> That is, EXCEPTION_RECORD64 is how EXCEPTION_RECORD normally looks in
> 64-bit process.
> 
> 3) EXCEPTION_RECORD that we receive is sized to *gdb* bitness. That is,
> casing it to EXCEPTION_RECORD32 in 64-bit gdb will always lead to bad
> interpretation, even if debugee is 32-bit.
> 
> 4) ExceptionInfromation array that we receive as part of EXCEPTION_RECORD
> is *also natively aligned for gdb*. I've made 32-bit debugee print out the
> addresses of fields of the THEADNAME_INFO structure, and it's aligned to 4
> bytes (as expected), but examining the EXCEPTION_RECORD structure that
> 64-bit gdb receives shows that the ExceptionInformation array is aligned to
> 8 bytes. Therefore, it's safe to always use EXCEPTION_RECORD as-is, without
> worrying about alignment of the ExceptionInformation data.
> 
> 5) 64-bit gdb receives an EXCEPTION_RECORD with NumberParameters == 6 when
> debugee is 64-bit. The contents of the extra 2 elements are a mystery (they
> seem to point to the stack, but that's all i can tell). Also, the 4-th
> element (which is "Reserved for future use, must be zero") is not zero when
> the exception is caught.
> In light of this, we should probably check for NumberParameters >= 4. Or
> even NumberParameters >= 3, given that we don't really look at the 4th
> parameter.
> 

Attaching the latest version of the patch:

* Treats ExceptionInformation[0] != 0x1000 or NumberParameters < 3 as
unknown exception.
* Uses (hopefully) correct datatypes for thread_name_target and
named_thread_id.
* Ensures thread name is 0-terminated, doesn't leak.
* Uses "MS_VC_EXCEPTION" as the exception name.

By the way, the realignment of the ExceptionInformation when it is passed
from a 32-bit process to a 64-bit one suggests that RaiseException()
documentation is actually precise: ExceptionInformation is an array of
pointer-sized values, and is treated as such. As a test, i've tried to pass
a struct with 12 separate char fields initialized into consecutive numbers
(and packed tightly, i've checked), and by the time gdb got it, the
"struct" was chopped into groups of 4 bytes, each of which was padded by 4
empty extra bytes.
MS uses THREADNAME_INFO struct in its example, but it really should have
used an array of ULONG_PTR, because that is what is being actually sent.

-- 
O< ascii ribbon - stop html email! - www.asciiribbon.org

Attachment: 0001-Support-settings-thread-name-MS-Windows.patch
Description: Text document

Attachment: 0x6759BA74.asc
Description: application/pgp-keys

Attachment: signature.asc
Description: OpenPGP digital signature