_Nullable and _Nonnull in GCC's analyzer (was: [PATCH v5] libio: Add nonnull attribute for most FILE * arguments in stdio.h)

_Nullable and _Nonnull in GCC's analyzer (was: [PATCH v5] libio: Add nonnull attribute for most FILE * arguments in stdio.h)

[Alejandro Colomar]

[-- Attachment #1.1: Type: text/plain, Size: 1359 bytes --]

[CC += Andrew]

Hi Xi, Andrew,

On 7/10/23 20:41, Xi Ruoyao wrote:
> Maybe we should have a weaker version of nonnull which only performs the
> diagnostic, not the optimization.  But it looks like they hate the idea:
> https://gcc.gnu.org/PR110617.
> 
This is the one thing that makes me use both Clang and GCC to compile,
because while any of them would be enough to build, I want as much
static analysis as I can get, and so I want -fanalyzer (so I need GCC),
but I also use _Nullable (so I need Clang).

If GCC had support for _Nullable, I would have in GCC the superset of
features that I need from both in a single vendor.  Moreover, Clang's
static analyzer is brain-damaged (sorry, but it doesn't have a simple
command line to run it, contrary to GCC's easy -fanalyzer), so having
GCC's analyzer get over those _Nullable qualifiers would be great.

Clang's _Nullable (and _Nonnull) are not very useful outside of analyzer
mode, as there are many cases where the compiler doesn't have enough
information, and the analyzer can get rid of false negatives and
positives.  See: <https://github.com/llvm/llvm-project/issues/57546>

I'll back the ask for the qualifiers in GCC, for compatibility with
Clang.

Thanks,
Alex

-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: _Nullable and _Nonnull in GCC's analyzer (was: [PATCH v5] libio: Add nonnull attribute for most FILE * arguments in stdio.h)

[Alejandro Colomar]

[-- Attachment #1.1: Type: text/plain, Size: 1706 bytes --]

On 7/10/23 22:14, Alejandro Colomar wrote:
> [CC += Andrew]
> 
> Hi Xi, Andrew,
> 
> On 7/10/23 20:41, Xi Ruoyao wrote:
>> Maybe we should have a weaker version of nonnull which only performs the
>> diagnostic, not the optimization.  But it looks like they hate the idea:
>> https://gcc.gnu.org/PR110617.
>>
> This is the one thing that makes me use both Clang and GCC to compile,
> because while any of them would be enough to build, I want as much
> static analysis as I can get, and so I want -fanalyzer (so I need GCC),
> but I also use _Nullable (so I need Clang).
> 
> If GCC had support for _Nullable, I would have in GCC the superset of
> features that I need from both in a single vendor.  Moreover, Clang's
> static analyzer is brain-damaged (sorry, but it doesn't have a simple
> command line to run it, contrary to GCC's easy -fanalyzer), so having
> GCC's analyzer get over those _Nullable qualifiers would be great.
> 
> Clang's _Nullable (and _Nonnull) are not very useful outside of analyzer
> mode, as there are many cases where the compiler doesn't have enough
> information, and the analyzer can get rid of false negatives and
> positives.  See: <https://github.com/llvm/llvm-project/issues/57546>
> 
> I'll back the ask for the qualifiers in GCC, for compatibility with
> Clang.

BTW, Bionic libc is adding those qualifiers:

<https://android-review.googlesource.com/c/platform/bionic/+/2552567/7/libc/include/netinet/ether.h#45>
<https://android-review.googlesource.com/q/owner:zijunzhao@google.com+Nullability>

> 
> Thanks,
> Alex
> 

-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: _Nullable and _Nonnull in GCC's analyzer (was: [PATCH v5] libio: Add nonnull attribute for most FILE * arguments in stdio.h)

[Martin Uecker]

Am Montag, dem 10.07.2023 um 22:16 +0200 schrieb Alejandro Colomar via Gcc:
> On 7/10/23 22:14, Alejandro Colomar wrote:
> > [CC += Andrew]
> > 
> > Hi Xi, Andrew,
> > 
> > On 7/10/23 20:41, Xi Ruoyao wrote:
> > > Maybe we should have a weaker version of nonnull which only performs the
> > > diagnostic, not the optimization.  But it looks like they hate the idea:
> > > https://gcc.gnu.org/PR110617.
> > > 
> > This is the one thing that makes me use both Clang and GCC to compile,
> > because while any of them would be enough to build, I want as much
> > static analysis as I can get, and so I want -fanalyzer (so I need GCC),
> > but I also use _Nullable (so I need Clang).
> > 
> > If GCC had support for _Nullable, I would have in GCC the superset of
> > features that I need from both in a single vendor.  Moreover, Clang's
> > static analyzer is brain-damaged (sorry, but it doesn't have a simple
> > command line to run it, contrary to GCC's easy -fanalyzer), so having
> > GCC's analyzer get over those _Nullable qualifiers would be great.
> > 
> > Clang's _Nullable (and _Nonnull) are not very useful outside of analyzer
> > mode, as there are many cases where the compiler doesn't have enough
> > information, and the analyzer can get rid of false negatives and
> > positives.  See: <https://github.com/llvm/llvm-project/issues/57546>
> > 
> > I'll back the ask for the qualifiers in GCC, for compatibility with
> > Clang.
> 
> BTW, Bionic libc is adding those qualifiers:
> 
> <https://android-review.googlesource.com/c/platform/bionic/+/2552567/7/libc/include/netinet/ether.h#45>
> <https://android-review.googlesource.com/q/owner:zijunzhao@google.com+Nullability>
> 
> 

I am sceptical about these qualifiers because they do
not fit well with this language mechanism.   Qualifiers take
effect at accesses to objects and are discarded at lvalue
conversion.  So a qualifier should ideally describe a property
that is relevant for accessing objects but is not relevant
for values.

Also there are built-in conversion rules a qualifier should
conform to.  A pointer pointing to a type without qualifier 
can implicitely convert to a pointer to a type with qualifier,
e.g.   int*  can be converted to const int*.

Together, this implies that a qualifier should add constraints
to a type that are relevant to how an object is accessed.

"const" and "volatile" or "_Atomic" are good examples.
("restricted" does not quite follow these rules and is 
considered weird and difficult to understand.)

In contrast, "nonnull" and "nullable" are properties that
affect the set of values of the pointer, but not how the
pointer itself is accessed. 


Martin

Re: _Nullable and _Nonnull in GCC's analyzer (was: [PATCH v5] libio: Add nonnull attribute for most FILE * arguments in stdio.h)

[enh]

(bionic maintainer here, mostly by accident...)

yeah, we tried the GCC attributes once before with _disastrous_
results (because GCC saw it as an excuse to delete explicit null
checks, it broke all kinds of things). the clang attributes are
"better" in that they don't confuse two entirely separate notions ...
but they're not "good" because the analysis is so limited. i think we
were hoping for something more like the "used but not set" kind of
diagnostic, but afaict it really only catches _direct_ use of a
literal nullptr. so:

  foo(nullptr); // caught

  void* p = nullptr;
  foo(p); // not caught

without getting on to anything fancy like:

  void* p;
  if (a) p = bar();
  foo(p);

we've done all the annotations anyway, but we're not expecting to
actually catch many bugs with them, and in fact did not catch any real
bugs in AOSP while adding the annotations. (though we did find several
"you're not _wrong_, but ..." bits of code :-) )

what i really want for christmas is the annotation that lets me say
"this size_t argument tells you the size of this array argument" and
actually does something usefully fortify-like with that. i've seen
proposals like https://discourse.llvm.org/t/rfc-enforcing-bounds-safety-in-c-fbounds-safety/70854/
but i haven't seen anything i can use yet, so you -- who do use GCC
which aiui has something along these lines already -- will find out
what's good/bad about it before i do...

On Tue, Aug 8, 2023 at 3:01 AM Martin Uecker <muecker@gwdg.de> wrote:
>
> Am Montag, dem 10.07.2023 um 22:16 +0200 schrieb Alejandro Colomar via Gcc:
> > On 7/10/23 22:14, Alejandro Colomar wrote:
> > > [CC += Andrew]
> > >
> > > Hi Xi, Andrew,
> > >
> > > On 7/10/23 20:41, Xi Ruoyao wrote:
> > > > Maybe we should have a weaker version of nonnull which only performs the
> > > > diagnostic, not the optimization.  But it looks like they hate the idea:
> > > > https://gcc.gnu.org/PR110617.
> > > >
> > > This is the one thing that makes me use both Clang and GCC to compile,
> > > because while any of them would be enough to build, I want as much
> > > static analysis as I can get, and so I want -fanalyzer (so I need GCC),
> > > but I also use _Nullable (so I need Clang).
> > >
> > > If GCC had support for _Nullable, I would have in GCC the superset of
> > > features that I need from both in a single vendor.  Moreover, Clang's
> > > static analyzer is brain-damaged (sorry, but it doesn't have a simple
> > > command line to run it, contrary to GCC's easy -fanalyzer), so having
> > > GCC's analyzer get over those _Nullable qualifiers would be great.
> > >
> > > Clang's _Nullable (and _Nonnull) are not very useful outside of analyzer
> > > mode, as there are many cases where the compiler doesn't have enough
> > > information, and the analyzer can get rid of false negatives and
> > > positives.  See: <https://github.com/llvm/llvm-project/issues/57546>
> > >
> > > I'll back the ask for the qualifiers in GCC, for compatibility with
> > > Clang.
> >
> > BTW, Bionic libc is adding those qualifiers:
> >
> > <https://android-review.googlesource.com/c/platform/bionic/+/2552567/7/libc/include/netinet/ether.h#45>
> > <https://android-review.googlesource.com/q/owner:zijunzhao@google.com+Nullability>
> >
> >
>
> I am sceptical about these qualifiers because they do
> not fit well with this language mechanism.   Qualifiers take
> effect at accesses to objects and are discarded at lvalue
> conversion.  So a qualifier should ideally describe a property
> that is relevant for accessing objects but is not relevant
> for values.
>
> Also there are built-in conversion rules a qualifier should
> conform to.  A pointer pointing to a type without qualifier
> can implicitely convert to a pointer to a type with qualifier,
> e.g.   int*  can be converted to const int*.
>
> Together, this implies that a qualifier should add constraints
> to a type that are relevant to how an object is accessed.
>
> "const" and "volatile" or "_Atomic" are good examples.
> ("restricted" does not quite follow these rules and is
> considered weird and difficult to understand.)
>
> In contrast, "nonnull" and "nullable" are properties that
> affect the set of values of the pointer, but not how the
> pointer itself is accessed.
>
>
> Martin
>
>
>
>
>

Re: _Nullable and _Nonnull in GCC's analyzer (was: [PATCH v5] libio: Add nonnull attribute for most FILE * arguments in stdio.h)

[Siddhesh Poyarekar]

On 2023-08-08 20:14, enh wrote:
> (bionic maintainer here, mostly by accident...)
> 
> yeah, we tried the GCC attributes once before with _disastrous_
> results (because GCC saw it as an excuse to delete explicit null
> checks, it broke all kinds of things). the clang attributes are

AFAICT based on earlier discussions around this patch, the NULL check 
deletion anomalies in gcc seem to have been fixed recently.

> "better" in that they don't confuse two entirely separate notions ...
> but they're not "good" because the analysis is so limited. i think we
> were hoping for something more like the "used but not set" kind of
> diagnostic, but afaict it really only catches _direct_ use of a
> literal nullptr. so:
> 
>    foo(nullptr); // caught
> 
>    void* p = nullptr;
>    foo(p); // not caught
> 
> without getting on to anything fancy like:
> 
>    void* p;
>    if (a) p = bar();
>    foo(p);
> 
> we've done all the annotations anyway, but we're not expecting to
> actually catch many bugs with them, and in fact did not catch any real
> bugs in AOSP while adding the annotations. (though we did find several
> "you're not _wrong_, but ..." bits of code :-) )

I believe it did catch a few issues in the glibc test cases when we 
enabled fortification internally in addition to flagging several "you're 
not _wrong_, but..." bits.  In any case, it's only enabled with 
fortification since we use that as a hint for wanting safer code.

> what i really want for christmas is the annotation that lets me say
> "this size_t argument tells you the size of this array argument" and
> actually does something usefully fortify-like with that. i've seen
> proposals like https://discourse.llvm.org/t/rfc-enforcing-bounds-safety-in-c-fbounds-safety/70854/
> but i haven't seen anything i can use yet, so you -- who do use GCC
> which aiui has something along these lines already -- will find out
> what's good/bad about it before i do...

I think a lot of this is covered by __access__, __alloc_size__ and the 
upcoming __counted_by__ attributes.  There are still gaps that something 
like -fbounds-safety would cover (I think -fsanitize=bounds and 
-fsanitize=object-size further cover some of those gaps but there's a 
general lack of confidence in using them in production due to 
performance concerns; fbounds-safety has those concerns too AFAICT) but 
that gap is getting narrower.

Thanks,
Sid

Re: _Nullable and _Nonnull in GCC's analyzer (was: [PATCH v5] libio: Add nonnull attribute for most FILE * arguments in stdio.h)

[Martin Uecker]

Am Dienstag, dem 08.08.2023 um 17:14 -0700 schrieb enh:
> (bionic maintainer here, mostly by accident...)
> 
> yeah, we tried the GCC attributes once before with _disastrous_
> results (because GCC saw it as an excuse to delete explicit null
> checks, it broke all kinds of things).

Thanks! I am currently exploring different options and what
could be done to improve the situation from the language
as well as compile side.  It looks we have some partial
solution but they do not work quite right or do not
fit together perfectly.


>  the clang attributes are
> "better" in that they don't confuse two entirely separate notions ...
> but they're not "good" because the analysis is so limited. i think we
> were hoping for something more like the "used but not set" kind of
> diagnostic, but afaict it really only catches _direct_ use of a
> literal nullptr. so:
> 
>   foo(nullptr); // caught
> 
>   void* p = nullptr;
>   foo(p); // not caught
> 
> without getting on to anything fancy like:
> 
>   void* p;
>   if (a) p = bar();
>   foo(p);
> 
> we've done all the annotations anyway, but we're not expecting to
> actually catch many bugs with them, and in fact did not catch any real
> bugs in AOSP while adding the annotations. (though we did find several
> "you're not _wrong_, but ..." bits of code :-) )
> 
> what i really want for christmas is the annotation that lets me say
> "this size_t argument tells you the size of this array argument" and
> actually does something usefully fortify-like with that.
> 

What is your opinion about the access attribute?

https://godbolt.org/z/PPfajefvP

it is a bit cumbersome to use, but one can use [static]
instead, which gives you the same static warnings.

[static] does not work with __builtin_dynamic_object_size,
but maybe this could be changed (there is a bug filed.)

I am not sure whether [static] should imply [[gnu::nonnull]]
which would then also trigger the optimization. I think
clang uses it for optimization.

Martin


>  i've seen
> proposals like https://discourse.llvm.org/t/rfc-enforcing-bounds-safety-in-c-fbounds-safety/70854/
> but i haven't seen anything i can use yet, so you -- who do use GCC
> which aiui has something along these lines already -- will find out
> what's good/bad about it before i do...



> 
> On Tue, Aug 8, 2023 at 3:01 AM Martin Uecker <muecker@gwdg.de> wrote:
> > 
> > Am Montag, dem 10.07.2023 um 22:16 +0200 schrieb Alejandro Colomar via Gcc:
> > > On 7/10/23 22:14, Alejandro Colomar wrote:
> > > > [CC += Andrew]
> > > > 
> > > > Hi Xi, Andrew,
> > > > 
> > > > On 7/10/23 20:41, Xi Ruoyao wrote:
> > > > > Maybe we should have a weaker version of nonnull which only performs the
> > > > > diagnostic, not the optimization.  But it looks like they hate the idea:
> > > > > https://gcc.gnu.org/PR110617.
> > > > > 
> > > > This is the one thing that makes me use both Clang and GCC to compile,
> > > > because while any of them would be enough to build, I want as much
> > > > static analysis as I can get, and so I want -fanalyzer (so I need GCC),
> > > > but I also use _Nullable (so I need Clang).
> > > > 
> > > > If GCC had support for _Nullable, I would have in GCC the superset of
> > > > features that I need from both in a single vendor.  Moreover, Clang's
> > > > static analyzer is brain-damaged (sorry, but it doesn't have a simple
> > > > command line to run it, contrary to GCC's easy -fanalyzer), so having
> > > > GCC's analyzer get over those _Nullable qualifiers would be great.
> > > > 
> > > > Clang's _Nullable (and _Nonnull) are not very useful outside of analyzer
> > > > mode, as there are many cases where the compiler doesn't have enough
> > > > information, and the analyzer can get rid of false negatives and
> > > > positives.  See: <https://github.com/llvm/llvm-project/issues/57546>
> > > > 
> > > > I'll back the ask for the qualifiers in GCC, for compatibility with
> > > > Clang.
> > > 
> > > BTW, Bionic libc is adding those qualifiers:
> > > 
> > > <https://android-review.googlesource.com/c/platform/bionic/+/2552567/7/libc/include/netinet/ether.h#45>
> > > <https://android-review.googlesource.com/q/owner:zijunzhao@google.com+Nullability>
> > > 
> > > 
> > 
> > I am sceptical about these qualifiers because they do
> > not fit well with this language mechanism.   Qualifiers take
> > effect at accesses to objects and are discarded at lvalue
> > conversion.  So a qualifier should ideally describe a property
> > that is relevant for accessing objects but is not relevant
> > for values.
> > 
> > Also there are built-in conversion rules a qualifier should
> > conform to.  A pointer pointing to a type without qualifier
> > can implicitely convert to a pointer to a type with qualifier,
> > e.g.   int*  can be converted to const int*.
> > 
> > Together, this implies that a qualifier should add constraints
> > to a type that are relevant to how an object is accessed.
> > 
> > "const" and "volatile" or "_Atomic" are good examples.
> > ("restricted" does not quite follow these rules and is
> > considered weird and difficult to understand.)
> > 
> > In contrast, "nonnull" and "nullable" are properties that
> > affect the set of values of the pointer, but not how the
> > pointer itself is accessed.
> > 
> > 
> > Martin
> > 
> > 
> > 
> > 
> > 

ISO C's [static] (was: _Nullable and _Nonnull in GCC's analyzer)

[Alejandro Colomar]

[-- Attachment #1.1: Type: text/plain, Size: 2551 bytes --]

Hi Martin,

On 2023-08-09 09:26, Martin Uecker wrote:
> it is a bit cumbersome to use, but one can use [static]
> instead, which gives you the same static warnings.
> 
> [static] does not work with __builtin_dynamic_object_size,
> but maybe this could be changed (there is a bug filed.)
> 
> I am not sure whether [static] should imply [[gnu::nonnull]]

I have a gripe with ISO C's [static].  As you mention, ISO
conflated two functionalities in [static]:

-  The size of the array passed as argument must not be less
   than the size specified in the parameter's [].

-  The pointer must not be NULL.

And there are valid cases where you may want the first but
not the second.  Or the second but not the first (that's the
case for _Nonnull, of course).

In fact, it's so badly damaged, that it prompted a proposal
to ISO C of using [static 1] as an equivalent of _Nonnull in
the prototypes that accepted a pointer that should not be
NULL.  However, that proposal didn't include the functions
that actually take arrays as input (because they are taken
in the opposite order, so array syntax is not legal).  Don't
you find it ironic that ISO C could have used array syntax
for pointers and pointer syntax for arrays?  I do.

As for when one would want to mean the first (size of array)
but not _Nonnull: for a function where you may pass either
an array (which should not be smaller than the size), or a
sentinel NULL value.

Nevertheless, I floated the idea that [static] is completely
unnecessary, and nobody has yet been against it.

GCC could perfectly add a warning for the following case:

    void foo(size_t n, int a[n]);

    int
    main(void)
    {
        int a[7];

        foo(42, a);
    }

Nobody in their right mind would specify a size of an array
in a parameter and expect that passing a smaller array than
that can produce a valid program.  So, why not make that a
Wall warning?

And so [static] would be irrelevant in GNU C, because well,
what does it add?  In fact, I like that [static] is so badly
designed, because then we can repurpose plain [size] to mean
the right thing, which would produce cleaner programs
([static] just adds noise to the source).

What do you think of giving [42] a meaning, instead of just
ignoring it?

Cheers,
Alex

> which would then also trigger the optimization. I think
> clang uses it for optimization.
> 
> Martin

-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: ISO C's [static] (was: _Nullable and _Nonnull in GCC's analyzer)

[Martin Uecker]

Hi Alejandro!

Am Mittwoch, dem 09.08.2023 um 12:42 +0200 schrieb Alejandro Colomar:

...

> 
> As for when one would want to mean the first (size of array)
> but not _Nonnull: for a function where you may pass either
> an array (which should not be smaller than the size), or a
> sentinel NULL value.
> 
> Nevertheless, I floated the idea that [static] is completely
> unnecessary, and nobody has yet been against it.
> 
> GCC could perfectly add a warning for the following case:
> 
>     void foo(size_t n, int a[n]);
> 
>     int
>     main(void)
>     {
>         int a[7];
> 
>         foo(42, a);
>     }
> 
> Nobody in their right mind would specify a size of an array
> in a parameter and expect that passing a smaller array than
> that can produce a valid program.  So, why not make that a
> Wall warning?

But we have this warning! is even activated by 
default without -Wall and already since GCC 11:





https://godbolt.org/z/sMbTon458

But this is for minimum required elements. How do 
we differentiate between null and non-null?

We have:

int[] or int* // no bound, nullable
int[N]	      // at least N, nullable
int[static N] // at least N, nonnull

The 'static' implies nonnull, so we could 
use 'static' to diffentiate between nonnull 
and nullable. 

What is missing something which implies bounds
also inside the callee.  You can use the "access"
attribute or we extend the meaning of int[N]
and int[static N] also imply a maximum bound.


Martin

Re: ISO C's [static] (was: _Nullable and _Nonnull in GCC's analyzer)

[Alejandro Colomar]

[-- Attachment #1.1: Type: text/plain, Size: 3822 bytes --]

Hi Martin!

On 2023-08-09 14:03, Martin Uecker wrote:
[...]

>> GCC could perfectly add a warning for the following case:
>>
>>     void foo(size_t n, int a[n]);
>>
>>     int
>>     main(void)
>>     {
>>         int a[7];
>>
>>         foo(42, a);
>>     }
>>
>> Nobody in their right mind would specify a size of an array
>> in a parameter and expect that passing a smaller array than
>> that can produce a valid program.  So, why not make that a
>> Wall warning?
> 
> But we have this warning! is even activated by 
> default without -Wall and already since GCC 11:

Ahh, I hadn't realized that was added (relatievly) recently.
That's great news!  Thanks!

> https://godbolt.org/z/sMbTon458
> 
> But this is for minimum required elements. How do 
> we differentiate between null and non-null?
> 
> We have:
> 
> int[] or int* // no bound, nullable
> int[N]	      // at least N, nullable
> int[static N] // at least N, nonnull
> 
> The 'static' implies nonnull, so we could 
> use 'static' to diffentiate between nonnull 
> and nullable. 

Since static is only for arrays (okay, they're all pointers, but
it's only usable with array syntax), and nullability is a thing
of pointers, I think static is not the right choice.

I oppose using array syntax for pointers, as it can confuse
programmers.

In any case, [static] is not that different from [_Nonnull], and
_Nonnull is more flexible, since you can also use it as
*_Nonnull.

Regarding the issues you have with _Nonnull being a qualifier,
I've been thinking about it for a long time and don't yet have
a concrete answer.  The more I think about it, the more I'm
convinced it is like `restrict`.  You can't have null correctness
as we can do with `const`.  With const, we know it's always bad
to store a const object in a non-const pointer.  With NULL, it
depends on the context: if you've checked for NULL in the lines
above, then it's fine to do the conversion.

There is a proposal for Clang to add `_Optional`, a qualifier to
the pointee, as a more correct version of _Nullable.  The
following would be equivalent:

    int *_Nullable  i;
    _Optional int   *j;

However, I don't believe it's a good choice, because of the above.
Instead, I think _Nullable or _Nonnull should be like `restrict`
and used only in function prototypes.  Let the analyzer do the
job.  I know `restrict` is hard to grasp, but I couldn't think of
anything better.

> 
> What is missing something which implies bounds
> also inside the callee.  You can use the "access"
> attribute or we extend the meaning of int[N]
> and int[static N] also imply a maximum bound.

Why is the upper bound important?  It's usually irrelevant.

In the case where one wants to make sure that an array is of an
exact size (instead of just "at least"), pointers to arrays can be
used.  They're just not often used, because you don't need that
so often.  I don't think we need a new addition to the language,
do we?

    $ cat ap.c
    #include <stddef.h>

    void
    foo(size_t n, int (*a)[n])
    {
        for (size_t i = 0; i < n; i++)
            (*a)[i] = 42;
    }
    $ gcc-13 -S -Wall -Wextra ap.c 
    $ 

In the function above, n is not a lower bound, but an exact bound.

GCC should add a warning for the following code:

    $ cat ap2.c
    #include <stddef.h>

    void foo(size_t n, int (*a)[n]);

    void
    bar(void)
    {
        int x[7];

        foo(3, &x);
        foo(9, &x);
    }
    $ gcc-13 -S -Wall -Wextra ap2.c
    $ 

Neither GCC nor Clang warn about that.  Not even with -fanalyzer.

Cheers,
Alex

-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: ISO C's [static] (was: _Nullable and _Nonnull in GCC's analyzer)

[Martin Uecker]

Am Mittwoch, dem 09.08.2023 um 14:37 +0200 schrieb Alejandro Colomar:
> Hi Martin!
> 
> On 2023-08-09 14:03, Martin Uecker wrote:


> 
> Regarding the issues you have with _Nonnull being a qualifier,
> I've been thinking about it for a long time and don't yet have
> a concrete answer.  The more I think about it, the more I'm
> convinced it is like `restrict`.  You can't have null correctness
> as we can do with `const`.  With const, we know it's always bad
> to store a const object in a non-const pointer.  With NULL, it
> depends on the context: if you've checked for NULL in the lines
> above, then it's fine to do the conversion.
> 
> There is a proposal for Clang to add `_Optional`, a qualifier to
> the pointee, as a more correct version of _Nullable.  The
> following would be equivalent:
> 
>     int *_Nullable  i;
>     _Optional int   *j;
> 
> However, I don't believe it's a good choice, because of the above.
> Instead, I think _Nullable or _Nonnull should be like `restrict`
> and used only in function prototypes.  Let the analyzer do the
> job.  I know `restrict` is hard to grasp, but I couldn't think of
> anything better.

I think this would be bad. More confusion is the least we need.
I would definitely prefer an attribute over a confusing qualifier 
which is not really a qualifier. 

_Optional is a something else and maybe not directly
comparable.  I think it is an interesting idea. First, it 
would fit well with rules the language.  One gets 
conversion errors exactly as with other qualifiers. So
one could add it to selected pointer in an existing code
base and incrementally improve the code by adding checks.

It would not be useful for existing interfaces that
need stay compatible.

> > 
> > What is missing something which implies bounds
> > also inside the callee.  You can use the "access"
> > attribute or we extend the meaning of int[N]
> > and int[static N] also imply a maximum bound.
> 
> Why is the upper bound important?  It's usually irrelevant.
> 
> In the case where one wants to make sure that an array is of an
> exact size (instead of just "at least"), pointers to arrays can be
> used.  They're just not often used, because you don't need that
> so often.  I don't think we need a new addition to the language,
> do we?
> 
>     $ cat ap.c
>     #include <stddef.h>
> 
>     void
>     foo(size_t n, int (*a)[n])
>     {
>         for (size_t i = 0; i < n; i++)
>             (*a)[i] = 42;
>     }
>     $ gcc-13 -S -Wall -Wextra ap.c 
>     $ 
> 
> In the function above, n is not a lower bound, but an exact bound.
> 
> GCC should add a warning for the following code:
> 
>     $ cat ap2.c
>     #include <stddef.h>
> 
>     void foo(size_t n, int (*a)[n]);
> 
>     void
>     bar(void)
>     {
>         int x[7];
> 
>         foo(3, &x);
>         foo(9, &x);
>     }
>     $ gcc-13 -S -Wall -Wextra ap2.c
>     $ 
> 
> Neither GCC nor Clang warn about that.  Not even with -fanalyzer.

I know. One can already get bounds checking side the callee with
this using UBSan. I have UBSan patch which would protect the
call itself and make sure the bounds are correct. Compile-time
warnings would also be good.

In any case, this does not work for existing interfaces.

Martin

Re: ISO C's [static] (was: _Nullable and _Nonnull in GCC's analyzer)

[Xi Ruoyao]

On Wed, 2023-08-09 at 12:42 +0200, Alejandro Colomar wrote:
> I have a gripe with ISO C's [static].  As you mention, ISO
> conflated two functionalities in [static]:
> 
> -  The size of the array passed as argument must not be less
>    than the size specified in the parameter's [].
> 
> -  The pointer must not be NULL.

https://gcc.gnu.org/pipermail/gcc-patches/2023-July/625559.html

If this patch is merged, they'll just become __nonnull and have all
advantages and disadvantages as __nonnull.

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

Re: _Nullable and _Nonnull in GCC's analyzer (was: [PATCH v5] libio: Add nonnull attribute for most FILE * arguments in stdio.h)

[enh]

On Wed, Aug 9, 2023 at 12:26 AM Martin Uecker <muecker@gwdg.de> wrote:
>
> Am Dienstag, dem 08.08.2023 um 17:14 -0700 schrieb enh:
> > (bionic maintainer here, mostly by accident...)
> >
> > yeah, we tried the GCC attributes once before with _disastrous_
> > results (because GCC saw it as an excuse to delete explicit null
> > checks, it broke all kinds of things).
>
> Thanks! I am currently exploring different options and what
> could be done to improve the situation from the language
> as well as compile side.  It looks we have some partial
> solution but they do not work quite right or do not
> fit together perfectly.
>
>
> >  the clang attributes are
> > "better" in that they don't confuse two entirely separate notions ...
> > but they're not "good" because the analysis is so limited. i think we
> > were hoping for something more like the "used but not set" kind of
> > diagnostic, but afaict it really only catches _direct_ use of a
> > literal nullptr. so:
> >
> >   foo(nullptr); // caught
> >
> >   void* p = nullptr;
> >   foo(p); // not caught
> >
> > without getting on to anything fancy like:
> >
> >   void* p;
> >   if (a) p = bar();
> >   foo(p);
> >
> > we've done all the annotations anyway, but we're not expecting to
> > actually catch many bugs with them, and in fact did not catch any real
> > bugs in AOSP while adding the annotations. (though we did find several
> > "you're not _wrong_, but ..." bits of code :-) )
> >
> > what i really want for christmas is the annotation that lets me say
> > "this size_t argument tells you the size of this array argument" and
> > actually does something usefully fortify-like with that.
> >
>
> What is your opinion about the access attribute?
>
> https://godbolt.org/z/PPfajefvP
>
> it is a bit cumbersome to use, but one can use [static]
> instead, which gives you the same static warnings.

yeah, "that's hard to read" was actually my first reaction. maybe it's
just because i'm a libc maintainer used to the printf_like attribute,
but i actually find the regular __attribute__() style more readable.
you probably need to ask people who _consume_ more headers than they
write :-)

> [static] does not work with __builtin_dynamic_object_size,
> but maybe this could be changed (there is a bug filed.)
>
> I am not sure whether [static] should imply [[gnu::nonnull]]
> which would then also trigger the optimization. I think
> clang uses it for optimization.

yeah, that was what made us revert the old gcc nonnull annotations ---
we don't have any use case for "please use this to omit checks"
because we're generally dealing with public API. having the compiler
dead-code eliminate our attempts to return sensible errors was counter
to our goals, and seems like it would be in any place where we'd use a
bounds annotation too --- it'll be those worried about security (who
want to fail fast, and _before_ adding a potentially caller-controlled
index to an array base address!) who i'd expect to see using this kind
of thing first.

> Martin
>
>
> >  i've seen
> > proposals like https://discourse.llvm.org/t/rfc-enforcing-bounds-safety-in-c-fbounds-safety/70854/
> > but i haven't seen anything i can use yet, so you -- who do use GCC
> > which aiui has something along these lines already -- will find out
> > what's good/bad about it before i do...
>
>
>
> >
> > On Tue, Aug 8, 2023 at 3:01 AM Martin Uecker <muecker@gwdg.de> wrote:
> > >
> > > Am Montag, dem 10.07.2023 um 22:16 +0200 schrieb Alejandro Colomar via Gcc:
> > > > On 7/10/23 22:14, Alejandro Colomar wrote:
> > > > > [CC += Andrew]
> > > > >
> > > > > Hi Xi, Andrew,
> > > > >
> > > > > On 7/10/23 20:41, Xi Ruoyao wrote:
> > > > > > Maybe we should have a weaker version of nonnull which only performs the
> > > > > > diagnostic, not the optimization.  But it looks like they hate the idea:
> > > > > > https://gcc.gnu.org/PR110617.
> > > > > >
> > > > > This is the one thing that makes me use both Clang and GCC to compile,
> > > > > because while any of them would be enough to build, I want as much
> > > > > static analysis as I can get, and so I want -fanalyzer (so I need GCC),
> > > > > but I also use _Nullable (so I need Clang).
> > > > >
> > > > > If GCC had support for _Nullable, I would have in GCC the superset of
> > > > > features that I need from both in a single vendor.  Moreover, Clang's
> > > > > static analyzer is brain-damaged (sorry, but it doesn't have a simple
> > > > > command line to run it, contrary to GCC's easy -fanalyzer), so having
> > > > > GCC's analyzer get over those _Nullable qualifiers would be great.
> > > > >
> > > > > Clang's _Nullable (and _Nonnull) are not very useful outside of analyzer
> > > > > mode, as there are many cases where the compiler doesn't have enough
> > > > > information, and the analyzer can get rid of false negatives and
> > > > > positives.  See: <https://github.com/llvm/llvm-project/issues/57546>
> > > > >
> > > > > I'll back the ask for the qualifiers in GCC, for compatibility with
> > > > > Clang.
> > > >
> > > > BTW, Bionic libc is adding those qualifiers:
> > > >
> > > > <https://android-review.googlesource.com/c/platform/bionic/+/2552567/7/libc/include/netinet/ether.h#45>
> > > > <https://android-review.googlesource.com/q/owner:zijunzhao@google.com+Nullability>
> > > >
> > > >
> > >
> > > I am sceptical about these qualifiers because they do
> > > not fit well with this language mechanism.   Qualifiers take
> > > effect at accesses to objects and are discarded at lvalue
> > > conversion.  So a qualifier should ideally describe a property
> > > that is relevant for accessing objects but is not relevant
> > > for values.
> > >
> > > Also there are built-in conversion rules a qualifier should
> > > conform to.  A pointer pointing to a type without qualifier
> > > can implicitely convert to a pointer to a type with qualifier,
> > > e.g.   int*  can be converted to const int*.
> > >
> > > Together, this implies that a qualifier should add constraints
> > > to a type that are relevant to how an object is accessed.
> > >
> > > "const" and "volatile" or "_Atomic" are good examples.
> > > ("restricted" does not quite follow these rules and is
> > > considered weird and difficult to understand.)
> > >
> > > In contrast, "nonnull" and "nullable" are properties that
> > > affect the set of values of the pointer, but not how the
> > > pointer itself is accessed.
> > >
> > >
> > > Martin
> > >
> > >
> > >
> > >
> > >
>