Program-assigned thread names on Windows
Jon Turney
jon.turney@dronecode.org.uk
Tue Jul 26 13:18:00 GMT 2016
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. */
More information about the Gdb-patches
mailing list