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