Skip to content

Commit

Permalink
Make ASAN actually do something and fix all the things
Browse files Browse the repository at this point in the history
We've had the ability to build with ASAN for quite a while, but
at some point the LLVM pass stopped doing anything unless you
also annotated the function with the `sanitize_address` attribute,
so if you built Julia with asan enabled, very few things were
actually built with asan and those that were didn't have one
of asan's most crucial features enabled: Tracking out of bounds
stack accesses.

This PR enabled asan-stack handling, adds the appropriate asan
poison handling to our tasks and fixes a plethora of other things
that didn't work with asan enabled. The result of this PR (together
with a few other PRs still pending) should be that asan passes
tests on master.

This was a significant amount of work, and the changes required
quite subtle. As a result, I think we should make sure to quickly
set up CI to test this configuration and make sure it doesn't
regress.
  • Loading branch information
Keno committed Aug 13, 2022
1 parent 0e3e00d commit 1acecb7
Show file tree
Hide file tree
Showing 14 changed files with 289 additions and 89 deletions.
2 changes: 1 addition & 1 deletion Make.inc
Original file line number Diff line number Diff line change
Expand Up @@ -689,7 +689,7 @@ SANITIZE_OPTS += -fsanitize=memory -fsanitize-memory-track-origins -fno-omit-fra
SANITIZE_LDFLAGS += $(SANITIZE_OPTS)
endif
ifeq ($(SANITIZE_ADDRESS),1)
SANITIZE_OPTS += -fsanitize=address -mllvm -asan-stack=0
SANITIZE_OPTS += -fsanitize=address
SANITIZE_LDFLAGS += -fsanitize=address
endif
ifeq ($(SANITIZE_THREAD),1)
Expand Down
4 changes: 3 additions & 1 deletion src/ccalltest.c
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@
#ifdef _OS_WINDOWS_
# define DLLEXPORT __declspec(dllexport)
#else
# if defined(_OS_LINUX_)
# if defined(_OS_LINUX_) && !defined(_COMPILER_CLANG_)
// Clang and ld disagree about the proper relocation for STV_PROTECTED, causing
// linker errors.
# define DLLEXPORT __attribute__ ((visibility("protected")))
# else
# define DLLEXPORT __attribute__ ((visibility("default")))
Expand Down
3 changes: 3 additions & 0 deletions src/codegen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2119,6 +2119,9 @@ static void jl_init_function(Function *F)
attr.addAttribute("probe-stack", "inline-asm");
//attr.addAttribute("stack-probe-size", "4096"); // can use this to change the default
#endif
#if defined(_COMPILER_ASAN_ENABLED_)
attr.addAttribute(Attribute::SanitizeAddress);
#endif
#if JL_LLVM_VERSION >= 140000
F->addFnAttrs(attr);
#else
Expand Down
1 change: 1 addition & 0 deletions src/datatype.c
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ JL_DLLEXPORT jl_typename_t *jl_new_typename_in(jl_sym_t *name, jl_module_t *modu
jl_atomic_store_relaxed(&tn->linearcache, jl_emptysvec);
tn->names = NULL;
tn->hash = bitmix(bitmix(module ? module->build_id : 0, name->hash), 0xa1ada1da);
tn->_reserved = 0;
tn->abstract = abstract;
tn->mutabl = mutabl;
tn->mayinlinealloc = 0;
Expand Down
1 change: 1 addition & 0 deletions src/dump.c
Original file line number Diff line number Diff line change
Expand Up @@ -2090,6 +2090,7 @@ static jl_value_t *jl_deserialize_value_any(jl_serializer_state *s, uint8_t tag,
jl_gc_wb(tn, tn->mt);
ios_read(s->s, (char*)&tn->hash, sizeof(tn->hash));
int8_t flags = read_int8(s->s);
tn->_reserved = 0;
tn->abstract = flags & 1;
tn->mutabl = (flags>>1) & 1;
tn->mayinlinealloc = (flags>>2) & 1;
Expand Down
8 changes: 0 additions & 8 deletions src/flisp/flmain.c
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,6 @@
extern "C" {
#endif

#if defined(__has_feature)
#if __has_feature(address_sanitizer)
const char* __asan_default_options() {
return "detect_leaks=0";
}
#endif
#endif

static value_t argv_list(fl_context_t *fl_ctx, int argc, char *argv[])
{
int i;
Expand Down
6 changes: 6 additions & 0 deletions src/gc-stacks.c
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,9 @@ static unsigned select_pool(size_t nb) JL_NOTSAFEPOINT

static void _jl_free_stack(jl_ptls_t ptls, void *stkbuf, size_t bufsz)
{
#ifdef _COMPILER_ASAN_ENABLED_
__asan_unpoison_stack_memory((uintptr_t)stkbuf, bufsz);
#endif
if (bufsz <= pool_sizes[JL_N_STACK_POOLS - 1]) {
unsigned pool_id = select_pool(bufsz);
if (pool_sizes[pool_id] == bufsz) {
Expand Down Expand Up @@ -135,6 +138,9 @@ void jl_release_task_stack(jl_ptls_t ptls, jl_task_t *task)
unsigned pool_id = select_pool(bufsz);
if (pool_sizes[pool_id] == bufsz) {
task->stkbuf = NULL;
#ifdef _COMPILER_ASAN_ENABLED_
__asan_unpoison_stack_memory((uintptr_t)stkbuf, bufsz);
#endif
arraylist_push(&ptls->heap.free_stacks[pool_id], stkbuf);
}
}
Expand Down
20 changes: 4 additions & 16 deletions src/init.c
Original file line number Diff line number Diff line change
Expand Up @@ -76,17 +76,16 @@ void jl_init_stack_limits(int ismaster, void **stack_lo, void **stack_hi)
#if defined(_COMPILER_GCC_) && __GNUC__ >= 12
#pragma GCC diagnostic ignored "-Wdangling-pointer"
#endif
*stack_hi = (void*)&stacksize;
*stack_hi = (void*)__builtin_frame_address(0);
#pragma GCC diagnostic pop
return;
# elif defined(_OS_DARWIN_)
extern void *pthread_get_stackaddr_np(pthread_t thread);
extern size_t pthread_get_stacksize_np(pthread_t thread);
pthread_t thread = pthread_self();
void *stackaddr = pthread_get_stackaddr_np(thread);
size_t stacksize = pthread_get_stacksize_np(thread);
*stack_lo = (void*)stackaddr;
*stack_hi = (void*)&stacksize;
*stack_hi = (void*)__builtin_frame_address(0);
return;
# elif defined(_OS_FREEBSD_)
pthread_attr_t attr;
Expand All @@ -97,7 +96,7 @@ void jl_init_stack_limits(int ismaster, void **stack_lo, void **stack_hi)
pthread_attr_getstack(&attr, &stackaddr, &stacksize);
pthread_attr_destroy(&attr);
*stack_lo = (void*)stackaddr;
*stack_hi = (void*)&stacksize;
*stack_hi = (void*)__builtin_frame_address(0);
return;
# else
# warning "Getting precise stack size for thread is not supported."
Expand All @@ -106,19 +105,8 @@ void jl_init_stack_limits(int ismaster, void **stack_lo, void **stack_hi)
struct rlimit rl;
getrlimit(RLIMIT_STACK, &rl);
size_t stacksize = rl.rlim_cur;
// We intentionally leak a stack address here core.StackAddressEscape
# ifndef __clang_analyzer__
*stack_hi = (void*)&stacksize;
#pragma GCC diagnostic push
#if defined(_COMPILER_GCC_) && __GNUC__ >= 12
#pragma GCC diagnostic ignored "-Wdangling-pointer"
#endif
*stack_hi = __builtin_frame_address(0);
*stack_lo = (void*)((char*)*stack_hi - stacksize);
#pragma GCC diagnostic pop
# else
*stack_hi = 0;
*stack_lo = 0;
# endif
#endif
}

Expand Down
1 change: 1 addition & 0 deletions src/interpreter.c
Original file line number Diff line number Diff line change
Expand Up @@ -542,6 +542,7 @@ static jl_value_t *eval_body(jl_array_t *stmts, interpreter_state *s, size_t ip,
// leave happens during normal control flow, but we must
// longjmp to pop the eval_body call for each enter.
s->continue_at = next_ip;
asan_unpoison_task_stack(ct, &eh->eh_ctx);
jl_longjmp(eh->eh_ctx, 1);
}
else if (head == jl_pop_exception_sym) {
Expand Down
7 changes: 7 additions & 0 deletions src/julia.h
Original file line number Diff line number Diff line change
Expand Up @@ -485,6 +485,7 @@ typedef struct {
uint8_t abstract:1;
uint8_t mutabl:1;
uint8_t mayinlinealloc:1;
uint8_t _reserved:5;
uint8_t max_methods; // override for inference's max_methods setting (0 = no additional limit or relaxation)
} jl_typename_t;

Expand Down Expand Up @@ -1984,8 +1985,14 @@ void (ijl_longjmp)(jmp_buf _Buf, int _Value);
#define jl_setjmp_name "sigsetjmp"
#endif
#define jl_setjmp(a,b) sigsetjmp(a,b)
#if defined(_COMPILER_ASAN_ENABLED_) && __GLIBC__
// Bypass the ASAN longjmp wrapper - we're unpoisoning the stack ourselves.
extern int __attribute__ ((nothrow)) (__libc_siglongjmp)(jl_jmp_buf buf, int val);
#define jl_longjmp(a,b) __libc_siglongjmp(a,b)
#else
#define jl_longjmp(a,b) siglongjmp(a,b)
#endif
#endif


#ifdef __clang_gcanalyzer__
Expand Down
37 changes: 37 additions & 0 deletions src/julia_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,45 @@
extern "C" {
#endif
#ifdef _COMPILER_ASAN_ENABLED_
#if defined(__GLIBC__) && defined(_CPU_X86_64_)
/* TODO: This is terrible - we're reaching deep into glibc internals here.
We should probably just switch to our own setjmp/longjmp implementation. */
#define JB_RSP 6
static inline uintptr_t demangle_ptr(uintptr_t var)
{
asm ("ror $17, %0\n\t"
"xor %%fs:0x30, %0\n\t"
: "=r" (var)
: "0" (var));
return var;
}
static inline uintptr_t jmpbuf_sp(jl_jmp_buf *buf)
{
return demangle_ptr((uintptr_t)(*buf)[0].__jmpbuf[JB_RSP]);
}
#else
#error Need to implement jmpbuf_sp for this architecture
#endif
void __sanitizer_start_switch_fiber(void**, const void*, size_t);
void __sanitizer_finish_switch_fiber(void*, const void**, size_t*);
extern void __asan_unpoison_stack_memory(uintptr_t addr, size_t size);
static inline void asan_unpoison_task_stack(jl_task_t *ct, jl_jmp_buf *buf)
{
if (!ct)
return;
/* Unpoison everything from the base of the stack allocation to the address
that we're resetting to. The idea is to remove the posion from the frames
that we're skipping over, since they won't be unwound. */
uintptr_t top = jmpbuf_sp(buf);
uintptr_t bottom = (uintptr_t)ct->stkbuf;
__asan_unpoison_stack_memory(bottom, top - bottom);
}
static inline void asan_unpoison_stack_memory(uintptr_t addr, size_t size) {
__asan_unpoison_stack_memory(addr, size);
}
#else
static inline void asan_unpoison_task_stack(jl_task_t *ct, jl_jmp_buf *buf) JL_NOTSAFEPOINT {}
static inline void asan_unpoison_stack_memory(uintptr_t addr, size_t size) JL_NOTSAFEPOINT {}
#endif
#ifdef _COMPILER_TSAN_ENABLED_
void *__tsan_create_fiber(unsigned flags);
Expand Down
3 changes: 3 additions & 0 deletions src/julia_threads.h
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,9 @@ typedef struct {
#if defined(_COMPILER_TSAN_ENABLED_)
void *tsan_state;
#endif
#if defined(_COMPILER_ASAN_ENABLED_)
void *asan_fake_stack;
#endif
} jl_ucontext_t;


Expand Down
4 changes: 3 additions & 1 deletion src/options.h
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,9 @@

// When not using COPY_STACKS the task-system is less memory efficient so
// you probably want to choose a smaller default stack size (factor of 8-10)
#ifdef _P64
#if defined(_COMPILER_ASAN_ENABLED_) || defined(_COMPILER_MSAN_ENABLED_)
#define JL_STACK_SIZE (64*1024*1024)
#elif defined(_P64)
#define JL_STACK_SIZE (4*1024*1024)
#else
#define JL_STACK_SIZE (2*1024*1024)
Expand Down
Loading

0 comments on commit 1acecb7

Please sign in to comment.