[Bug ipa/109914] New: --suggest-attribute=pure misdiagnoses static functions

[Bug ipa/109914] New: --suggest-attribute=pure misdiagnoses static functions

[bruno at clisp dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109914

            Bug ID: 109914
           Summary: --suggest-attribute=pure misdiagnoses static functions
           Product: gcc
           Version: 13.1.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: ipa
          Assignee: unassigned at gcc dot gnu.org
          Reporter: bruno at clisp dot org
                CC: marxin at gcc dot gnu.org
  Target Milestone: ---

Created attachment 55125
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=55125&action=edit
test case

GCC's --suggest-attribute=pure diagnoses static functions, even though the
'pure' attribute is useless for static functions (after all, the compiler has
deduced the property on its own). This is leading to my having to litter code
with '__attribute__ (pure)' declarations merely to pacify GCC. GCC should treat
the 'pure' attribute like other attributes (e.g., malloc, const), and should
issue the diagnostic only for non-static functions where the attribute is in
fact useful.

How to reproduce:
$ gcc --version
gcc (GCC) 13.1.0
...
$ gcc -Wsuggest-attribute=pure -O2 -S file-has-acl.c
file-has-acl.c: In function ‘have_xattr’:
file-has-acl.c:3385:14: warning: function might be candidate for attribute
‘pure’ if it is known to return normally [-Wsuggest-attribute=pure]
 3385 | static _Bool have_xattr (char const *attr, char const *listbuf, ssize_t
listsize)
      |              ^~~~~~~~~~

[Bug ipa/109914] --suggest-attribute=pure misdiagnoses static functions

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109914

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
*** Bug 109915 has been marked as a duplicate of this bug. ***

[Bug ipa/109914] --suggest-attribute=pure misdiagnoses static functions

[hubicka at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109914

--- Comment #2 from Jan Hubicka <hubicka at gcc dot gnu.org> ---
The reason why gcc warns is that it is unable to prove that the function is
always finite. This means that it can not auto-detect pure attribute since
optimizing the call out may turn infinite program to finite one. 
So adding the attribute would still help compiler to know that the loops are
indeed finite.

[Bug ipa/109914] --suggest-attribute=pure misdiagnoses static functions

[bruno at clisp dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=109914

--- Comment #3 from Bruno Haible <bruno at clisp dot org> ---
(In reply to Jan Hubicka from comment #2)
> The reason why gcc warns is that it is unable to prove that the function is
> always finite. This means that it can not auto-detect pure attribute since
> optimizing the call out may turn infinite program to finite one. 
> So adding the attribute would still help compiler to know that the loops are
> indeed finite.

Thanks for explaining. So, the warning asks the developer not only to add an
__attribute__((__pure__)) marker, but also to verify that the function
terminates. In this case, it does, but it took me a minute of reflection to
convince myself.

For what purpose shall the developer make this effort? The documentation
https://gcc.gnu.org/onlinedocs/gcc-13.1.0/gcc/Common-Function-Attributes.html
says that it's to allow the compiler to do common subexpression elimination.
But in this case, the compiler could easily find out that it cannot do common
subexpression elimination anyway, because:
  - The only caller of this function (have_xattr) is file_has_acl.
  - In this function, there are three calls to have_xattr.
  - Each of them is executed only at most once. Control flow analysis shows
this.
  - Each of them has different argument lists: The first argument is a string
literal in each case, namely "system.nfs4_acl", "system.posix_acl_access",
"system.posix_acl_default" respectively.
So, there is no possibility for common subexpression elimination here, even if
the function was marked "pure".

Therefore it is pointless to suggest to the developer that it would be a gain
to mark the function as "pure" and that it is worth spending brain cycles on
that.