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