From c5cce00ab258fff868b80049871aad6bcf83484b Mon Sep 17 00:00:00 2001 From: Jameson Nash Date: Tue, 29 Nov 2022 18:46:22 -0500 Subject: [PATCH] strengthen setglobal to default to release-consume ordering In looking at a TSAN report recently, I noticed that globals were getting stored as atomic-unordered (since c92ab5e79ea #44182), instead of atomic-release as intended (since 46135dfce9074e5bf94eb277de28a33cad9cc14f #45484). --- src/builtins.c | 2 +- src/codegen.cpp | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/builtins.c b/src/builtins.c index bf9f886d92ba8..90cc544b47986 100644 --- a/src/builtins.c +++ b/src/builtins.c @@ -1201,7 +1201,7 @@ JL_CALLABLE(jl_f_getglobal) JL_CALLABLE(jl_f_setglobal) { - enum jl_memory_order order = jl_memory_order_monotonic; + enum jl_memory_order order = jl_memory_order_release; JL_NARGS(setglobal!, 3, 4); if (nargs == 4) { JL_TYPECHK(setglobal!, symbol, args[3]); diff --git a/src/codegen.cpp b/src/codegen.cpp index f02815df37e73..c938651059ca7 100644 --- a/src/codegen.cpp +++ b/src/codegen.cpp @@ -2879,6 +2879,7 @@ static bool emit_f_opglobal(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f, const jl_cgval_t &sym = argv[2]; const jl_cgval_t &val = argv[3]; enum jl_memory_order order = jl_memory_order_unspecified; + assert(f == jl_builtin_setglobal && modifyop == nullptr && "unimplemented"); if (nargs == 4) { const jl_cgval_t &arg4 = argv[4]; @@ -2888,7 +2889,7 @@ static bool emit_f_opglobal(jl_codectx_t &ctx, jl_cgval_t *ret, jl_value_t *f, return false; } else - order = jl_memory_order_monotonic; + order = jl_memory_order_release; if (order == jl_memory_order_invalid || order == jl_memory_order_notatomic) { emit_atomic_error(ctx, order == jl_memory_order_invalid ? "invalid atomic ordering" : "setglobal!: module binding cannot be written non-atomically"); @@ -4686,7 +4687,7 @@ static void emit_assignment(jl_codectx_t &ctx, jl_value_t *l, jl_value_t *r, ssi bp = global_binding_pointer(ctx, jl_globalref_mod(l), jl_globalref_name(l), &bnd, true); } if (bp != NULL) { - emit_globalset(ctx, bnd, bp, rval_info, AtomicOrdering::Unordered); + emit_globalset(ctx, bnd, bp, rval_info, AtomicOrdering::Release); // Global variable. Does not need debug info because the debugger knows about // its memory location. }