[Bug c++/108871] New: attribute nonnull does not spot nullptr O2 and above when function inlined

[Bug c++/108871] New: attribute nonnull does not spot nullptr O2 and above when function inlined

[jg at jguk dot org]

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

            Bug ID: 108871
           Summary: attribute nonnull does not spot nullptr O2 and above
                    when function inlined
           Product: gcc
           Version: 12.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: jg at jguk dot org
  Target Milestone: ---

With Optimizations level 1 it still works.

https://godbolt.org/z/6axdfchr8

-fanalyzer -std=c++23 -O3 -Wnonnull -Wall
-Wno-analyzer-use-of-uninitialized-value


#include <string>

std::string make_std_string(const char * const str) __attribute__ ((nonnull));

std::string make_std_string(const char * const str)
{
    return std::string(str);
}

int main()
{
    const char * a = NULL;
    std::string result = make_std_string(a);
    __builtin_puts(result.c_str());
}

[Bug c++/108871] attribute nonnull does not spot nullptr O2 and above when function inlined

[jg at jguk dot org]

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

--- Comment #1 from Jonny Grant <jg at jguk dot org> ---
gcc-help mailing list discussion
https://gcc.gnu.org/pipermail/gcc-help/2023-February/142279.html

[Bug ipa/108871] attribute nonnull does not spot nullptr O2 and above when function inlined

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |marxin at gcc dot gnu.org
           Keywords|                            |missed-optimization
          Component|c++                         |ipa

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
What could be done is insert assertions into the IL when inlining functions
with attributes that assert something on their arguments (or return value) via
function attributes.

Note one nice thing about having it in attributes and across calls only is
that you do not have to worry about code motion across assertions creating
wrong code.

[Bug ipa/108871] attribute nonnull does not spot nullptr O2 and above when function inlined

[pinskia at gcc dot gnu.org]

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

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

[Bug ipa/108871] attribute nonnull does not spot nullptr O2 and above when function inlined

[jg at jguk dot org]

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

--- Comment #4 from Jonny Grant <jg at jguk dot org> ---
(In reply to Andrew Pinski from comment #3)
> *** Bug 108893 has been marked as a duplicate of this bug. ***

Hello Andrew
May I check, I thought attribute access read_only was different from attribute
nonnull?

https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html

"Note that the access attribute merely specifies how an object referenced by
the pointer argument can be accessed; it does not imply that an access will
happen. Also, the access attribute does not imply the attribute nonnull; it may
be appropriate to add both attributes at the declaration of a function that
unconditionally manipulates a buffer via a pointer argument. See the nonnull
attribute for more information and caveats."

Seems to be stating, nonnull is not the same as access read_only.  Or am I
missing something?



I had understood __attribute__((access(read_only, 1)));  would be enough on a
-O3 -Wall build to generate a warning that a nullptr had been passed (even if
it was not actually dereferenced)



Here is an example that actually does derefernence, and runtime SEGV, but it
the access read_only itself doesn't give any build warning. Just an example,
I'm only hoping to get build warnings when optimizer sees that nullptr got
through

g++ -std=c++23 -O3 -Wall
https://godbolt.org/z/KbzcjEjYj

void f(const char * const str) __attribute__((access(read_only,1)));
void f(const char * const str)
{
    if(*str == 'a')
    {
        __builtin_puts("a1");
    }
    else
    {
        __builtin_puts("a2");
    }
}

int main()
{
    const char * a = nullptr;
    f(a);
}

[Bug ipa/108871] attribute nonnull does not spot nullptr O2 and above when function inlined

[redi at gcc dot gnu.org]

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

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #3)
> *** Bug 108893 has been marked as a duplicate of this bug. ***

N.B. this one is about __attribute__((access(read_only, 1))) not nonnull. The
docs already seem to imply that it requires a non-null pointer:

  The read_only access mode specifies that the pointer to which it applies is
used to
  read the referenced object but not write to it. Unless the argument
specifying the
  size of the access denoted by size-index is zero, the referenced object must
be
  initialized.

If a non-zero size implies an initialized object, then it also implies a
non-null pointer (since a null pointer doesn't point to an initialized object).

I don't know if we want this PR to be specific to the nonnull attribute, or if
it makes sense to use it for access(read_only) too.

[Bug ipa/108871] attribute nonnull does not spot nullptr O2 and above when function inlined

[jakub at gcc dot gnu.org]

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

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

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

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #5)
> (In reply to Andrew Pinski from comment #3)
> > *** Bug 108893 has been marked as a duplicate of this bug. ***
> 
> N.B. this one is about __attribute__((access(read_only, 1))) not nonnull.
> The docs already seem to imply that it requires a non-null pointer:
> 
>   The read_only access mode specifies that the pointer to which it applies
> is used to
>   read the referenced object but not write to it. Unless the argument
> specifying the
>   size of the access denoted by size-index is zero, the referenced object
> must be
>   initialized.
> 
> If a non-zero size implies an initialized object, then it also implies a
> non-null pointer (since a null pointer doesn't point to an initialized
> object).
> 
> I don't know if we want this PR to be specific to the nonnull attribute, or
> if it makes sense to use it for access(read_only) too.

On the other side, looking at glibc sources, access attributes there are used
on many functions together with nonnull attributes for the same arguments and
in many cases in places where there are not nonnull attributes.  So, making
access attribute imply non-null might not be desirable in real-world unless it
is already implied.
So it might be just that it is badly documented.

[Bug ipa/108871] attribute nonnull does not spot nullptr O2 and above when function inlined

[jg at jguk dot org]

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

--- Comment #7 from Jonny Grant <jg at jguk dot org> ---
(In reply to Jakub Jelinek from comment #6)
> (In reply to Jonathan Wakely from comment #5)
> > (In reply to Andrew Pinski from comment #3)
> > > *** Bug 108893 has been marked as a duplicate of this bug. ***
> > 
> > N.B. this one is about __attribute__((access(read_only, 1))) not nonnull.
> > The docs already seem to imply that it requires a non-null pointer:
> > 
> >   The read_only access mode specifies that the pointer to which it applies
> > is used to
> >   read the referenced object but not write to it. Unless the argument
> > specifying the
> >   size of the access denoted by size-index is zero, the referenced object
> > must be
> >   initialized.
> > 
> > If a non-zero size implies an initialized object, then it also implies a
> > non-null pointer (since a null pointer doesn't point to an initialized
> > object).
> > 
> > I don't know if we want this PR to be specific to the nonnull attribute, or
> > if it makes sense to use it for access(read_only) too.
> 
> On the other side, looking at glibc sources, access attributes there are
> used on many functions together with nonnull attributes for the same
> arguments and in many cases in places where there are not nonnull
> attributes.  So, making access attribute imply non-null might not be
> desirable in real-world unless it is already implied.
> So it might be just that it is badly documented.

Hi Jakub, Did you manage to get gcc to give a build warning from those
__attr_access read_only lines in glibc?  I haven't found a way to activate any
build warnings.  

attribute nonnull seems to be used by optimizer to remove any NULL checks it
finds.

[Bug ipa/108871] attribute nonnull does not spot nullptr O2 and above when function inlined

[jg at jguk dot org]

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

--- Comment #8 from Jonny Grant <jg at jguk dot org> ---
Another test case.

https://godbolt.org/z/qss7jj51x


I noticed when not using -fanalyzer gcc still warns about __builtin_puts being
passed NULL. However gcc doesn't warn about my own function with attribute
nullptr.  Maybe I'm missing something.


With puts() it gives a nice warning

In function 'void f2(const char*)',
    inlined from 'void f1(const char*)' at <source>:22:7,
    inlined from 'int main()' at <source>:28:7:
<source>:11:19: warning: argument 1 null where non-null expected [-Wnonnull]
   11 |     __builtin_puts(str);
      |     ~~~~~~~~~~~~~~^~~~~


// -std=c++23 -O1 -Wnonnull -Wall

// Test case that shows a difference in behavour,
// __builtin_puts generates a warning but not f1()
// Note, not using -fanalyzer

// Change this to 1 to see nonnull warning
#define USE_PUTS 0

void f2(const char * str)
{
#if USE_PUTS
    __builtin_puts(str);
#else
    char a = *str;
    __builtin_printf("%c", a);
#endif
}

void f1(const char * const str) __attribute__ ((nonnull));
void f1(const char * const str)
{

    f2(str);
}

int main()
{
    const char * a = nullptr;
    f1(a);
}