Skip to content

Commit

Permalink
strengthen setglobal to default to release-consume ordering (#47742)
Browse files Browse the repository at this point in the history
In looking at a TSAN report recently, I noticed that globals were
getting stored as atomic-unordered (since c92ab5e #44182), instead
of atomic-release as intended (since
46135df #45484).
  • Loading branch information
vtjnash authored Nov 30, 2022
1 parent 060a492 commit f4534d1
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 3 deletions.
2 changes: 1 addition & 1 deletion src/builtins.c
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand Down
5 changes: 3 additions & 2 deletions src/codegen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2881,6 +2881,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];
Expand All @@ -2890,7 +2891,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");
Expand Down Expand Up @@ -4690,7 +4691,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.
}
Expand Down

0 comments on commit f4534d1

Please sign in to comment.