Re: [PATCH] Use GCC5/DWARF5 DW_AT_noreturn to mark functions that don't return normally.

[Mark Wielaard <mjw at redhat dot com>]

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 <http://www.gnu.org/licenses/>.
> > +
> > +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 <stdlib.h>
> > +
> > +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 <mjw@redhat.com>
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  <mjw@redhat.com>
+
+	* 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  <palves@redhat.com>
 
 	* 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  <mjw@redhat.com>
+
+	* 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  <palves@redhat.com>
 
 	* 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 <http://www.gnu.org/licenses/>.  */
+
+#include <stdlib.h>
+
+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 <http://www.gnu.org/licenses/>.
+
+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 <http://www.gnu.org/licenses/>.  */
+
+#include <stdlib.h>
+
+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 <http://www.gnu.org/licenses/>.
+
+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  <mjw@redhat.com>
+
+	* dwarf2.def (DW_AT_noreturn): New DWARF5 attribute.
+
 2015-01-14  Jan-Benedict Glaw  <jbglaw@lug-owl.de>
 
 	* 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