From patchwork Tue Jul 26 06:07:39 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: LRN X-Patchwork-Id: 13981 Received: (qmail 4170 invoked by alias); 26 Jul 2016 06:08:03 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 4157 invoked by uid 89); 26 Jul 2016 06:08:02 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.5 required=5.0 tests=AWL, BAYES_00, FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FROM, RCVD_IN_DNSWL_LOW, SPF_PASS autolearn=ham version=3.3.2 spammy=sk:_cygwin, 1749, 26072016, 26.07.2016 X-HELO: mail-lf0-f46.google.com Received: from mail-lf0-f46.google.com (HELO mail-lf0-f46.google.com) (209.85.215.46) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Tue, 26 Jul 2016 06:07:48 +0000 Received: by mail-lf0-f46.google.com with SMTP id l69so142770213lfg.1 for ; Mon, 25 Jul 2016 23:07:48 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:subject:from:to:references:message-id:date :user-agent:mime-version:in-reply-to; bh=IFyWcFMqghk7VPA9iwH767CsCCMRcPY3J2aBZ4VQK0M=; b=OgLJ4GDoiNFbNFE6RNyinb/KeFkMr1t7Fx8OuE9ciZqP3F1Ip6C1B30HeQ1qei0AdX CeOqdr6RbiVexXUdvMS1zuuSmBD3lQcDtkxuydctkt7ewvzlu7N665p1oAAWDtSJFTBQ wjbkfxAZWFSVw2eliol7y6ILDLQfRRw0DL2MRyZNxLNOGHU7yylrpNDXOTkJrePl3e8d jKqOgCVVnsf1upIScXM7B8Kcxro9vFt2uPLeO2kUBgto2/qzwY+IpdZ/9qnAf9niEm77 8NTFP/nEKmeHDlkjda0k70L1hXHZXoQQdlW4INvSmD4lByko8KafZdxfcqBwcH9tlEAY 4BjA== X-Gm-Message-State: AEkooutzEJ55zi9UnskNNVdfvDFY/hRw/YGo+O5MstbDu9vfdvQ65aUeZraB3A6w27jjpg== X-Received: by 10.25.81.137 with SMTP id f131mr7536017lfb.206.1469513264515; Mon, 25 Jul 2016 23:07:44 -0700 (PDT) Received: from [192.168.4.39] (broadband-95-84-200-6.nationalcablenetworks.ru. [95.84.200.6]) by smtp.gmail.com with ESMTPSA id p102sm6030266lfi.9.2016.07.25.23.07.43 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 25 Jul 2016 23:07:43 -0700 (PDT) Subject: Re: Program-assigned thread names on Windows From: LRN To: gdb-patches@sourceware.org References: <5052d495-ea40-b364-96ea-9e68c90bd747@gmail.com> <14995502.J10EtrK3xV@ralph.baldwin.cx> <6a3446f9-63dc-67a1-3702-203d77c8d85d@gmail.com> <0cabec98-8411-2c3a-98d0-3d950de02bc5@gmail.com> <28023f06-f99c-77d1-10cf-5243f2a082a4@gmail.com> Message-ID: Date: Tue, 26 Jul 2016 09:07:39 +0300 User-Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:50.0) Gecko/20100101 Thunderbird/50.0a1 MIME-Version: 1.0 In-Reply-To: <28023f06-f99c-77d1-10cf-5243f2a082a4@gmail.com> X-IsSubscribed: yes 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. 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. */