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

[Pedro Alves <palves at redhat dot com>]

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

> 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).


> +void __attribute__((noreturn))
> +noreturn_func ()

noreturn_func (void)


> +{
> +  while (1)
> +    ;

Please don't make the test loop forever if GDB crashes.
Does e.g., "abort()" like the other test work here too?

> +}
> +
> +int
> +main ()

likewise (void)

> +{
> +  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.

> +
> +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
}


> +
> +proc noreturn_test { } {
> +    global gdb_prompt
> +
> +    if { ! [ runto_main ] } then {
> +	untested noreturn.exp

Use $testfile, like "untested ${testfile}.exp".

> +	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"

> +
> +    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
	}

> +   }
> +}
> +
> +clean_restart ${binfile}

prepare_for_testing does this for you.

> +
> +set timeout 30

I don't see why this is necessary.  I think this is just a copy/paste?

> +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.

Otherwise OK.

Thanks,
Pedro Alves