On Tue, Jun 3, 2014 at 1:33 PM, Yury Gribov <y.gri...@samsung.com> wrote:
>> Any reason why the BUILT_IN_* names so differ from the actual function
>> names? I.e. why not use BUILT_IN_ASAN_{LOAD,STORE}{1,2,4,8,16,N}
>> (no underscore before N, no CHECK_)?
>
>
> Makes sense.
>
>> Wouldn't it be better to do
>> ...
>>
>> so that you don't risk signed overflow? Maybe also
>
>
> Yeah, that looks cleaner.
>
>
>> Also note (but, already preexisting issue) is that the
>> __asan_report* and __asan_{load,store}* calls are declared in
>> sanitizer.def as having 1st argument PTR type, while in the library
>> it is uptr (aka PTRMODE). So, either we should fix up sanitizer.def
>> to match that (we were passing already base_addr which is integral,
>> not pointer), or if we keep sanitizer.def as is, we should pass
>> to the calls base_ssa instead of base_addr.
>
>
> Which solution is better for middle-end? Would casting to uptr complicate
> alias analysis or somehow damage optimization capabilities of further
> passes?
>
> Frankly I don't understand why libsanitizer prefers uptr to void*'s and
> size_t's...
We can not use size_t because we do not include system headers.
>
>
>> Furthermore, this looks just like a partial transition, I don't see
>> why we should emit one __asan_loadN call but two separate
>> __asan_report_load_1 for the instrumented stringops.
>
>
> I was too lazy to bother with strlen. But ok, I'll optimize this piece.
Note that in clang we stopped instrumenting stringops in compiler and
fully rely on interceptors
({memset,memcpy,memmove} are replaced with
__asan_{memset,memcpy,memmove} to avoid late inlining)
>
>
>> Instead, build_check_stmt should be changed, so that it is possible to
>> pass
>> it a variable length (checked by the caller for non-zero already),
>> and in that case force using the slow_p stuff, but instead of adding a
>> constant size add the variable length - 1.
>
>
> +1, this will simplify code.
>
> -Y
