From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 41340 invoked by alias); 8 Feb 2020 20:04:27 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 41331 invoked by uid 89); 8 Feb 2020 20:04:27 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-17.8 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.1 spammy= X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 08 Feb 2020 20:04:25 +0000 Received: from [10.0.0.11] (unknown [192.222.164.54]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id AD4C21E08E; Sat, 8 Feb 2020 15:04:23 -0500 (EST) Subject: Re: [PATCH v2 2/2] Use enums for human-readable exception information. To: Hannes Domani , gdb-patches@sourceware.org, Eli Zaretskii References: <20200208162614.4918-1-ssbssa@yahoo.de> <20200208162614.4918-2-ssbssa@yahoo.de> From: Simon Marchi Message-ID: Date: Sat, 08 Feb 2020 20:04:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.4.2 MIME-Version: 1.0 In-Reply-To: <20200208162614.4918-2-ssbssa@yahoo.de> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2020-02/txt/msg00244.txt.bz2 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 , > NumberParameters = 2, > { > ExceptionInformation = {1, 291, 0 }, > AccessViolationInformation = { > Type = WRITE_ACCESS_VIOLATION, > Address = 0x123 > } > } > } > > gdb/ChangeLog: > > 2020-02-08 Hannes Domani > > * 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