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 <ja...@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
>
--
Richard Biener <rguent...@suse.de>
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany;
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG Nürnberg)
