From b12195bebfd15fb721a03aab7dfe119e2dd4db72 Mon Sep 17 00:00:00 2001 From: gingerBill Date: Wed, 7 Oct 2026 22:43:08 +0100 Subject: [PATCH] Add an atomic memory ordering analysis (`-no-atomic-analysis`, `#+atomic-analysis`, `#+no-atomic-analysis`) warning where release or acquire ordering pairs with nothing, and `-vet-atomic-access` for plain reads of what is accessed atomically --- src/build_settings.cpp | 4 + src/check_atomics.cpp | 625 +++++++++++++++++++++++++++++++++++++++++ src/check_builtin.cpp | 36 ++- src/check_expr.cpp | 12 + src/check_stmt.cpp | 5 + src/checker.cpp | 13 +- src/checker.hpp | 15 + src/main.cpp | 23 ++ src/parser.cpp | 34 ++- src/parser.hpp | 2 + 10 files changed, 748 insertions(+), 21 deletions(-) create mode 100644 src/check_atomics.cpp diff --git a/src/build_settings.cpp b/src/build_settings.cpp index 510bc402b..03ca80194 100644 --- a/src/build_settings.cpp +++ b/src/build_settings.cpp @@ -320,6 +320,7 @@ enum VetFlags : u64 { VetFlag_WhenShadowing = 1u<<12, VetFlag_NilDeref = 1u<<13, VetFlag_Uninitialized = 1u<<14, + VetFlag_AtomicAccess = 1u<<15, VetFlag_Unused = VetFlag_UnusedVariables|VetFlag_UnusedImports, @@ -361,6 +362,8 @@ u64 get_vet_flag_from_name(String const &name) { return VetFlag_NilDeref; } else if (name == "uninitialized") { return VetFlag_Uninitialized; + } else if (name == "atomic-access") { + return VetFlag_AtomicAccess; } return VetFlag_NONE; } @@ -564,6 +567,7 @@ struct BuildContext { bool no_rpath; bool no_entry_point; bool no_escape_analysis; + bool no_atomic_analysis; bool no_thread_local; bool cross_compiling; bool different_os; diff --git a/src/check_atomics.cpp b/src/check_atomics.cpp new file mode 100644 index 000000000..f13145fa2 --- /dev/null +++ b/src/check_atomics.cpp @@ -0,0 +1,625 @@ +// The analysis of atomic memory orderings, after every procedure body is checked. Each atomic operation on `&x` is +// on a location: a global, a static, or a field for every value of its struct, as which value it is cannot be known. +// What is written with release ordering but only loaded with relaxed ordering, or read with acquire ordering but only +// stored with relaxed ordering, orders nothing, and is warned about, unless the address of the location is taken +// elsewhere, as it may then be accessed through it. Only loads and stores are what it would pair with, as a +// read-modify-write asking for an ordering which nothing pairs with, e.g. to count, is harmless. +// With -vet-atomic-access, a plain read of a location accessed atomically is an error, unless a lock is taken. + +struct AtomicUses { + Ast *first; // the first atomic operation of each, by position + Ast *load; + Ast *store; + Ast *asks_acquire; // explicitly, in a file with the analysis + Ast *asks_release; + bool acquires; // by any read, including with seq_cst ordering or a fence + bool releases; + bool escaped; +}; + +struct AtomicFences { + bool acquire; + bool release; +}; + +struct AtomicReport { + Ast *site; // what asks for an ordering which nothing pairs with + Ast *other; // what would pair with it, with that ordering + bool release; +}; + +struct AtomicScan { + Array reads; // plain reads of what is accessed atomically + bool locks; +}; + +gb_global PtrMap atomic_uses; + + +// what an atomic operation on `&expr` is on, if anything +gb_internal Entity *atomic_location(Ast *expr) { + for (;;) { + expr = unparen_expr(expr); + switch (expr->kind) { + case_ast_node(i, Ident, expr); + Entity *e = entity_of_node(expr); + if (e == nullptr || e->kind != Entity_Variable) { + return nullptr; + } + if (e->using_parent != nullptr) { + Type *t = base_type(type_deref(e->using_parent->type)); + if (t == nullptr || t->kind != Type_Struct) { + return nullptr; + } + for (Entity *f : t->Struct.fields) { + if (f->token.string == e->token.string) { + return f; + } + } + return nullptr; + } + if (!e->Variable.is_global && (e->flags & EntityFlag_Static) == 0) { + return nullptr; + } + if (e->Variable.thread_local_model.len != 0) { + return nullptr; + } + return e; + case_end; + + case_ast_node(se, SelectorExpr, expr); + if (se->swizzle_count > 0 || se->is_bit_field) { + return nullptr; + } + Entity *pkg = entity_of_node(se->expr); + if (pkg != nullptr && pkg->kind == Entity_ImportName) { + expr = se->selector; + continue; + } + Entity *f = entity_of_node(se->selector); + if (f == nullptr || f->kind != Entity_Variable || (f->flags & EntityFlag_Field) == 0) { + return nullptr; + } + return f; + case_end; + + case_ast_node(ie, IndexExpr, expr); + // an element of an array is within it, but not one of a slice, as it may be reached through any copy of it + Type *t = base_type(ie->expr->tav.type); + if (t == nullptr || (t->kind != Type_Array && t->kind != Type_EnumeratedArray && t->kind != Type_FixedCapacityDynamicArray)) { + return nullptr; + } + expr = ie->expr; + case_end; + + default: + return nullptr; + } + } +} + +gb_internal void atomic_first(Ast **site, Ast *call) { + if (*site == nullptr || token_pos_cmp(ast_token(call).pos, ast_token(*site).pos) < 0) { + *site = call; + } +} + +gb_internal OdinAtomicMemoryOrder atomic_order_of(Ast *expr) { + return cast(OdinAtomicMemoryOrder)exact_value_to_i64(expr->tav.value); +} + +gb_internal bool atomic_order_acquires(OdinAtomicMemoryOrder order) { + switch (order) { + case OdinAtomicMemoryOrder_consume: + case OdinAtomicMemoryOrder_acquire: + case OdinAtomicMemoryOrder_acq_rel: + case OdinAtomicMemoryOrder_seq_cst: + return true; + } + return false; +} + +gb_internal bool atomic_order_releases(OdinAtomicMemoryOrder order) { + switch (order) { + case OdinAtomicMemoryOrder_release: + case OdinAtomicMemoryOrder_acq_rel: + case OdinAtomicMemoryOrder_seq_cst: + return true; + } + return false; +} + +gb_internal int atomic_report_cmp(void const *a, void const *b) { + AtomicReport const *x = cast(AtomicReport const *)a; + AtomicReport const *y = cast(AtomicReport const *)b; + return token_pos_cmp(ast_token(x->site).pos, ast_token(y->site).pos); +} + +// the plain reads within a statement or an expression, of which only the address is used when `addr` +gb_internal void atomic_scan(AtomicScan *s, Ast *node, bool addr) { + if (node == nullptr || node->tav.mode == Addressing_Constant || node->tav.mode == Addressing_Type) { + return; + } + switch (node->kind) { + case Ast_Ident: + case Ast_SelectorExpr: + case Ast_IndexExpr: + if (!addr) { + Entity *e = atomic_location(node); + if (e != nullptr && map_get(&atomic_uses, e) != nullptr) { + array_add(&s->reads, node); + } + } + break; + } + + switch (node->kind) { + case_ast_node(pe, ParenExpr, node); + atomic_scan(s, pe->expr, addr); + case_end; + + case_ast_node(ue, UnaryExpr, node); + atomic_scan(s, ue->expr, ue->op.kind == Token_And); + case_end; + + case_ast_node(be, BinaryExpr, node); + atomic_scan(s, be->left, false); + atomic_scan(s, be->right, false); + case_end; + + case_ast_node(se, SelectorExpr, node); + Entity *pkg = entity_of_node(se->expr); + if (pkg != nullptr && pkg->kind == Entity_ImportName) { + break; + } + // reading a field reads only it, of what it is within + if (is_type_pointer(se->expr->tav.type)) { + atomic_scan(s, se->expr, false); + } else { + atomic_scan(s, se->expr, addr || se->swizzle_count == 0); + } + case_end; + + case_ast_node(ie, IndexExpr, node); + Type *t = base_type(ie->expr->tav.type); + if (t != nullptr && (t->kind == Type_Array || t->kind == Type_EnumeratedArray || t->kind == Type_FixedCapacityDynamicArray || t->kind == Type_Matrix || t->kind == Type_Struct)) { + atomic_scan(s, ie->expr, true); + } else { + atomic_scan(s, ie->expr, false); + } + atomic_scan(s, ie->index, false); + case_end; + + case_ast_node(mie, MatrixIndexExpr, node); + atomic_scan(s, mie->expr, true); + atomic_scan(s, mie->row_index, false); + atomic_scan(s, mie->column_index, false); + case_end; + + case_ast_node(de, DerefExpr, node); + atomic_scan(s, de->expr, false); + case_end; + + case_ast_node(se, SliceExpr, node); + atomic_scan(s, se->expr, is_type_array_like(se->expr->tav.type)); + atomic_scan(s, se->low, false); + atomic_scan(s, se->high, false); + case_end; + + case_ast_node(ce, CallExpr, node); + Ast *proc = unparen_expr(ce->proc); + if (proc->tav.mode != Addressing_Type && proc->tav.mode != Addressing_Builtin) { + Entity *e = nullptr; + if (proc->kind == Ast_Ident || proc->kind == Ast_SelectorExpr) { + e = entity_of_node(proc); + } + // e.g. `sync.mutex_lock` or `sync.guard`, after which plain reads are assumed to be under the lock + if (e != nullptr && e->kind == Entity_Procedure && e->pkg != nullptr && e->pkg->name == "sync" && + (string_contains_string(e->token.string, str_lit("lock")) || string_contains_string(e->token.string, str_lit("guard")))) { + s->locks = true; + } + atomic_scan(s, proc, false); + } + for (Ast *arg : ce->args) { + atomic_scan(s, arg, false); + } + case_end; + + case_ast_node(sce, SelectorCallExpr, node); + atomic_scan(s, sce->call, false); + case_end; + + case_ast_node(cl, CompoundLit, node); + for (Ast *elem : cl->elems) { + atomic_scan(s, elem, false); + } + case_end; + + case_ast_node(fv, FieldValue, node); + atomic_scan(s, fv->value, false); + case_end; + + case_ast_node(te, TernaryIfExpr, node); + atomic_scan(s, te->cond, false); + atomic_scan(s, te->x, false); + atomic_scan(s, te->y, false); + case_end; + + case_ast_node(te, TernaryWhenExpr, node); + if (te->cond != nullptr && te->cond->tav.value.kind == ExactValue_Bool) { + if (te->cond->tav.value.value_bool) { + atomic_scan(s, te->x, false); + } else { + atomic_scan(s, te->y, false); + } + } + case_end; + + case_ast_node(oe, OrElseExpr, node); + atomic_scan(s, oe->x, false); + atomic_scan(s, oe->y, false); + case_end; + + case_ast_node(re, OrReturnExpr, node); + atomic_scan(s, re->expr, false); + case_end; + + case_ast_node(be, OrBranchExpr, node); + atomic_scan(s, be->expr, false); + case_end; + + case_ast_node(ta, TypeAssertion, node); + atomic_scan(s, ta->expr, false); + case_end; + + case_ast_node(tc, TypeCast, node); + atomic_scan(s, tc->expr, false); + case_end; + + case_ast_node(ac, AutoCast, node); + atomic_scan(s, ac->expr, false); + case_end; + + case_ast_node(te, TagExpr, node); + atomic_scan(s, te->expr, false); + case_end; + + case_ast_node(es, ExprStmt, node); + atomic_scan(s, es->expr, false); + case_end; + + case_ast_node(vd, ValueDecl, node); + if (vd->is_mutable) { + for (Ast *value : vd->values) { + atomic_scan(s, value, false); + } + } + case_end; + + case_ast_node(as, AssignStmt, node); + for (Ast *rhs : as->rhs) { + atomic_scan(s, rhs, false); + } + for (Ast *lhs : as->lhs) { + atomic_scan(s, lhs, as->op.kind == Token_Eq); + } + case_end; + + case_ast_node(bs, BlockStmt, node); + for (Ast *stmt : bs->stmts) { + atomic_scan(s, stmt, false); + } + case_end; + + case_ast_node(is, IfStmt, node); + atomic_scan(s, is->init, false); + atomic_scan(s, is->cond, false); + atomic_scan(s, is->body, false); + atomic_scan(s, is->else_stmt, false); + case_end; + + case_ast_node(ws, WhenStmt, node); + if (ws->is_cond_determined) { + if (ws->determined_cond) { + atomic_scan(s, ws->body, false); + } else { + atomic_scan(s, ws->else_stmt, false); + } + } + case_end; + + case_ast_node(rs, ReturnStmt, node); + for (Ast *result : rs->results) { + atomic_scan(s, result, false); + } + case_end; + + case_ast_node(fs, ForStmt, node); + atomic_scan(s, fs->init, false); + atomic_scan(s, fs->cond, false); + atomic_scan(s, fs->post, false); + atomic_scan(s, fs->body, false); + case_end; + + case_ast_node(rs, RangeStmt, node); + bool by_ref = false; + for (Ast *val : rs->vals) { + by_ref |= val->kind == Ast_UnaryExpr && val->UnaryExpr.op.kind == Token_And; + } + atomic_scan(s, rs->expr, by_ref && is_type_array_like(rs->expr->tav.type)); + atomic_scan(s, rs->body, false); + case_end; + + case_ast_node(rs, UnrollRangeStmt, node); + bool by_ref = false; + Ast *vals[2] = {rs->val0, rs->val1}; + for (Ast *val : vals) { + by_ref |= val != nullptr && val->kind == Ast_UnaryExpr && val->UnaryExpr.op.kind == Token_And; + } + atomic_scan(s, rs->init, false); + atomic_scan(s, rs->expr, by_ref && is_type_array_like(rs->expr->tav.type)); + atomic_scan(s, rs->body, false); + case_end; + + case_ast_node(ss, SwitchStmt, node); + atomic_scan(s, ss->init, false); + atomic_scan(s, ss->tag, false); + atomic_scan(s, ss->body, false); + case_end; + + case_ast_node(ss, TypeSwitchStmt, node); + if (ss->tag != nullptr && ss->tag->kind == Ast_AssignStmt) { + for (Ast *rhs : ss->tag->AssignStmt.rhs) { + atomic_scan(s, rhs, false); + } + } + atomic_scan(s, ss->body, false); + case_end; + + case_ast_node(cc, CaseClause, node); + for (Ast *expr : cc->list) { + atomic_scan(s, expr, false); + } + for (Ast *stmt : cc->stmts) { + atomic_scan(s, stmt, false); + } + case_end; + + case_ast_node(ds, DeferStmt, node); + atomic_scan(s, ds->stmt, false); + case_end; + } +} + +gb_internal void atomic_check_bodies(ProcInfo **procs, isize count) { + TEMPORARY_ALLOCATOR_GUARD(); + for (isize i = 0; i < count; i++) { + ProcInfo *pi = procs[i]; + Ast *body = pi->body; + if (body == nullptr || !ast_file_atomic_analysis(body->file()) || (ast_file_vet_flags(body->file()) & VetFlag_AtomicAccess) == 0) { + continue; + } + AtomicScan s = {}; + s.reads = array_make(temporary_allocator(), 0, 0); + atomic_scan(&s, body, false); + if (s.locks || s.reads.count == 0) { + continue; + } + + ErrorInstantiations prev_instantiations = global_error_context.instantiations; + global_error_context.instantiations = {pi->generated_from_polymorphic ? pi : pi->poly_parent, nullptr}; + for (Ast *read : s.reads) { + AtomicUses *uses = map_get(&atomic_uses, atomic_location(read)); + ERROR_BLOCK(); + gbString str = expr_to_string(read); + error(read, "'%s' is read plainly, but it is accessed atomically, e.g. at %s", str, token_pos_to_string(ast_token(uses->first).pos)); + error_line("\tSuggestion: Read it with 'atomic_load_explicit(&%s, .Relaxed)', or take a lock around it\n", str); + gb_string_free(str); + } + global_error_context.instantiations = prev_instantiations; + } +} + +gb_internal void check_atomics(Checker *c) { + TEMPORARY_ALLOCATOR_GUARD(); + + auto atomics = array_make(heap_allocator()); + auto addresses = array_make(heap_allocator()); + defer (array_free(&atomics)); + defer (array_free(&addresses)); + per_thread_array_gather(&c->info.checked_atomics_queue, &atomics); + per_thread_array_gather(&c->info.checked_addresses_queue, &addresses); + + map_init(&atomic_uses, 0); + defer ({ + map_destroy(&atomic_uses); + atomic_uses = {}; + }); + + PtrMap fences = {}; + map_init(&fences, 0); + defer (map_destroy(&fences)); + for (CheckedAtomic const &a : atomics) { + if (a.id != BuiltinProc_atomic_thread_fence || a.decl == nullptr) { + continue; + } + OdinAtomicMemoryOrder order = atomic_order_of(a.call->CallExpr.args[0]); + AtomicFences f = {}; + if (AtomicFences *found = map_get(&fences, a.decl)) { + f = *found; + } + f.acquire |= atomic_order_acquires(order); + f.release |= atomic_order_releases(order); + map_set(&fences, a.decl, f); + } + + PtrSet operated = {}; + ptr_set_init(&operated, atomics.count); + defer (ptr_set_destroy(&operated)); + + for (CheckedAtomic const &a : atomics) { + if (a.id == BuiltinProc_atomic_thread_fence || a.id == BuiltinProc_atomic_signal_fence) { + continue; + } + Ast *call = a.call; + Ast *ptr = atomic_address_of(call->CallExpr.args[0]); + if (ptr == nullptr) { + continue; + } + ptr_set_add(&operated, ptr); + Entity *e = atomic_location(ptr->UnaryExpr.expr); + if (e == nullptr) { + continue; + } + + // what the operation does, with the orderings it has, explicitly or by default + bool explicit_order = true; + OdinAtomicMemoryOrder order = OdinAtomicMemoryOrder_seq_cst; + OdinAtomicMemoryOrder failure = OdinAtomicMemoryOrder_relaxed; + bool reads = true; + bool writes = true; + switch (a.id) { + case BuiltinProc_atomic_store: + explicit_order = false; + reads = false; + break; + case BuiltinProc_atomic_store_explicit: + order = atomic_order_of(call->CallExpr.args[2]); + reads = false; + break; + case BuiltinProc_atomic_load: + explicit_order = false; + writes = false; + break; + case BuiltinProc_atomic_load_explicit: + order = atomic_order_of(call->CallExpr.args[1]); + writes = false; + break; + case BuiltinProc_atomic_add_explicit: + case BuiltinProc_atomic_sub_explicit: + case BuiltinProc_atomic_and_explicit: + case BuiltinProc_atomic_nand_explicit: + case BuiltinProc_atomic_or_explicit: + case BuiltinProc_atomic_xor_explicit: + case BuiltinProc_atomic_exchange_explicit: + order = atomic_order_of(call->CallExpr.args[2]); + break; + case BuiltinProc_atomic_compare_exchange_strong_explicit: + case BuiltinProc_atomic_compare_exchange_weak_explicit: + order = atomic_order_of(call->CallExpr.args[3]); + failure = atomic_order_of(call->CallExpr.args[4]); + break; + default: + explicit_order = false; + break; + } + + AtomicFences f = {}; + if (a.decl != nullptr) { + if (AtomicFences *found = map_get(&fences, a.decl)) { + f = *found; + } + } + + AtomicUses uses = {}; + if (AtomicUses *found = map_get(&atomic_uses, e)) { + uses = *found; + } else { + // what is foreign or exported may be accessed by what is not checked + uses.escaped = e->Variable.is_foreign || e->Variable.is_export; + } + // seq_cst, by default or not, is not asked for as acquire or release ordering is, so it is never reported + bool reported = explicit_order && order != OdinAtomicMemoryOrder_seq_cst && ast_file_atomic_analysis(call->file()); + atomic_first(&uses.first, call); + if (reads && !writes) { + atomic_first(&uses.load, call); + } + if (writes && !reads) { + atomic_first(&uses.store, call); + } + if (reads) { + uses.acquires |= atomic_order_acquires(order) || atomic_order_acquires(failure) || f.acquire; + if (reported && atomic_order_acquires(order)) { + atomic_first(&uses.asks_acquire, call); + } + } + if (writes) { + uses.releases |= atomic_order_releases(order) || f.release; + if (reported && atomic_order_releases(order)) { + atomic_first(&uses.asks_release, call); + } + } + map_set(&atomic_uses, e, uses); + } + if (atomic_uses.count == 0) { + return; + } + + for (CheckedAddress const &a : addresses) { + if (ptr_set_exists(&operated, a.node)) { + continue; + } + if (AtomicUses *uses = map_get(&atomic_uses, a.location)) { + uses->escaped = true; + } + } + + if (!global_ignore_warnings()) { + auto reports = array_make(temporary_allocator(), 0, 0); + for (auto const &entry : atomic_uses) { + AtomicUses const &uses = entry.value; + if (uses.escaped) { + continue; + } + // asking for acquire ordering acquires, so at most one of these + if (uses.asks_release != nullptr && uses.load != nullptr && !uses.acquires) { + array_add(&reports, AtomicReport{uses.asks_release, uses.load, true}); + } else if (uses.asks_acquire != nullptr && uses.store != nullptr && !uses.releases) { + array_add(&reports, AtomicReport{uses.asks_acquire, uses.store, false}); + } + } + // in order, and once for what is at the same place in each instantiation of a polymorphic procedure + array_sort(reports, atomic_report_cmp); + TokenPos last = {}; + for (AtomicReport const &r : reports) { + TokenPos pos = ast_token(r.site).pos; + if (pos == last) { + continue; + } + last = pos; + + ERROR_BLOCK(); + gbString str = expr_to_string(atomic_address_of(r.site->CallExpr.args[0])->UnaryExpr.expr); + char const *other = token_pos_to_string(ast_token(r.other).pos); + if (r.release) { + warning(r.site, "'%s' is written with release ordering, but it is only loaded with relaxed ordering, so the release orders nothing", str); + error_line("\tSuggestion: Load it with .Acquire, e.g. at %s, or write it with .Relaxed\n", other); + } else { + warning(r.site, "'%s' is read with acquire ordering, but it is only stored with relaxed ordering, so the acquire orders nothing", str); + error_line("\tSuggestion: Store it with .Release, e.g. at %s, or read it with .Relaxed\n", other); + } + gb_string_free(str); + } + } + + // only when a file has -vet-atomic-access + bool vetted = (build_context.vet_flags & VetFlag_AtomicAccess) != 0; + for (auto const &entry : c->info.files) { + AstFile *f = entry.value; + vetted |= f->vet_flags_set && (f->vet_flags & VetFlag_AtomicAccess) != 0; + } + if (!vetted) { + return; + } + + auto procs = array_make(heap_allocator()); + defer (array_free(&procs)); + for (PerThreadArraySlot &slot : c->info.checked_bodies_queue.slots) { + array_add_elems(&procs, slot.array.data, slot.array.count); + } + if (!build_context.no_threaded_checker && build_context.thread_count > 1) { + thread_pool_for_chunks(procs.data, procs.count, 1024, atomic_check_bodies); + } else { + atomic_check_bodies(procs.data, procs.count); + } +} diff --git a/src/check_builtin.cpp b/src/check_builtin.cpp index 3c7cf880a..7f2f955f8 100644 --- a/src/check_builtin.cpp +++ b/src/check_builtin.cpp @@ -2169,6 +2169,26 @@ gb_internal bool is_valid_type_for_load(Type *type) { return false; } +// the `&x` an atomic operation's pointer is, through any conversions of it, unless it is a pointer from elsewhere +gb_internal Ast *atomic_address_of(Ast *ptr) { + ptr = unparen_expr(ptr); + for (;;) { + if (ptr->kind == Ast_CallExpr && ptr->CallExpr.proc->tav.mode == Addressing_Type && ptr->CallExpr.args.count == 1) { + ptr = unparen_expr(ptr->CallExpr.args[0]); + } else if (ptr->kind == Ast_TypeCast) { + ptr = unparen_expr(ptr->TypeCast.expr); + } else if (ptr->kind == Ast_AutoCast) { + ptr = unparen_expr(ptr->AutoCast.expr); + } else { + break; + } + } + if (ptr->kind != Ast_UnaryExpr || ptr->UnaryExpr.op.kind != Token_And) { + return nullptr; + } + return ptr; +} + gb_internal bool check_atomic_ptr_argument(Operand *operand, String const &builtin_name, Type *elem) { if (!is_type_valid_atomic_type(elem)) { error(operand->expr, "Only an integer, floating-point, boolean, or pointer can be used as an atomic for '%.*s'", LIT(builtin_name)); @@ -2185,20 +2205,8 @@ gb_internal bool check_atomic_ptr_argument(Operand *operand, String const &built return false; } - // the address, through any conversions of it - Ast *ptr = unparen_expr(operand->expr); - for (;;) { - if (ptr->kind == Ast_CallExpr && ptr->CallExpr.proc->tav.mode == Addressing_Type && ptr->CallExpr.args.count == 1) { - ptr = unparen_expr(ptr->CallExpr.args[0]); - } else if (ptr->kind == Ast_TypeCast) { - ptr = unparen_expr(ptr->TypeCast.expr); - } else if (ptr->kind == Ast_AutoCast) { - ptr = unparen_expr(ptr->AutoCast.expr); - } else { - break; - } - } - if (ptr->kind != Ast_UnaryExpr || ptr->UnaryExpr.op.kind != Token_And) { + Ast *ptr = atomic_address_of(operand->expr); + if (ptr == nullptr) { return true; } diff --git a/src/check_expr.cpp b/src/check_expr.cpp index 6af3b394f..3ea468533 100644 --- a/src/check_expr.cpp +++ b/src/check_expr.cpp @@ -3066,6 +3066,11 @@ gb_internal void check_unary_expr(CheckerContext *c, Operand *o, Token op, Ast * o->mode = Addressing_Invalid; return; } + if (atomic_analysis_in_use()) { + if (Entity *e = atomic_location(o->expr)) { + per_thread_array_add(&c->info->checked_addresses_queue, CheckedAddress{node, e}); + } + } Type *soa_for_in_type = nullptr; if (node->kind == Ast_UnaryExpr) { @@ -9492,6 +9497,8 @@ gb_internal ExprKind check_call_expr(CheckerContext *c, Operand *operand, Ast *c if (!check_builtin_procedure(c, operand, call, id, type_hint)) { operand->mode = Addressing_Invalid; operand->type = t_invalid; + } else if (BuiltinProc_atomic_thread_fence <= id && id <= BuiltinProc_atomic_compare_exchange_weak_explicit && atomic_analysis_in_use()) { + per_thread_array_add(&c->info->checked_atomics_queue, CheckedAtomic{call, c->curr_proc_decl, id}); } operand->expr = call; return builtin_procs[id].kind; @@ -12889,6 +12896,11 @@ gb_internal ExprKind check_slice_expr(CheckerContext *c, Operand *o, Ast *node, o->expr = node; return kind; } + if (atomic_analysis_in_use() && !is_type_pointer(o->type)) { + if (Entity *e = atomic_location(node->SliceExpr.expr)) { + per_thread_array_add(&c->info->checked_addresses_queue, CheckedAddress{node, e}); + } + } o->type = alloc_type_slice(t->Array.elem); break; diff --git a/src/check_stmt.cpp b/src/check_stmt.cpp index a748209b1..4062fe0d4 100644 --- a/src/check_stmt.cpp +++ b/src/check_stmt.cpp @@ -2109,6 +2109,11 @@ gb_internal void check_range_stmt(CheckerContext *ctx, Ast *node, u32 mod_flags) if (is_addressed) { if (is_possibly_addressable && i == addressable_index) { entity->flags &= ~EntityFlag_Value; + if (atomic_analysis_in_use()) { + if (Entity *e = atomic_location(expr)) { + per_thread_array_add(&ctx->info->checked_addresses_queue, CheckedAddress{node, e}); + } + } } else { char const *idx_name = is_map ? "key" : (is_bit_set || i == 0) ? "element" : "index"; error(token, "The %s variable '%.*s' cannot be made addressable", idx_name, LIT(str)); diff --git a/src/checker.cpp b/src/checker.cpp index d4df6e880..5c9c9e0b9 100644 --- a/src/checker.cpp +++ b/src/checker.cpp @@ -1708,6 +1708,8 @@ gb_internal void init_checker_info(CheckerInfo *i) { per_thread_array_init(&i->definition_queue, global_thread_pool.threads.count); per_thread_array_init(&i->checked_bodies_queue, global_thread_pool.threads.count); per_thread_array_init(&i->checked_calls_queue, global_thread_pool.threads.count); + per_thread_array_init(&i->checked_atomics_queue, global_thread_pool.threads.count); + per_thread_array_init(&i->checked_addresses_queue, global_thread_pool.threads.count); mpsc_init(&i->required_global_variable_queue, a); // 1<<10); mpsc_init(&i->required_foreign_imports_through_force_queue, a); // 1<<10); mpsc_init(&i->foreign_imports_to_check_fullpaths, a); // 1<<10); @@ -1744,6 +1746,8 @@ gb_internal void destroy_checker_info(CheckerInfo *i) { per_thread_array_destroy(&i->definition_queue); per_thread_array_destroy(&i->checked_bodies_queue); per_thread_array_destroy(&i->checked_calls_queue); + per_thread_array_destroy(&i->checked_atomics_queue); + per_thread_array_destroy(&i->checked_addresses_queue); mpsc_destroy(&i->required_global_variable_queue); mpsc_destroy(&i->required_foreign_imports_through_force_queue); mpsc_destroy(&i->foreign_imports_to_check_fullpaths); @@ -4935,6 +4939,7 @@ gb_internal DECL_ATTRIBUTE_PROC(asm_decl_attribute) { #include "check_decl.cpp" #include "check_stmt.cpp" #include "check_escape.cpp" +#include "check_atomics.cpp" @@ -6874,7 +6879,7 @@ gb_internal bool check_proc_info(Checker *c, ProcInfo *pi, UntypedExprInfoMap *u if (body_was_checked) { pi->decl->proc_info = pi; - if (escape_analysis_in_use()) { + if (escape_analysis_in_use() || atomic_analysis_in_use()) { per_thread_array_add(&c->info.checked_bodies_queue, pi); } pi->decl->proc_checked_state.store(ProcCheckedState_Checked); @@ -8096,6 +8101,12 @@ gb_internal void check_parsed_files(Checker *c) { debugf("Total Procedure Bodies Checked: %td\n", total_bodies_checked.load(std::memory_order_relaxed)); + // before the escape analysis, which takes the checked bodies + if (atomic_analysis_in_use()) { + TIME_SECTION("check atomics"); + check_atomics(c); + } + if (escape_analysis_in_use()) { TIME_SECTION("check escapes"); check_escapes(c); diff --git a/src/checker.hpp b/src/checker.hpp index cf4c8f12e..05db715e4 100644 --- a/src/checker.hpp +++ b/src/checker.hpp @@ -282,6 +282,17 @@ struct CheckedCall { Entity * callee; }; +struct CheckedAtomic { + Ast * call; + DeclInfo *decl; + i32 id; // BuiltinProcId +}; + +struct CheckedAddress { + Ast * node; // `&x`, or what is sliced or iterated by reference + Entity *location; // see `atomic_location` +}; + enum LinkNameUseKind : u8 { LinkNameUse_ForeignProcedure, @@ -846,6 +857,9 @@ struct CheckerInfo { PerThreadArray checked_bodies_queue; // for `check_escapes` PerThreadArray checked_calls_queue; // for `check_escapes` + PerThreadArray checked_atomics_queue; // for `check_atomics` + PerThreadArray checked_addresses_queue; // for `check_atomics`, what has its address taken, by `&` or otherwise + BlockingMutex instrumentation_mutex; Entity *instrumentation_enter_entity; Entity *instrumentation_exit_entity; @@ -952,6 +966,7 @@ gb_internal isize type_info_index (CheckerInfo *info, TypeInfoPair // Will return nullptr if not found gb_internal Entity *entity_of_node(Ast *expr); +gb_internal Entity *atomic_location(Ast *expr); // gb_internal Entity *scope_lookup_current(Scope *s, String const &name, u32 hash=0); diff --git a/src/main.cpp b/src/main.cpp index 6df6fda11..9483ced7f 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -431,6 +431,7 @@ enum BuildFlagKind { BuildFlag_NoRPath, BuildFlag_NoEntryPoint, BuildFlag_NoEscapeAnalysis, + BuildFlag_NoAtomicAnalysis, BuildFlag_Linker, BuildFlag_UseSeparateModules, BuildFlag_UseSingleModule, @@ -457,6 +458,7 @@ enum BuildFlagKind { BuildFlag_VetWhenShadowing, BuildFlag_VetNilDeref, BuildFlag_VetUninitialized, + BuildFlag_VetAtomicAccess, BuildFlag_VetPackages, BuildFlag_CustomAttribute, @@ -705,6 +707,7 @@ gb_internal bool parse_build_flags(Array args) { add_flag(&build_flags, BuildFlag_NoRPath, str_lit("no-rpath"), BuildFlagParam_None, Command__does_build); add_flag(&build_flags, BuildFlag_NoEntryPoint, str_lit("no-entry-point"), BuildFlagParam_None, Command__does_check &~ Command_test); add_flag(&build_flags, BuildFlag_NoEscapeAnalysis, str_lit("no-escape-analysis"), BuildFlagParam_None, Command__does_check); + add_flag(&build_flags, BuildFlag_NoAtomicAnalysis, str_lit("no-atomic-analysis"), BuildFlagParam_None, Command__does_check); add_flag(&build_flags, BuildFlag_Linker, str_lit("linker"), BuildFlagParam_String, Command__does_build); add_flag(&build_flags, BuildFlag_UseSeparateModules, str_lit("use-separate-modules"), BuildFlagParam_None, Command__does_build); add_flag(&build_flags, BuildFlag_UseSingleModule, str_lit("use-single-module"), BuildFlagParam_None, Command__does_build); @@ -731,6 +734,7 @@ gb_internal bool parse_build_flags(Array args) { add_flag(&build_flags, BuildFlag_VetWhenShadowing, str_lit("vet-when-shadowing"), BuildFlagParam_None, Command__does_check); add_flag(&build_flags, BuildFlag_VetNilDeref, str_lit("vet-nil-deref"), BuildFlagParam_None, Command__does_check); add_flag(&build_flags, BuildFlag_VetUninitialized, str_lit("vet-uninitialized"), BuildFlagParam_None, Command__does_check); + add_flag(&build_flags, BuildFlag_VetAtomicAccess, str_lit("vet-atomic-access"), BuildFlagParam_None, Command__does_check); add_flag(&build_flags, BuildFlag_VetPackages, str_lit("vet-packages"), BuildFlagParam_String, Command__does_check); add_flag(&build_flags, BuildFlag_CustomAttribute, str_lit("custom-attribute"), BuildFlagParam_String, Command__does_check, true); @@ -1431,6 +1435,9 @@ gb_internal bool parse_build_flags(Array args) { case BuildFlag_NoEscapeAnalysis: build_context.no_escape_analysis = true; break; + case BuildFlag_NoAtomicAnalysis: + build_context.no_atomic_analysis = true; + break; case BuildFlag_NoThreadLocal: build_context.no_thread_local = true; break; @@ -1510,6 +1517,7 @@ gb_internal bool parse_build_flags(Array args) { case BuildFlag_VetWhenShadowing: build_context.vet_flags |= VetFlag_WhenShadowing; break; case BuildFlag_VetNilDeref: build_context.vet_flags |= VetFlag_NilDeref; break; case BuildFlag_VetUninitialized: build_context.vet_flags |= VetFlag_Uninitialized; break; + case BuildFlag_VetAtomicAccess: build_context.vet_flags |= VetFlag_AtomicAccess; break; case BuildFlag_VetUnusedProcedures: build_context.vet_flags |= VetFlag_UnusedProcedures; break; case BuildFlag_VetPackages: @@ -2087,6 +2095,11 @@ gb_internal bool parse_build_flags(Array args) { bad_flags = true; } + if (set_flags[BuildFlag_NoAtomicAnalysis] && set_flags[BuildFlag_VetAtomicAccess]) { + gb_printf_err("-vet-atomic-access cannot be used with -no-atomic-analysis, as it is part of it\n"); + bad_flags = true; + } + if ((!(build_context.export_timings_format == TimingsExportUnspecified)) && (build_context.export_timings_file.len == 0)) { gb_printf_err("`-export-timings:` requires `-export-timings-file:` to be specified as well\n"); bad_flags = true; @@ -3268,6 +3281,12 @@ gb_internal int print_show_help(String const arg0, String command, String option } if (check) { + if (print_flag("-no-atomic-analysis")) { + print_usage_line(2, "Disables the analysis of atomic memory orderings, except in files with '#+atomic-analysis'."); + print_usage_line(2, "It warns where what is written with release ordering is only loaded with relaxed ordering, or the reverse."); + print_usage_line(2, "Cannot be used with -vet-atomic-access."); + } + if (print_flag("-no-escape-analysis")) { print_usage_line(2, "Disables the escape analysis of stack memory, except in files with '#+escape-analysis'."); print_usage_line(2, "Where it is disabled, by this or by '#+no-escape-analysis', only returning the address of a local or similar is an error."); @@ -3539,6 +3558,10 @@ gb_internal int print_show_help(String const arg0, String command, String option print_usage_line(3, "-vet-when-shadowing"); } + if (print_flag("-vet-atomic-access")) { + print_usage_line(2, "Errs on a plain read of a variable or field which is accessed atomically elsewhere, except in procedures which take a lock."); + } + if (print_flag("-vet-cast")) { print_usage_line(2, "Errs on casting a value to its own type or using `transmute` rather than `cast`."); } diff --git a/src/parser.cpp b/src/parser.cpp index aee72de4f..50f9b7569 100644 --- a/src/parser.cpp +++ b/src/parser.cpp @@ -91,26 +91,40 @@ gb_internal u64 ast_file_vet_flags(AstFile *f) { return 0; } -// whether any file has `#+escape-analysis`, so that it runs for them when `-no-escape-analysis` disables it otherwise +// whether any file has `#+escape-analysis` or `#+atomic-analysis`, so that it runs for them when disabled otherwise gb_global std::atomic global_escape_analysis_tagged; +gb_global std::atomic global_atomic_analysis_tagged; -gb_internal bool ast_file_escape_analysis(AstFile *f) { +// whether a file has an analysis, which its `#+` or `#+no-` tag decides over the command line +gb_internal bool ast_file_has_analysis(AstFile *f, u32 on_flag, u32 off_flag, bool disabled) { if (f == nullptr) { - return !build_context.no_escape_analysis; + return !disabled; } - if (f->flags & AstFile_EscapeAnalysis) { + if (f->flags & on_flag) { return true; } - if (f->flags & AstFile_NoEscapeAnalysis) { + if (f->flags & off_flag) { return false; } - return !build_context.no_escape_analysis; + return !disabled; +} + +gb_internal bool ast_file_escape_analysis(AstFile *f) { + return ast_file_has_analysis(f, AstFile_EscapeAnalysis, AstFile_NoEscapeAnalysis, build_context.no_escape_analysis); +} + +gb_internal bool ast_file_atomic_analysis(AstFile *f) { + return ast_file_has_analysis(f, AstFile_AtomicAnalysis, AstFile_NoAtomicAnalysis, build_context.no_atomic_analysis); } gb_internal bool escape_analysis_in_use(void) { return !build_context.no_escape_analysis || global_escape_analysis_tagged.load(std::memory_order_relaxed); } +gb_internal bool atomic_analysis_in_use(void) { + return !build_context.no_atomic_analysis || global_atomic_analysis_tagged.load(std::memory_order_relaxed); +} + gb_internal bool ast_file_vet_style(AstFile *f) { return (ast_file_vet_flags(f) & VetFlag_Style) != 0; } @@ -7283,6 +7297,7 @@ gb_internal u64 parse_vet_tag(Token token_for_pos, String s, u64 base_vet_flags) error_line("\twhen-shadowing\n"); error_line("\tnil-deref\n"); error_line("\tuninitialized\n"); + error_line("\tatomic-access\n"); return vet_flags; } } @@ -7511,6 +7526,13 @@ gb_internal bool parse_file_tag(const String &lc, const Token &tok, AstFile *f) } else if (lc == "no-escape-analysis") { f->flags |= AstFile_NoEscapeAnalysis; f->flags &= ~AstFile_EscapeAnalysis; + } else if (lc == "atomic-analysis") { + f->flags |= AstFile_AtomicAnalysis; + f->flags &= ~AstFile_NoAtomicAnalysis; + global_atomic_analysis_tagged.store(true, std::memory_order_relaxed); + } else if (lc == "no-atomic-analysis") { + f->flags |= AstFile_NoAtomicAnalysis; + f->flags &= ~AstFile_AtomicAnalysis; } else { syntax_error(tok, "Unknown tag '%.*s'", LIT(lc)); } diff --git a/src/parser.hpp b/src/parser.hpp index 8d4d9ceb1..1070ffe85 100644 --- a/src/parser.hpp +++ b/src/parser.hpp @@ -99,6 +99,8 @@ enum AstFileFlag : u32 { AstFile_EscapeAnalysis = 1<<6, // `#+escape-analysis` AstFile_NoEscapeAnalysis = 1<<7, // `#+no-escape-analysis` + AstFile_AtomicAnalysis = 1<<8, // `#+atomic-analysis` + AstFile_NoAtomicAnalysis = 1<<9, // `#+no-atomic-analysis` }; enum AstDelayQueueKind {