From patchwork Fri Jan 23 16:33:25 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mark Wielaard X-Patchwork-Id: 4778 Received: (qmail 27777 invoked by alias); 23 Jan 2015 16:33:45 -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 27709 invoked by uid 89); 23 Jan 2015 16:33:40 -0000 Authentication-Results: sourceware.org; auth=none 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; Fri, 23 Jan 2015 16:33:32 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t0NGXSc5026425 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL); Fri, 23 Jan 2015 11:33:28 -0500 Received: from bordewijk.wildebeest.org (ovpn-116-65.ams2.redhat.com [10.36.116.65]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t0NGXQmA022481 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Fri, 23 Jan 2015 11:33:27 -0500 Received: by bordewijk.wildebeest.org (Postfix, from userid 1000) id 210F4813383E; Fri, 23 Jan 2015 17:33:25 +0100 (CET) Message-ID: <1422030805.4858.16.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: Stan Shebs , gdb-patches@sourceware.org Date: Fri, 23 Jan 2015 17:33:25 +0100 In-Reply-To: <54BFDCD8.9090709@redhat.com> References: <1417099980-31834-1-git-send-email-mjw@redhat.com> <5480696F.1060308@redhat.com> <1418122161.18974.42.camel@bordewijk.wildebeest.org> <548745FD.40000@earthlink.net> <1418210696.5011.10.camel@bordewijk.wildebeest.org> <548AC7C3.9020000@redhat.com> <1421366172.26117.29.camel@bordewijk.wildebeest.org> <54BFDCD8.9090709@redhat.com> Mime-Version: 1.0 On Wed, 2015-01-21 at 17:07 +0000, Pedro Alves wrote: > > On 01/15/2015 11:56 PM, Mark Wielaard wrote: > > > create mode 100644 gdb/testsuite/gdb.base/noreturn.c > > create mode 100644 gdb/testsuite/gdb.base/noreturn.exp > > How about "noreturn-return.{c|exp}", to go with noreturn_finish ? > > > create mode 100644 gdb/testsuite/gdb.base/noreturn_finish.c > > create mode 100644 gdb/testsuite/gdb.base/noreturn_finish.exp > > But please use '-' instead of '_': > > gdb/testsuite/gdb.base/noreturn-finish.c > gdb/testsuite/gdb.base/noreturn-finish.exp OK, changed all file and test names. > > diff --git a/gdb/testsuite/gdb.base/noreturn.c b/gdb/testsuite/gdb.base/noreturn.c > > new file mode 100644 > > index 0000000..e39cf15 > > --- /dev/null > > +++ b/gdb/testsuite/gdb.base/noreturn.c > > @@ -0,0 +1,13 @@ > > Please add a copyright header. Even though some of our old > files don't have it, all new files should, even if the file > is small (so that we don't have to recall adding it back > if the file grows in future). Added. Note that most .c files in gdb.base don't have such a header. > > +void __attribute__((noreturn)) > > +noreturn_func () > > noreturn_func (void) Added the void. Note most existing .c tests in gdb.base don't declare functions with void arguments. > > +{ > > + while (1) > > + ; > > Please don't make the test loop forever if GDB crashes. > Does e.g., "abort()" like the other test work here too? Changed. It isn't semantically the same, but it does work. > > +} > > + > > +int > > +main () > > likewise (void) Added. > > +{ > > + noreturn_func (); > > + return 0; > > +} > > diff --git a/gdb/testsuite/gdb.base/noreturn.exp b/gdb/testsuite/gdb.base/noreturn.exp > > new file mode 100644 > > index 0000000..885642f > > --- /dev/null > > +++ b/gdb/testsuite/gdb.base/noreturn.exp > > @@ -0,0 +1,54 @@ > > +# Copyright 2015 Free Software Foundation, Inc. > > + > > +# This program is free software; you can redistribute it and/or modify > > +# it under the terms of the GNU General Public License as published by > > +# the Free Software Foundation; either version 3 of the License, or > > +# (at your option) any later version. > > +# > > +# This program is distributed in the hope that it will be useful, > > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > +# GNU General Public License for more details. > > +# > > +# You should have received a copy of the GNU General Public License > > +# along with this program. If not, see . > > + > > +standard_testfile .c > > You can drop the ".c", as it's the default. Dropped. It is used in other .exp files though. > > + > > +if { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } { > > + untested noreturn.exp > > + return -1 > > +} > > Use prepare_for_testing, like: > > if [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} {debug}] { > return -1 > } Changed. This idiom comes from existing .exp files though. > > + > > +proc noreturn_test { } { > > + global gdb_prompt > > + > > + if { ! [ runto_main ] } then { > > + untested noreturn.exp > > Use $testfile, like "untested ${testfile}.exp". Changed. > > + return -1 > > + } > > + > > + gdb_test "break noreturn_func" "Breakpoint \[0123456789\].*" \ > > + "set break on noreturn_func" > > + gdb_test "continue" "Breakpoint.* noreturn_func.*" \ > > + "continue to noreturn_func" > > gdb_breakpoint "noreturn_func" > gdb_continue_to_breakpoint "noreturn_func" Changed. Note that the above comes from existing .exp testcases. > > + > > + gdb_test_multiple "return" "return from noreturn_func" { > > + -re "warning: Function does not return normally to caller" { > > + verbose -log "saw warning" > > + exp_continue > > + } > > + -re "Make noreturn_func return now.*y or n. $" { > > + send_gdb "n\n" > > + exp_continue > > + } > > + -re "Not confirmed.*$gdb_prompt $" { > > + pass "noreturn_func return cancelled" > > + } > > Make the test message be the same in all paths, like: > > set test "return from noreturn_func" > gdb_test_multiple "return" $test { > -re "warning: Function does not return normally to caller" { > verbose -log "saw warning" > exp_continue > } > -re "Make noreturn_func return now.*y or n. $" { > send_gdb "n\n" > exp_continue > } > -re "Not confirmed.*$gdb_prompt $" { > pass $test > } Changed > > + } > > +} > > + > > +clean_restart ${binfile} > > prepare_for_testing does this for you. Dropped. > > + > > +set timeout 30 > > I don't see why this is necessary. Dropped. > I think this is just a copy/paste? Yes, I just took the testcases you recommended and changed them to test the new warning messages added. > > +noreturn_test > > diff --git a/gdb/testsuite/gdb.base/noreturn_finish.c b/gdb/testsuite/gdb.base/noreturn_finish.c > > new file mode 100644 > > index 0000000..cd52769 > > --- /dev/null > > +++ b/gdb/testsuite/gdb.base/noreturn_finish.c > > @@ -0,0 +1,14 @@ > > +#include > > + > > +void __attribute__((noreturn)) > > +noreturn_func () > > +{ > > + abort (); > > +} > > + > > +int > > +main () > > +{ > > + noreturn_func (); > > + return 0; > > +} > > Same comments apply to noreturn_finish.c|exp. Same changes made. > Otherwise OK. Pushed as attached after retesting under GCC 5 and 4.8.2. Thanks, Mark From 743649fd80776de922475362bf3ac8b44511bb24 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. gdb/testsuite/ChangeLog * gdb.base/noreturn-return.c: New file. * gdb.base/noreturn-return.exp: New file. * gdb.base/noreturn-finish.c: New file. * gdb.base/noreturn-finish.exp: New file. 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 +++-- gdb/testsuite/ChangeLog | 7 ++++ gdb/testsuite/gdb.base/noreturn-finish.c | 31 ++++++++++++++++++ gdb/testsuite/gdb.base/noreturn-finish.exp | 51 ++++++++++++++++++++++++++++++ gdb/testsuite/gdb.base/noreturn-return.c | 31 ++++++++++++++++++ gdb/testsuite/gdb.base/noreturn-return.exp | 51 ++++++++++++++++++++++++++++++ include/ChangeLog | 4 +++ include/dwarf2.def | 2 ++ 12 files changed, 218 insertions(+), 5 deletions(-) create mode 100644 gdb/testsuite/gdb.base/noreturn-finish.c create mode 100644 gdb/testsuite/gdb.base/noreturn-finish.exp create mode 100644 gdb/testsuite/gdb.base/noreturn-return.c create mode 100644 gdb/testsuite/gdb.base/noreturn-return.exp diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 818eb7c..c195f5d 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,14 @@ +2015-01-15 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. + 2015-01-23 Pedro Alves * linux-nat.c (linux_is_async_p): New macro. diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c index 96b2537..9d765c5 100644 --- a/gdb/dwarf2read.c +++ b/gdb/dwarf2read.c @@ -14343,6 +14343,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 7c06a4f..ba5c857 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 7b26663..9a1fb8d 100644 --- a/gdb/infcmd.c +++ b/gdb/infcmd.c @@ -1872,7 +1872,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 0201d0a..5f2a3dc 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/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 1a17ba4..5b53cdd 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,3 +1,10 @@ +2015-01-15 Mark Wielaard + + * gdb.base/noreturn-return.c: New file. + * gdb.base/noreturn-return.exp: New file. + * gdb.base/noreturn-finish.c: New file. + * gdb.base/noreturn-finish.exp: New file. + 2015-01-23 Pedro Alves * gdb.threads/continue-pending-after-query.c: New file. diff --git a/gdb/testsuite/gdb.base/noreturn-finish.c b/gdb/testsuite/gdb.base/noreturn-finish.c new file mode 100644 index 0000000..3ec48cd --- /dev/null +++ b/gdb/testsuite/gdb.base/noreturn-finish.c @@ -0,0 +1,31 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2015 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +#include + +void __attribute__((noreturn)) +noreturn_func (void) +{ + abort (); +} + +int +main (void) +{ + noreturn_func (); + return 0; +} diff --git a/gdb/testsuite/gdb.base/noreturn-finish.exp b/gdb/testsuite/gdb.base/noreturn-finish.exp new file mode 100644 index 0000000..a2ae0db --- /dev/null +++ b/gdb/testsuite/gdb.base/noreturn-finish.exp @@ -0,0 +1,51 @@ +# Copyright 2015 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +standard_testfile + +if [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} {debug}] { + return -1 +} + +proc noreturn_finish_test { } { + global gdb_prompt + + if { ! [ runto_main ] } then { + untested ${testfile}.exp + return -1 + } + + gdb_breakpoint "noreturn_func" + gdb_continue_to_breakpoint "noreturn_func" + + set test "cancel finish from noreturn_func" + gdb_test_multiple "finish" $test { + -re "warning: Function noreturn_func does not return normally" { + verbose -log "saw warning" + exp_continue + } + -re "Try to finish anyway.*y or n.* $" { + send_gdb "n\n" + exp_continue + } + -re ".*$gdb_prompt $" { + pass $test + } + } +} + +clean_restart ${binfile} + +noreturn_finish_test diff --git a/gdb/testsuite/gdb.base/noreturn-return.c b/gdb/testsuite/gdb.base/noreturn-return.c new file mode 100644 index 0000000..3ec48cd --- /dev/null +++ b/gdb/testsuite/gdb.base/noreturn-return.c @@ -0,0 +1,31 @@ +/* This testcase is part of GDB, the GNU debugger. + + Copyright 2015 Free Software Foundation, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see . */ + +#include + +void __attribute__((noreturn)) +noreturn_func (void) +{ + abort (); +} + +int +main (void) +{ + noreturn_func (); + return 0; +} diff --git a/gdb/testsuite/gdb.base/noreturn-return.exp b/gdb/testsuite/gdb.base/noreturn-return.exp new file mode 100644 index 0000000..8a64a3e --- /dev/null +++ b/gdb/testsuite/gdb.base/noreturn-return.exp @@ -0,0 +1,51 @@ +# Copyright 2015 Free Software Foundation, Inc. + +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +standard_testfile + +if [prepare_for_testing "failed to prepare" ${testfile} ${srcfile} {debug}] { + return -1 +} + +proc noreturn_test { } { + global gdb_prompt + + if { ! [ runto_main ] } then { + untested ${testfile}.exp + return -1 + } + + gdb_breakpoint "noreturn_func" + gdb_continue_to_breakpoint "noreturn_func" + + set test "cancel return from noreturn_func" + gdb_test_multiple "return" $test { + -re "warning: Function does not return normally to caller" { + verbose -log "saw warning" + exp_continue + } + -re "Make noreturn_func return now.*y or n. $" { + send_gdb "n\n" + exp_continue + } + -re "Not confirmed.*$gdb_prompt $" { + pass $test + } + } +} + +clean_restart ${binfile} + +noreturn_test diff --git a/include/ChangeLog b/include/ChangeLog index 31f1e18..21b6ae6 100644 --- a/include/ChangeLog +++ b/include/ChangeLog @@ -1,3 +1,7 @@ +2015-01-15 Mark Wielaard + + * dwarf2.def (DW_AT_noreturn): New DWARF5 attribute. + 2015-01-14 Jan-Benedict Glaw * libiberty.h: Merge from GCC. diff --git a/include/dwarf2.def b/include/dwarf2.def index 5da3ae0..3aecdfc 100644 --- a/include/dwarf2.def +++ b/include/dwarf2.def @@ -306,6 +306,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