Fix patchable_function_entry attribute handling (PR c/89946)

Hi!

The following patch fixes various issues in patchable_function_entry
handling:
1) most importantly, it adds a warning if the argument(s) aren't integer
   constants or if they are negative and drops the attribute
2) if (tree_fits_uhwi_p (x)) y = tree_to_uhwi (x); else gcc_unreachable ();
   makes no sense, as tree_to_uhwi has the same gcc_assert already
3) list_length () > 1 might look cool, but we should have guaranteed it
   is only 1 or 2 argument and even if it was longer, computing whole
   length and then comparing it against 1 is inefficient
4) warnings shouldn't start with a capital letter
5) formatting fixes

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2019-04-12  Jakub Jelinek  <jakub@redhat.com>

	PR c/89946
	* varasm.c (assemble_start_function): Don't use tree_fits_uhwi_p
	and gcc_unreachable if it fails, just call tree_to_uhwi which
	verifies that too.  Test TREE_CHAIN instead of list_length > 1.
	Start warning message with a lower-case letter.  Formatting fixes.
c-family/
	* c-attribs.c (handle_patchable_function_entry_attribute): Add
	function comment.  Warn if arguments of the attribute are not positive
	integer constants.
testsuite/
	* c-c++-common/pr89946.c: New test.


	Jakub

On Fri, 12 Apr 2019, Jakub Jelinek wrote:

> Hi!
> 
> The following patch fixes various issues in patchable_function_entry
> handling:
> 1) most importantly, it adds a warning if the argument(s) aren't integer
>    constants or if they are negative and drops the attribute
> 2) if (tree_fits_uhwi_p (x)) y = tree_to_uhwi (x); else gcc_unreachable ();
>    makes no sense, as tree_to_uhwi has the same gcc_assert already
> 3) list_length () > 1 might look cool, but we should have guaranteed it
>    is only 1 or 2 argument and even if it was longer, computing whole
>    length and then comparing it against 1 is inefficient
> 4) warnings shouldn't start with a capital letter
> 5) formatting fixes
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Richard.

> 2019-04-12  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c/89946
> 	* varasm.c (assemble_start_function): Don't use tree_fits_uhwi_p
> 	and gcc_unreachable if it fails, just call tree_to_uhwi which
> 	verifies that too.  Test TREE_CHAIN instead of list_length > 1.
> 	Start warning message with a lower-case letter.  Formatting fixes.
> c-family/
> 	* c-attribs.c (handle_patchable_function_entry_attribute): Add
> 	function comment.  Warn if arguments of the attribute are not positive
> 	integer constants.
> testsuite/
> 	* c-c++-common/pr89946.c: New test.
> 
> --- gcc/varasm.c.jj	2019-02-28 08:14:58.438559862 +0100
> +++ gcc/varasm.c	2019-04-11 15:34:46.933248354 +0200
> @@ -1865,28 +1865,20 @@ assemble_start_function (tree decl, cons
>        tree pp_val = TREE_VALUE (patchable_function_entry_attr);
>        tree patchable_function_entry_value1 = TREE_VALUE (pp_val);
>  
> -      if (tree_fits_uhwi_p (patchable_function_entry_value1))
> -	patch_area_size = tree_to_uhwi (patchable_function_entry_value1);
> -      else
> -	gcc_unreachable ();
> -
> +      patch_area_size = tree_to_uhwi (patchable_function_entry_value1);
>        patch_area_entry = 0;
> -      if (list_length (pp_val) > 1)
> +      if (TREE_CHAIN (pp_val) != NULL_TREE)
>  	{
> -	  tree patchable_function_entry_value2 =
> -	    TREE_VALUE (TREE_CHAIN (pp_val));
> -
> -	  if (tree_fits_uhwi_p (patchable_function_entry_value2))
> -	    patch_area_entry = tree_to_uhwi (patchable_function_entry_value2);
> -	  else
> -	    gcc_unreachable ();
> +	  tree patchable_function_entry_value2
> +	    = TREE_VALUE (TREE_CHAIN (pp_val));
> +	  patch_area_entry = tree_to_uhwi (patchable_function_entry_value2);
>  	}
>      }
>  
>    if (patch_area_entry > patch_area_size)
>      {
>        if (patch_area_size > 0)
> -	warning (OPT_Wattributes, "Patchable function entry > size");
> +	warning (OPT_Wattributes, "patchable function entry > size");
>        patch_area_entry = 0;
>      }
>  
> @@ -1906,7 +1898,8 @@ assemble_start_function (tree decl, cons
>    /* And the area after the label.  Record it if we haven't done so yet.  */
>    if (patch_area_size > patch_area_entry)
>      targetm.asm_out.print_patchable_function_entry (asm_out_file,
> -					     patch_area_size-patch_area_entry,
> +						    patch_area_size
> +						    - patch_area_entry,
>  						    patch_area_entry == 0);
>  
>    if (lookup_attribute ("no_split_stack", DECL_ATTRIBUTES (decl)))
> --- gcc/c-family/c-attribs.c.jj	2019-04-08 10:11:26.706250584 +0200
> +++ gcc/c-family/c-attribs.c	2019-04-11 15:51:45.851326928 +0200
> @@ -3988,10 +3988,29 @@ handle_fallthrough_attribute (tree *, tr
>    return NULL_TREE;
>  }
>  
> +/* Handle a "patchable_function_entry" attributes; arguments as in
> +   struct attribute_spec.handler.  */
> +
>  static tree
> -handle_patchable_function_entry_attribute (tree *, tree, tree, int, bool *)
> +handle_patchable_function_entry_attribute (tree *, tree name, tree args,
> +					   int, bool *no_add_attrs)
>  {
> -  /* Nothing to be done here.  */
> +  for (; args; args = TREE_CHAIN (args))
> +    {
> +      tree val = TREE_VALUE (args);
> +      if (val && TREE_CODE (val) != IDENTIFIER_NODE
> +	  && TREE_CODE (val) != FUNCTION_DECL)
> +	val = default_conversion (val);
> +
> +      if (!tree_fits_uhwi_p (val))
> +	{
> +	  warning (OPT_Wattributes,
> +		   "%qE attribute argument %qE is not an integer constant",
> +		   name, val);
> +	  *no_add_attrs = true;
> +	  return NULL_TREE;
> +	}
> +    }
>    return NULL_TREE;
>  }
>  
> --- gcc/testsuite/c-c++-common/pr89946.c.jj	2019-04-11 15:52:33.616532035 +0200
> +++ gcc/testsuite/c-c++-common/pr89946.c	2019-04-11 15:51:29.303602307 +0200
> @@ -0,0 +1,7 @@
> +/* PR c/89946 */
> +
> +__attribute__((patchable_function_entry (-1))) void foo (void) {}	/* { dg-warning "'patchable_function_entry' attribute argument '-1' is not an integer constant" } */
> +__attribute__((patchable_function_entry (5, -5))) void bar (void) {}	/* { dg-warning "'patchable_function_entry' attribute argument '-5' is not an integer constant" } */
> +int i, j;
> +__attribute__((patchable_function_entry (i))) void baz (void) {}	/* { dg-warning "'patchable_function_entry' attribute argument 'i' is not an integer constant" } */
> +__attribute__((patchable_function_entry (2, j))) void qux (void) {}	/* { dg-warning "'patchable_function_entry' attribute argument 'j' is not an integer constant" } */
> 
> 	Jakub
>