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

[Dmitry Vyukov]

On Mon, Mar 9, 2015 at 2:15 AM, Nuno Lopes <nunoplopes at sapo.pt> wrote:
> 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).


Mailed http://reviews.llvm.org/D8193 with a fix.