[llvm] r231241 - asan: do not instrument direct inbounds accesses tostack variables

[Nuno Lopes]

Hi Dmitry,

Sorry for the late review. Please see comments below.


> Author: dvyukov
> Date: Wed Mar  4 07:27:53 2015
> New Revision: 231241
>
> URL: http://llvm.org/viewvc/llvm-project?rev=231241&view=rev
> Log:
> asan: do not instrument direct inbounds accesses to stack variables
>
> Do not instrument direct accesses to stack variables that can be
> proven to be inbounds, e.g. accesses to fields of structs on stack.
>
> But it eliminates 33% of instrumentation on webrtc/modules_unittests
> (number of memory accesses goes down from 290152 to 193998) and
> reduces binary size by 15% (from 74M to 64M) and improved compilation time 
> by 6-12%.
>
> The optimization is guarded by asan-opt-stack flag that is off by default.
>
> http://reviews.llvm.org/D7583

> +// isSafeAccess returns true if Addr is always inbounds with respect to 
> its
> +// base object. For example, it is a field access or an array access with
> +// constant inbounds index.
> +bool AddressSanitizer::isSafeAccess(ObjectSizeOffsetVisitor &ObjSizeVis,
> +                                    Value *Addr, uint64_t TypeSize) const 
> {
> +  SizeOffsetType SizeOffset = ObjSizeVis.compute(Addr);
> +  if (!ObjSizeVis.bothKnown(SizeOffset)) return false;
> +  int64_t Size = SizeOffset.first.getSExtValue();

Size is unsigned. You should getZExtValue() instead.  Or do the checks below 
with APInts (more future-proof, but slower).


> +  int64_t Offset = SizeOffset.second.getSExtValue();
> +  // Three checks are required to ensure safety:
> +  // . Offset >= 0  (since the offset is given from the base ptr)
> +  // . Size >= Offset  (unsigned)
> +  // . Size - Offset >= NeededSize  (unsigned)
> +  return Offset >= 0 && Size >= Offset &&
> +         uint64_t(Size - Offset) >= TypeSize / 8;
> +}

The 2nd check is being done with signed operands, which is incorrect (and 
doesn't match the comment).

Nuno