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

["Zack Weinberg" <zack@owlfolio.org>]

On Mon, Jul 10, 2023, at 1:51 PM, 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.

That would be fine with me.

> Would it be more acceptable to you if this gets wrapped into fortify,
> i.e. it gets enabled if _FORTIFY_SOURCE is defined?

I tend to agree with Xi that having the presence of __nonnull depend on
_FORTIFY_SOURCE would cause more problems than it solves.  Also, since
several Linux distributions enable _FORTIFY_SOURCE by default, we'd
still be risking significant breakage if we shipped that in 2.38.

> In fact, the wrappers in stdio2.h and the _chk variants of those
> functions should likely also get the __nonnull annotation.

Yes, divergence between the _chk variants and the unfortified variants
should be avoided as much as possible.

zw