On Fri, Oct 17, 2014 at 03:45:52PM +0400, Maxim Ostapenko wrote:
> Patch also changes logic in asan_mem_ref_hash updating. I eliminated memory
> ref access size from hash computing, so all accesses for same memory
> reference have the same hash. Updating of asan_mem_ref_hash occurs only if
> new access size is greater then saved one.
I guess that is reasonable.
> -/* Instrument an access to a contiguous memory region that starts at
> - the address pointed to by BASE, over a length of LEN (expressed in
> - the sizeof (*BASE) bytes). ITER points to the instruction before
> - which the instrumentation instructions must be inserted. LOCATION
> - is the source location that the instrumentation instructions must
> - have. If IS_STORE is true, then the memory access is a store;
> - otherwise, it's a load. */
> +/* Insert a memory reference into the hash table if access length
> + can be determined in compile time. */
...
If you don't expand the memops builtins inline, I'd expect you start with
get_mem_refs_of_builtin_call and remove all the builtins you stop
expanding specially (i.e. emit a libcall instead unconditionally) that are
handled by libsanitizer (only a subset of them are apparently, perhaps
something to fix) from there.
There are builtins that must be kept instrumented (e.g. all the
sync/atomic builtins). There are builtins which might need first additions
to libsanitizer (e.g. I see no __*_chk functions in libsanitizer).
> +/* Returns TRUE if given FCODE corresponds to string or memory builtin
> function.
> + */
> +
> +static inline bool
> +is_memory_builtin (enum built_in_function fcode)
> +{
> + return fcode <= BUILT_IN_STRSTR && fcode >= BUILT_IN_BCMP;
This is too fragile and ugly.
IMHO you should list (supposedly not in a special inline, but directly
where you use it) in a switch all the builtins you don't want to expand.
Jakub
