Re: Program-assigned thread names on Windows

LRN
2016-07-26T06:08:03Z

[-- Attachment #1.1.1: Type: text/plain, Size: 4814 bytes --]

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 #1.1.2: 0001-Support-settings-thread-name-MS-Windows.patch --]
[-- Type: text/plain, Size: 4303 bytes --]

From 141c4ff8f185dd2ee1a8ffbf4d26a21e16c852bd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=D0=A0=D1=83=D1=81=D0=BB=D0=B0=D0=BD=20=D0=98=D0=B6=D0=B1?=
=?UTF-8?q?=D1=83=D0=BB=D0=B0=D1=82=D0=BE=D0=B2?= <lrn1986@gmail.com>
Date: Sun, 26 Jun 2016 11:14:49 +0000
Subject: [PATCH 1/3] Support settings thread name (MS-Windows)

This is done by catching an exception number 0x406D1388
(it has no documented name), which is thrown by the program.
The exception record contains an ID of a thread and a name to
give it.

This requires rolling back some changes in handle_exception(),
which now again returns more than two distinct values. The code
2 means that gdb should just continue, without returning
thread ID up the stack (which will result in further handling
of the exception, which is not what we want).
---
gdb/windows-nat.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 61 insertions(+), 5 deletions(-)

diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 3f67486..084d5a9 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -174,6 +174,9 @@ static int debug_registers_used;
static int windows_initialization_done;
#define DR6_CLEAR_VALUE 0xffff0ff0

+#define MS_VC_EXCEPTION 0x406D1388
+#define MS_VC_EXCEPTION_S "0x406D1388"
+
/* The string sent by cygwin when it processes a signal.
FIXME: This should be in a cygwin include file. */
#ifndef _CYGWIN_SIGNAL_STRING
@@ -1035,6 +1038,7 @@ static int
handle_exception (struct target_waitstatus *ourstatus)
{
DWORD code = current_event.u.Exception.ExceptionRecord.ExceptionCode;
+ int result = 1;

ourstatus->kind = TARGET_WAITKIND_STOPPED;

@@ -1140,6 +1144,49 @@ handle_exception (struct target_waitstatus *ourstatus)
DEBUG_EXCEPTION_SIMPLE ("EXCEPTION_NONCONTINUABLE_EXCEPTION");
ourstatus->value.sig = GDB_SIGNAL_ILL;
break;
+ case MS_VC_EXCEPTION:
+ if (current_event.u.Exception.ExceptionRecord.NumberParameters >= 3
+ && current_event.u.Exception.ExceptionRecord.ExceptionInformation[0] == 0x1000)
+ {
+ long named_thread_id;
+ ptid_t named_thread_ptid;
+ struct thread_info *named_thread;
+ CORE_ADDR thread_name_target;
+ char *thread_name;
+ int thread_name_len;
+
+ DEBUG_EXCEPTION_SIMPLE (MS_VC_EXCEPTION_S);
+
+ named_thread_id = (long) current_event.u.Exception.ExceptionRecord.ExceptionInformation[2];
+ thread_name_target = current_event.u.Exception.ExceptionRecord.ExceptionInformation[1];
+
+ if (named_thread_id == (DWORD) -1)
+ named_thread_id = current_event.dwThreadId;
+
+ named_thread_ptid = ptid_build (current_event.dwProcessId, 0, named_thread_id),
+ named_thread = find_thread_ptid (named_thread_ptid);
+
+ thread_name = NULL;
+ thread_name_len = target_read_string (thread_name_target, &thread_name, 1025, 0);
+ if (thread_name_len > 0 && thread_name != NULL)
+ {
+ if (thread_name[thread_name_len - 1] != '\0')
+ thread_name[thread_name_len - 1] = '\0';
+ if (thread_name[0] != '\0')
+ {
+ xfree (named_thread->name);
+ named_thread->name = thread_name;
+ }
+ else
+ {
+ xfree (thread_name);
+ }
+ }
+ ourstatus->value.sig = GDB_SIGNAL_TRAP;
+ result = 2;
+ break;
+ }
+ /* treat improperly formed exception as unknown, fallthrough */
default:
/* Treat unhandled first chance exceptions specially. */
if (current_event.u.Exception.dwFirstChance)
@@ -1153,7 +1200,7 @@ handle_exception (struct target_waitstatus *ourstatus)
}
exception_count++;
last_sig = ourstatus->value.sig;
- return 1;
+ return result;
}

/* Resume thread specified by ID, or all artificially suspended
@@ -1510,10 +1557,19 @@ get_windows_debug_event (struct target_ops *ops,
"EXCEPTION_DEBUG_EVENT"));
if (saw_create != 1)
break;
- if (handle_exception (ourstatus))
- thread_id = current_event.dwThreadId;
- else
- continue_status = DBG_EXCEPTION_NOT_HANDLED;
+ switch (handle_exception (ourstatus))
+ {
+ case 0:
+ default:
+ continue_status = DBG_EXCEPTION_NOT_HANDLED;
+ break;
+ case 1:
+ thread_id = current_event.dwThreadId;
+ break;
+ case 2:
+ continue_status = DBG_CONTINUE;
+ break;
+ }
break;

case OUTPUT_DEBUG_STRING_EVENT: /* Message from the kernel. */
--
2.4.0

[-- Attachment #1.1.3: 0x6759BA74.asc --]