From patchwork Wed Aug 10 12:15:17 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Pedro Alves X-Patchwork-Id: 14467 Received: (qmail 1389 invoked by alias); 10 Aug 2016 12:15:32 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 1376 invoked by uid 89); 10 Aug 2016 12:15:31 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: =?ISO-8859-1?Q?No, score=4.3 required=5.0 tests=BAYES_50, BODY_8BITS, GARBLED_BODY, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=no version=3.3.2 spammy==d0=be=d0=b2, =d0=bb=d0=b0=d1, segment, hell?= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 10 Aug 2016 12:15:20 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 75D088111C; Wed, 10 Aug 2016 12:15:19 +0000 (UTC) Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u7ACFI5t013016; Wed, 10 Aug 2016 08:15:18 -0400 Subject: Re: Program-assigned thread names on Windows To: LRN , gdb-patches@sourceware.org References: <5052d495-ea40-b364-96ea-9e68c90bd747@gmail.com> <14995502.J10EtrK3xV@ralph.baldwin.cx> <6a3446f9-63dc-67a1-3702-203d77c8d85d@gmail.com> <0cabec98-8411-2c3a-98d0-3d950de02bc5@gmail.com> <28023f06-f99c-77d1-10cf-5243f2a082a4@gmail.com> <0e59216f-77cb-608a-aa39-578c2610eda1@dronecode.org.uk> <0f064b2b-6b51-f132-caa6-a4c1a85585a3@gmail.com> <2c9f43ec-af6f-cdaf-8e45-b251588b9b89@gmail.com> <0515957c-dfd2-b119-d423-517917e8a5cd@gmail.com> <40aaee01-35a5-9d3f-ceae-2bf4ca53a7b9@gmail.com> <83twf3md9t.fsf@gnu.org> <96ea9dc1-ce52-532d-d733-97af33bb70b4@gmail.com> From: Pedro Alves Message-ID: Date: Wed, 10 Aug 2016 13:15:17 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 MIME-Version: 1.0 In-Reply-To: <96ea9dc1-ce52-532d-d733-97af33bb70b4@gmail.com> 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 = ¤t_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?= 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 Руслан Ижбулатов * 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 = ¤t_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; }