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