[llvm-commits] [llvm] r157261 - in /llvm/trunk: include/llvm/InitializePasses.h include/llvm/LinkAllPasses.h include/llvm/Transforms/Scalar.h include/llvm/Transforms/Utils/Local.h lib/Transforms/InstCombine/InstCombine.h lib/Transforms/InstCombin

[Eli Friedman]

On Wed, May 23, 2012 at 9:01 AM, Nuno Lopes <nunoplopes at sapo.pt> wrote:
>>> add a new pass to instrument loads and stores for run-time bounds checking
>>> move EmitGEPOffset from InstCombine to Transforms/Utils/Local.h
>>
>> if I understand right you are inserting code that does something like this:
>>
>> ...
>>    if (pointer < object_start || pointer > object_end) trap();
>>    %x = load pointer
>> ...
>>
>> I don't think this will work reliably.  Suppose the optimizers discover that
>> pointer is undef.  The undef will be propagated throughout:
>>
>>    if (undef < object_start || undef > object_end) trap();
>>    %x = load undef
>>
>> The optimizers can fold the comparisons to whatever they want, for example
>>
>>    if (false || false) trap();
>>    %x = load undef
>>
>> which then turns into
>>
>>    %x = load undef
>>
>> Oops - the bound check was removed but a wrong address is still loaded!
>> A similar problem may occur if the pointer is a trap value, due to an
>> inbounds GEP or something like that: the compares may be eliminated even
>> though the value lies outside the range.
>
> I believe a 'load undef' will be removed. So, if there is no
> load/store, then memory is not touched, and there is no buffer
> overflow (which is what we are trying to avoid).
> Of course that with this approach we will miss some bugs, namely those
> that incur in undefined behavior that is exploited by the compiler.
> But as far as security is concerned, this is fine.

That isn't how undef works... it's possible (but probably rare) we'll
eliminate the bounds check, but not the load.

-Eli