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

Reply via email to