Re: [PATCH v5] libio: Add nonnull attribute for most FILE * arguments in stdio.h

[Xi Ruoyao <xry111@xry111.site>]

On Mon, 2023-07-10 at 13:51 -0400, Siddhesh Poyarekar wrote:
> On 2023-07-10 13:12, Zack Weinberg wrote:
> > On Mon, Jul 10, 2023, at 12:13 PM, Xi Ruoyao via Libc-alpha wrote:
> > > During the review of a GCC analyzer test case, we found most stdio
> > > functions accepting a FILE * argument expect it to be nonnull and just
> > > segfault when the argument is NULL.  Add nonnull attribute for them.
> > 
> > I think this patchset has a high risk of breaking application code,
> > because "this function will promptly crash if passed a NULL pointer" is
> > a very different property from "any code path that would cause this
> > function to be passed a NULL pointer is necessarily unreachable."
> > 
> > If we take it at all -- and my current gut feeling is that we
> > *shouldn't* -- we should do so early in a release cycle to give us the
> > best chance of discovering broken applications before the release.
> 
> Thanks for your comment; it made me take a closer look at this.  I 
> suppose it makes sense to push it in right after we tag 2.38 then, so 
> that there's the rest of the year to test and fix broken applications 
> before 2.39.  It may make sense to backport to the release branch for 
> distributions if we find it to be stable enough.
> 
> Would it be more acceptable to you if this gets wrapped into fortify, 
> i.e. it gets enabled if _FORTIFY_SOURCE is defined?  In fact, the 
> wrappers in stdio2.h and the _chk variants of those functions should 
> likely also get the __nonnull annotation.

But it then means w/o -D_FORTIFY_SOURCE we'll still not see the warning,
and GCC analyzer developers will still implement a lot of special cases
:(.

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.

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