Re: Program-assigned thread names on Windows

Pedro Alves
2016-08-10T12:15:32Z

On 08/10/2016 08:11 AM, LRN wrote:
> No one seems to object.
>

I have some comments. See below.

> I've attached the version of the patch with the named_thread == NULL issue
> fixed, and also the ChangeLog and NEWS patches just for the hell of it.
>

> #define DR6_CLEAR_VALUE 0xffff0ff0
>
> +#define MS_VC_EXCEPTION 0x406D1388
> +

This should have a describing comment. There's one in the git commit
log we can reuse.

> @@ -1140,10 +1150,52 @@ 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] & 0xFFFFFFFF) == 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");
> +
> + named_thread_id = (long) (0xFFFFFFFF & current_event.u.Exception.ExceptionRecord.ExceptionInformation[2]);
> + thread_name_target = current_event.u.Exception.ExceptionRecord.ExceptionInformation[1];
> +

Lines too long. Easy to sort out by adding:

EXCEPTION_RECORD *rec = &current_event.u.Exception.ExceptionRecord;

at the top, and then using rec instead.

Hex constants are written in lowercase.

> + 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);

Overly long lines here too.

> + if (thread_name_len > 0 && thread_name != NULL)
> + {
> + thread_name[thread_name_len - 1] = '\0';
> + if (thread_name[0] != '\0' && named_thread != NULL)
> + {
> + xfree (named_thread->name);
> + named_thread->name = thread_name;

This doesn't look correct. This overwrites a thread name a user
has specified manually, with "thread name". Instead, store the
name somewhere else, and then implement a target_thread_name target
method that returns that name. Turns out there's a seemingly unused
name field in windows_thread_info that we can borrow.

> + }
> + else
> + {
> + xfree (thread_name);

Why bother to read the string at all if named_thread is NULL?

> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -77,6 +77,12 @@
> The "catch syscall" command now supports catching a group of related
> syscalls using the 'group:' or 'g:' prefix.
>
> +* Support for thread names on MS-Windows.
> +
> + GDB will catch and correctly handle the special exception that
> + programs running on MS-Windows use to assign names to threads
> + in the debugger.
> +

Should be moved to the "after 7.12" section. I did that for you.
(Slightly edited to avoid future tense, while at it.)

I was going to apply your patches, so I squashed them all, and
only after did I notice the problems, so I went ahead and did
the changes. I also fixed a couple typos in the commit log.

Could you give this updated patch a try? I build-tested it using
a cross compiler, but haven't tried it out on Windows.

--------------------------------------
From 4da942cfa605c82c292f08b0787378a0a2f82fcb 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: Wed, 10 Aug 2016 12:56:37 +0100
Subject: [PATCH] Support setting thread names (MS-Windows)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This is done by catching an exception number 0x406D1388 (it has no
documented name, though MSDN dubs it "MS_VC_EXCEPTION" in one code
example), 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 value
HANDLE_EXCEPTION_IGNORED means that gdb should just continue, without
returning thread ID up the stack (which would result in further
handling of the exception, which is not what we want).

gdb/ChangeLog:
2016-08-10 Ð ÑƒÑÐ»Ð°Ð½ Ð˜Ð¶Ð±ÑƒÐ»Ð°Ñ‚Ð¾Ð² <lrn1986@gmail.com>

* windows-nat.c (handle_exception): Handle exception 0x406D1388
that is used to set thread name.
* NEWS: Mention the thread naming support on MS-Windows.
---
gdb/NEWS | 6 ++++
gdb/windows-nat.c | 90 ++++++++++++++++++++++++++++++++++++++++++++++++-------
2 files changed, 85 insertions(+), 11 deletions(-)

diff --git a/gdb/NEWS b/gdb/NEWS
index b08d8a0..6f5feb1 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -3,6 +3,12 @@

*** Changes since GDB 7.12

+* Support for thread names on MS-Windows.
+
+ GDB now catches and handles the special exception that programs
+ running on MS-Windows use to assign names to threads in the
+ debugger.
+
*** Changes in GDB 7.12

* GDB and GDBserver now build with a C++ compiler by default.
diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 3f67486..3b0b3e3 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -174,6 +174,18 @@ static int debug_registers_used;
static int windows_initialization_done;
#define DR6_CLEAR_VALUE 0xffff0ff0

+/* The exception number which is thrown by the program to set a
+ thread's name in the debugger. The exception record contains an ID
+ of a thread and a name to give it. */
+#define MS_VC_EXCEPTION 0x406d1388
+
+typedef enum
+{
+ HANDLE_EXCEPTION_UNHANDLED = 0,
+ HANDLE_EXCEPTION_HANDLED,
+ HANDLE_EXCEPTION_IGNORED
+} handle_exception_result;
+
/* The string sent by cygwin when it processes a signal.
FIXME: This should be in a cygwin include file. */
#ifndef _CYGWIN_SIGNAL_STRING
@@ -441,6 +453,7 @@ windows_delete_thread (ptid_t ptid, DWORD exit_code)
{
windows_thread_info *here = th->next;
th->next = here->next;
+ xfree (here->name);
xfree (here);
}
}
@@ -1031,10 +1044,12 @@ display_selectors (char * args, int from_tty)
host_address_to_string (\
current_event.u.Exception.ExceptionRecord.ExceptionAddress))

-static int
+static handle_exception_result
handle_exception (struct target_waitstatus *ourstatus)
{
- DWORD code = current_event.u.Exception.ExceptionRecord.ExceptionCode;
+ EXCEPTION_RECORD *rec = &current_event.u.Exception.ExceptionRecord;
+ DWORD code = rec->ExceptionCode;
+ handle_exception_result result = HANDLE_EXCEPTION_HANDLED;

ourstatus->kind = TARGET_WAITKIND_STOPPED;

@@ -1057,14 +1072,13 @@ handle_exception (struct target_waitstatus *ourstatus)
cygwin-specific-signal. So, ignore SEGVs if they show up
within the text segment of the DLL itself. */
const char *fn;
- CORE_ADDR addr = (CORE_ADDR) (uintptr_t)
- current_event.u.Exception.ExceptionRecord.ExceptionAddress;
+ CORE_ADDR addr = (CORE_ADDR) (uintptr_t) rec->ExceptionAddress;

if ((!cygwin_exceptions && (addr >= cygwin_load_start
&& addr < cygwin_load_end))
|| (find_pc_partial_function (addr, &fn, NULL, NULL)
&& startswith (fn, "KERNEL32!IsBad")))
- return 0;
+ return HANDLE_EXCEPTION_UNHANDLED;
}
#endif
break;
@@ -1140,10 +1154,46 @@ handle_exception (struct target_waitstatus *ourstatus)
DEBUG_EXCEPTION_SIMPLE ("EXCEPTION_NONCONTINUABLE_EXCEPTION");
ourstatus->value.sig = GDB_SIGNAL_ILL;
break;
+ case MS_VC_EXCEPTION:
+ if (rec->NumberParameters >= 3
+ && (rec->ExceptionInformation[0] & 0xffffffff) == 0x1000)
+ {
+ long named_thread_id;
+ windows_thread_info* named_thread;
+ CORE_ADDR thread_name_target;
+
+ DEBUG_EXCEPTION_SIMPLE ("MS_VC_EXCEPTION");
+
+ thread_name_target = rec->ExceptionInformation[1];
+ named_thread_id = (long) (0xffffffff & rec->ExceptionInformation[2]);
+
+ if (named_thread_id == (DWORD) -1)
+ named_thread_id = current_event.dwThreadId;
+
+ named_thread = thread_rec (named_thread_id, 0);
+ if (named_thread != NULL)
+ {
+ int thread_name_len;
+ char *thread_name;
+
+ thread_name_len = target_read_string (thread_name_target,
+ &thread_name, 1025, NULL);
+ if (thread_name_len > 0)
+ {
+ thread_name[thread_name_len - 1] = '\0';
+ xfree (named_thread->name);
+ named_thread->name = thread_name;
+ }
+ }
+ ourstatus->value.sig = GDB_SIGNAL_TRAP;
+ result = HANDLE_EXCEPTION_IGNORED;
+ break;
+ }
+ /* treat improperly formed exception as unknown, fallthrough */
default:
/* Treat unhandled first chance exceptions specially. */
if (current_event.u.Exception.dwFirstChance)
- return 0;
+ return HANDLE_EXCEPTION_UNHANDLED;
printf_unfiltered ("gdb: unknown target exception 0x%08x at %s\n",
(unsigned) current_event.u.Exception.ExceptionRecord.ExceptionCode,
host_address_to_string (
@@ -1153,7 +1203,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 +1560,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 HANDLE_EXCEPTION_UNHANDLED:
+ default:
+ continue_status = DBG_EXCEPTION_NOT_HANDLED;
+ break;
+ case HANDLE_EXCEPTION_HANDLED:
+ thread_id = current_event.dwThreadId;
+ break;
+ case HANDLE_EXCEPTION_IGNORED:
+ continue_status = DBG_CONTINUE;
+ break;
+ }
break;

case OUTPUT_DEBUG_STRING_EVENT: /* Message from the kernel. */
@@ -2514,6 +2573,14 @@ windows_get_ada_task_ptid (struct target_ops *self, long lwp, long thread)
return ptid_build (ptid_get_pid (inferior_ptid), 0, lwp);
}

+/* Implementation of the to_thread_name method. */
+
+static const char *
+windows_thread_name (struct target_ops *self, struct thread_info *thr)
+{
+ return thread_rec (ptid_get_tid (thr->ptid), 0)->name;
+}
+
static struct target_ops *
windows_target (void)
{
@@ -2538,6 +2605,7 @@ windows_target (void)
t->to_pid_to_exec_file = windows_pid_to_exec_file;
t->to_get_ada_task_ptid = windows_get_ada_task_ptid;
t->to_get_tib_address = windows_get_tib_address;
+ t->to_thread_name = windows_thread_name;

return t;
}
--
2.5.5