This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |
Other format: | [Raw text] |
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:
0x6759BA74.asc
Description: application/pgp-keys
Attachment:
signature.asc
Description: OpenPGP digital signature
Index Nav: | [Date Index] [Subject Index] [Author Index] [Thread Index] | |
---|---|---|
Message Nav: | [Date Prev] [Date Next] | [Thread Prev] [Thread Next] |