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