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...

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.

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

Reply via email to