This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Program-assigned thread names on Windows


On 08/10/2016 06:54 PM, LRN wrote:

> Curses! I've been reading your message like a set of last-minute nitpicks,
> fixing things in my patch as i went along. I got to the named_thread->name
> and overriding "thread name" part, then went ahead and fixed that too. Then
> got to the end of the email and found out that you've done that already!
> :-\ Good job, me...

:-P  Sorry about that.

> 
> Anyway, i've applied your patch. gdb compiles. threadname-setting
> functionality works as expected: "thread name X" has precedence, "thread
> name" (with no name) allows app-assigned name (if any) to be shown once again.
> 
> I've also applied my version of the patch. gdb also compiles.
> threadname-setting functionality also works as expected. Therefore i'm
> attaching my version, in case you find it useful.

I based the end result off of mine, because it included several other
minor tweaks that I didn't mention explicitly (e.g., NULL instead of
0), and because I already had it handy and nicely squashed.  :-)

I've merged your comment describing the exception name with mine though.

Your windows_thread_name implementation was doing more than necessary.
There should always be a windows_thread_info object for each thread_info
object, the simple one-liner I had should do.  Also, that method shouldn't
return struct thread_info->name; it should concern itself only with returning
the target's view of the thread name.

> 
> I've only tested 32-bit version of gdb (hoping that none of the recent
> changes touch anything architecture-dependent).

Probably not.

In the final pushed version, I did a couple more tweaks:

- I changed the thread id type to DWORD, since we're no longer using
  ptid_build with it.

- target_read_string always returns an allocated buffer, even
  if it fails to read byte 0 and thus returns 0.  So I've added:

	      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;
		}
 +	      else
 +		xfree (thread_name);

- The ChangeLog was incomplete, but I've expanded it now.  We need to
  mention the new macro, enum, and all touched functions.

Below's what I pushed.

Maybe I could convince you to implement this in gdbserver/win32-low.c too?

--------------
>From 24cdb46e9f0a694b4fbc11085e094857f08c0419 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 19:22:45 +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 new
HANDLE_EXCEPTION_IGNORED value means that gdb should just continue,
without returning the 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>
	    Pedro Alves  <palves@redhat.com>

	* windows-nat.c (MS_VC_EXCEPTION): New define.
	(handle_exception_result): New enum.
	(windows_delete_thread): Free the thread's name.
	(handle_exception): Handle MS_VC_EXCEPTION.
	(get_windows_debug_event): Handle HANDLE_EXCEPTION_IGNORED.
	(windows_thread_name): New function.
	(windows_target): Install it as to_thread_name method.
	* NEWS: Mention the thread naming support on MS-Windows.
---
 gdb/ChangeLog     | 12 +++++++
 gdb/NEWS          |  6 ++++
 gdb/windows-nat.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++-------
 3 files changed, 100 insertions(+), 11 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index a1865f9..843da83 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,15 @@
+2016-08-10  Руслан Ижбулатов  <lrn1986@gmail.com>
+	    Pedro Alves  <palves@redhat.com>
+
+	* windows-nat.c (MS_VC_EXCEPTION): New define.
+	(handle_exception_result): New enum.
+	(windows_delete_thread): Free the thread's name.
+	(handle_exception): Handle MS_VC_EXCEPTION.
+	(get_windows_debug_event): Handle HANDLE_EXCEPTION_IGNORED.
+	(windows_thread_name): New function.
+	(windows_target): Install it as to_thread_name method.
+	* NEWS: Mention the thread naming support on MS-Windows.
+
 2016-08-10  Pedro Alves  <palves@redhat.com>
 
 	* common/signals-state-save-restore.c
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..0470f31 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -174,6 +174,19 @@ static int debug_registers_used;
 static int windows_initialization_done;
 #define DR6_CLEAR_VALUE 0xffff0ff0
 
+/* The exception thrown by a program to tell the debugger the name of
+   a thread.  The exception record contains an ID of a thread and a
+   name to give it.  This exception has no documented name, but MSDN
+   dubs it "MS_VC_EXCEPTION" in one code example.  */
+#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 +454,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 +1045,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 +1073,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 +1155,48 @@ 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)
+	{
+	  DWORD 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 = (DWORD) (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;
+		}
+	      else
+		xfree (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 +1206,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 +1563,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 +2576,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 +2608,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



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]