Re: LLVM Address Sanitizer (ASAN) and valgrind support

On Mon, Sep 07, 2015 at 05:05:10PM +0100, Greg Stark wrote:
> I feel like I remember hearing about this before but I can't find any
> mention of it in my mail archives. It seems pretty simple to add
> support for LLVM's Address Sanitizer (asan) by using the hooks we
> already have for valgrind.

Nice.

> In fact I think this would actually be sufficient. I'm not sure what
> the MEMPOOL valgrind stuff is though. I don't think it's relevant to
> address sanitizer which only tracks references to free'd or
> unallocated pointers.

Documentation: http://valgrind.org/docs/manual/mc-manual.html#mc-manual.mempools

A Valgrind memory pool is the precise analog of a PostgreSQL memory context.
The design of the PostgreSQL Valgrind support gave mcxt.c primary
responsibility for validating and invalidating pointers, and mcxt.c uses
MEMPOOL client requests to do so. While Valgrind could catch all the same
errors with just VALGRIND_MAKE_MEM_{DEFINED,NOACCESS,UNDEFINED}, the MEMPOOL
client requests improve the attribution of errors. You get this:

==00:00:14:08.472 471537== Address 0x8ecd848 is 24 bytes inside a block of size 25 client-defined
==00:00:14:08.472 471537== at 0x7717FD: MemoryContextAlloc (mcxt.c:589)

Instead of this:

==00:00:02:29.587 18940== Address 0x6a1b51b is 83,835 bytes inside a block of size 262,144 alloc'd
==00:00:02:29.587 18940== at 0x4C2353F: malloc (vg_replace_malloc.c:236)
==00:00:02:29.587 18940== by 0x7FD12F: AllocSetAlloc (aset.c:792)
==00:00:02:29.587 18940== by 0x7FEE9D: MemoryContextAlloc (mcxt.c:524)

> I don't even see any need offhand for a configure flag or autoconf
> test. We could have a configure flag just to be consistent with
> valgrind but it seems pointless. If you're compiling with asan I don't
> see any reason to not use it. I'm building this to see if it works
> now.

I agree. A flag guards Valgrind client requests, because we'd otherwise have
no idea whether the user plans to run the binary under Valgrind. For ASAN,
all relevant decisions happen at build time.

> --- a/src/include/utils/memdebug.h
> +++ b/src/include/utils/memdebug.h
> @@ -19,6 +19,27 @@
>
> #ifdef USE_VALGRIND
> #include <valgrind/memcheck.h>
> +
> +#elif __has_feature(address_sanitizer) || defined(__SANITIZE_ADDRESS__)
> +
> +#include <sanitizer/asan_interface.h>
> +
> +#define VALGRIND_MAKE_MEM_DEFINED(addr, size) \
> + ASAN_UNPOISON_MEMORY_REGION(addr, size)
> +
> +#define VALGRIND_MAKE_MEM_NOACCESS(addr, size) \
> + ASAN_POISON_MEMORY_REGION(addr, size)
> +
> +#define VALGRIND_MAKE_MEM_UNDEFINED(addr, size) \
> + ASAN_UNPOISON_MEMORY_REGION(addr, size)
> +
> +#define VALGRIND_CHECK_MEM_IS_DEFINED(addr, size) do {} while (0)
> +#define VALGRIND_CREATE_MEMPOOL(context, redzones, zeroed) do {} while (0)
> +#define VALGRIND_DESTROY_MEMPOOL(context) do {} while (0)
> +#define VALGRIND_MEMPOOL_ALLOC(context, addr, size) do {} while (0)
> +#define VALGRIND_MEMPOOL_FREE(context, addr) do {} while (0)
> +#define VALGRIND_MEMPOOL_CHANGE(context, optr, nptr, size) do {} while (0)

aset.c relies on the fact that VALGRIND_MEMPOOL_ALLOC() has an implicit
VALGRIND_MAKE_MEM_UNDEFINED() and VALGRIND_MEMPOOL_FREE() has an implicit
VALGRIND_MAKE_MEM_NOACCESS(). #define those two accordingly. If ASAN has no
equivalent of MEMPOOL client requests, it's fine to stub out the rest.