[cfe-dev] RFC: do not optimize on basis of __attribute__((nonnull)) in glibc headers

[Aaron Ballman via cfe-dev]

On Wed, Jan 4, 2017 at 2:01 PM, James Y Knight <jyknight at google.com> wrote:
>
>
> On Wed, Jan 4, 2017 at 12:54 PM, Hal Finkel <hfinkel at anl.gov> wrote:
>>
>>
>> On 01/04/2017 11:43 AM, James Y Knight via cfe-dev wrote:
>>
>> On Wed, Jan 4, 2017 at 11:12 AM, Aaron Ballman via cfe-dev
>> <cfe-dev at lists.llvm.org> wrote:
>>>
>>> So I would be opposed to ignoring those attributes in
>>>
>>> Sema (I think we should still warn when users do nonportable things),
>>> but in favor of not changing the optimizer to capitalize on this
>>> "opportunity."
>>
>>
>> I'd be opposed to ignoring the attributes only in some places and not in
>> others. It should be ignored totally, as if it wasn't present on those
>> functions. Doing anything else sends the wrong message -- that libc authors
>> should continue to use nonnull on these functions because they might be
>> helpful, and won't do anything bad.
>>
>>
>> I think that we have a responsibility to our users to continue to warn
>> (statically, in ubsan, etc.) on non-portable behavior, which this is and
>> will continue to be in practice for at least a decade or two, regardless of
>> the message we'd like to send libc authors. We cannot undo history here and
>> this will be relevant to production systems for at least a decade. We can
>> talk to libc developers directly -- they're a much smaller set -- and we can
>> pursue change at the standards level while still providing the most useful
>> set of tools to our users in the mean time.
>
>
> But, this is an entirely different question.
>
> - Should clang warn about non-portable usage of passing NULL to memcpy/etc?
>
> Sounds like a fine warning to add.
>
> - Should that warning be dependent on the libc headers having nonnull
> annotations on these functions, which will be used only for warnings, and
> ignored for semantics, on this given list of hardcoded functions?
>
> No.
>
> Firstly: I'd note that nearly all libc implementations don't use these
> attributes today. In some cases, because they've simply not thought about
> it, but in others because they explicitly decided to NOT break their users'
> code by introducing this problem! Glibc is the outlier, here.
>
> So: what portability do you want to warn for? Portability assuming the same
> libc, but a different compiler which might fail to ignore the nonnull
> attribute? Or portability to other libc? If the latter, depending on the
> nonnull attribute being present doesn't and can't work.

My preference is for portability for both, but you bring up a good
point about other libc implementations not being annotated. So long as
we retain the ability to tell users their code is not portable *and*
we get rid of the dangerous optimizations, I'm happy. I just don't
want to lose the diagnostics because of the optimizations.

~Aaron

> Secondly: if we already have a hardcoded list of functions to special case,
> that could just as well be used to generate a nonportable-stringfunc-null
> warning, as well.