On Sat, Jan 10, 2015 at 4:02 PM, Paul Eggert <egg...@cs.ucla.edu> wrote: > Jim Meyering wrote: >> >> +#if defined __clang__ >> +# if __has_feature(address_sanitizer) >> +# define HAVE_ASAN 1 >> +# endif >> +#elif defined __GNUC__ \ >> + && (((__GNUC__ == 4) && (__GNUC_MINOR__ >= 8)) || (__GNUC__ >= 5)) \ >> + && __SANITIZE_ADDRESS__ >> +# define HAVE_ASAN 1 >> +#endif > > > How about the following instead? > > #ifndef __has_feature > # define __has_feature(a) false > #endif > > #if defined __SANITIZE_ADDRESS__ || __has_feature (address_sanitizer) > # define HAVE_ASAN 1 > #else > # define HAVE_ASAN 0 > #endif > > This is what Emacs uses (its symbol is ADDRESS_SANITIZER instead of > HAVE_ASAN, for what that's worth). > >> + ASAN_POISON_MEMORY_REGION (buflim + sizeof(uword), >> + bufalloc - (buflim - buffer) - >> sizeof(uword)); >> > > The two 'sizeof's need spaces afterwards.
Fixed.
> I don't see the value of having macros here. How about the following
> instead?
>
> #ifndef HAVE_ASAN
> static void
> __asan_unpoison_memory_region (void const volatile *addr, size_t size)
> {
> }
>
> static void
> __asan_unpoison_memory_region (void const volatile *addr, size_t size)
> {
> }
> #endif
I agree. Adjusted via s/unpoison/poison/ in the first.
In addition, I have added _GL_UNUSED so as not to run afoul of grep's
use of -Werror=unused-function. Also, I have adapted not to
declare when using the static wrappers.
and to use #if, not #ifndef, since now HAVE_ASAN is always defined.
> And then have the callers invoke '__asan_poison_memory_region' instead of
> 'ASAN_POISON_MEMORY_REGION'. This way, there should be no need to pull in
> the ignore-value machinery, it's two less macros to worry about, and there's
> better type checking when address sanitization is not in use.
Thanks for all the good suggestions. Here's an updated version:
0001-tests-add-support-for-ASAN-memory-poisoning.patch
Description: Binary data
