<div dir="ltr">Thanks for the comments!<div class="gmail_extra"><br></div><div class="gmail_extra">> <span style="font-size:12.8px">Have you considered changing our existing behavior to match GCC's</span></div><span style="font-size:12.8px">> builtin_object_size instead of adding a new parameter</span><div><span style="font-size:12.8px"><br></span></div><div><span style="font-size:12.8px">Yup! My issue with turning `i1 %min` into `i8 %flags` is that __builtin_object_size would get lowered like:</span></div><div><span style="font-size:12.8px"><br></span></div><div><span style="font-size:12.8px">__builtin_object_size(p, 0) -> @llvm.objectsize(i8* %p, </span><span style="font-size:12.8px">i8</span><span style="font-size:12.8px"> </span><span style="font-size:12.8px">2)</span></div><div><span style="font-size:12.8px">__builtin_object_size(p, 1) -> @llvm.objectsize(</span><span style="font-size:12.8px">i8*</span><span style="font-size:12.8px"> </span><span style="font-size:12.8px">%p, </span><span style="font-size:12.8px">i8</span><span style="font-size:12.8px"> </span><span style="font-size:12.8px">2)</span></div><div><span style="font-size:12.8px">__builtin_object_size(p, 2) -> @llvm.objectsize(</span><span style="font-size:12.8px">i8*</span><span style="font-size:12.8px"> </span><span style="font-size:12.8px">%p, </span><span style="font-size:12.8px">i8</span><span style="font-size:12.8px"> </span><span style="font-size:12.8px">3)</span></div><div><span style="font-size:12.8px">(__builtin_object_size(p, 3) doesn't actually get lowered)</span></div><div><span style="font-size:12.8px"><br></span></div><div>...Which might be a bit surprising to some people. If we think that's a non-issue, I'm happy to take the simpler approach and use an i8.</div><div><div class="gmail_extra"><br></div><div class="gmail_extra"><div class="gmail_extra"><span style="font-size:12.8px">> We'll have to have some <min>-awareness either in clang (to decide if</span><br style="font-size:12.8px"><span style="font-size:12.8px">> the <null-is-unknown> bit needs to be set) or in the middle end.  What</span><br style="font-size:12.8px"><span style="font-size:12.8px">> is your plan here?</span></div><div class="gmail_extra"><br></div><div class="gmail_extra">My plan was just to always set the `null-is-unknown` bit when lowering a call to __builtin_object_size in clang. If `min` is true, we treat unknown and null values identically in @llvm.objectsize, so whether `null-is-unknown` is set in that case shouldn't matter.</div></div><div class="gmail_extra"><br></div><div class="gmail_extra"><div class="gmail_extra">> However, since Malloc can return null:</div><div class="gmail_extra"><br></div><div class="gmail_extra">I think I was unclear: this behavior would only exist if @llvm.objectsize was actually handed null. I wasn't planning on changing how we'd handle memory allocation functions that may return null (GCC gives back 2 for __builtin_object_size(malloc(2<wbr>), 0)). In other words, this `null-is-unknown` bit only makes the objectsize evaluator see `T* null` as though it was `call T* @opaque_user_defined_function(<wbr>)`, nothing else.</div><div class="gmail_extra"><br></div><div class="gmail_extra">This is also consistent with how @llvm.objectsize already acts: if the pointer it's given is the result of a call to `malloc(N)`, it'll return N regardless of the value of `min`.</div></div><div class="gmail_extra"><br></div><div class="gmail_extra">But yeah, I agree that some of the features of __builtin_object_size aren't entirely intuitive. :)</div><div class="gmail_extra"><br></div><div class="gmail_extra"><div class="gmail_quote">On Mon, Jan 2, 2017 at 12:01 AM, Sanjoy Das <span dir="ltr"><<a href="mailto:sanjoy@playingwithpointers.com" target="_blank">sanjoy@playingwithpointers.co<wbr>m</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi George,<br>
<br>
Have you considered changing our existing behavior to match GCC's<br>
builtin_object_size instead of adding a new parameter?  That may be<br>
simpler overall.  There's also a clear upgrade strategy -- fold every<br>
old style call to "<min> ? 0 : 1".<br>
<br>
You probably already know this, but GCC folds<br>
builtin_object_size(0, 0) to -1 and builtin_object_size(0, 2) to 0.<br>
We'll have to have some <min>-awareness either in clang (to decide if<br>
the <null-is-unknown> bit needs to be set) or in the middle end.  What<br>
is your plan here?<br>
<br>
I also found gcc's choice of folding builtin_object_size(0, 2) to 0 and<br>
builtin_object_size(0, 0) to -1 somewhat odd; I'd have expected the<br>
inverse.  This follows from the following "intuitive" rules<br>
<br>
ObjSizeMax(X) = UMAX(ObjSizeMax(A), ObjSizeMax(B))<br>
ObjSizeMin(X) = UMIN(ObjSizeMin(A), ObjSizeMin(B))<br>
<br>
(where X is a value that can either be A or B at runtime)<br>
<br>
and that we want to fold<br>
<br>
ObjSizeMax(Malloc(N)) = ObjSizeMin(Malloc(N)) = N<br>
<br>
However, since Malloc can return null:<br>
<br>
ObjSizeMax(Malloc(N)) = UMAX(N, ObjSizeMax(NULL)) = N<br>
ObjSizeMin(Malloc(N)) = UMIN(N, ObjSizeMin(NULL)) = N<br>
<br>
and for that to be true ObjSizeMax(NULL) =<br>
builtin_object_size(NULL, 0) needs to be 0 and ObjSizeMin(NULL) =<br>
builtin_object_size(NULL, 2) needs to be (unsigned)-1.<br>
<br>
However, I'm not sure if it is up to us to change that; given the very<br>
motivation of this thread is GCC compatibility.<br>
<br>
-- Sanjoy<br>
<div><div class="m_9192598381235594606m_8986242359694721517m_2193203331874465382gmail-m_-3654251363712103047m_7058043061164614700gmail-h5"><br>
On Sun, Jan 1, 2017 at 10:03 PM, George Burgess IV via llvm-dev<br>
<<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br>
> Happy New Year ping. :)<br>
><br>
> Will ping again on Wednesday. If I don't get comments by EOD Thursday, I'll<br>
> assume everyone's OK with this and put together a patch.<br>
><br>
> On Wed, Dec 21, 2016 at 11:44 AM, George Burgess IV<br>
> <<a href="mailto:george.burgess.iv@gmail.com" target="_blank">george.burgess.iv@gmail.com</a>> wrote:<br>
>><br>
>> tl;dr: We'd like to add a bit to @llvm.objectsize to make it optionally be<br>
>> conservative when it's handed a null pointer.<br>
>><br>
>> Happy Wednesday!<br>
>><br>
>> We're trying to fix PR23277, which is a bug about how clang+LLVM treat<br>
>> __builtin_object_size when it's given a null pointer. For compatibility with<br>
>> GCC, clang would like to be able to hand back a conservative result (e.g.<br>
>> (size_t)-1), but the LLVM intrinsic that clang lowers<br>
>> __builtin_object_size(p, N) to, @llvm.objectsize, only hands back 0 when<br>
>> given a null pointer. Because it's often assumed that __builtin_object_size<br>
>> always folds to a constant, handling this only in clang may be tricky: if we<br>
>> emit the IR equivalent to ((p) ? __builtin_object_size(p, N) : -1ULL) and<br>
>> LLVM can't fold away the null check, we've failed to emit a constant.<br>
>><br>
>> So, the best path forward that I can see is to add a "null is unknown<br>
>> size" bit to @llvm.objectsize, much like the "min" bit it already has. If<br>
>> this bit is true, null would be treated like an opaque pointer. Otherwise,<br>
>> @llvm.objectsize would act no differently than it does today.<br>
>><br>
>> If we like this approach, I'm assuming it would be best to have this bit<br>
>> as a third argument to @llvm.objectsize, rather than making the second<br>
>> argument an i8 and using it as a bag of bits.<br>
>><br>
>> All thoughts/comments/alternative approaches/etc. highly appreciated. :)<br>
>><br>
>> Thanks (and Happy Holidays)!<br>
>> George<br>
><br>
><br>
><br>
</div></div>> ______________________________<wbr>_________________<br>
> LLVM Developers mailing list<br>
> <a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-dev</a><br>
><br>
<span class="m_9192598381235594606m_8986242359694721517m_2193203331874465382gmail-m_-3654251363712103047m_7058043061164614700gmail-HOEnZb"><font color="#888888"><br>
<br>
<br>
--<br>
Sanjoy Das<br>
<a href="http://playingwithpointers.com" rel="noreferrer" target="_blank">http://playingwithpointers.com</a><br>
</font></span></blockquote></div><br></div></div></div>