Re: Program-assigned thread names on Windows

LRN
2016-07-23T17:01:41Z

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

On 23.07.2016 19:39, John Baldwin wrote:
> On Saturday, July 23, 2016 12:25:15 PM LRN wrote:
>> The attached patch adds thread naming support on Windows.
>>
>> 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/
>
> Does this leak 'thread_name' if the first character is '\0'?
>
> + thread_name = NULL;
> + if (!target_read_string ((CORE_ADDR) thread_name_target, &thread_name, 1024, 0)
> + || !thread_name || !*thread_name)
> + /* nothing to do */;
> + else
> + {
> + xfree (named_thread->name);
> + named_thread->name = thread_name;
> + }
> + result = 2;
>
> Maybe restructure as:
>
> if (target_read_string (...))
> {
> if (thread_name && thread_name[0] != '\0')
> {
> xfree (named_thread->name);
> named_thread->name = thread_name;
> }
> else
> xfree (thread_name);
> }
>

You're right, it can leak. Fixed patch attached.

That said, handle_output_debug_string() does the same thing (and, judging
by the "nothing to do" comment, i've copied this call from there back when
i made wrote this code).

--
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: 4117 bytes --]

From 83320cdb4dc349ffefd777bfcbef0848bc661c8b 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 | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 55 insertions(+), 5 deletions(-)

diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 3f67486..458790b 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 WINDOWS_THREADNAME_EXCEPTION 0x406D1388
+#define WINDOWS_THREADNAME_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,43 @@ handle_exception (struct target_waitstatus *ourstatus)
DEBUG_EXCEPTION_SIMPLE ("EXCEPTION_NONCONTINUABLE_EXCEPTION");
ourstatus->value.sig = GDB_SIGNAL_ILL;
break;
+ 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;
+
+ named_thread_id = (DWORD) current_event.u.Exception.ExceptionRecord.ExceptionInformation[2];
+ thread_name_target = (uintptr_t) 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;
+ if (target_read_string ((CORE_ADDR) thread_name_target, &thread_name, 1024, 0))
+ {
+ if (thread_name != NULL && thread_name[0] != '\0')
+ {
+ xfree (named_thread->name);
+ named_thread->name = thread_name;
+ }
+ else if (thread_name != NULL)
+ {
+ /* nothing to do */;
+ xfree (thread_name);
+ }
+ }
+ result = 2;
+ }
+ break;
default:
/* Treat unhandled first chance exceptions specially. */
if (current_event.u.Exception.dwFirstChance)
@@ -1153,7 +1194,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 +1551,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 --]