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

[Aaron Ballman via cfe-dev]

On Tue, Jan 3, 2017 at 5:06 PM, Richard Smith via cfe-dev
<cfe-dev at lists.llvm.org> wrote:
> Via https://reviews.llvm.org/D27855, LLVM is likely to gain the ability to
> delete null checks in callers based on __attribute__((nonnull)) on the
> callee. This interacts badly with glibc's choice to mark the pointer
> parameters of memcpy, memmove, etc. as __attribute__((nonnull)) -- it is
> relatively common for programs to pass a null pointer and a zero size to
> these functions, and in practice libc implementations accept such usage.
> Indeed, LLVM's lowering of @llvm.memcpy intrinsics relies on these calls
> working.
>
> Deleting a null pointer check on p after a memcpy(p, q, 0) call seems
> extremely user-hostile, and very unlikely to result in a valuable
> improvement to the program, so I propose that we stop lowering
> __attribute__((nonnull)) on these builtin library functions to the llvm
> nonnull attribute.
>
> (Chandler is working on a paper for the C++ committee proposing to give
> these functions defined behavior when given a null pointer and a zero size,
> but optimizing on the basis of these particular nonnull attributes seems
> like a bad idea regardless of the C or C++ committees' decisions.)
>
> Thoughts?

On the one hand, I think it's reasonable because we don't want the
optimizer to delete those null checks. On the other hand, the
functions are properly attributed as far as the standard is concerned.
C11 7.24.1p2 is very clear that even with a 0 size, the pointer *must*
still be valid. 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." We've seen real exploits in the wild from this sort of
thing in the past. See
https://www.securecoding.cert.org/confluence/display/c/EXP34-C.+Do+not+dereference+null+pointers
for an example where deleting a null point check based on the
presumption of a nonnull pointer caused the optimizer to introduce a
security exploit in the Linux kernel. While the example does not
involve memcpy() or friends, it does demonstrate that real exploits
happen from this kind of optimization.

Failing to lower these attributes to LLVM IR may impact security
researchers who are relying on that information from the source, but I
think they could either carry a local patch or request this behavior
to have a flag.

~Aaron