[PATCH 1/2] Display ExceptionRecord for $_siginfo

[PATCH 1/2] Display ExceptionRecord for $_siginfo

[Hannes Domani via gdb-patches]

Uses the $_siginfo convenience variable to show the last exception.

The type looks like this:

(gdb) pt $_siginfo
type = struct EXCEPTION_RECORD {
    unsigned int ExceptionCode;
    unsigned int ExceptionFlags;
    struct EXCEPTION_RECORD *ExceptionRecord;
    void *ExceptionAddress;
    unsigned int NumberParameters;
    void *ExceptionInformation[15];
}

EXCEPTION_RECORD is documented at [1].

Example:

Program received signal SIGSEGV, Segmentation fault.
main () at crasher.c:4
4         *(int*)0x123 = 0;
(gdb) p $_siginfo
$1 = {
  ExceptionCode = 3221225477,
  ExceptionFlags = 0,
  ExceptionRecord = 0x0,
  ExceptionAddress = 0x401632 <main+18>,
  NumberParameters = 2,
  ExceptionInformation = {0x1, 0x123, 0x0 <repeats 13 times>}
}
(gdb) p/x $_siginfo.ExceptionCode
$2 = 0xc0000005

And 0xc0000005 is the value of EXCEPTION_ACCESS_VIOLATION.

[1] https://docs.microsoft.com/en-us/windows/win32/api/winnt/ns-winnt-exception_record

gdb/ChangeLog:

2020-01-17  Hannes Domani  <ssbssa@yahoo.de>

	* windows-nat.c (handle_exception): Set siginfo_er.
	(windows_nat_target::mourn_inferior): Reset siginfo_er.
	(windows_xfer_siginfo): New function.
	(windows_nat_target::xfer_partial): Call windows_xfer_siginfo.
	* windows-tdep.c (struct windows_gdbarch_data): New struct.
	(init_windows_gdbarch_data): New function.
	(get_windows_gdbarch_data): New function.
	(windows_get_siginfo_type): New function.
	(windows_init_abi): Register windows_get_siginfo_type.
	(_initialize_windows_tdep): Register init_windows_gdbarch_data.

gdb/gdbserver/ChangeLog:

2020-01-17  Hannes Domani  <ssbssa@yahoo.de>

	* win32-low.c (win32_clear_inferiors): Reset siginfo_er.
	(handle_exception): Set siginfo_er.
	(win32_xfer_siginfo): New function.
---
 gdb/gdbserver/win32-low.c | 28 +++++++++++++++++-
 gdb/windows-nat.c         | 29 ++++++++++++++++++
 gdb/windows-tdep.c        | 62 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 118 insertions(+), 1 deletion(-)

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, &current_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."),
-- 
2.25.0

[PATCH 2/2] Use enums for human-readable exception information.

[Hannes Domani via gdb-patches]

Changes to $_siginfo type to this:

(gdb) pt $_siginfo
type = struct EXCEPTION_RECORD {
    enum ExceptionCode ExceptionCode;
    unsigned int ExceptionFlags;
    struct EXCEPTION_RECORD *ExceptionRecord;
    void *ExceptionAddress;
    unsigned int NumberParameters;
    union {
        void *ExceptionInformation[15];
        struct {...} AccessViolationInformation;
    };
}
(gdb) pt $_siginfo.ExceptionCode
type = enum ExceptionCode {FATAL_APP_EXIT = 1073741845,
    DBG_CONTROL_C = 1073807365, DBG_CONTROL_BREAK = 1073807368,
    DATATYPE_MISALIGNMENT = 2147483650, BREAKPOINT, SINGLE_STEP,
    ACCESS_VIOLATION = 3221225477, IN_PAGE_ERROR,
    ILLEGAL_INSTRUCTION = 3221225501, NONCONTINUABLE_EXCEPTION = 3221225509,
    INVALID_DISPOSITION, ARRAY_BOUNDS_EXCEEDED = 3221225612,
    FLOAT_DENORMAL_OPERAND, FLOAT_DIVIDE_BY_ZERO, FLOAT_INEXACT_RESULT,
    FLOAT_INVALID_OPERATION, FLOAT_OVERFLOW, FLOAT_STACK_CHECK,
    FLOAT_UNDERFLOW, INTEGER_DIVIDE_BY_ZERO, INTEGER_OVERFLOW,
    PRIV_INSTRUCTION, STACK_OVERFLOW = 3221225725, FAST_FAIL = 3221226505}
(gdb) pt $_siginfo.AccessViolationInformation
type = struct {
    enum ViolationType Type;
    void *Address;
}
(gdb) pt $_siginfo.AccessViolationInformation.Type
type = enum ViolationType {READ_ACCESS_VIOLATION, WRITE_ACCESS_VIOLATION,
    DATA_EXECUTION_PREVENTION_VIOLATION = 8}

Which makes it easier to understand the reason of the exception:

(gdb) p $_siginfo
$1 = {
  ExceptionCode = ACCESS_VIOLATION,
  ExceptionFlags = 0,
  ExceptionRecord = 0x0,
  ExceptionAddress = 0x401632 <main+18>,
  NumberParameters = 2,
  {
    ExceptionInformation = {0x1, 0x123, 0x0 <repeats 13 times>},
    AccessViolationInformation = {
      Type = WRITE_ACCESS_VIOLATION,
      Address = 0x123
    }
  }
}

gdb/ChangeLog:

2020-01-17  Hannes Domani  <ssbssa@yahoo.de>

	* windows-tdep.c (struct enum_value_name): New struct.
	(create_enum): New function.
	(windows_get_siginfo_type): Create and use enum types.
---
 gdb/windows-tdep.c | 93 ++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 86 insertions(+), 7 deletions(-)

diff --git a/gdb/windows-tdep.c b/gdb/windows-tdep.c
index 50efa31709..080b3a89d2 100644
--- a/gdb/windows-tdep.c
+++ b/gdb/windows-tdep.c
@@ -668,12 +668,75 @@ windows_gdb_signal_to_target (struct gdbarch *gdbarch, enum gdb_signal signal)
   return -1;
 }
 
+struct enum_value_name
+{
+  uint32_t value;
+  const char *name;
+};
+
+static struct type *
+create_enum (struct gdbarch *gdbarch, int bit, const char *name,
+	     const struct enum_value_name *values, int count)
+{
+  struct type *type;
+  int i;
+
+  type = arch_type (gdbarch, TYPE_CODE_ENUM, bit, name);
+  TYPE_NFIELDS (type) = count;
+  TYPE_FIELDS (type) = (struct field *)
+    TYPE_ZALLOC (type, sizeof (struct field) * count);
+  TYPE_UNSIGNED (type) = 1;
+
+  for (i = 0; i < count; i++)
+  {
+    TYPE_FIELD_NAME (type, i) = values[i].name;
+    SET_FIELD_ENUMVAL (TYPE_FIELD (type, i), values[i].value);
+  }
+
+  return type;
+}
+
+static const struct enum_value_name exception_values[] =
+{
+  { 0x40000015, "FATAL_APP_EXIT" },
+  { 0x40010005, "DBG_CONTROL_C" },
+  { 0x40010008, "DBG_CONTROL_BREAK" },
+  { 0x80000002, "DATATYPE_MISALIGNMENT" },
+  { 0x80000003, "BREAKPOINT" },
+  { 0x80000004, "SINGLE_STEP" },
+  { 0xC0000005, "ACCESS_VIOLATION" },
+  { 0xC0000006, "IN_PAGE_ERROR" },
+  { 0xC000001D, "ILLEGAL_INSTRUCTION" },
+  { 0xC0000025, "NONCONTINUABLE_EXCEPTION" },
+  { 0xC0000026, "INVALID_DISPOSITION" },
+  { 0xC000008C, "ARRAY_BOUNDS_EXCEEDED" },
+  { 0xC000008D, "FLOAT_DENORMAL_OPERAND" },
+  { 0xC000008E, "FLOAT_DIVIDE_BY_ZERO" },
+  { 0xC000008F, "FLOAT_INEXACT_RESULT" },
+  { 0xC0000090, "FLOAT_INVALID_OPERATION" },
+  { 0xC0000091, "FLOAT_OVERFLOW" },
+  { 0xC0000092, "FLOAT_STACK_CHECK" },
+  { 0xC0000093, "FLOAT_UNDERFLOW" },
+  { 0xC0000094, "INTEGER_DIVIDE_BY_ZERO" },
+  { 0xC0000095, "INTEGER_OVERFLOW" },
+  { 0xC0000096, "PRIV_INSTRUCTION" },
+  { 0xC00000FD, "STACK_OVERFLOW" },
+  { 0xC0000409, "FAST_FAIL" },
+};
+
+static const struct enum_value_name violation_values[] =
+{
+  { 0, "READ_ACCESS_VIOLATION" },
+  { 1, "WRITE_ACCESS_VIOLATION" },
+  { 8, "DATA_EXECUTION_PREVENTION_VIOLATION" },
+};
+
 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;
+  struct type *uint_type, *void_ptr_type, *code_enum, *violation_enum;
+  struct type *violation_type, *para_type, *siginfo_ptr_type, *siginfo_type;
 
   windows_gdbarch_data = get_windows_gdbarch_data (gdbarch);
   if (windows_gdbarch_data->siginfo_type != NULL)
@@ -683,22 +746,38 @@ windows_get_siginfo_type (struct gdbarch *gdbarch)
 				 1, "unsigned int");
   void_ptr_type = lookup_pointer_type (builtin_type (gdbarch)->builtin_void);
 
+  code_enum = create_enum (gdbarch, gdbarch_int_bit (gdbarch),
+			   "ExceptionCode", exception_values,
+			   ARRAY_SIZE (exception_values));
+
+  violation_enum = create_enum (gdbarch, gdbarch_ptr_bit (gdbarch),
+				"ViolationType", violation_values,
+				ARRAY_SIZE (violation_values));
+
+  violation_type = arch_composite_type (gdbarch, NULL, TYPE_CODE_STRUCT);
+  append_composite_type_field (violation_type, "Type", violation_enum);
+  append_composite_type_field (violation_type, "Address", void_ptr_type);
+
+  para_type = arch_composite_type (gdbarch, NULL, TYPE_CODE_UNION);
+  append_composite_type_field (para_type, "ExceptionInformation",
+			       lookup_array_range_type (void_ptr_type, 0, 14));
+  append_composite_type_field (para_type, "AccessViolationInformation",
+			       violation_type);
+
   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, "ExceptionCode", code_enum);
   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));
+  append_composite_type_field_aligned (siginfo_type, "",
+				       para_type, TYPE_LENGTH (void_ptr_type));
 
   windows_gdbarch_data->siginfo_type = siginfo_type;
 
-- 
2.25.0

Re: [PATCH 1/2] Display ExceptionRecord for $_siginfo

[Simon Marchi]

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

Re: [PATCH 2/2] Use enums for human-readable exception information.

[Simon Marchi]

On 2020-01-17 10:31 a.m., Hannes Domani via gdb-patches wrote:
> Changes to $_siginfo type to this:
> 
> (gdb) pt $_siginfo
> type = struct EXCEPTION_RECORD {
>     enum ExceptionCode ExceptionCode;
>     unsigned int ExceptionFlags;
>     struct EXCEPTION_RECORD *ExceptionRecord;
>     void *ExceptionAddress;
>     unsigned int NumberParameters;
>     union {
>         void *ExceptionInformation[15];
>         struct {...} AccessViolationInformation;
>     };
> }
> (gdb) pt $_siginfo.ExceptionCode
> type = enum ExceptionCode {FATAL_APP_EXIT = 1073741845,
>     DBG_CONTROL_C = 1073807365, DBG_CONTROL_BREAK = 1073807368,
>     DATATYPE_MISALIGNMENT = 2147483650, BREAKPOINT, SINGLE_STEP,
>     ACCESS_VIOLATION = 3221225477, IN_PAGE_ERROR,
>     ILLEGAL_INSTRUCTION = 3221225501, NONCONTINUABLE_EXCEPTION = 3221225509,
>     INVALID_DISPOSITION, ARRAY_BOUNDS_EXCEEDED = 3221225612,
>     FLOAT_DENORMAL_OPERAND, FLOAT_DIVIDE_BY_ZERO, FLOAT_INEXACT_RESULT,
>     FLOAT_INVALID_OPERATION, FLOAT_OVERFLOW, FLOAT_STACK_CHECK,
>     FLOAT_UNDERFLOW, INTEGER_DIVIDE_BY_ZERO, INTEGER_OVERFLOW,
>     PRIV_INSTRUCTION, STACK_OVERFLOW = 3221225725, FAST_FAIL = 3221226505}
> (gdb) pt $_siginfo.AccessViolationInformation
> type = struct {
>     enum ViolationType Type;
>     void *Address;
> }
> (gdb) pt $_siginfo.AccessViolationInformation.Type
> type = enum ViolationType {READ_ACCESS_VIOLATION, WRITE_ACCESS_VIOLATION,
>     DATA_EXECUTION_PREVENTION_VIOLATION = 8}
> 
> Which makes it easier to understand the reason of the exception:
> 
> (gdb) p $_siginfo
> $1 = {
>   ExceptionCode = ACCESS_VIOLATION,
>   ExceptionFlags = 0,
>   ExceptionRecord = 0x0,
>   ExceptionAddress = 0x401632 <main+18>,
>   NumberParameters = 2,
>   {
>     ExceptionInformation = {0x1, 0x123, 0x0 <repeats 13 times>},
>     AccessViolationInformation = {
>       Type = WRITE_ACCESS_VIOLATION,
>       Address = 0x123
>     }
>   }
> }

Well, this kind of goes against the last comment I left on patch 1 (to use the
actual types found in the real structure), but I think that what you are
suggesting here is quite convenient, so I wouldn't be against doing something
like that.

Please add a bit of comments to your code to explain what you are doing.  Since
you are inventing types that don't exist in the documentation, it's not obvious
to know what this is doing.

Simon

Re: [PATCH 1/2] Display ExceptionRecord for $_siginfo

[Simon Marchi]

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

Re: [PATCH 1/2] Display ExceptionRecord for $_siginfo

[Hannes Domani via gdb-patches]

 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

Re: [PATCH 1/2] Display ExceptionRecord for $_siginfo

[Hannes Domani via gdb-patches]

 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

Re: [PATCH 1/2] Display ExceptionRecord for $_siginfo

[Tom Tromey]

>>>>> "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

Re: [PATCH 1/2] Display ExceptionRecord for $_siginfo

[Hannes Domani via gdb-patches]

 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