[PATCH] Checked arithmetic builtins

Eli Friedman eli.friedman at gmail.com
Thu Jun 20 15:06:18 PDT 2013


On Thu, Jun 20, 2013 at 3:05 PM, Eli Friedman <eli.friedman at gmail.com>wrote:

> On Thu, Jun 20, 2013 at 2:39 PM, Michael Gottesman <mgottesman at apple.com>wrote:
>
>>
>> On Jun 20, 2013, at 2:16 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
>>
>> On Tue, Jun 18, 2013 at 3:11 PM, Michael Gottesman <mgottesman at apple.com>
>>  wrote:
>>
>>> Hello cfe commits!
>>>
>>> The attached patch adds checked-arithmetic builtins to ameliorate such
>>> code in security critical applications (for instance webkit). It simply
>>> exposes {u,s}{add,sub,mul}.
>>>
>>> *NOTE* The u{add,sub} overlaps with the multi precision built-ins. I
>>> decided to add in the additional builtin since users are going to see
>>> s{add,sub} and look for u{add,sub}. We could add in a Builtins.h header
>>> where I could implement the checked arithmetic with the multi precision
>>> arithmetic builtins but I felt that was a bigger change than this.
>>>
>>>
>> It looks like you copy-pasted some stuff you didn't mean to in the
>> changes to LanguageExtensions.rst.
>>
>>
>> = /. Fixed.
>>
>>
>> +    return RValue::get(Builder.CreateZExt(Carry, X->getType()));
>>
>> These builtins all return bool, right?  Why are you zero-extending here?
>>  (You might be able to get away with this in C because of integer
>> promotions, but I'm pretty sure it'll explode in C++.)
>>
>>
>> = /. Fixed. The reason I was zero extending was that I was basing this
>> off of the multiprecision builtin code and forgot to remove it.
>>
>>
>> IIRC, we don't actually support CodeGen of llvm.smul.with.overflow with
>> 64-bit operands on x86-32; it would be nice to print a proper error message
>> instead of crashing.  (At least, we didn't at one point; I don't recall if
>> it ever got fixed.)
>>
>>
>> I just codegened my test file on OS X with -arch i386 and it codegened
>> fine. So it looks like it was fixed.
>>
>> Hows this look:
>>
>
>
> LGTM.
>


Oh, one more thing I forgot: please put a note into the release notes.

-Eli
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130620/2f32c31d/attachment.html>


More information about the cfe-commits mailing list