This is an archive of the discontinued LLVM Phabricator instance.

Add ASan hooks to BumpPtrAllocator
AbandonedPublic

Authored by pete on May 13 2015, 11:20 AM.

Details

Reviewers
None
Summary

BumpPtrAllocator already has MSan hooks to teach the sanitizer about what memory is valid.

This patch adds the equivalent for ASan. New slabs are initially poisoned then each individual allocation is unopposed.

BumpPtrAllocator has an empty Deallocate method, however, we can use that here to ensure that once a region has been Deallocated, we poison it and don't continue to use it. A similar change should probably be made for MSan.

Unfortunately this patch can't yet be committed due to a bug found in clang with this applied (https://llvm.org/bugs/show_bug.cgi?id=23516). I'm putting this up on Phab in case anyone else wants to try it out. It may also be better to move most of the #ifdefs to Compiler.h where __msan_allocated_memory is currently defined, so i expect to change this patch in future.

Diff Detail

Event Timeline

pete updated this revision to Diff 25716.May 13 2015, 11:20 AM
pete retitled this revision from to Add ASan hooks to BumpPtrAllocator.
pete updated this object.
pete edited the test plan for this revision. (Show Details)
pete added a subscriber: Unknown Object (MLST).
kcc added a subscriber: kcc.May 14 2015, 1:59 PM

<grumble>
the annotations are provided for exactly this purpose,
but still asan handles allocation much better if it fully controls malloc.
</grumble>

include/llvm/Support/Allocator.h
229

I don't like so many ifdefs.
Maybe like this?

#if LLVM_ADDRESS_SANITIZER_BUILD

include ...

#else

#define ASAN_POISON_MEMORY_REGION(a,b)  /*empty*/

#endif

pete added a comment.May 14 2015, 2:08 PM

Ah yeah, that is much better. Thanks!

samsonov added inline comments.
include/llvm/Support/Allocator.h
229

In fact, it's worth defining this macro in llvm/Support/Compiler.h, similar to what we do for MSan interface function __msan_allocated_memory

pete abandoned this revision.Dec 7 2015, 3:16 PM

Justin fixed this in r254964 by finishing up the patch here with Alexey's comments.