Message ID | 20200117153140.2231-1-ssbssa@yahoo.de |
---|---|
State | New |
Headers | show |
On 2020-01-17 10:31 a.m., Hannes Domani via gdb-patches wrote: > diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c > index 901e64263c..824ff4b322 100644 > --- a/gdb/windows-nat.c > +++ b/gdb/windows-nat.c > @@ -236,6 +236,7 @@ static DEBUG_EVENT current_event; /* The current debug event from > WaitForDebugEvent */ > static HANDLE current_process_handle; /* Currently executing process */ > static windows_thread_info *current_thread; /* Info on currently selected thread */ > +static EXCEPTION_RECORD siginfo_er; /* Contents of $_siginfo */ Huh... I was going to say that it shouldn't be a global variable, but a per-inferior thing (or is it per-thread?), but pretty much all the state is already global... so I guess it's fine. I gather that the windows-nat does not support debugging multiple inferiors? Same for win32-low in gdbserver? > > /* Counts of things. */ > static int exception_count = 0; > @@ -1166,6 +1167,8 @@ handle_exception (struct target_waitstatus *ourstatus) > DWORD code = rec->ExceptionCode; > handle_exception_result result = HANDLE_EXCEPTION_HANDLED; > > + memcpy (&siginfo_er, rec, sizeof siginfo_er); > + > ourstatus->kind = TARGET_WAITKIND_STOPPED; > > /* Record the context of the current thread. */ > @@ -2862,6 +2865,7 @@ windows_nat_target::mourn_inferior () > CHECK (CloseHandle (current_process_handle)); > open_process_used = 0; > } > + siginfo_er.ExceptionCode = 0; > inf_child_target::mourn_inferior (); > } > > @@ -2994,6 +2998,28 @@ windows_xfer_shared_libraries (struct target_ops *ops, > return len != 0 ? TARGET_XFER_OK : TARGET_XFER_EOF; > } > > +static enum target_xfer_status > +windows_xfer_siginfo (gdb_byte *readbuf, ULONGEST offset, ULONGEST len, > + ULONGEST *xfered_len) > +{ > + if (!siginfo_er.ExceptionCode) siginfo_er.ExceptionCode != 0 > + return TARGET_XFER_E_IO; > + > + if (!readbuf) readbuf == nullptr > + return TARGET_XFER_E_IO; > + > + if (offset > sizeof (siginfo_er)) > + return TARGET_XFER_E_IO; > + > + if (offset + len > sizeof (siginfo_er)) > + len = sizeof (siginfo_er) - offset; > + > + memcpy (readbuf, (char *) &siginfo_er + offset, len); > + *xfered_len = len; > + > + return TARGET_XFER_OK; > +} > + > enum target_xfer_status > windows_nat_target::xfer_partial (enum target_object object, > const char *annex, gdb_byte *readbuf, > @@ -3009,6 +3035,9 @@ windows_nat_target::xfer_partial (enum target_object object, > return windows_xfer_shared_libraries (this, object, annex, readbuf, > writebuf, offset, len, xfered_len); > > + case TARGET_OBJECT_SIGNAL_INFO: > + return windows_xfer_siginfo (readbuf, offset, len, xfered_len); > + > default: > if (beneath () == NULL) > { > diff --git a/gdb/windows-tdep.c b/gdb/windows-tdep.c > index 1fc2748581..50efa31709 100644 > --- a/gdb/windows-tdep.c > +++ b/gdb/windows-tdep.c > @@ -153,6 +153,26 @@ static const int FULL_TIB_SIZE = 0x1000; > > static bool maint_display_all_tib = false; > > +static struct gdbarch_data *windows_gdbarch_data_handle; > + > +struct windows_gdbarch_data > + { > + struct type *siginfo_type; > + }; Unindent the curly braces and the field: { struct type *siginfo_type; } I know there are some structures formatted this way, but the new ones we add are aligned on column 0. > + > +static void * > +init_windows_gdbarch_data (struct gdbarch *gdbarch) > +{ > + return GDBARCH_OBSTACK_ZALLOC (gdbarch, struct windows_gdbarch_data); > +} > + > +static struct windows_gdbarch_data * > +get_windows_gdbarch_data (struct gdbarch *gdbarch) > +{ > + return ((struct windows_gdbarch_data *) > + gdbarch_data (gdbarch, windows_gdbarch_data_handle)); > +} > + > /* Define Thread Local Base pointer type. */ > > static struct type * > @@ -648,6 +668,43 @@ windows_gdb_signal_to_target (struct gdbarch *gdbarch, enum gdb_signal signal) > return -1; > } > > +static struct type * > +windows_get_siginfo_type (struct gdbarch *gdbarch) > +{ > + struct windows_gdbarch_data *windows_gdbarch_data; > + struct type *uint_type, *void_ptr_type; > + struct type *siginfo_ptr_type, *siginfo_type; > + > + windows_gdbarch_data = get_windows_gdbarch_data (gdbarch); > + if (windows_gdbarch_data->siginfo_type != NULL) > + return windows_gdbarch_data->siginfo_type; > + > + uint_type = arch_integer_type (gdbarch, gdbarch_int_bit (gdbarch), > + 1, "unsigned int"); You should be able to get this one from builtin_type (gdbarch)->builtin_unsigned_int. > + void_ptr_type = lookup_pointer_type (builtin_type (gdbarch)->builtin_void); > + > + siginfo_type = arch_composite_type (gdbarch, "EXCEPTION_RECORD", > + TYPE_CODE_STRUCT); > + siginfo_ptr_type = arch_pointer_type (gdbarch, gdbarch_ptr_bit (gdbarch), > + NULL, siginfo_type); > + > + append_composite_type_field (siginfo_type, "ExceptionCode", uint_type); > + append_composite_type_field (siginfo_type, "ExceptionFlags", uint_type); > + append_composite_type_field (siginfo_type, "ExceptionRecord", > + siginfo_ptr_type); > + append_composite_type_field (siginfo_type, "ExceptionAddress", > + void_ptr_type); > + append_composite_type_field (siginfo_type, "NumberParameters", uint_type); > + append_composite_type_field_aligned (siginfo_type, "ExceptionInformation", > + lookup_array_range_type (void_ptr_type, > + 0, 14), > + TYPE_LENGTH (void_ptr_type)); Shouldn't you use "DWORD" and other types named like what is found in the real structure, instead of plain "unsigned int"? Like what is done in windows_get_tlb_type? As a user, I would expect that "ptype $_siginfo" shows me "DWORD" and not "unsigned int", don't you think? Simon
Btw, as mentioned here: https://docs.microsoft.com/en-us/windows/win32/api/winnt/ns-winnt-exception_record#remarks The layout of the structure is different depending on if the debugged process is 32 or 64 bits. From what I understand, you code adapts to both, since it uses gdbarch_ptr_bit and uses proper alignment for the ExceptionInformation field, but I wanted to point it out just to be sure. Simon
Am Freitag, 7. Februar 2020, 22:56:18 MEZ hat Simon Marchi <simark@simark.ca> Folgendes geschrieben: > On 2020-01-17 10:31 a.m., Hannes Domani via gdb-patches wrote: > > diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c > > index 901e64263c..824ff4b322 100644 > > --- a/gdb/windows-nat.c > > +++ b/gdb/windows-nat.c > > @@ -236,6 +236,7 @@ static DEBUG_EVENT current_event; /* The current debug event from > > WaitForDebugEvent */ > > static HANDLE current_process_handle; /* Currently executing process */ > > static windows_thread_info *current_thread; /* Info on currently selected thread */ > > +static EXCEPTION_RECORD siginfo_er; /* Contents of $_siginfo */ > > Huh... I was going to say that it shouldn't be a global variable, but a per-inferior > thing (or is it per-thread?), but pretty much all the state is already global... so > I guess it's fine. I gather that the windows-nat does not support debugging multiple > inferiors? Same for win32-low in gdbserver? > > > > > /* Counts of things. */ > > static int exception_count = 0; > > @@ -1166,6 +1167,8 @@ handle_exception (struct target_waitstatus *ourstatus) > > DWORD code = rec->ExceptionCode; > > handle_exception_result result = HANDLE_EXCEPTION_HANDLED; > > > > + memcpy (&siginfo_er, rec, sizeof siginfo_er); > > + > > ourstatus->kind = TARGET_WAITKIND_STOPPED; > > > > /* Record the context of the current thread. */ > > @@ -2862,6 +2865,7 @@ windows_nat_target::mourn_inferior () > > CHECK (CloseHandle (current_process_handle)); > > open_process_used = 0; > > } > > + siginfo_er.ExceptionCode = 0; > > inf_child_target::mourn_inferior (); > > } > > > > @@ -2994,6 +2998,28 @@ windows_xfer_shared_libraries (struct target_ops *ops, > > return len != 0 ? TARGET_XFER_OK : TARGET_XFER_EOF; > > } > > > > +static enum target_xfer_status > > +windows_xfer_siginfo (gdb_byte *readbuf, ULONGEST offset, ULONGEST len, > > + ULONGEST *xfered_len) > > +{ > > + if (!siginfo_er.ExceptionCode) > > siginfo_er.ExceptionCode != 0 > > > + return TARGET_XFER_E_IO; > > + > > + if (!readbuf) > > readbuf == nullptr > > > + return TARGET_XFER_E_IO; > > + > > + if (offset > sizeof (siginfo_er)) > > + return TARGET_XFER_E_IO; > > + > > + if (offset + len > sizeof (siginfo_er)) > > + len = sizeof (siginfo_er) - offset; > > + > > + memcpy (readbuf, (char *) &siginfo_er + offset, len); > > + *xfered_len = len; > > + > > + return TARGET_XFER_OK; > > +} > > + > > enum target_xfer_status > > windows_nat_target::xfer_partial (enum target_object object, > > const char *annex, gdb_byte *readbuf, > > @@ -3009,6 +3035,9 @@ windows_nat_target::xfer_partial (enum target_object object, > > return windows_xfer_shared_libraries (this, object, annex, readbuf, > > writebuf, offset, len, xfered_len); > > > > + case TARGET_OBJECT_SIGNAL_INFO: > > + return windows_xfer_siginfo (readbuf, offset, len, xfered_len); > > + > > default: > > if (beneath () == NULL) > > { > > diff --git a/gdb/windows-tdep.c b/gdb/windows-tdep.c > > index 1fc2748581..50efa31709 100644 > > --- a/gdb/windows-tdep.c > > +++ b/gdb/windows-tdep.c > > @@ -153,6 +153,26 @@ static const int FULL_TIB_SIZE = 0x1000; > > > > static bool maint_display_all_tib = false; > > > > +static struct gdbarch_data *windows_gdbarch_data_handle; > > + > > +struct windows_gdbarch_data > > + { > > + struct type *siginfo_type; > > + }; > > Unindent the curly braces and the field: > > { > struct type *siginfo_type; > } > > I know there are some structures formatted this way, but the new ones we add are > aligned on column 0. OK, I fill fix all these coding style problems and try to remember them. > > + > > +static void * > > +init_windows_gdbarch_data (struct gdbarch *gdbarch) > > +{ > > + return GDBARCH_OBSTACK_ZALLOC (gdbarch, struct windows_gdbarch_data); > > +} > > + > > +static struct windows_gdbarch_data * > > +get_windows_gdbarch_data (struct gdbarch *gdbarch) > > +{ > > + return ((struct windows_gdbarch_data *) > > + gdbarch_data (gdbarch, windows_gdbarch_data_handle)); > > +} > > + > > /* Define Thread Local Base pointer type. */ > > > > static struct type * > > @@ -648,6 +668,43 @@ windows_gdb_signal_to_target (struct gdbarch *gdbarch, enum gdb_signal signal) > > return -1; > > } > > > > +static struct type * > > +windows_get_siginfo_type (struct gdbarch *gdbarch) > > +{ > > + struct windows_gdbarch_data *windows_gdbarch_data; > > + struct type *uint_type, *void_ptr_type; > > + struct type *siginfo_ptr_type, *siginfo_type; > > + > > + windows_gdbarch_data = get_windows_gdbarch_data (gdbarch); > > + if (windows_gdbarch_data->siginfo_type != NULL) > > + return windows_gdbarch_data->siginfo_type; > > + > > + uint_type = arch_integer_type (gdbarch, gdbarch_int_bit (gdbarch), > > + 1, "unsigned int"); > > You should be able to get this one from builtin_type (gdbarch)->builtin_unsigned_int. Good to know. > > + void_ptr_type = lookup_pointer_type (builtin_type (gdbarch)->builtin_void); > > + > > + siginfo_type = arch_composite_type (gdbarch, "EXCEPTION_RECORD", > > + TYPE_CODE_STRUCT); > > + siginfo_ptr_type = arch_pointer_type (gdbarch, gdbarch_ptr_bit (gdbarch), > > + NULL, siginfo_type); > > + > > + append_composite_type_field (siginfo_type, "ExceptionCode", uint_type); > > + append_composite_type_field (siginfo_type, "ExceptionFlags", uint_type); > > + append_composite_type_field (siginfo_type, "ExceptionRecord", > > + siginfo_ptr_type); > > + append_composite_type_field (siginfo_type, "ExceptionAddress", > > + void_ptr_type); > > + append_composite_type_field (siginfo_type, "NumberParameters", uint_type); > > + append_composite_type_field_aligned (siginfo_type, "ExceptionInformation", > > + lookup_array_range_type (void_ptr_type, > > + 0, 14), > > + TYPE_LENGTH (void_ptr_type)); > > > Shouldn't you use "DWORD" and other types named like what is found in the > real structure, instead of plain "unsigned int"? Like what is done in > windows_get_tlb_type? > > As a user, I would expect that "ptype $_siginfo" shows me "DWORD" and not > "unsigned int", don't you think? Personally I didn't really care what the name of the type is, but you are right, it probably is what users expect, so I will change the type names accordingly. Regards Hannes Domani
Am Freitag, 7. Februar 2020, 23:07:08 MEZ hat Simon Marchi <simark@simark.ca> Folgendes geschrieben: > Btw, as mentioned here: > > https://docs.microsoft.com/en-us/windows/win32/api/winnt/ns-winnt-exception_record#remarks > > The layout of the structure is different depending on if the debugged process is 32 or 64 bits. > > From what I understand, you code adapts to both, since it uses gdbarch_ptr_bit and uses proper > alignment for the ExceptionInformation field, but I wanted to point it out just to be sure. Yes, that's right. I will add some comments to make this more clear. Regards Hannes Domani
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:
Simon> Huh... I was going to say that it shouldn't be a global variable,
Simon> but a per-inferior thing (or is it per-thread?), but pretty much
Simon> all the state is already global... so I guess it's fine. I
Simon> gather that the windows-nat does not support debugging multiple
Simon> inferiors? Same for win32-low in gdbserver?
Yes, that's correct. I think it could be implemented, just nobody has
done it yet.
Tom
Am Dienstag, 11. Februar 2020, 16:28:37 MEZ hat Tom Tromey <tom@tromey.com> Folgendes geschrieben: > >>>>> "Simon" == Simon Marchi <simark@simark.ca> writes: > > Simon> Huh... I was going to say that it shouldn't be a global variable, > Simon> but a per-inferior thing (or is it per-thread?), but pretty much > Simon> all the state is already global... so I guess it's fine. I > Simon> gather that the windows-nat does not support debugging multiple > Simon> inferiors? Same for win32-low in gdbserver? > > Yes, that's correct. I think it could be implemented, just nobody has > done it yet. How does debugging multiple inferiors work exactly? Is this only possible with async mode (which doesn't work on Windows)? Regards Hannes Domani
diff --git a/gdb/gdbserver/win32-low.c b/gdb/gdbserver/win32-low.c index 2c4a9b1074..223561be54 100644 --- a/gdb/gdbserver/win32-low.c +++ b/gdb/gdbserver/win32-low.c @@ -75,6 +75,7 @@ static int attaching = 0; static HANDLE current_process_handle = NULL; static DWORD current_process_id = 0; static DWORD main_thread_id = 0; +static EXCEPTION_RECORD siginfo_er; /* Contents of $_siginfo */ static enum gdb_signal last_sig = GDB_SIGNAL_0; /* The current debug event from WaitForDebugEvent. */ @@ -801,6 +802,7 @@ win32_clear_inferiors (void) CloseHandle (current_process_handle); for_each_thread (delete_thread_info); + siginfo_er.ExceptionCode = 0; clear_inferiors (); } @@ -1230,6 +1232,9 @@ handle_exception (struct target_waitstatus *ourstatus) { DWORD code = current_event.u.Exception.ExceptionRecord.ExceptionCode; + memcpy (&siginfo_er, ¤t_event.u.Exception.ExceptionRecord, + sizeof siginfo_er); + ourstatus->kind = TARGET_WAITKIND_STOPPED; switch (code) @@ -1772,6 +1777,27 @@ wince_hostio_last_error (char *buf) } #endif +static int +win32_xfer_siginfo (const char *annex, unsigned char *readbuf, + unsigned const char *writebuf, CORE_ADDR offset, int len) +{ + if (!siginfo_er.ExceptionCode) + return -1; + + if (!readbuf) + return -1; + + if (offset > sizeof (siginfo_er)) + return -1; + + if (offset + len > sizeof (siginfo_er)) + len = sizeof (siginfo_er) - offset; + + memcpy (readbuf, (char *) &siginfo_er + offset, len); + + return len; +} + /* Write Windows OS Thread Information Block address. */ static int @@ -1833,7 +1859,7 @@ static process_stratum_target win32_target_ops = { hostio_last_error_from_errno, #endif NULL, /* qxfer_osdata */ - NULL, /* qxfer_siginfo */ + win32_xfer_siginfo, NULL, /* supports_non_stop */ NULL, /* async */ NULL, /* start_non_stop */ diff --git a/gdb/windows-nat.c b/gdb/windows-nat.c index 901e64263c..824ff4b322 100644 --- a/gdb/windows-nat.c +++ b/gdb/windows-nat.c @@ -236,6 +236,7 @@ static DEBUG_EVENT current_event; /* The current debug event from WaitForDebugEvent */ static HANDLE current_process_handle; /* Currently executing process */ static windows_thread_info *current_thread; /* Info on currently selected thread */ +static EXCEPTION_RECORD siginfo_er; /* Contents of $_siginfo */ /* Counts of things. */ static int exception_count = 0; @@ -1166,6 +1167,8 @@ handle_exception (struct target_waitstatus *ourstatus) DWORD code = rec->ExceptionCode; handle_exception_result result = HANDLE_EXCEPTION_HANDLED; + memcpy (&siginfo_er, rec, sizeof siginfo_er); + ourstatus->kind = TARGET_WAITKIND_STOPPED; /* Record the context of the current thread. */ @@ -2862,6 +2865,7 @@ windows_nat_target::mourn_inferior () CHECK (CloseHandle (current_process_handle)); open_process_used = 0; } + siginfo_er.ExceptionCode = 0; inf_child_target::mourn_inferior (); } @@ -2994,6 +2998,28 @@ windows_xfer_shared_libraries (struct target_ops *ops, return len != 0 ? TARGET_XFER_OK : TARGET_XFER_EOF; } +static enum target_xfer_status +windows_xfer_siginfo (gdb_byte *readbuf, ULONGEST offset, ULONGEST len, + ULONGEST *xfered_len) +{ + if (!siginfo_er.ExceptionCode) + return TARGET_XFER_E_IO; + + if (!readbuf) + return TARGET_XFER_E_IO; + + if (offset > sizeof (siginfo_er)) + return TARGET_XFER_E_IO; + + if (offset + len > sizeof (siginfo_er)) + len = sizeof (siginfo_er) - offset; + + memcpy (readbuf, (char *) &siginfo_er + offset, len); + *xfered_len = len; + + return TARGET_XFER_OK; +} + enum target_xfer_status windows_nat_target::xfer_partial (enum target_object object, const char *annex, gdb_byte *readbuf, @@ -3009,6 +3035,9 @@ windows_nat_target::xfer_partial (enum target_object object, return windows_xfer_shared_libraries (this, object, annex, readbuf, writebuf, offset, len, xfered_len); + case TARGET_OBJECT_SIGNAL_INFO: + return windows_xfer_siginfo (readbuf, offset, len, xfered_len); + default: if (beneath () == NULL) { diff --git a/gdb/windows-tdep.c b/gdb/windows-tdep.c index 1fc2748581..50efa31709 100644 --- a/gdb/windows-tdep.c +++ b/gdb/windows-tdep.c @@ -153,6 +153,26 @@ static const int FULL_TIB_SIZE = 0x1000; static bool maint_display_all_tib = false; +static struct gdbarch_data *windows_gdbarch_data_handle; + +struct windows_gdbarch_data + { + struct type *siginfo_type; + }; + +static void * +init_windows_gdbarch_data (struct gdbarch *gdbarch) +{ + return GDBARCH_OBSTACK_ZALLOC (gdbarch, struct windows_gdbarch_data); +} + +static struct windows_gdbarch_data * +get_windows_gdbarch_data (struct gdbarch *gdbarch) +{ + return ((struct windows_gdbarch_data *) + gdbarch_data (gdbarch, windows_gdbarch_data_handle)); +} + /* Define Thread Local Base pointer type. */ static struct type * @@ -648,6 +668,43 @@ windows_gdb_signal_to_target (struct gdbarch *gdbarch, enum gdb_signal signal) return -1; } +static struct type * +windows_get_siginfo_type (struct gdbarch *gdbarch) +{ + struct windows_gdbarch_data *windows_gdbarch_data; + struct type *uint_type, *void_ptr_type; + struct type *siginfo_ptr_type, *siginfo_type; + + windows_gdbarch_data = get_windows_gdbarch_data (gdbarch); + if (windows_gdbarch_data->siginfo_type != NULL) + return windows_gdbarch_data->siginfo_type; + + uint_type = arch_integer_type (gdbarch, gdbarch_int_bit (gdbarch), + 1, "unsigned int"); + void_ptr_type = lookup_pointer_type (builtin_type (gdbarch)->builtin_void); + + siginfo_type = arch_composite_type (gdbarch, "EXCEPTION_RECORD", + TYPE_CODE_STRUCT); + siginfo_ptr_type = arch_pointer_type (gdbarch, gdbarch_ptr_bit (gdbarch), + NULL, siginfo_type); + + append_composite_type_field (siginfo_type, "ExceptionCode", uint_type); + append_composite_type_field (siginfo_type, "ExceptionFlags", uint_type); + append_composite_type_field (siginfo_type, "ExceptionRecord", + siginfo_ptr_type); + append_composite_type_field (siginfo_type, "ExceptionAddress", + void_ptr_type); + append_composite_type_field (siginfo_type, "NumberParameters", uint_type); + append_composite_type_field_aligned (siginfo_type, "ExceptionInformation", + lookup_array_range_type (void_ptr_type, + 0, 14), + TYPE_LENGTH (void_ptr_type)); + + windows_gdbarch_data->siginfo_type = siginfo_type; + + return siginfo_type; +} + /* To be called from the various GDB_OSABI_CYGWIN handlers for the various Windows architectures and machine types. */ @@ -667,6 +724,8 @@ windows_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch) set_gdbarch_gdb_signal_to_target (gdbarch, windows_gdb_signal_to_target); set_solib_ops (gdbarch, &solib_target_so_ops); + + set_gdbarch_get_siginfo_type (gdbarch, windows_get_siginfo_type); } /* Implementation of `tlb' variable. */ @@ -682,6 +741,9 @@ void _initialize_windows_tdep (); void _initialize_windows_tdep () { + windows_gdbarch_data_handle = + gdbarch_data_register_post_init (init_windows_gdbarch_data); + init_w32_command_list (); add_cmd ("thread-information-block", class_info, display_tib, _("Display thread information block."),