llvm dev - Dec 2016 - RFC: Allowing @llvm.objectsize to be more conservative with null.

tl;dr: We'd like to add a bit to @llvm.objectsize to make it optionally be
conservative when it's handed a null pointer.

Happy Wednesday!

We're trying to fix PR23277, which is a bug about how clang+LLVM treat
__builtin_object_size when it's given a null pointer. For compatibility
with GCC, clang would like to be able to hand back a conservative result
(e.g. (size_t)-1), but the LLVM intrinsic that clang lowers
__builtin_object_size(p, N) to, @llvm.objectsize, only hands back 0 when
given a null pointer. Because it's often assumed that __builtin_object_size
always folds to a constant, handling this only in clang may be tricky: if
we emit the IR equivalent to ((p) ? __builtin_object_size(p, N) : -1ULL)
and LLVM can't fold away the null check, we've failed to emit a
constant.

So, the best path forward that I can see is to add a "null is unknown
size"
bit to @llvm.objectsize, much like the "min" bit it already has. If
this
bit is true, null would be treated like an opaque pointer. Otherwise,
@llvm.objectsize would act no differently than it does today.

If we like this approach, I'm assuming it would be best to have this bit as
a third argument to @llvm.objectsize, rather than making the second
argument an i8 and using it as a bag of bits.

All thoughts/comments/alternative approaches/etc. highly appreciated. :)

Thanks (and Happy Holidays)!
George
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20161221/d68f9c6b/attachment.html>

Happy New Year ping. :)

Will ping again on Wednesday. If I don't get comments by EOD Thursday,
I'll
assume everyone's OK with this and put together a patch.

On Wed, Dec 21, 2016 at 11:44 AM, George Burgess IV <
george.burgess.iv at gmail.com> wrote:
> tl;dr: We'd like to add a bit to @llvm.objectsize to make it optionally
be
> conservative when it's handed a null pointer.
>
> Happy Wednesday!
>
> We're trying to fix PR23277, which is a bug about how clang+LLVM treat
> __builtin_object_size when it's given a null pointer. For compatibility
> with GCC, clang would like to be able to hand back a conservative result
> (e.g. (size_t)-1), but the LLVM intrinsic that clang lowers
> __builtin_object_size(p, N) to, @llvm.objectsize, only hands back 0 when
> given a null pointer. Because it's often assumed that
__builtin_object_size
> always folds to a constant, handling this only in clang may be tricky: if
> we emit the IR equivalent to ((p) ? __builtin_object_size(p, N) : -1ULL)
> and LLVM can't fold away the null check, we've failed to emit a
constant.
>
> So, the best path forward that I can see is to add a "null is unknown
> size" bit to @llvm.objectsize, much like the "min" bit it
already has. If
> this bit is true, null would be treated like an opaque pointer. Otherwise,
> @llvm.objectsize would act no differently than it does today.
>
> If we like this approach, I'm assuming it would be best to have this
bit
> as a third argument to @llvm.objectsize, rather than making the second
> argument an i8 and using it as a bag of bits.
>
> All thoughts/comments/alternative approaches/etc. highly appreciated. :)
>
> Thanks (and Happy Holidays)!
> George
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170101/803feaa5/attachment.html>

Hi George,

Have you considered changing our existing behavior to match GCC's
builtin_object_size instead of adding a new parameter?  That may be
simpler overall.  There's also a clear upgrade strategy -- fold every
old style call to "<min> ? 0 : 1".

You probably already know this, but GCC folds
builtin_object_size(0, 0) to -1 and builtin_object_size(0, 2) to 0.
We'll have to have some <min>-awareness either in clang (to decide if
the <null-is-unknown> bit needs to be set) or in the middle end.  What
is your plan here?

I also found gcc's choice of folding builtin_object_size(0, 2) to 0 and
builtin_object_size(0, 0) to -1 somewhat odd; I'd have expected the
inverse.  This follows from the following "intuitive" rules

ObjSizeMax(X) = UMAX(ObjSizeMax(A), ObjSizeMax(B))
ObjSizeMin(X) = UMIN(ObjSizeMin(A), ObjSizeMin(B))

(where X is a value that can either be A or B at runtime)

and that we want to fold

ObjSizeMax(Malloc(N)) = ObjSizeMin(Malloc(N)) = N

However, since Malloc can return null:

ObjSizeMax(Malloc(N)) = UMAX(N, ObjSizeMax(NULL)) = N
ObjSizeMin(Malloc(N)) = UMIN(N, ObjSizeMin(NULL)) = N

and for that to be true ObjSizeMax(NULL) builtin_object_size(NULL, 0) needs to
be 0 and ObjSizeMin(NULL) builtin_object_size(NULL, 2) needs to be (unsigned)-1.

However, I'm not sure if it is up to us to change that; given the very
motivation of this thread is GCC compatibility.

-- Sanjoy

On Sun, Jan 1, 2017 at 10:03 PM, George Burgess IV via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
> Happy New Year ping. :)
>
> Will ping again on Wednesday. If I don't get comments by EOD Thursday,
I'll
> assume everyone's OK with this and put together a patch.
>
> On Wed, Dec 21, 2016 at 11:44 AM, George Burgess IV
> <george.burgess.iv at gmail.com> wrote:
>>
>> tl;dr: We'd like to add a bit to @llvm.objectsize to make it
optionally be
>> conservative when it's handed a null pointer.
>>
>> Happy Wednesday!
>>
>> We're trying to fix PR23277, which is a bug about how clang+LLVM
treat
>> __builtin_object_size when it's given a null pointer. For
compatibility with
>> GCC, clang would like to be able to hand back a conservative result
(e.g.
>> (size_t)-1), but the LLVM intrinsic that clang lowers
>> __builtin_object_size(p, N) to, @llvm.objectsize, only hands back 0
when
>> given a null pointer. Because it's often assumed that
__builtin_object_size
>> always folds to a constant, handling this only in clang may be tricky:
if we
>> emit the IR equivalent to ((p) ? __builtin_object_size(p, N) : -1ULL)
and
>> LLVM can't fold away the null check, we've failed to emit a
constant.
>>
>> So, the best path forward that I can see is to add a "null is
unknown
>> size" bit to @llvm.objectsize, much like the "min" bit
it already has. If
>> this bit is true, null would be treated like an opaque pointer.
Otherwise,
>> @llvm.objectsize would act no differently than it does today.
>>
>> If we like this approach, I'm assuming it would be best to have
this bit
>> as a third argument to @llvm.objectsize, rather than making the second
>> argument an i8 and using it as a bag of bits.
>>
>> All thoughts/comments/alternative approaches/etc. highly appreciated.
:)
>>
>> Thanks (and Happy Holidays)!
>> George
>
>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>


-- 
Sanjoy Das
http://playingwithpointers.com