[Bug middle-end/88397] New: attribute malloc ignored on function pointers when alloc_size is accepted

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[Bug middle-end/88397] New: attribute malloc ignored on function pointers when alloc_size is accepted

kargl at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88397

            Bug ID: 88397
           Summary: attribute malloc ignored on function pointers when
                    alloc_size is accepted
           Product: gcc
           Version: 9.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
          Assignee: unassigned at gcc dot gnu.org
          Reporter: msebor at gcc dot gnu.org
  Target Milestone: ---

I noticed this while testing a fix for bug 88372.

GCC accepts attributes alloc_align and alloc_size on declarations of function
pointers but ignores (with a warning) attribute malloc.  Besides being
inconsistent, it makes it impossible to get GCC to emit optimally efficient
code for calls via function pointers to malloc-like functions (such as malloc
or aligned_alloc).  The same limitation applies to defining function types with
attribute malloc.

$ cat u.c && gcc -O2 -S -Wall -Wextra u.c
typedef __SIZE_TYPE__ size_t;

__attribute__ ((alloc_align (1), alloc_size (2)))
void* (*p)(size_t, size_t) = __builtin_aligned_alloc;

__attribute__ ((alloc_align (1), alloc_size (2), malloc))
void* (*q)(size_t, size_t) = __builtin_aligned_alloc;

u.c:7:1: warning: ‘malloc’ attribute ignored [-Wattributes]
    7 | void* (*q)(size_t, size_t) = __builtin_aligned_alloc;
      | ^~~~
Reply | Threaded
Open this post in threaded view
|

[Bug middle-end/88397] attribute malloc ignored on function pointers when alloc_size is accepted

kargl at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88397

Martin Sebor <msebor at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2018-12-07
           Assignee|unassigned at gcc dot gnu.org      |msebor at gcc dot gnu.org
     Ever confirmed|0                           |1

--- Comment #1 from Martin Sebor <msebor at gcc dot gnu.org> ---
Based on quick review of c-attribs.c it looks like attributes noreturn and
nothrow suffer from the same limitation as malloc.  It should be easy to fix.
I'm testing the patch below.  If it doesn't cause major problems I'll add
tests, update the manual, and post it.

Index: gcc/c-family/c-attribs.c
===================================================================
--- gcc/c-family/c-attribs.c    (revision 266881)
+++ gcc/c-family/c-attribs.c    (working copy)
@@ -257,7 +257,7 @@ const struct attribute_spec c_common_attribute_tab
      would require all the places in the compiler that use TREE_THIS_VOLATILE
      on a decl to identify non-returning functions to be located and fixed
      to check the function type instead.  */
-  { "noreturn",               0, 0, true,  false, false, false,
+  { "noreturn",               0, 0, false, true, true, false,
                              handle_noreturn_attribute,
                              attr_noreturn_exclusions },
   { "volatile",               0, 0, true,  false, false, false,
@@ -330,7 +330,7 @@ const struct attribute_spec c_common_attribute_tab
   { "no_profile_instrument_function",  0, 0, true, false, false, false,
                              handle_no_profile_instrument_function_attribute,
                              NULL },
-  { "malloc",                 0, 0, true,  false, false, false,
+  { "malloc",                 0, 0, false, true, true, false,
                              handle_malloc_attribute, attr_alloc_exclusions },
   { "returns_twice",          0, 0, true,  false, false, false,
                              handle_returns_twice_attribute,
@@ -372,7 +372,7 @@ const struct attribute_spec c_common_attribute_tab
                              handle_nonnull_attribute, NULL },
   { "nonstring",              0, 0, true, false, false, false,
                              handle_nonstring_attribute, NULL },
-  { "nothrow",                0, 0, true,  false, false, false,
+  { "nothrow",                0, 0, false, true, true, false,
                              handle_nothrow_attribute, NULL },
   { "may_alias",             0, 0, false, true, false, false, NULL, NULL },
   { "cleanup",               1, 1, true, false, false, false,
Reply | Threaded
Open this post in threaded view
|

[Bug middle-end/88397] attribute malloc ignored on function pointers when alloc_size is accepted

kargl at gcc dot gnu.org
In reply to this post by kargl at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88397

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
> noreturn
See the FIXME right above it:
  /* FIXME: logically, noreturn attributes should be listed as
     "false, true, true" and apply to function types.  But implementing this
     would require all the places in the compiler that use TREE_THIS_VOLATILE
     on a decl to identify non-returning functions to be located and fixed
     to check the function type instead.  */
Reply | Threaded
Open this post in threaded view
|

[Bug middle-end/88397] attribute malloc ignored on function pointers when alloc_size is accepted

kargl at gcc dot gnu.org
In reply to this post by kargl at gcc dot gnu.org
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88397

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I'm not sure we want to change how the attributes are defined, that is pretty
major change for the attributes.  For noreturn you just can't do that.  And,
the #c0 testcase shouldn't use __builtin other than in a direct call.