From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qv1-xf33.google.com (mail-qv1-xf33.google.com [IPv6:2607:f8b0:4864:20::f33]) by sourceware.org (Postfix) with ESMTPS id C31C03858D20 for ; Fri, 11 Aug 2023 23:34:13 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org C31C03858D20 Authentication-Results: sourceware.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=google.com Received: by mail-qv1-xf33.google.com with SMTP id 6a1803df08f44-641882b73b5so11681006d6.3 for ; Fri, 11 Aug 2023 16:34:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1691796853; x=1692401653; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=h4d87UZF/Cglbg0Z3EfFOeQ3sTZevulT/bXF0jwdc2I=; b=XBRxBGyhBc7PRr/vVKfh6v5P7k0dnegluKu0JmhAQDUyvfhVA6221BBn5Eev3T0KhG NH1w+oTcmOuPgqia3TENHMhV6LdBZLgS95eW322ZTSrgUC/UtDlZKoSfU8FXpx0/67/G a+Rw3YW690eviYpWS2/n8ON14unUW65UwQuJzAfPcJqG+vX+6Za9+R+DSgSIDikqR8Ns bjqR+zXukZEqCdmEuzmb70PooDeSRzXVgte7aimnbpgsaQLJjKYoGH6Tq4Sf9cbTyAvh h/+fFVlJlUPBc2s19LS3+fkGbi0NT0bLaayjF5+XyUeVdC+4NyDBvMJZlYS1JfBMatwR hK1Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1691796853; x=1692401653; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=h4d87UZF/Cglbg0Z3EfFOeQ3sTZevulT/bXF0jwdc2I=; b=MeAwoo7SyBCg7XHc7J0WuwoRYW5qgrJn0yjOsfS4xp63+O+wHIHIL8z4VHXQsZJaq7 bXuYpDH6UIqopsrpcAoUCwJG60hxXJAstL9+DNdrrPQaWXCd+7Q3lXs1OpFzP0NitEIC hqYt09vOXW7uBLCJ7nsjdlOYJnN5Iqv3RlXb4NuUmbVesPr5DPyeVtkQp+1DxIwg+I5v q7MDu2SEBqLawN/Vr9lbJ9j64gbx2l7m3QLYsWVkGAJKlgELS9oUxB1tZiclPrec//+V H07ewDC+T4j9cCKw1r0c+yHyEkmfj+/rUEdHbJpm7Fc/oaEoVezJJ8LGWyc1HsLP7czD 1aNg== X-Gm-Message-State: AOJu0YzEtt6f6jD6aArYPtKWOXqY3bjeg9htzvqlhLRPWCOkXNQdgMCN 1qDY2Xybmc4A926zFll6HjYEItcd8NTqRhOC5Y6U4Q== X-Google-Smtp-Source: AGHT+IGHcpHkoTPOiXGomUIzI2/S2KKWcTk8rf8aGdxwvpHDfTZSYyvbG1NQgkdVPNlQQLebJ22xDaHDehtThPE0v/4= X-Received: by 2002:a05:6214:bd3:b0:63c:d763:77b6 with SMTP id ff19-20020a0562140bd300b0063cd76377b6mr3110051qvb.55.1691796852927; Fri, 11 Aug 2023 16:34:12 -0700 (PDT) MIME-Version: 1.0 References: <20230710161300.1678172-1-xry111@xry111.site> <1efbe0b2dd8fefffc945c6734222c7d6e04cf465.camel@xry111.site> <10994861-c244-ba4f-70ad-86d66acf7277@kernel.org> <08d7552c-d90a-ae84-4b7e-2f6f2136dd66@kernel.org> In-Reply-To: From: enh Date: Fri, 11 Aug 2023 16:34:01 -0700 Message-ID: Subject: Re: _Nullable and _Nonnull in GCC's analyzer (was: [PATCH v5] libio: Add nonnull attribute for most FILE * arguments in stdio.h) To: Martin Uecker Cc: Alejandro Colomar , Xi Ruoyao , Andrew Pinski , GNU libc development , Adhemerval Zanella , "Carlos O'Donell" , Andreas Schwab , Siddhesh Poyarekar , Zack Weinberg , "gcc@gcc.gnu.org" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-18.4 required=5.0 tests=BAYES_00,DKIMWL_WL_MED,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,ENV_AND_HDR_SPF_MATCH,KAM_SHORT,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,USER_IN_DEF_DKIM_WL,USER_IN_DEF_SPF_WL autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Wed, Aug 9, 2023 at 12:26=E2=80=AFAM Martin Uecker wro= te: > > 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 =3D nullptr; > > foo(p); // not caught > > > > without getting on to anything fancy like: > > > > void* p; > > if (a) p =3D 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=E2=80=AFAM Martin Uecker = wrote: > > > > > > Am Montag, dem 10.07.2023 um 22:16 +0200 schrieb Alejandro Colomar vi= a Gcc: > > > > On 7/10/23 22:14, Alejandro Colomar wrote: > > > > > [CC +=3D Andrew] > > > > > > > > > > Hi Xi, Andrew, > > > > > > > > > > On 7/10/23 20:41, Xi Ruoyao wrote: > > > > > > Maybe we should have a weaker version of nonnull which only per= forms 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 com= pile, > > > > > because while any of them would be enough to build, I want as muc= h > > > > > 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 superse= t of > > > > > features that I need from both in a single vendor. Moreover, Cla= ng's > > > > > static analyzer is brain-damaged (sorry, but it doesn't have a si= mple > > > > > command line to run it, contrary to GCC's easy -fanalyzer), so ha= ving > > > > > GCC's analyzer get over those _Nullable qualifiers would be great= . > > > > > > > > > > Clang's _Nullable (and _Nonnull) are not very useful outside of a= nalyzer > > > > > mode, as there are many cases where the compiler doesn't have eno= ugh > > > > > information, and the analyzer can get rid of false negatives and > > > > > positives. See: > > > > > > > > > > I'll back the ask for the qualifiers in GCC, for compatibility wi= th > > > > > Clang. > > > > > > > > BTW, Bionic libc is adding those qualifiers: > > > > > > > > > > > > > > > > > > > > > > > > > > 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 > > > > > > > > > > > > > > > >