[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/InstCombine/InstCombineAddSub.cpp lib/Transforms/InstCombine/InstructionCombining.cpp lib/Transforms/Scalar/BoundsChecking.cpp lib/Transforms/Scalar/Scalar.cpp

[John Criswell]

On 5/23/12 2:06 AM, Duncan Sands wrote:
> Hi Nuno,
>
>> 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.
>
> A possible solution is to introduce a new readnone intrinsic "llvm.identity"
> which simply returns its argument but is opaque to the optimizers.  You then
> insert

Duncan, would the llvm.identity intrinsic be useful for other things, or 
is it only useful for this particular application?

The SAFECode checks in my proposal do not suffer from this problem 
because the comparisons are hidden away within the run-time check functions.

-- John T.