From patchwork Tue Dec 9 10:49:21 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mark Wielaard X-Patchwork-Id: 4119 Received: (qmail 24333 invoked by alias); 9 Dec 2014 10:49:27 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 24323 invoked by uid 89); 9 Dec 2014 10:49:26 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.7 required=5.0 tests=AWL, BAYES_00, KAM_STOCKGEN, SPF_HELO_PASS, SPF_PASS, T_RP_MATCHES_RCVD autolearn=no version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Tue, 09 Dec 2014 10:49:25 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id sB9AnNXe019908 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Tue, 9 Dec 2014 05:49:23 -0500 Received: from bordewijk.wildebeest.org (ovpn-116-48.ams2.redhat.com [10.36.116.48]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id sB9AnMb1032369 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Tue, 9 Dec 2014 05:49:23 -0500 Received: by bordewijk.wildebeest.org (Postfix, from userid 1000) id C1FCC8062001; Tue, 9 Dec 2014 11:49:21 +0100 (CET) Message-ID: <1418122161.18974.42.camel@bordewijk.wildebeest.org> Subject: Re: [PATCH] Use GCC5/DWARF5 DW_AT_noreturn to mark functions that don't return normally. From: Mark Wielaard To: Pedro Alves Cc: gdb-patches@sourceware.org Date: Tue, 09 Dec 2014 11:49:21 +0100 In-Reply-To: <5480696F.1060308@redhat.com> References: <1417099980-31834-1-git-send-email-mjw@redhat.com> <5480696F.1060308@redhat.com> Mime-Version: 1.0 On Thu, 2014-12-04 at 14:02 +0000, Pedro Alves wrote: > On 11/27/2014 02:53 PM, Mark Wielaard wrote: > > Add a flag field is_noreturn to struct func_type. Make calling_convention > > a small bit field to not increase the size of the struct. Set is_noreturn > > if the new GCC5/DWARF5 DW_AT_noreturn is set on a DW_TAG_subprogram. > > Use this information to warn the user before doing a finish or return from > > a function that does not return normally to its caller. > > > > (gdb) finish > > Warning. Function endless does not return normally. > > Try to finish anyway? (y or n) > > > > (gdb) return > > warning: Function does not return normally to caller! > > Make endless return now? (y or n) > > I'd suggest making the warnings a bit more consistent. > > - "Warning." vs "warning: " > > Prefer the latter, as that's what the "warning" function uses. > > - "." vs "!" > > I'd keep it calm and get rid of the "!". :-) Fixed both. > > > > gdb/ChangeLog > > > > * dwarf2read.c (read_subroutine_type): Set TYPE_NO_RETURN from > > DW_AT_noreturn. > > * gdbtypes.h (struct func_type): Add is_noreturn field flag. Make > > calling_convention an 8 bit bit field. > > (TYPE_NO_RETURN): New macro. > > * infcmd.c (finish_command): Query if function does not return > > normally. > > * stack.c (return_command): Likewise. > > > > include/ChangeLog > > > > * dwarf2.def (DW_AT_noreturn): New DWARF5 attribute. > > I wonder if we could have a test? Could e.g., make sure we don't > crash when the user confirms a return in a noreturn function. I am not sure how to write such a test. This is mainly interactive code, which will only trigger from_tty. I also am not sure such a test really tests this new feature. Trying to return from a noreturn function triggers undefined behavior. GDB probably won't crash, but the inferior might since the result is unpredictable (that is precisely why I added this, you forcibly return from a function and end up somewhere unexpected). Which makes testing the expected output of the user ignoring the warning somewhat hard. > > --- a/gdb/infcmd.c > > +++ b/gdb/infcmd.c > > @@ -1869,7 +1869,14 @@ finish_command (char *arg, int from_tty) > > if (execution_direction == EXEC_REVERSE) > > printf_filtered (_("Run back to call of ")); > > else > > - printf_filtered (_("Run till exit from ")); > > + { > > + if (function != NULL && TYPE_NO_RETURN (function->type) > > + && ! query (_("Warning. Function %s does not return normally.\n" > > + "Try to finish anyway? "), > > + SYMBOL_PRINT_NAME (function))) > > + error (_("Not confirmed.")); > > + printf_filtered (_("Run till exit from ")); > > + } > > No space between "!" and query: > > && !query ... Fixed. > > --- a/gdb/stack.c > > +++ b/gdb/stack.c > > @@ -2462,8 +2462,12 @@ return_command (char *retval_exp, int from_tty) > > confirmed = query (_("%sMake selected stack frame return now? "), > > query_prefix); > > else > > - confirmed = query (_("%sMake %s return now? "), query_prefix, > > - SYMBOL_PRINT_NAME (thisfun)); > > + { > > + if (TYPE_NO_RETURN (thisfun->type)) > > + warning ("Function does not return normally to caller!"); > > i18n / _() . Added. Rebased patch attached. Thanks, Mark From 6b454703a5a53bee1355b5a871a10f6bbfcde64e Mon Sep 17 00:00:00 2001 From: Mark Wielaard Date: Tue, 9 Dec 2014 11:45:41 +0100 Subject: [PATCH] Use GCC5/DWARF5 DW_AT_noreturn to mark functions that don't return normally. Add a flag field is_noreturn to struct func_type. Make calling_convention a small bit field to not increase the size of the struct. Set is_noreturn if the new GCC5/DWARF5 DW_AT_noreturn is set on a DW_TAG_subprogram. Use this information to warn the user before doing a finish or return from a function that does not return normally to its caller. (gdb) finish warning: Function endless does not return normally. Try to finish anyway? (y or n) (gdb) return warning: Function does not return normally to caller. Make endless return now? (y or n) gdb/ChangeLog * dwarf2read.c (read_subroutine_type): Set TYPE_NO_RETURN from DW_AT_noreturn. * gdbtypes.h (struct func_type): Add is_noreturn field flag. Make calling_convention an 8 bit bit field. (TYPE_NO_RETURN): New macro. * infcmd.c (finish_command): Query if function does not return normally. * stack.c (return_command): Likewise. include/ChangeLog * dwarf2.def (DW_AT_noreturn): New DWARF5 attribute. The dwarf2.h addition and the code to emit the new attribute is already in the gcc tree. --- gdb/ChangeLog | 11 +++++++++++ gdb/dwarf2read.c | 6 ++++++ gdb/gdbtypes.h | 12 ++++++++++-- gdb/infcmd.c | 9 ++++++++- gdb/stack.c | 8 ++++++-- include/ChangeLog | 4 ++++ include/dwarf2.def | 2 ++ 7 files changed, 47 insertions(+), 5 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index ad845a7..43a62af 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,14 @@ +2014-12-09 Mark Wielaard + + * dwarf2read.c (read_subroutine_type): Set TYPE_NO_RETURN from + DW_AT_noreturn. + * gdbtypes.h (struct func_type): Add is_noreturn field flag. Make + calling_convention an 8 bit bit field. + (TYPE_NO_RETURN): New macro. + * infcmd.c (finish_command): Query if function does not return + normally. + * stack.c (return_command): Likewise. + 2014-12-08 Doug Evans * python/py-objfile.c (objfpy_get_owner): Increment refcount of result. diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c index 829611d..90e7893 100644 --- a/gdb/dwarf2read.c +++ b/gdb/dwarf2read.c @@ -14308,6 +14308,12 @@ read_subroutine_type (struct die_info *die, struct dwarf2_cu *cu) else TYPE_CALLING_CONVENTION (ftype) = DW_CC_normal; + /* Record whether the function returns normally to its caller or not + if the DWARF producer set that information. */ + attr = dwarf2_attr (die, DW_AT_noreturn, cu); + if (attr && (DW_UNSND (attr) != 0)) + TYPE_NO_RETURN (ftype) = 1; + /* We need to add the subroutine type to the die immediately so we don't infinitely recurse when dealing with parameters declared as the same subroutine type. */ diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h index a56f543..2806c4d 100644 --- a/gdb/gdbtypes.h +++ b/gdb/gdbtypes.h @@ -1019,9 +1019,16 @@ struct func_type { /* * The calling convention for targets supporting multiple ABIs. Right now this is only fetched from the Dwarf-2 - DW_AT_calling_convention attribute. */ + DW_AT_calling_convention attribute. The value is one of the + DW_CC enum dwarf_calling_convention constants. */ - unsigned calling_convention; + unsigned calling_convention : 8; + + /* * Whether this function normally returns to its caller. It is + set from the DW_AT_noreturn attribute if set on the + DW_TAG_subprogram. */ + + unsigned int is_noreturn : 1; /* * Only those DW_TAG_GNU_call_site's in this function that have DW_AT_GNU_tail_call set are linked in this list. Function @@ -1245,6 +1252,7 @@ extern void allocate_gnat_aux_type (struct type *); #define TYPE_GNAT_SPECIFIC(thistype) TYPE_MAIN_TYPE(thistype)->type_specific.gnat_stuff #define TYPE_DESCRIPTIVE_TYPE(thistype) TYPE_GNAT_SPECIFIC(thistype)->descriptive_type #define TYPE_CALLING_CONVENTION(thistype) TYPE_MAIN_TYPE(thistype)->type_specific.func_stuff->calling_convention +#define TYPE_NO_RETURN(thistype) TYPE_MAIN_TYPE(thistype)->type_specific.func_stuff->is_noreturn #define TYPE_TAIL_CALL_LIST(thistype) TYPE_MAIN_TYPE(thistype)->type_specific.func_stuff->tail_call_list #define TYPE_BASECLASS(thistype,index) TYPE_FIELD_TYPE(thistype, index) #define TYPE_N_BASECLASSES(thistype) TYPE_CPLUS_SPECIFIC(thistype)->n_baseclasses diff --git a/gdb/infcmd.c b/gdb/infcmd.c index 4415b31..68d9ef9 100644 --- a/gdb/infcmd.c +++ b/gdb/infcmd.c @@ -1869,7 +1869,14 @@ finish_command (char *arg, int from_tty) if (execution_direction == EXEC_REVERSE) printf_filtered (_("Run back to call of ")); else - printf_filtered (_("Run till exit from ")); + { + if (function != NULL && TYPE_NO_RETURN (function->type) + && !query (_("warning: Function %s does not return normally.\n" + "Try to finish anyway? "), + SYMBOL_PRINT_NAME (function))) + error (_("Not confirmed.")); + printf_filtered (_("Run till exit from ")); + } print_stack_frame (get_selected_frame (NULL), 1, LOCATION, 0); } diff --git a/gdb/stack.c b/gdb/stack.c index 2834801..f7e77e4 100644 --- a/gdb/stack.c +++ b/gdb/stack.c @@ -2462,8 +2462,12 @@ return_command (char *retval_exp, int from_tty) confirmed = query (_("%sMake selected stack frame return now? "), query_prefix); else - confirmed = query (_("%sMake %s return now? "), query_prefix, - SYMBOL_PRINT_NAME (thisfun)); + { + if (TYPE_NO_RETURN (thisfun->type)) + warning ("Function does not return normally to caller."); + confirmed = query (_("%sMake %s return now? "), query_prefix, + SYMBOL_PRINT_NAME (thisfun)); + } if (!confirmed) error (_("Not confirmed")); } diff --git a/include/ChangeLog b/include/ChangeLog index e80d2ec..69ed996 100644 --- a/include/ChangeLog +++ b/include/ChangeLog @@ -1,3 +1,7 @@ +2014-12-09 Mark Wielaard + + * dwarf2.def (DW_AT_noreturn): New DWARF5 attribute. + 2014-12-06 Eric Botcazou * dis-asm.h (print_insn_visium): Declare. diff --git a/include/dwarf2.def b/include/dwarf2.def index 8ca143c..8533a3e 100644 --- a/include/dwarf2.def +++ b/include/dwarf2.def @@ -308,6 +308,8 @@ DW_AT (DW_AT_data_bit_offset, 0x6b) DW_AT (DW_AT_const_expr, 0x6c) DW_AT (DW_AT_enum_class, 0x6d) DW_AT (DW_AT_linkage_name, 0x6e) +/* DWARF 5. */ +DW_AT (DW_AT_noreturn, 0x87) DW_AT_DUP (DW_AT_lo_user, 0x2000) /* Implementation-defined range start. */ DW_AT_DUP (DW_AT_hi_user, 0x3fff) /* Implementation-defined range end. */ -- 1.8.3.1