Merge pull request #42556 from JuliaLang/jn/tsan-build

threading correctness improvements

base/asyncevent.jl

@@ -15,7 +15,7 @@ mutable struct AsyncCondition
     handle::Ptr{Cvoid}
     cond::ThreadSynchronizer
     isopen::Bool
-    set::Bool
+    @atomic set::Bool
 
     function AsyncCondition()
         this = new(Libc.malloc(_sizeof_uv_async), ThreadSynchronizer(), true, false)
@@ -69,7 +69,7 @@ mutable struct Timer
     handle::Ptr{Cvoid}
     cond::ThreadSynchronizer
     isopen::Bool
-    set::Bool
+    @atomic set::Bool
 
     function Timer(timeout::Real; interval::Real = 0.0)
         timeout ≥ 0 || throw(ArgumentError("timer cannot have negative timeout of $timeout seconds"))
@@ -124,7 +124,7 @@ function _trywait(t::Union{Timer, AsyncCondition})
         end
         iolock_end()
     end
-    t.set = false
+    @atomic :monotonic t.set = false
     return set
 end
 
@@ -182,7 +182,7 @@ function uv_asynccb(handle::Ptr{Cvoid})
     async = @handle_as handle AsyncCondition
     lock(async.cond)
     try
-        async.set = true
+        @atomic :monotonic async.set = true
         notify(async.cond, true)
     finally
         unlock(async.cond)
@@ -194,7 +194,7 @@ function uv_timercb(handle::Ptr{Cvoid})
     t = @handle_as handle Timer
     lock(t.cond)
     try
-        t.set = true
+        @atomic :monotonic t.set = true
         if ccall(:uv_timer_get_repeat, UInt64, (Ptr{Cvoid},), t) == 0
             # timer is stopped now
             close(t)

base/gcutils.jl

@@ -50,8 +50,7 @@ function finalizer(@nospecialize(f), @nospecialize(o))
     return o
 end
 
-function finalizer(f::Ptr{Cvoid}, o::T) where T
-    @inline
+function finalizer(f::Ptr{Cvoid}, o::T) where T @inline
     if !ismutable(o)
         error("objects of type ", typeof(o), " cannot be finalized")
     end
@@ -116,16 +115,14 @@ another Task or thread.
 """
 enable_finalizers(on::Bool) = on ? enable_finalizers() : disable_finalizers()
 
-function enable_finalizers()
-    Base.@inline
+function enable_finalizers() @inline
     ccall(:jl_gc_enable_finalizers_internal, Cvoid, ())
-    if unsafe_load(cglobal(:jl_gc_have_pending_finalizers, Cint)) != 0
+    if Core.Intrinsics.atomic_pointerref(cglobal(:jl_gc_have_pending_finalizers, Cint), :monotonic) != 0
         ccall(:jl_gc_run_pending_finalizers, Cvoid, (Ptr{Cvoid},), C_NULL)
     end
 end
 
-function disable_finalizers()
-    Base.@inline
+function disable_finalizers() @inline
     ccall(:jl_gc_disable_finalizers_internal, Cvoid, ())
 end
 

deps/libuv.mk

@@ -7,16 +7,17 @@ $(eval $(call git-external,libuv,LIBUV,configure,,$(SRCCACHE)))
 UV_CFLAGS := -O2
 
 UV_FLAGS := LDFLAGS="$(LDFLAGS) $(CLDFLAGS) -v"
-ifneq ($(UV_CFLAGS),)
-UV_FLAGS += CFLAGS="$(CFLAGS) $(UV_CFLAGS)"
-endif
+UV_FLAGS += CFLAGS="$(CFLAGS) $(UV_CFLAGS) $(SANITIZE_OPTS)"
 
 ifneq ($(VERBOSE), 0)
 UV_MFLAGS += V=1
 endif
 
 LIBUV_BUILDDIR := $(BUILDDIR)/$(LIBUV_SRC_DIR)
 
+ifneq ($(CLDFLAGS)$(SANITIZE_LDFLAGS),)
+$(LIBUV_BUILDDIR)/build-configured: LDFLAGS:=$(LDFLAGS) $(CLDFLAGS) $(SANITIZE_LDFLAGS)
+endif
 $(LIBUV_BUILDDIR)/build-configured: $(SRCCACHE)/$(LIBUV_SRC_DIR)/source-extracted
 	touch -c $(SRCCACHE)/$(LIBUV_SRC_DIR)/aclocal.m4 # touch a few files to prevent autogen from getting called
 	touch -c $(SRCCACHE)/$(LIBUV_SRC_DIR)/Makefile.in

deps/libwhich.mk

@@ -4,7 +4,7 @@ LIBWHICH_TAR_URL = https://api.github.com/repos/vtjnash/libwhich/tarball/$1
 $(eval $(call git-external,libwhich,LIBWHICH,,,$(BUILDDIR)))
 
 LIBWHICH_OBJ_LIB := $(build_depsbindir)/libwhich
-LIBWHICH_MFLAGS := CC="$(CC)"
+LIBWHICH_MFLAGS := CC="$(HOSTCC)"
 
 $(BUILDDIR)/$(LIBWHICH_SRC_DIR)/build-compiled: $(BUILDDIR)/$(LIBWHICH_SRC_DIR)/source-extracted
 	$(MAKE) -C $(dir $<) $(LIBWHICH_MFLAGS) libwhich

deps/patchelf.mk

@@ -12,6 +12,9 @@ $(SRCCACHE)/patchelf-$(PATCHELF_VER)/source-extracted: $(SRCCACHE)/patchelf-$(PA
 checksum-patchelf: $(SRCCACHE)/patchelf-$(PATCHELF_VER).tar.gz
 	$(JLCHECKSUM) $<
 
+$(BUILDDIR)/patchelf-$(PATCHELF_VER)/build-configured: CC:=$(HOSTCC)
+$(BUILDDIR)/patchelf-$(PATCHELF_VER)/build-configured: CXX:=$(HOSTCXX)
+$(BUILDDIR)/patchelf-$(PATCHELF_VER)/build-configured: XC_HOST:=$(BUILD_MACHINE)
 $(BUILDDIR)/patchelf-$(PATCHELF_VER)/build-configured: $(SRCCACHE)/patchelf-$(PATCHELF_VER)/source-extracted
 	mkdir -p $(dir $@)
 	cd $(dir $@) && \

deps/tools/common.mk

@@ -4,7 +4,7 @@
 # it will make its way into the LLVM build flags, and LLVM is picky about RPATH (though
 # apparently not on FreeBSD). Ref PR #22352
 
-CONFIGURE_COMMON := --prefix=$(abspath $(build_prefix)) --build=$(BUILD_MACHINE) --libdir=$(abspath $(build_libdir)) --bindir=$(abspath $(build_depsbindir)) $(CUSTOM_LD_LIBRARY_PATH)
+CONFIGURE_COMMON = --prefix=$(abspath $(build_prefix)) --build=$(BUILD_MACHINE) --libdir=$(abspath $(build_libdir)) --bindir=$(abspath $(build_depsbindir)) $(CUSTOM_LD_LIBRARY_PATH)
 ifneq ($(XC_HOST),)
 CONFIGURE_COMMON += --host=$(XC_HOST)
 endif

src/aotcompile.cpp

@@ -290,7 +290,7 @@ void *jl_create_native_impl(jl_array_t *methods, const jl_cgparams_t *cgparams,
     std::unique_ptr<Module> clone(jl_create_llvm_module("text"));
 
     // compile all methods for the current world and type-inference world
-    size_t compile_for[] = { jl_typeinf_world, jl_world_counter };
+    size_t compile_for[] = { jl_typeinf_world, jl_atomic_load_acquire(&jl_world_counter) };
     for (int worlds = 0; worlds < 2; worlds++) {
         params.world = compile_for[worlds];
         if (!params.world)

src/ast.c

@@ -118,7 +118,7 @@ JL_DLLEXPORT jl_sym_t *jl_acquire_release_sym;
 JL_DLLEXPORT jl_sym_t *jl_sequentially_consistent_sym;
 
 
-static uint8_t flisp_system_image[] = {
+static const uint8_t flisp_system_image[] = {
 #include <julia_flisp.boot.inc>
 };
 
@@ -197,19 +197,19 @@ static value_t fl_defined_julia_global(fl_context_t *fl_ctx, value_t *args, uint
     return (b != NULL && b->owner == ctx->module) ? fl_ctx->T : fl_ctx->F;
 }
 
-static value_t fl_current_module_counter(fl_context_t *fl_ctx, value_t *args, uint32_t nargs)
+static value_t fl_current_module_counter(fl_context_t *fl_ctx, value_t *args, uint32_t nargs) JL_NOTSAFEPOINT
 {
     jl_ast_context_t *ctx = jl_ast_ctx(fl_ctx);
     assert(ctx->module);
     return fixnum(jl_module_next_counter(ctx->module));
 }
 
-static value_t fl_julia_current_file(fl_context_t *fl_ctx, value_t *args, uint32_t nargs)
+static value_t fl_julia_current_file(fl_context_t *fl_ctx, value_t *args, uint32_t nargs) JL_NOTSAFEPOINT
 {
     return symbol(fl_ctx, jl_filename);
 }
 
-static value_t fl_julia_current_line(fl_context_t *fl_ctx, value_t *args, uint32_t nargs)
+static value_t fl_julia_current_line(fl_context_t *fl_ctx, value_t *args, uint32_t nargs) JL_NOTSAFEPOINT
 {
     return fixnum(jl_lineno);
 }
@@ -273,10 +273,10 @@ static value_t fl_julia_logmsg(fl_context_t *fl_ctx, value_t *args, uint32_t nar
 }
 
 static const builtinspec_t julia_flisp_ast_ext[] = {
-    { "defined-julia-global", fl_defined_julia_global },
+    { "defined-julia-global", fl_defined_julia_global }, // TODO: can we kill this safepoint
     { "current-julia-module-counter", fl_current_module_counter },
-    { "julia-scalar?", fl_julia_scalar },
-    { "julia-logmsg", fl_julia_logmsg },
+    { "julia-scalar?", fl_julia_scalar }, // TODO: can we kill this safepoint? (from jl_isa)
+    { "julia-logmsg", fl_julia_logmsg }, // TODO: kill this safepoint
     { "julia-current-file", fl_julia_current_file },
     { "julia-current-line", fl_julia_current_line },
     { NULL, NULL }
@@ -310,23 +310,23 @@ static void jl_init_ast_ctx(jl_ast_context_t *ast_ctx) JL_NOTSAFEPOINT
 }
 
 // There should be no GC allocation while holding this lock
-static jl_mutex_t flisp_lock;
+static uv_mutex_t flisp_lock;
 static jl_ast_context_list_t *jl_ast_ctx_using = NULL;
 static jl_ast_context_list_t *jl_ast_ctx_freed = NULL;
 
 static jl_ast_context_t *jl_ast_ctx_enter(void) JL_GLOBALLY_ROOTED JL_NOTSAFEPOINT
 {
     jl_task_t *ct = jl_current_task;
     JL_SIGATOMIC_BEGIN();
-    JL_LOCK_NOGC(&flisp_lock);
+    uv_mutex_lock(&flisp_lock);
     jl_ast_context_list_t *node;
     jl_ast_context_t *ctx;
     // First check if the current task is using one of the contexts
     for (node = jl_ast_ctx_using;node;(node = node->next)) {
         ctx = jl_ast_context_list_item(node);
         if (ctx->task == ct) {
             ctx->ref++;
-            JL_UNLOCK_NOGC(&flisp_lock);
+            uv_mutex_unlock(&flisp_lock);
             return ctx;
         }
     }
@@ -338,7 +338,7 @@ static jl_ast_context_t *jl_ast_ctx_enter(void) JL_GLOBALLY_ROOTED JL_NOTSAFEPOI
         ctx->ref = 1;
         ctx->task = ct;
         ctx->module = NULL;
-        JL_UNLOCK_NOGC(&flisp_lock);
+        uv_mutex_unlock(&flisp_lock);
         return ctx;
     }
     // Construct a new one if we can't find any
@@ -347,7 +347,7 @@ static jl_ast_context_t *jl_ast_ctx_enter(void) JL_GLOBALLY_ROOTED JL_NOTSAFEPOI
     ctx->task = ct;
     node = &ctx->list;
     jl_ast_context_list_insert(&jl_ast_ctx_using, node);
-    JL_UNLOCK_NOGC(&flisp_lock);
+    uv_mutex_unlock(&flisp_lock);
     jl_init_ast_ctx(ctx);
     return ctx;
 }
@@ -357,19 +357,20 @@ static void jl_ast_ctx_leave(jl_ast_context_t *ctx)
     JL_SIGATOMIC_END();
     if (--ctx->ref)
         return;
-    JL_LOCK_NOGC(&flisp_lock);
+    uv_mutex_lock(&flisp_lock);
     ctx->task = NULL;
     jl_ast_context_list_t *node = &ctx->list;
     jl_ast_context_list_delete(node);
     jl_ast_context_list_insert(&jl_ast_ctx_freed, node);
-    JL_UNLOCK_NOGC(&flisp_lock);
+    uv_mutex_unlock(&flisp_lock);
 }
 
 void jl_init_flisp(void)
 {
     jl_task_t *ct = jl_current_task;
     if (jl_ast_ctx_using || jl_ast_ctx_freed)
         return;
+    uv_mutex_init(&flisp_lock);
     jl_ast_main_ctx.ref = 1;
     jl_ast_main_ctx.task = ct;
     jl_ast_context_list_insert(&jl_ast_ctx_using, &jl_ast_main_ctx.list);
@@ -1114,7 +1115,9 @@ static jl_value_t *jl_invoke_julia_macro(jl_array_t *args, jl_module_t *inmodule
         margs[i] = jl_array_ptr_ref(args, i - 1);
 
     size_t last_age = ct->world_age;
-    ct->world_age = world < jl_world_counter ? world : jl_world_counter;
+    ct->world_age = jl_atomic_load_acquire(&jl_world_counter);
+    if (ct->world_age > world)
+        ct->world_age = world;
     jl_value_t *result;
     JL_TRY {
         margs[0] = jl_toplevel_eval(*ctx, margs[0]);
@@ -1236,7 +1239,7 @@ JL_DLLEXPORT jl_value_t *jl_macroexpand(jl_value_t *expr, jl_module_t *inmodule)
     JL_TIMING(LOWERING);
     JL_GC_PUSH1(&expr);
     expr = jl_copy_ast(expr);
-    expr = jl_expand_macros(expr, inmodule, NULL, 0, jl_world_counter, 0);
+    expr = jl_expand_macros(expr, inmodule, NULL, 0, jl_atomic_load_acquire(&jl_world_counter), 0);
     expr = jl_call_scm_on_ast("jl-expand-macroscope", expr, inmodule);
     JL_GC_POP();
     return expr;
@@ -1247,7 +1250,7 @@ JL_DLLEXPORT jl_value_t *jl_macroexpand1(jl_value_t *expr, jl_module_t *inmodule
     JL_TIMING(LOWERING);
     JL_GC_PUSH1(&expr);
     expr = jl_copy_ast(expr);
-    expr = jl_expand_macros(expr, inmodule, NULL, 1, jl_world_counter, 0);
+    expr = jl_expand_macros(expr, inmodule, NULL, 1, jl_atomic_load_acquire(&jl_world_counter), 0);
     expr = jl_call_scm_on_ast("jl-expand-macroscope", expr, inmodule);
     JL_GC_POP();
     return expr;
@@ -1348,7 +1351,7 @@ JL_DLLEXPORT jl_value_t *jl_parse(const char *text, size_t text_len, jl_value_t
     args[4] = options;
     jl_task_t *ct = jl_current_task;
     size_t last_age = ct->world_age;
-    ct->world_age = jl_world_counter;
+    ct->world_age = jl_atomic_load_acquire(&jl_world_counter);
     jl_value_t *result = jl_apply(args, 5);
     ct->world_age = last_age;
     args[0] = result; // root during error checks below

src/builtins.c

@@ -735,7 +735,7 @@ JL_CALLABLE(jl_f__apply_pure)
         // because, why not :)
         // and `promote` works better this way
         size_t last_age = ct->world_age;
-        ct->world_age = jl_world_counter;
+        ct->world_age = jl_atomic_load_acquire(&jl_world_counter);
         ret = do_apply(args, nargs, NULL);
         ct->world_age = last_age;
         ct->ptls->in_pure_callback = last_in;
@@ -753,24 +753,26 @@ JL_CALLABLE(jl_f__call_latest)
     jl_task_t *ct = jl_current_task;
     size_t last_age = ct->world_age;
     if (!ct->ptls->in_pure_callback)
-        ct->world_age = jl_world_counter;
+        ct->world_age = jl_atomic_load_acquire(&jl_world_counter);
     jl_value_t *ret = jl_apply(args, nargs);
     ct->world_age = last_age;
     return ret;
 }
 
 // Like call_in_world, but runs in the specified world.
-// If world > jl_world_counter, run in the latest world.
+// If world > jl_atomic_load_acquire(&jl_world_counter), run in the latest world.
 JL_CALLABLE(jl_f__call_in_world)
 {
     JL_NARGSV(_apply_in_world, 2);
     jl_task_t *ct = jl_current_task;
     size_t last_age = ct->world_age;
     JL_TYPECHK(_apply_in_world, ulong, args[0]);
     size_t world = jl_unbox_ulong(args[0]);
-    world = world <= jl_world_counter ? world : jl_world_counter;
-    if (!ct->ptls->in_pure_callback)
-        ct->world_age = world;
+    if (!ct->ptls->in_pure_callback) {
+        ct->world_age = jl_atomic_load_acquire(&jl_world_counter);
+        if (ct->world_age > world)
+            ct->world_age = world;
+    }
     jl_value_t *ret = jl_apply(&args[1], nargs - 1);
     ct->world_age = last_age;
     return ret;

src/cgmemmgr.cpp

@@ -210,7 +210,7 @@ static _Atomic(size_t) map_offset{0};
 // Hopefully no one will set a ulimit for this to be a problem...
 static constexpr size_t map_size_inc_default = 128 * 1024 * 1024;
 static size_t map_size = 0;
-static jl_mutex_t shared_map_lock;
+static uv_mutex_t shared_map_lock;
 
 static size_t get_map_size_inc()
 {
@@ -256,7 +256,7 @@ static void *alloc_shared_page(size_t size, size_t *id, bool exec)
     *id = off;
     size_t map_size_inc = get_map_size_inc();
     if (__unlikely(off + size > map_size)) {
-        JL_LOCK_NOGC(&shared_map_lock);
+        uv_mutex_lock(&shared_map_lock);
         size_t old_size = map_size;
         while (off + size > map_size)
             map_size += map_size_inc;
@@ -267,7 +267,7 @@ static void *alloc_shared_page(size_t size, size_t *id, bool exec)
                 abort();
             }
         }
-        JL_UNLOCK_NOGC(&shared_map_lock);
+        uv_mutex_unlock(&shared_map_lock);
     }
     return create_shared_map(size, off);
 }
@@ -305,6 +305,7 @@ ssize_t pwrite_addr(int fd, const void *buf, size_t nbyte, uintptr_t addr)
 // Use `get_self_mem_fd` which has a guard to call this only once.
 static int _init_self_mem()
 {
+    uv_mutex_init(&shared_map_lock);
     struct utsname kernel;
     uname(&kernel);
     int major, minor;

src/clangsa/GCChecker.cpp

@@ -1455,6 +1455,23 @@ bool GCChecker::evalCall(const CallEvent &Call, CheckerContext &C) const {
     C.addTransition(State->BindExpr(CE, C.getLocationContext(), Result));
     return true;
   }
+  else if (name == "uv_mutex_lock") {
+    ProgramStateRef State = C.getState();
+    if (State->get<SafepointDisabledAt>() == (unsigned)-1) {
+      C.addTransition(State->set<SafepointDisabledAt>(C.getStackFrame()->getIndex()));
+      return true;
+    }
+  }
+  else if (name == "uv_mutex_unlock") {
+    ProgramStateRef State = C.getState();
+    const auto *LCtx = C.getLocationContext();
+    const auto *FD = dyn_cast<FunctionDecl>(LCtx->getDecl());
+    if (State->get<SafepointDisabledAt>() == (unsigned)C.getStackFrame()->getIndex() &&
+        !isFDAnnotatedNotSafepoint(FD)) {
+      C.addTransition(State->set<SafepointDisabledAt>(-1));
+      return true;
+    }
+  }
   return false;
 }