Make ASAN actually do something and fix all the things

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.
JuliaLang · Aug 16, 2022 · 2d17301 · 2d17301
1 parent e358a17
commit 2d17301
Show file tree

Hide file tree

Showing 14 changed files with 289 additions and 89 deletions.
diff --git a/Make.inc b/Make.inc
@@ -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)

diff --git a/src/ccalltest.c b/src/ccalltest.c
@@ -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")))

diff --git a/src/codegen.cpp b/src/codegen.cpp
@@ -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

diff --git a/src/datatype.c b/src/datatype.c
@@ -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;

diff --git a/src/dump.c b/src/dump.c
@@ -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;

diff --git a/src/flisp/flmain.c b/src/flisp/flmain.c
@@ -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;

diff --git a/src/gc-stacks.c b/src/gc-stacks.c
@@ -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) {
@@ -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);
         }
     }

diff --git a/src/init.c b/src/init.c
@@ -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;
@@ -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."
@@ -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
 }
 

diff --git a/src/interpreter.c b/src/interpreter.c
@@ -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) {

diff --git a/src/julia.h b/src/julia.h
@@ -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;
 
@@ -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__

diff --git a/src/julia_internal.h b/src/julia_internal.h
@@ -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);

diff --git a/src/julia_threads.h b/src/julia_threads.h
@@ -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;
 
 

diff --git a/src/options.h b/src/options.h
@@ -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)