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

[Richard Smith via cfe-dev]

On 3 January 2017 at 14:46, Friedman, Eli <efriedma at codeaurora.org> wrote:

> On 1/3/2017 2:06 PM, Richard Smith via cfe-dev 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?
>>
>
> "memcpy, memmove, etc." seems to be hiding some details here; do you have
> a complete list of functions you want to special-case?  There are a lot of
> functions which could potentially go on that list, and some of them are
> POSIX or GNU extensions.
>

The following glibc functions in <string.h> take both a pointer and a size
providing an upper limit on the number of bytes that may be accessed
through that pointer, and mark the pointer as __attribute__((nonnull)):

ISO C:
  memcpy, memmove, memset, memcmp, memchr, memrchr
  strncpy [param 1 only], strncat [param 1 only], strncmp

Extensions:
  strndup, memccpy, mempcpy, strnlen, bcopy, bzero, bcmp, memfrob,
  strncasecmp, strncasecmp_l [not parameter 4], stpncpy

I would be interested in covering at least the ISO C functions. I don't
have strong opinions about the extensions (any code built using those is
likely also compiled with GCC, and so presumably has to fix this
regardless).

Why do we care about keeping these null pointer checks in particular, as
> opposed to providing a flag which stops the optimizer from deleting null
> pointer checks in general (like gcc's -fno-delete-null-pointer-checks).


Deleting null checks when we can prove a pointer logically cannot possibly
be null is still useful -- for instance, deleting a null check after the
pointer was already passed to strlen, or when it's the address of a local
variable, both seem fine.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20170103/bab480e4/attachment.html>