Program-assigned thread names on Windows

Program-assigned thread names on Windows

[LRN]

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

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/

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

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

diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 3f67486..01e7954 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,38 @@ 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)
+	      || !thread_name || !*thread_name)
+	    /* nothing to do */;
+	  else
+	    {
+	      xfree (named_thread->name);
+	      named_thread->name = thread_name;
+	    }
+	  result = 2;
+	}
+      break;
     default:
       /* Treat unhandled first chance exceptions specially.  */
       if (current_event.u.Exception.dwFirstChance)
@@ -1153,7 +1189,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 +1546,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: 0002-Add-the-thread-naming-support-to-NEWS-file.patch --]
[-- Type: text/plain, Size: 815 bytes --]

From d5b4daafe8ee23ef69b47048aa68b9a2542ee740 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: Sat, 23 Jul 2016 08:59:05 +0000
Subject: [PATCH 2/3] Add the thread naming support to NEWS file

---
 gdb/NEWS | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/gdb/NEWS b/gdb/NEWS
index 0e339dd..b26df2f 100644
--- a/gdb/NEWS
+++ b/gdb/NEWS
@@ -60,6 +60,12 @@
   for its own control and synchronization, invisible to the command
   line.
 
+* 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.
+
 * New commands
 
 skip -file file
-- 
2.4.0


[-- Attachment #1.1.4: 0003-Add-a-ChangeLog-entry-for-thread-naming-on-MS-Window.patch --]
[-- Type: text/plain, Size: 848 bytes --]

From 4e7e912d84620950e249c0f10a00e0aad49f7f50 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: Sat, 23 Jul 2016 09:00:38 +0000
Subject: [PATCH 3/3] Add a ChangeLog entry for thread naming on MS-Windows

---
 gdb/ChangeLog | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 0f4a8b6..bd09cb5 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2016-07-23  Руслан Ижбулатов  <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.
+
 2016-06-30  Руслан Ижбулатов  <lrn1986@gmail.com>
 
 	PR gdb/14529
-- 
2.4.0


[-- Attachment #1.1.5: 0x6759BA74.asc --]
[-- Type: application/pgp-keys, Size: 3540 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Program-assigned thread names on Windows

[Eli Zaretskii]

> From: LRN <lrn1986@gmail.com>
> Date: Sat, 23 Jul 2016 12:25:15 +0300
> 
> 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/

Thanks.  But I don't think what that means in terms of the "thread
name", "thread find", and "info threads" commands in GDB.  Can you
tell?

Re: Program-assigned thread names on Windows

[LRN]

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

On 23.07.2016 12:33, Eli Zaretskii wrote:
> On 23.07.2016 12:25, 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/
>> 
> 
> Thanks.  But I don't think what that means in terms of the "thread
> name", "thread find", and "info threads" commands in GDB.  Can you
> tell?
> 

"info thread" will show the thread name, if it is set

"thread name" will change the thread name (but the debugee will not be
aware of that; i haven't looked for a way to communicate name change
back to the debugee, and i doubt that such way exists)

"thread find" will be able to find threads by their name, if they have it set

-- 
O< ascii ribbon - stop html email! - www.asciiribbon.org

[-- Attachment #1.1.2: 0x6759BA74.asc --]
[-- Type: application/pgp-keys, Size: 3540 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Program-assigned thread names on Windows

[Eli Zaretskii]

> From: LRN <lrn1986@gmail.com>
> Date: Sat, 23 Jul 2016 12:43:05 +0300
> 
> "info thread" will show the thread name, if it is set
> 
> "thread name" will change the thread name (but the debugee will not be
> aware of that; i haven't looked for a way to communicate name change
> back to the debugee, and i doubt that such way exists)
> 
> "thread find" will be able to find threads by their name, if they have it set

Sounds useful, thanks.

Re: Program-assigned thread names on Windows

[John Baldwin]

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

-- 
John Baldwin

Re: Program-assigned thread names on Windows

[LRN]

[-- 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 --]
[-- Type: application/pgp-keys, Size: 3540 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Program-assigned thread names on Windows

[Jon Turney]

On 23/07/2016 18:01, LRN wrote:
> 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.

Nice.  A few comments below.

>>> >>
>>> >> 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/
>> >
>
> This is done by catching an exception number 0x406D1388
> (it has no documented name), which is thrown by the program.

The name used in the MSDN article [1] is 'MS_VC_EXCEPTION', so why not 
use that?

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

Shouldn't this check for ExceptionInformation[0] = 0x1000, and treat 
this as an unknown exception otherwise?

> +	  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?

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

Does thread_name end up not being null terminated if the thread name 
length really exceeds 1024? (or is improperly not null terminated in the 
target process...)

> +	    {
> +	      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
>

Re: Program-assigned thread names on Windows

[LRN]

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

On 25.07.2016 15:17, Jon Turney wrote:
> On 23/07/2016 18:01, LRN wrote:
>> On 23.07.2016 19:39, John Baldwin wrote:
>>>> On Saturday, July 23, 2016 12:25:15 PM LRN wrote:
>>>>>>
>>>>>> 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/
>>>>
>>
>> This is done by catching an exception number 0x406D1388
>> (it has no documented name), which is thrown by the program.
> 
> The name used in the MSDN article [1] is 'MS_VC_EXCEPTION', so why not 
> use that?

No reason. If you want, run sed -e
's/WINDOWS_THREADNAME_EXCEPTION/MS_VC_EXCEPTION' over the patch file prior
to committing it.

That said, i think "MS_VC_EXCEPTION" does not offer a good enough
description (doesn't mention threads, does mention VisualC).

> 
>> +    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;
>> +
> 
> Shouldn't this check for ExceptionInformation[0] = 0x1000, and treat 
> this as an unknown exception otherwise?

Yes, it should.

> 
>> +	  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?

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).

On the other hand, the example code for throwing the exception uses 32-bit
DWORD fields explicitly. I don't know what the OS does between the
exception being thrown and given to gdb.

I'll try to use i686 gdb to debug an x86_64 process, but the reverse would
be difficult, as i lack an established buildsystem for building x86_64 gdb.

EXCEPTION_RECORD layout aside, casting thread ID into 32-bit DWORD should
be OK, because thread IDs are 32-bit even on 64-bit Windows.

Casting thread_name_target to uintptr_t is less clear. On one hand, it
could be x86_64 pointer in debugee address space. On the other hand, there
are some calls in windows-nat.c (to WriteProcessMemory(), for example) that
do this kind of casting. Most likely the correct way to do this is to cast
it to CORE_ADDR...

This will most likely produce extra warnings after EXCEPTION_RECORD ->
EXCEPTION_RECORD32/64 change, i'll see what gcc has to say about this.

> 
>> +
>> +	  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))
> 
> Does thread_name end up not being null terminated if the thread name 
> length really exceeds 1024? (or is improperly not null terminated in the 
> target process...)

Good point. I think it's best to check the last byte in the string to be 0,
using the value returned by target_read_string(), and set it to 0 if it
isn't. Give it 1025 instead of 1024 as the maximum (although either is
arbitrary, really).


-- 
O< ascii ribbon - stop html email! - www.asciiribbon.org

[-- Attachment #1.1.2: 0x6759BA74.asc --]
[-- Type: application/pgp-keys, Size: 3540 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Program-assigned thread names on Windows

[Jon Turney]

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:
>>> On 23.07.2016 19:39, John Baldwin wrote:
>>>>> On Saturday, July 23, 2016 12:25:15 PM LRN wrote:
>>>>>>>
>>>>>>> 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/
>>>>>
>>>
>>> This is done by catching an exception number 0x406D1388
>>> (it has no documented name), which is thrown by the program.
>>
>> The name used in the MSDN article [1] is 'MS_VC_EXCEPTION', so why not
>> use that?
>
> No reason. If you want, run sed -e
> 's/WINDOWS_THREADNAME_EXCEPTION/MS_VC_EXCEPTION' over the patch file prior
> to committing it.
>
> That said, i think "MS_VC_EXCEPTION" does not offer a good enough
> description (doesn't mention threads, does mention VisualC).

There might be current or future uses of this exception with type other 
than 0x1000 which aren't related to threads at all.

>>> +    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;
>>> +
>>
>> Shouldn't this check for ExceptionInformation[0] = 0x1000, and treat
>> this as an unknown exception otherwise?
>
> Yes, it should.
>
>>
>>> +	  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)

Re: Program-assigned thread names on Windows

[LRN]

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

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.

-- 
O< ascii ribbon - stop html email! - www.asciiribbon.org

[-- Attachment #1.1.2: 0x6759BA74.asc --]
[-- Type: application/pgp-keys, Size: 3540 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Program-assigned thread names on Windows

[LRN]

[-- 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 --]
[-- Type: application/pgp-keys, Size: 3540 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Program-assigned thread names on Windows

[Jon Turney]

On 26/07/2016 07:07, LRN wrote:
> 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:

>> > 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.

Ah yes, I see.

I was thrown off by your references [2], [3], which compute 
nNumberOfArguments for RaiseException() as sizeof (info) / sizeof 
(DWORD), which I think is incorrect on 64-bit, and should be 
sizeof(info) / sizeof(ULONG_PTR), as the MSDN example code has.

>> > 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

I think this is a bug in the code you are testing with, as mentioned 
above, which doubles nNumberOfArguments ...

>> > 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.

It seems on x86_64, the structure is laid out by gcc as:

4 DWORD dwType
4 padding
8 LPCSTR szName
4 DWORD dwThreadID
4 DWORD dwFlags

total size 24, so nNumberOfArguments = 3, so this is passed to the 
debugger as an array of 3 DWORD64s

Of course, the 'correct' layout is defined by how the sample code is 
laid out by MSVC, which I'm guessing is the same, but haven't checked...

So dwThreadID and dwFlags get packed together into 
ExceptionInformation[2].  Fortunately, dwFlags should be set to 0.

Furthermore, accessing dwType as a DWORD64 value via 
ExceptionInformation[0] relies on the padding being zero initialized in 
the debugee to have useful values! I guess you'll have to mask with 0xffff?

> 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.

Great, thanks.  A few minor comments below.

> 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
>
>
> 0001-Support-settings-thread-name-MS-Windows.patch
>
>
> 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;

Since this holds a Win32 thread id, should it be DWORD type?

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

	  DEBUG_EXCEPTION_SIMPLE ("MS_VC_EXCEPTION"); ?

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

I'd just null-terminate unconditionally.

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

Would it be clearer to use an enum to name these return cases from 
handle_exception()?

> +	{
> +	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.  */

Re: Program-assigned thread names on Windows

[LRN]

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

On 26.07.2016 16:18, Jon Turney wrote:
> On 26/07/2016 07:07, LRN wrote:
>> 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:
> 
>>>> 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.
> 
> Ah yes, I see.
> 
> I was thrown off by your references [2], [3], which compute 
> nNumberOfArguments for RaiseException() as sizeof (info) / sizeof 
> (DWORD), which I think is incorrect on 64-bit, and should be 
> sizeof(info) / sizeof(ULONG_PTR), as the MSDN example code has.

Ah, that must be where '6' came from. Indeed, sizeof (THEADNAME_INFO) is
[4 +4padding] + [8] + [4 + 4] = 24.
24 / 4 = 6.

Also, i stand corrected. I've claimed that the array is realigned when it
passes the 32-bit/64-bit barrier, and it is. However, there's no such thing
when 64-bit process throws an exception and 64-bit debugger caches it. So,
because, as i've shown above, not all fields are 8-byte aligned (first two
fields are aligned, because one of them is a self-aligned pointer, but the
last two fields are packed together into one 8-byte slot), it can look
weird when ExceptionInformation[] is interpreted as an array of
pointer-sized values.

This is concerning, as i want the code that throws the exception to be
compatible with MSVS and WinDbg, and for gdb to support *that* version.

> 
>>>> 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
> 
> I think this is a bug in the code you are testing with, as mentioned 
> above, which doubles nNumberOfArguments ...

Yep.

> 
>>>> 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.
> 
> It seems on x86_64, the structure is laid out by gcc as:
> 
> 4 DWORD dwType
> 4 padding
> 8 LPCSTR szName
> 4 DWORD dwThreadID
> 4 DWORD dwFlags
> 
> total size 24, so nNumberOfArguments = 3, so this is passed to the 
> debugger as an array of 3 DWORD64s
> 
> Of course, the 'correct' layout is defined by how the sample code is 
> laid out by MSVC, which I'm guessing is the same, but haven't checked...
> 
> So dwThreadID and dwFlags get packed together into 
> ExceptionInformation[2].  Fortunately, dwFlags should be set to 0.
> 
> Furthermore, accessing dwType as a DWORD64 value via 
> ExceptionInformation[0] relies on the padding being zero initialized in 
> the debugee to have useful values! I guess you'll have to mask with 0xffff?

I'll play a bit with the 64-bit exception-throwing example and see how
WinDbg reacts to various combinations of alignment and argument counting,
and will make gdb support the layout that WinDbg supports.

>> +	{
>> +	  long named_thread_id;
> 
> Since this holds a Win32 thread id, should it be DWORD type?

I've changed it into long, because long is what ptid_build() takes. If
there's some kind of typecast going on, it'll happen when we assign things
to named_thread_id, not when we pass them to ptid_build().

>> +	  DEBUG_EXCEPTION_SIMPLE (MS_VC_EXCEPTION_S);
>> +
> 
> 	  DEBUG_EXCEPTION_SIMPLE ("MS_VC_EXCEPTION"); ?

I would actually prefer:
DEBUG_EXCEPTION_SIMPLE (STRINGIFY(MS_VC_EXCEPTION))
, but i don't know if gdb has a stringifying macro somewhere, and haven't
bothered to look. As i've said previously, MS_VC_EXCEPTION is not a
fully-documented name. It certainly is not in any SDK. But if you *want* to
show "MS_VC_EXCEPTION", that's easily done, obviously.

>> +	  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';
> 
> I'd just null-terminate unconditionally.

Okay.

>> @@ -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))
> 
> Would it be clearer to use an enum to name these return cases from 
> handle_exception()?

It would be. Where should id put its definition and how do i name it and
its values?

-- 
O< ascii ribbon - stop html email! - www.asciiribbon.org

[-- Attachment #1.1.2: 0x6759BA74.asc --]
[-- Type: application/pgp-keys, Size: 3540 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Program-assigned thread names on Windows

[LRN]

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

On 26.07.2016 17:17, LRN wrote:
> On 26.07.2016 16:18, Jon Turney wrote:
>> On 26/07/2016 07:07, LRN wrote:
>>> 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:
>>>>> 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.
>>
>> It seems on x86_64, the structure is laid out by gcc as:
>>
>> 4 DWORD dwType
>> 4 padding
>> 8 LPCSTR szName
>> 4 DWORD dwThreadID
>> 4 DWORD dwFlags
>>
>> total size 24, so nNumberOfArguments = 3, so this is passed to the 
>> debugger as an array of 3 DWORD64s
>>
>> Of course, the 'correct' layout is defined by how the sample code is 
>> laid out by MSVC, which I'm guessing is the same, but haven't checked...
>>
>> So dwThreadID and dwFlags get packed together into 
>> ExceptionInformation[2].  Fortunately, dwFlags should be set to 0.
>>
>> Furthermore, accessing dwType as a DWORD64 value via 
>> ExceptionInformation[0] relies on the padding being zero initialized in 
>> the debugee to have useful values! I guess you'll have to mask with 0xffff?
> 
> I'll play a bit with the 64-bit exception-throwing example and see how
> WinDbg reacts to various combinations of alignment and argument counting,
> and will make gdb support the layout that WinDbg supports.

Played around with 64-bit WinDbg.
It worked with the code that i've used originally (from MSDN with no
significant changes). Also:

1) WinDbg (of either bitness) doesn't care what the argument count is, as
long as it's at least 3 (0x1000, string pointer and thread ID); giving it 2
makes it silently drop the exception and not set the thread name

2) WinDbg (of either bitness) currently doesn't care what you put in
dwFlags. I've tried filling dwFlags with garbage (a copy of the dwThreadID
value, for example), and WinDbg still set the thread name correctly,
without misidentifying the thread.
This leads me to believe that, as you've suggested, 64-bit WinDbg does &
0x00000000FFFFFFFF on ExceptionInformation[2] (32-bit WinDbg doesn't have to).

Also of note, 32-bit WinDbg can't debug 64-bit executables, but 64-bit
WinDbg can debug 32-bit executables.

Maybe they foresaw the use of 64-bit architectures (i can't dig deeper into
the MSDN than MSVC 2003, not sure how the thread-name example looked in
MSVC 6.0 era) and padded the struct size to be a multiple of 8, reserving
the last DWORD for future use; later realized that due to struct packing a
64-bit debugger would get 3 64-bit pointer-sized values, with the last one
being a combination of threadid and flags, and adapted their debugger to
handle exactly this case.

Anyway, for gdb:
1) Look for at least 3 arguments.
2) In 64-bit gdb do the & 0xFFFFFFFF on the 3rd member to get thread id.

And that's it.

-- 
O< ascii ribbon - stop html email! - www.asciiribbon.org

[-- Attachment #1.1.2: 0x6759BA74.asc --]
[-- Type: application/pgp-keys, Size: 3540 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Program-assigned thread names on Windows

[LRN]

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

On 26.07.2016 18:40, LRN wrote:
> On 26.07.2016 17:17, LRN wrote:
>> On 26.07.2016 16:18, Jon Turney wrote:
>>> On 26/07/2016 07:07, LRN wrote:
>>>> 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:
>>>>>> 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.
>>>
>>> It seems on x86_64, the structure is laid out by gcc as:
>>>
>>> 4 DWORD dwType
>>> 4 padding
>>> 8 LPCSTR szName
>>> 4 DWORD dwThreadID
>>> 4 DWORD dwFlags
>>>
>>> total size 24, so nNumberOfArguments = 3, so this is passed to the 
>>> debugger as an array of 3 DWORD64s
>>>
>>> Of course, the 'correct' layout is defined by how the sample code is 
>>> laid out by MSVC, which I'm guessing is the same, but haven't checked...
>>>
>>> So dwThreadID and dwFlags get packed together into 
>>> ExceptionInformation[2].  Fortunately, dwFlags should be set to 0.
>>>
>>> Furthermore, accessing dwType as a DWORD64 value via 
>>> ExceptionInformation[0] relies on the padding being zero initialized in 
>>> the debugee to have useful values! I guess you'll have to mask with 0xffff?
>>
>> I'll play a bit with the 64-bit exception-throwing example and see how
>> WinDbg reacts to various combinations of alignment and argument counting,
>> and will make gdb support the layout that WinDbg supports.
> 
> Played around with 64-bit WinDbg.
> It worked with the code that i've used originally (from MSDN with no
> significant changes). Also:
> 
> 1) WinDbg (of either bitness) doesn't care what the argument count is, as
> long as it's at least 3 (0x1000, string pointer and thread ID); giving it 2
> makes it silently drop the exception and not set the thread name
> 
> 2) WinDbg (of either bitness) currently doesn't care what you put in
> dwFlags. I've tried filling dwFlags with garbage (a copy of the dwThreadID
> value, for example), and WinDbg still set the thread name correctly,
> without misidentifying the thread.
> This leads me to believe that, as you've suggested, 64-bit WinDbg does &
> 0x00000000FFFFFFFF on ExceptionInformation[2] (32-bit WinDbg doesn't have to).
> 
> Also of note, 32-bit WinDbg can't debug 64-bit executables, but 64-bit
> WinDbg can debug 32-bit executables.
> 
> Maybe they foresaw the use of 64-bit architectures (i can't dig deeper into
> the MSDN than MSVC 2003, not sure how the thread-name example looked in
> MSVC 6.0 era) and padded the struct size to be a multiple of 8, reserving
> the last DWORD for future use; later realized that due to struct packing a
> 64-bit debugger would get 3 64-bit pointer-sized values, with the last one
> being a combination of threadid and flags, and adapted their debugger to
> handle exactly this case.
> 
> Anyway, for gdb:
> 1) Look for at least 3 arguments.
> 2) In 64-bit gdb do the & 0xFFFFFFFF on the 3rd member to get thread id.
> 
> And that's it.
> 

Attached is the last (hopefully) iteration of the patch.

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

From 50ee2eee310c3c19bea520e384564762e9172f35 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, 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 will result
in further handling of the exception, which is not what we want).
---
 gdb/windows-nat.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 69 insertions(+), 8 deletions(-)

diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 3f67486..82630db 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -174,6 +174,15 @@ static int debug_registers_used;
 static int windows_initialization_done;
 #define DR6_CLEAR_VALUE 0xffff0ff0
 
+#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
@@ -1031,10 +1040,11 @@ 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;
+  handle_exception_result result = HANDLE_EXCEPTION_HANDLED;
 
   ourstatus->kind = TARGET_WAITKIND_STOPPED;
 
@@ -1064,7 +1074,7 @@ handle_exception (struct target_waitstatus *ourstatus)
 				    && 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 +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];
+
+	  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)
+	    {
+	      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 = 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 +1205,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 +1562,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.  */
-- 
2.4.0


[-- Attachment #1.1.3: 0x6759BA74.asc --]
[-- Type: application/pgp-keys, Size: 3540 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Program-assigned thread names on Windows

[Jon Turney]

On 26/07/2016 18:15, LRN wrote:
> Attached is the last (hopefully) iteration of the patch.
>
> 0001-Support-settings-thread-name-MS-Windows.patch

Thanks.  That addresses all my comments.

Re: Program-assigned thread names on Windows

[Jon Turney]

Doing a bit of testing with this patch...

On 26/07/2016 18:15, LRN wrote:
> +	  named_thread = find_thread_ptid (named_thread_ptid);

... it seems this may return NULL (e.g. if the thread we are naming was 
short-lived and has already exited, or if the thread id was invalid) ...

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

... so this becomes a null dereference.

> +		  named_thread->name = thread_name;
> +		}
> +	      else
> +		{
> +		  xfree (thread_name);
> +		}

Re: Program-assigned thread names on Windows

[LRN]

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

On 28.07.2016 0:35, Jon Turney wrote:
> 
> Doing a bit of testing with this patch...
> 
> On 26/07/2016 18:15, LRN wrote:
>> +	  named_thread = find_thread_ptid (named_thread_ptid);
> 
> ... it seems this may return NULL (e.g. if the thread we are naming was 
> short-lived and has already exited, or if the thread id was invalid) ...
> 
>> +
>> +	  thread_name = NULL;
>> +	  thread_name_len = target_read_string (thread_name_target, &thread_name, 1025, 0);
>> +	  if (thread_name_len > 0 && thread_name != NULL)
>> +	    {
>> +	      thread_name[thread_name_len - 1] = '\0';
>> +	      if (thread_name[0] != '\0')
>> +		{
>> +		  xfree (named_thread->name);
> 
> ... so this becomes a null dereference.
> 
>> +		  named_thread->name = thread_name;
>> +		}
>> +	      else
>> +		{
>> +		  xfree (thread_name);
>> +		}
> 

True.

The simplest fix for this is to turn the check
> thread_name[0] != '\0'
into
> thread_name[0] != '\0' && named_thread != NULL


-- 
O< ascii ribbon - stop html email! - www.asciiribbon.org

[-- Attachment #1.1.2: 0x6759BA74.asc --]
[-- Type: application/pgp-keys, Size: 3540 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Program-assigned thread names on Windows

[LRN]

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

On 28.07.2016 10:21, LRN wrote:
> On 28.07.2016 0:35, Jon Turney wrote:
>>
>> Doing a bit of testing with this patch...
>>
>> On 26/07/2016 18:15, LRN wrote:
>>> +	  named_thread = find_thread_ptid (named_thread_ptid);
>>
>> ... it seems this may return NULL (e.g. if the thread we are naming was 
>> short-lived and has already exited, or if the thread id was invalid) ...
>>
>>> +
>>> +	  thread_name = NULL;
>>> +	  thread_name_len = target_read_string (thread_name_target, &thread_name, 1025, 0);
>>> +	  if (thread_name_len > 0 && thread_name != NULL)
>>> +	    {
>>> +	      thread_name[thread_name_len - 1] = '\0';
>>> +	      if (thread_name[0] != '\0')
>>> +		{
>>> +		  xfree (named_thread->name);
>>
>> ... so this becomes a null dereference.
>>
>>> +		  named_thread->name = thread_name;
>>> +		}
>>> +	      else
>>> +		{
>>> +		  xfree (thread_name);
>>> +		}
>>
> 
> True.
> 
> The simplest fix for this is to turn the check
>> thread_name[0] != '\0'
> into
>> thread_name[0] != '\0' && named_thread != NULL
> 
> 

So, what happens now?

-- 
O< ascii ribbon - stop html email! - www.asciiribbon.org

[-- Attachment #1.1.2: 0x6759BA74.asc --]
[-- Type: application/pgp-keys, Size: 3540 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Program-assigned thread names on Windows

[Eli Zaretskii]

> From: LRN <lrn1986@gmail.com>
> Date: Tue, 2 Aug 2016 12:46:47 +0300
> 
> So, what happens now?

If no one objects in a while, we push.

Re: Program-assigned thread names on Windows

[LRN]

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

On 02.08.2016 17:55, Eli Zaretskii wrote:
>> From: LRN
>> Date: Tue, 2 Aug 2016 12:46:47 +0300
>>
>> So, what happens now?
> 
> If no one objects in a while, we push.
> 

No one seems to object.

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.

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

From 7a4fac770864393b1610b6bccc2e190620e4543e 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, 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 will result
in further handling of the exception, which is not what we want).
---
 gdb/windows-nat.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 69 insertions(+), 8 deletions(-)

diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 3f67486..8917997 100644
--- a/gdb/windows-nat.c
+++ b/gdb/windows-nat.c
@@ -174,6 +174,15 @@ static int debug_registers_used;
 static int windows_initialization_done;
 #define DR6_CLEAR_VALUE 0xffff0ff0
 
+#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
@@ -1031,10 +1040,11 @@ 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;
+  handle_exception_result result = HANDLE_EXCEPTION_HANDLED;
 
   ourstatus->kind = TARGET_WAITKIND_STOPPED;
 
@@ -1064,7 +1074,7 @@ handle_exception (struct target_waitstatus *ourstatus)
 				    && 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 +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];
+
+	  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)
+	    {
+	      thread_name[thread_name_len - 1] = '\0';
+	      if (thread_name[0] != '\0' && named_thread != NULL)
+		{
+		  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 +1205,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 +1562,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.  */
-- 
2.4.0


[-- Attachment #1.1.3: 0002-Add-the-thread-naming-support-to-NEWS-file.patch --]
[-- Type: text/plain, Size: 856 bytes --]

From 36200e518eb3645b476ef71d9c897369a9015edb 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: Sat, 23 Jul 2016 08:59:05 +0000
Subject: [PATCH 2/3] Add the thread naming support to NEWS file

---
 gdb/NEWS | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/gdb/NEWS b/gdb/NEWS
index b08d8a0..ec813f2 100644
--- 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.
+
 * New commands
 
 skip -file file
-- 
2.4.0


[-- Attachment #1.1.4: 0003-Add-a-ChangeLog-entry-for-thread-naming-on-MS-Window.patch --]
[-- Type: text/plain, Size: 828 bytes --]

From f8908dfdc6326ac13ddaed290d3bef336d003b7d 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: Sat, 23 Jul 2016 09:00:38 +0000
Subject: [PATCH 3/3] Add a ChangeLog entry for thread naming on MS-Windows

---
 gdb/ChangeLog | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 17c799e..d41d326 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+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.
+
 2016-08-09  Pedro Alves  <palves@redhat.com>
 
 	PR gdb/20418
-- 
2.4.0


[-- Attachment #1.1.5: 0x6759BA74.asc --]
[-- Type: application/pgp-keys, Size: 3540 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Program-assigned thread names on Windows

[Pedro Alves]

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

Re: Program-assigned thread names on Windows

[LRN]

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

On 10.08.2016 15:15, Pedro Alves wrote:
> On 08/10/2016 08:11 AM, LRN wrote:
>> No one seems to object.
>>
> 
> 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.


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...

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've only tested 32-bit version of gdb (hoping that none of the recent
changes touch anything architecture-dependent).

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

From 34c05e6eb6e9d94700f8dc9695a214fdf6fabaea 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, 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 will result
in further handling of the exception, which is not what we want).
---
 gdb/windows-nat.c | 121 +++++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 107 insertions(+), 14 deletions(-)

diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c
index 3f67486..6315303 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
 
+/* This exception has no documented name, but MSDN dubs it "MS_VC_EXCEPTION"
+   in one code example. It's thrown by a program to tell the debugger the name
+   of a thread. */
+#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,20 +1154,64 @@ 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_windows_thread;
+	  CORE_ADDR thread_name_target;
+	  char *thread_name;
+	  int thread_name_len;
+
+	  DEBUG_EXCEPTION_SIMPLE ("MS_VC_EXCEPTION");
+
+	  named_thread_id = (long) (0xffffffff & rec->ExceptionInformation[2]);
+	  thread_name_target = rec->ExceptionInformation[1];
+
+	  if (named_thread_id == (DWORD) -1)
+	    named_thread_id = current_event.dwThreadId;
+
+	  named_windows_thread = thread_rec (named_thread_id, 0);
+
+	  thread_name = NULL;
+	  thread_name_len = 0;
+
+	  if (named_windows_thread != NULL)
+	    thread_name_len = target_read_string (thread_name_target,
+					          &thread_name, 1025, 0);
+
+	  if (thread_name_len > 0 && thread_name != NULL)
+	    {
+	      thread_name[thread_name_len - 1] = '\0';
+	      if (thread_name[0] != '\0')
+		{
+		  xfree (named_windows_thread->name);
+		  named_windows_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 (
-	  current_event.u.Exception.ExceptionRecord.ExceptionAddress));
+	(unsigned) rec->ExceptionCode,
+	host_address_to_string (rec->ExceptionAddress));
       ourstatus->value.sig = GDB_SIGNAL_UNKNOWN;
       break;
     }
   exception_count++;
   last_sig = ourstatus->value.sig;
-  return 1;
+  return result;
 }
 
 /* Resume thread specified by ID, or all artificially suspended
@@ -1510,10 +1568,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 +2581,31 @@ windows_get_ada_task_ptid (struct target_ops *self, long lwp, long thread)
   return ptid_build (ptid_get_pid (inferior_ptid), 0, lwp);
 }
 
+static const char *
+windows_get_thread_name (struct target_ops *self, struct thread_info *thr)
+{
+  struct thread_info *threadinfo;
+  windows_thread_info *windows_threadinfo;
+  DWORD id = ptid_get_tid (thr->ptid);
+  char *name;
+
+  name = NULL;
+  threadinfo = find_thread_ptid (thr->ptid);
+
+  if (threadinfo != NULL)
+    name = threadinfo->name;
+
+  if (name == NULL && id != 0)
+    {
+      windows_threadinfo = thread_rec (id, 0);
+
+      if (windows_threadinfo != NULL)
+        name = windows_threadinfo->name;
+    }
+
+  return (const char *) name;
+}
+
 static struct target_ops *
 windows_target (void)
 {
@@ -2538,6 +2630,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_get_thread_name;
 
   return t;
 }
-- 
2.4.0


[-- Attachment #1.1.3: 0x6759BA74.asc --]
[-- Type: application/pgp-keys, Size: 3540 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Program-assigned thread names on Windows

[Pedro Alves]

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

Re: Program-assigned thread names on Windows

[LRN]

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

On 10.08.2016 21:45, Pedro Alves wrote:
> On 08/10/2016 06:54 PM, LRN wrote:
> 
> Maybe I could convince you to implement this in gdbserver/win32-low.c too?

You can't. I've never used gdbserver, and i don't see any need for me to
start using it. And since i don't use it, i wouldn't be able to test
anything code changes i could introduce there.

TBH, i don't have any plans for further contributions to gdb. There is only
one more feature that i thought could be added (using Windows API to obtain
stack traces, enhancing the built-in gdb stack walker; thus gdb would be
able to show useful stack trace even while being deep inside W32 API
calls), but the code was so complicated for me that i've failed to even
*find* where to start.

-- 
O< ascii ribbon - stop html email! - www.asciiribbon.org

[-- Attachment #1.1.2: 0x6759BA74.asc --]
[-- Type: application/pgp-keys, Size: 3540 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Program-assigned thread names on Windows

[Pedro Alves]

On 08/11/2016 12:42 AM, LRN wrote:
> On 10.08.2016 21:45, Pedro Alves wrote:
>> On 08/10/2016 06:54 PM, LRN wrote:
>>
>> Maybe I could convince you to implement this in gdbserver/win32-low.c too?
> 
> You can't. I've never used gdbserver, and i don't see any need for me to
> start using it. And since i don't use it, i wouldn't be able to test
> anything code changes i could introduce there.

It's as simple as:

shell1:

$ gdbserver :9999 testprog.exe

shell2:

$ gdb testprog.exe -ex "tar rem :9999" -ex "b main" -ex "c"

> 
> TBH, i don't have any plans for further contributions to gdb. There is only
> one more feature that i thought could be added (using Windows API to obtain
> stack traces, enhancing the built-in gdb stack walker; thus gdb would be
> able to show useful stack trace even while being deep inside W32 API
> calls), but the code was so complicated for me that i've failed to even
> *find* where to start.
> 

Sounds like you'd need to write a new frame unwinder.
Grep for "const struct frame_unwind .* =" to find the
many instances.  

The IA64 one, in ia64-libunwind-tdep.c / ia64-tdep.c, is
probably the closest, as that uses an external library
(libunwind) to do the heavy lifting.

Our Windows x64 SEH unwinder is in amd64-windows-tdep.c.

I think I read that you're on 32-bit, though, right?

An unwinder independent of the system one is better for
being usable when cross debugging as well.  I wonder
whether between Microsoft's recent opening of the PDB format,
and what the community might have learned in the past
years itself, there's enough to avoid relying on MS APIs.
I'd go look at Wine's implementation of said APIs too.

Thanks,
Pedro Alves