public inbox for gcc@gcc.gnu.org
 help / color / mirror / Atom feed
From: enh <enh@google.com>
To: Martin Uecker <muecker@gwdg.de>
Cc: Alejandro Colomar <alx@kernel.org>,
	Xi Ruoyao <xry111@xry111.site>,
	 Andrew Pinski <pinskia@gmail.com>,
	GNU libc development <libc-alpha@sourceware.org>,
	 Adhemerval Zanella <adhemerval.zanella@linaro.org>,
	"Carlos O'Donell" <carlos@redhat.com>,
	 Andreas Schwab <schwab@suse.de>,
	Siddhesh Poyarekar <siddhesh@gotplt.org>,
	Zack Weinberg <zack@owlfolio.org>,
	 "gcc@gcc.gnu.org" <gcc@gcc.gnu.org>
Subject: Re: _Nullable and _Nonnull in GCC's analyzer (was: [PATCH v5] libio: Add nonnull attribute for most FILE * arguments in stdio.h)
Date: Tue, 8 Aug 2023 17:14:02 -0700	[thread overview]
Message-ID: <CAJgzZopb5_fPCT7_Oovi+CDUjX17ULn+qgN70qYOxUpXCTRS4w@mail.gmail.com> (raw)
In-Reply-To: <b7e850f967e935cee9b836333dbbefeb785060d2.camel@gwdg.de>

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

  reply	other threads:[~2023-08-09  0:14 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20230710161300.1678172-1-xry111@xry111.site>
     [not found] ` <a3a0c195-1149-461b-807e-46eaa3d68fcc@app.fastmail.com>
     [not found]   ` <ed86d013-1df5-2880-3e39-0caf8f49a999@gotplt.org>
     [not found]     ` <1efbe0b2dd8fefffc945c6734222c7d6e04cf465.camel@xry111.site>
2023-07-10 20:14       ` Alejandro Colomar
2023-07-10 20:16         ` Alejandro Colomar
2023-08-08 10:01           ` Martin Uecker
2023-08-09  0:14             ` enh [this message]
2023-08-09  1:11               ` Siddhesh Poyarekar
2023-08-09  7:26               ` Martin Uecker
2023-08-09 10:42                 ` ISO C's [static] (was: _Nullable and _Nonnull in GCC's analyzer) Alejandro Colomar
2023-08-09 12:03                   ` Martin Uecker
2023-08-09 12:37                     ` Alejandro Colomar
2023-08-09 14:24                       ` Martin Uecker
2023-08-09 13:46                   ` Xi Ruoyao
2023-08-11 23:34                 ` _Nullable and _Nonnull in GCC's analyzer (was: [PATCH v5] libio: Add nonnull attribute for most FILE * arguments in stdio.h) enh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAJgzZopb5_fPCT7_Oovi+CDUjX17ULn+qgN70qYOxUpXCTRS4w@mail.gmail.com \
    --to=enh@google.com \
    --cc=adhemerval.zanella@linaro.org \
    --cc=alx@kernel.org \
    --cc=carlos@redhat.com \
    --cc=gcc@gcc.gnu.org \
    --cc=libc-alpha@sourceware.org \
    --cc=muecker@gwdg.de \
    --cc=pinskia@gmail.com \
    --cc=schwab@suse.de \
    --cc=siddhesh@gotplt.org \
    --cc=xry111@xry111.site \
    --cc=zack@owlfolio.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).