Subject: bug#19563: grep -F: fix a heap buffer (read) overrun

From: Paul Eggert <[email-masked]>
Date: Sat, 10 Jan 2015 16:02:46 -0800

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.

> +#ifdef HAVE_ASAN
> +# define ASAN_POISON_MEMORY_REGION(addr, size) \
> +  __asan_poison_memory_region ((addr), (size))
> +# define ASAN_UNPOISON_MEMORY_REGION(addr, size) \
> +  __asan_unpoison_memory_region ((addr), (size))
> +#else
> +# define ASAN_POISON_MEMORY_REGION(addr, size) \
> +  (ignore_value (addr), ignore_value (size))
> +# define ASAN_UNPOISON_MEMORY_REGION(addr, size) \
> +  (ignore_value (addr), ignore_value (size))
> +#endif

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

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.