Re: [PATCH] Don't expand string/memory builtins if ASan is enabled.

[Jakub Jelinek <jakub at redhat dot com>]

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