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

[Pedro Alves <palves@redhat.com>]

Hi Mark,

Thanks for doing this.

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 "!".  :-)

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

> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
> index 36cbbd9..56212e5 100644
> --- a/gdb/dwarf2read.c
> +++ b/gdb/dwarf2read.c
> @@ -14312,6 +14312,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))

 if (attr != NULL && (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.  */



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

> --- 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 / _() .

Thanks,
Pedro Alves