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

Message ID 20200208162614.4918-2-ssbssa@yahoo.de
State New, archived
Headers

Commit Message

Terekhov, Mikhail via Gdb-patches Feb. 8, 2020, 4:26 p.m. UTC
  Changes to $_siginfo type to this:

(gdb) pt $_siginfo
type = struct EXCEPTION_RECORD {
    enum ExceptionCode ExceptionCode;
    DWORD ExceptionFlags;
    struct EXCEPTION_RECORD *ExceptionRecord;
    PVOID ExceptionAddress;
    DWORD NumberParameters;
    union {
        ULONG_PTR 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;
    PVOID 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 = {1, 291, 0 <repeats 13 times>},
    AccessViolationInformation = {
      Type = WRITE_ACCESS_VIOLATION,
      Address = 0x123
    }
  }
}

gdb/ChangeLog:

2020-02-08  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.
---
v2:
- more comments
---
 gdb/windows-tdep.c | 102 ++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 96 insertions(+), 6 deletions(-)
  

Comments

Simon Marchi Feb. 8, 2020, 8:04 p.m. UTC | #1
On 2020-02-08 11:26 a.m., Hannes Domani via gdb-patches wrote:
> Changes to $_siginfo type to this:
> 
> (gdb) pt $_siginfo
> type = struct EXCEPTION_RECORD {
>     enum ExceptionCode ExceptionCode;
>     DWORD ExceptionFlags;
>     struct EXCEPTION_RECORD *ExceptionRecord;
>     PVOID ExceptionAddress;
>     DWORD NumberParameters;
>     union {
>         ULONG_PTR 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;
>     PVOID 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 = {1, 291, 0 <repeats 13 times>},
>     AccessViolationInformation = {
>       Type = WRITE_ACCESS_VIOLATION,
>       Address = 0x123
>     }
>   }
> }
> 
> gdb/ChangeLog:
> 
> 2020-02-08  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.
> ---
> v2:
> - more comments
> ---
>  gdb/windows-tdep.c | 102 ++++++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 96 insertions(+), 6 deletions(-)
> 
> diff --git a/gdb/windows-tdep.c b/gdb/windows-tdep.c
> index ad65b1b403..e9787887a4 100644
> --- a/gdb/windows-tdep.c
> +++ b/gdb/windows-tdep.c
> @@ -680,6 +680,71 @@ windows_gdb_signal_to_target (struct gdbarch *gdbarch, enum gdb_signal signal)
>    return -1;
>  }
>  
> +struct enum_value_name
> +{
> +  uint32_t value;
> +  const char *name;
> +};
> +
> +/* Allocate a TYPE_CODE_ENUM type structure with its named values.  */
> +
> +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" },
> +};
> +
>  /* Implement the "get_siginfo_type" gdbarch method.  */
>  
>  static struct type *
> @@ -687,7 +752,8 @@ windows_get_siginfo_type (struct gdbarch *gdbarch)
>  {
>    struct windows_gdbarch_data *windows_gdbarch_data;
>    struct type *dword_type, *pvoid_type, *ulongptr_type;
> -  struct type *siginfo_ptr_type, *siginfo_type;
> +  struct 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)
> @@ -700,12 +766,38 @@ windows_get_siginfo_type (struct gdbarch *gdbarch)
>    ulongptr_type = arch_integer_type (gdbarch, gdbarch_ptr_bit (gdbarch),
>  				     1, "ULONG_PTR");
>  
> +  /* ExceptionCode value names */
> +  code_enum = create_enum (gdbarch, gdbarch_int_bit (gdbarch),
> +			   "ExceptionCode", exception_values,
> +			   ARRAY_SIZE (exception_values));
> +
> +  /* ACCESS_VIOLATION type names */
> +  violation_enum = create_enum (gdbarch, gdbarch_ptr_bit (gdbarch),
> +				"ViolationType", violation_values,
> +				ARRAY_SIZE (violation_values));
> +
> +  /* ACCESS_VIOLATION information */
> +  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", pvoid_type);
> +
> +  /* Unnamed union of the documented field ExceptionInformation,
> +     and the alternative AccessViolationInformation (which displays
> +     human-readable values for ExceptionCode ACCESS_VIOLATION).  */
> +  para_type = arch_composite_type (gdbarch, NULL, TYPE_CODE_UNION);
> +  append_composite_type_field (para_type, "ExceptionInformation",
> +			       lookup_array_range_type (ulongptr_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", dword_type);
> +  /* ExceptionCode is documented as type DWORD, but here a helper
> +     enum type is used instead to display a human-readable value.  */
> +  append_composite_type_field (siginfo_type, "ExceptionCode", code_enum);
>    append_composite_type_field (siginfo_type, "ExceptionFlags", dword_type);
>    append_composite_type_field (siginfo_type, "ExceptionRecord",
>  			       siginfo_ptr_type);
> @@ -713,10 +805,8 @@ windows_get_siginfo_type (struct gdbarch *gdbarch)
>  			       pvoid_type);
>    append_composite_type_field (siginfo_type, "NumberParameters", dword_type);
>    /* The 64-bit variant needs some padding.  */
> -  append_composite_type_field_aligned (siginfo_type, "ExceptionInformation",
> -				       lookup_array_range_type (ulongptr_type,
> -								0, 14),
> -				       TYPE_LENGTH (ulongptr_type));
> +  append_composite_type_field_aligned (siginfo_type, "",
> +				       para_type, TYPE_LENGTH (ulongptr_type));
>  
>    windows_gdbarch_data->siginfo_type = siginfo_type;
>  
> -- 
> 2.25.0
> 

Eli, same question here.  This makes GDB's EXCEPTION_RECORD type deviate from
the official one, but just to improve readability when you print it.   I think
it's a good idea, but I'd like to hear what you think.

And again, should we document this behavior in the manual, or is it obvious and
discoverable enough by itself (when the user prints $_siginfo, it should be
self-explanatory, if they already know about the real EXCEPTION_RECORD type).

Simon
  
Eli Zaretskii Feb. 8, 2020, 8:14 p.m. UTC | #2
> From: Simon Marchi <simark@simark.ca>
> Date: Sat, 8 Feb 2020 15:04:23 -0500
> 
> Eli, same question here.  This makes GDB's EXCEPTION_RECORD type deviate from
> the official one, but just to improve readability when you print it.   I think
> it's a good idea, but I'd like to hear what you think.

This LGTM.

> And again, should we document this behavior in the manual, or is it obvious and
> discoverable enough by itself (when the user prints $_siginfo, it should be
> self-explanatory, if they already know about the real EXCEPTION_RECORD type).

Only in NEWS, I think.

Thanks.
  

Patch

diff --git a/gdb/windows-tdep.c b/gdb/windows-tdep.c
index ad65b1b403..e9787887a4 100644
--- a/gdb/windows-tdep.c
+++ b/gdb/windows-tdep.c
@@ -680,6 +680,71 @@  windows_gdb_signal_to_target (struct gdbarch *gdbarch, enum gdb_signal signal)
   return -1;
 }
 
+struct enum_value_name
+{
+  uint32_t value;
+  const char *name;
+};
+
+/* Allocate a TYPE_CODE_ENUM type structure with its named values.  */
+
+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" },
+};
+
 /* Implement the "get_siginfo_type" gdbarch method.  */
 
 static struct type *
@@ -687,7 +752,8 @@  windows_get_siginfo_type (struct gdbarch *gdbarch)
 {
   struct windows_gdbarch_data *windows_gdbarch_data;
   struct type *dword_type, *pvoid_type, *ulongptr_type;
-  struct type *siginfo_ptr_type, *siginfo_type;
+  struct 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)
@@ -700,12 +766,38 @@  windows_get_siginfo_type (struct gdbarch *gdbarch)
   ulongptr_type = arch_integer_type (gdbarch, gdbarch_ptr_bit (gdbarch),
 				     1, "ULONG_PTR");
 
+  /* ExceptionCode value names */
+  code_enum = create_enum (gdbarch, gdbarch_int_bit (gdbarch),
+			   "ExceptionCode", exception_values,
+			   ARRAY_SIZE (exception_values));
+
+  /* ACCESS_VIOLATION type names */
+  violation_enum = create_enum (gdbarch, gdbarch_ptr_bit (gdbarch),
+				"ViolationType", violation_values,
+				ARRAY_SIZE (violation_values));
+
+  /* ACCESS_VIOLATION information */
+  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", pvoid_type);
+
+  /* Unnamed union of the documented field ExceptionInformation,
+     and the alternative AccessViolationInformation (which displays
+     human-readable values for ExceptionCode ACCESS_VIOLATION).  */
+  para_type = arch_composite_type (gdbarch, NULL, TYPE_CODE_UNION);
+  append_composite_type_field (para_type, "ExceptionInformation",
+			       lookup_array_range_type (ulongptr_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", dword_type);
+  /* ExceptionCode is documented as type DWORD, but here a helper
+     enum type is used instead to display a human-readable value.  */
+  append_composite_type_field (siginfo_type, "ExceptionCode", code_enum);
   append_composite_type_field (siginfo_type, "ExceptionFlags", dword_type);
   append_composite_type_field (siginfo_type, "ExceptionRecord",
 			       siginfo_ptr_type);
@@ -713,10 +805,8 @@  windows_get_siginfo_type (struct gdbarch *gdbarch)
 			       pvoid_type);
   append_composite_type_field (siginfo_type, "NumberParameters", dword_type);
   /* The 64-bit variant needs some padding.  */
-  append_composite_type_field_aligned (siginfo_type, "ExceptionInformation",
-				       lookup_array_range_type (ulongptr_type,
-								0, 14),
-				       TYPE_LENGTH (ulongptr_type));
+  append_composite_type_field_aligned (siginfo_type, "",
+				       para_type, TYPE_LENGTH (ulongptr_type));
 
   windows_gdbarch_data->siginfo_type = siginfo_type;