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

[Hal Finkel via cfe-dev]

On 01/03/2017 04:55 PM, Chandler Carruth via cfe-dev wrote:
>
>
> On Tue, Jan 3, 2017 at 2:46 PM Friedman, Eli via cfe-dev 
> <cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>> wrote:
>
>     "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 only ones I'm immediately concerned with are the mem* and str* 
> functions which accept an explicit size argument governing the 
> pointers. I would be happy to restrict it further if that helps.
>
>     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).
>
>
> There are two reasons really.
>
> First is pragmatic: all of the security vulnerabilities I happen to be 
> aware of stem from deleting null pointer checks around the mem* 
> functions, and the functions that LLVM's own intrinsics specify null 
> pointers as allowed for are the mem* functions. Thus they seem likely 
> to be the most risky. Certainly, there are security issues stemming 
> from other null checks being eliminated, but in a purely pragmatic 
> sense, these seems like a useful first step.
>
> Second is perhaps more interesting. I think there are a large number 
> of null pointer checks that we can and should eliminate. I'll give one 
> example here of an entire class: references to objects with non-zero 
> size. Taking the address of this cannot produce a null pointer, and 
> code checking for null I think should be optimized away IMO.
>
> There is something very special about the nature of the mem* 
> intrinsics and that is that they can accept a *zero* size. It is that 
> zero size case that I think makes a null pointer a reasonable argument 
> to them. I would like to suggest that whenever we have a pointer and 
> size where the size could reasonably be zero, the pointer could also 
> reasonably be null.
>
> While this will cover some number of the current uses of nonnull 
> attribute, I think there are a reasonably large number of other uses 
> that I'm not sure such sweeping statements can be made about. With a 
> reference to a non-zero sized object, I don't see any reason to 
> eliminate the nonnull attribute or to stop optimizing away. Indeed, 
> the way these occur in C++ as part of the *type system* can make this 
> an incredibly important optimization in conjunction with proper inlining.
>
> I'm not really opposed to having a mode that disables *all* non-null 
> optimizations, but I think that is a very different thing and I'm 
> personally much less interested in such a mode. I don't think we could 
> reasonably make that the default for example, whereas I think we could 
> reasonably drop these particular nonnull attributes from glibc headers 
> by default.

I agree with this. -fno-delete-null-pointer-checks is too heavy a hammer 
to ideally address this problem. We can and should provide a 
more-targeted solution (regardless of whether it is on by default).

  -Hal

>
> -Chandler
>
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

-- 
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20170103/ff9f025f/attachment.html>