From 21b1173076cec12f97c5779556509ef1b908c644 Mon Sep 17 00:00:00 2001 From: gingerBill Date: Wed, 10 Sep 2025 18:20:20 +0100 Subject: Minor clean up of permanent/temporary arena usage --- src/check_stmt.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/check_stmt.cpp') diff --git a/src/check_stmt.cpp b/src/check_stmt.cpp index ae88ff333..e03d4a7ae 100644 --- a/src/check_stmt.cpp +++ b/src/check_stmt.cpp @@ -1963,7 +1963,7 @@ gb_internal void check_range_stmt(CheckerContext *ctx, Ast *node, u32 mod_flags) } auto rhs = slice_from_array(vals); - auto lhs = slice_make(temporary_allocator(), rhs.count); + auto lhs = temporary_slice_make(rhs.count); slice_copy(&lhs, rs->vals); isize addressable_index = cast(isize)is_map; -- cgit v1.2.3 From a36a8722dc823c6fe143f7935e79467c6569bc00 Mon Sep 17 00:00:00 2001 From: gingerBill Date: Wed, 10 Sep 2025 19:30:32 +0100 Subject: Minimize more thread contention --- src/build_settings.cpp | 2 +- src/check_decl.cpp | 4 ++-- src/check_expr.cpp | 16 ++++++-------- src/check_stmt.cpp | 5 +++-- src/check_type.cpp | 9 ++++---- src/checker.cpp | 58 +++++++++++++++++++++++++++----------------------- src/checker.hpp | 15 +++++++------ src/entity.cpp | 2 +- src/error.cpp | 11 ++++++++++ src/threading.cpp | 10 ++++----- 10 files changed, 74 insertions(+), 58 deletions(-) (limited to 'src/check_stmt.cpp') diff --git a/src/build_settings.cpp b/src/build_settings.cpp index 0081fabee..0c88f3d13 100644 --- a/src/build_settings.cpp +++ b/src/build_settings.cpp @@ -1163,7 +1163,7 @@ gb_internal String internal_odin_root_dir(void) { return global_module_path; } - auto path_buf = array_make(heap_allocator(), 300); + auto path_buf = array_make(temporary_allocator(), 300); defer (array_free(&path_buf)); len = 0; diff --git a/src/check_decl.cpp b/src/check_decl.cpp index 7dd9db105..49731ad60 100644 --- a/src/check_decl.cpp +++ b/src/check_decl.cpp @@ -1981,9 +1981,9 @@ gb_internal bool check_proc_body(CheckerContext *ctx_, Token token, DeclInfo *de ast_node(bs, BlockStmt, body); + TEMPORARY_ALLOCATOR_GUARD(); Array using_entities = {}; - using_entities.allocator = heap_allocator(); - defer (array_free(&using_entities)); + using_entities.allocator = temporary_allocator(); { if (type->Proc.param_count > 0) { diff --git a/src/check_expr.cpp b/src/check_expr.cpp index 84f1c6f0a..bdbccb4f8 100644 --- a/src/check_expr.cpp +++ b/src/check_expr.cpp @@ -7516,11 +7516,10 @@ gb_internal CallArgumentData check_call_arguments(CheckerContext *c, Operand *op return check_call_arguments_proc_group(c, operand, call); } - auto positional_operands = array_make(heap_allocator(), 0, positional_args.count); - auto named_operands = array_make(heap_allocator(), 0, 0); + TEMPORARY_ALLOCATOR_GUARD(); - defer (array_free(&positional_operands)); - defer (array_free(&named_operands)); + auto positional_operands = array_make(temporary_allocator(), 0, positional_args.count); + auto named_operands = array_make(temporary_allocator(), 0, 0); if (positional_args.count > 0) { Entity **lhs = nullptr; @@ -7623,11 +7622,10 @@ gb_internal CallArgumentError check_polymorphic_record_type(CheckerContext *c, O { // NOTE(bill, 2019-10-26): Allow a cycle in the parameters but not in the fields themselves auto prev_type_path = c->type_path; - c->type_path = new_checker_type_path(); - defer ({ - destroy_checker_type_path(c->type_path); - c->type_path = prev_type_path; - }); + TEMPORARY_ALLOCATOR_GUARD(); + + c->type_path = new_checker_type_path(temporary_allocator()); + defer (c->type_path = prev_type_path); if (is_call_expr_field_value(ce)) { named_fields = true; diff --git a/src/check_stmt.cpp b/src/check_stmt.cpp index e03d4a7ae..ba9c08fed 100644 --- a/src/check_stmt.cpp +++ b/src/check_stmt.cpp @@ -2567,8 +2567,9 @@ gb_internal void check_return_stmt(CheckerContext *ctx, Ast *node) { result_count = proc_type->Proc.results->Tuple.variables.count; } - auto operands = array_make(heap_allocator(), 0, 2*rs->results.count); - defer (array_free(&operands)); + TEMPORARY_ALLOCATOR_GUARD(); + + auto operands = array_make(temporary_allocator(), 0, 2*rs->results.count); check_unpack_arguments(ctx, result_entities, result_count, &operands, rs->results, UnpackFlag_AllowOk); diff --git a/src/check_type.cpp b/src/check_type.cpp index 4c995588f..e99909d6b 100644 --- a/src/check_type.cpp +++ b/src/check_type.cpp @@ -3512,8 +3512,9 @@ gb_internal bool check_type_internal(CheckerContext *ctx, Ast *e, Type **type, T case_ast_node(pt, PointerType, e); CheckerContext c = *ctx; - c.type_path = new_checker_type_path(); - defer (destroy_checker_type_path(c.type_path)); + + TEMPORARY_ALLOCATOR_GUARD(); + c.type_path = new_checker_type_path(temporary_allocator()); Type *elem = t_invalid; Operand o = {}; @@ -3747,8 +3748,8 @@ gb_internal bool check_type_internal(CheckerContext *ctx, Ast *e, Type **type, T gb_internal Type *check_type(CheckerContext *ctx, Ast *e) { CheckerContext c = *ctx; - c.type_path = new_checker_type_path(); - defer (destroy_checker_type_path(c.type_path)); + TEMPORARY_ALLOCATOR_GUARD(); + c.type_path = new_checker_type_path(temporary_allocator()); return check_type_expr(&c, e, nullptr); } diff --git a/src/checker.cpp b/src/checker.cpp index 04e46d0e6..c1d6302ad 100644 --- a/src/checker.cpp +++ b/src/checker.cpp @@ -1441,7 +1441,8 @@ gb_internal void init_checker_info(CheckerInfo *i) { map_init(&i->objc_method_implementations); string_map_init(&i->load_file_cache); - array_init(&i->all_procedures, heap_allocator()); + array_init(&i->all_procedures, a); + mpsc_init(&i->all_procedures_queue, a); mpsc_init(&i->entity_queue, a); // 1<<20); mpsc_init(&i->definition_queue, a); //); // 1<<20); @@ -1475,6 +1476,10 @@ gb_internal void destroy_checker_info(CheckerInfo *i) { array_free(&i->required_foreign_imports_through_force); array_free(&i->defineables); + array_free(&i->all_procedures); + + mpsc_destroy(&i->all_procedures_queue); + mpsc_destroy(&i->entity_queue); mpsc_destroy(&i->definition_queue); mpsc_destroy(&i->required_global_variable_queue); @@ -1502,12 +1507,12 @@ gb_internal CheckerContext make_checker_context(Checker *c) { ctx.scope = builtin_pkg->scope; ctx.pkg = builtin_pkg; - ctx.type_path = new_checker_type_path(); + ctx.type_path = new_checker_type_path(heap_allocator()); ctx.type_level = 0; return ctx; } gb_internal void destroy_checker_context(CheckerContext *ctx) { - destroy_checker_type_path(ctx->type_path); + destroy_checker_type_path(ctx->type_path, heap_allocator()); } gb_internal bool add_curr_ast_file(CheckerContext *ctx, AstFile *file) { @@ -1758,7 +1763,7 @@ gb_internal void add_untyped(CheckerContext *c, Ast *expr, AddressingMode mode, check_set_expr_info(c, expr, mode, type, value); } -gb_internal void add_type_and_value(CheckerContext *ctx, Ast *expr, AddressingMode mode, Type *type, ExactValue const &value) { +gb_internal void add_type_and_value(CheckerContext *ctx, Ast *expr, AddressingMode mode, Type *type, ExactValue const &value, bool use_mutex) { if (expr == nullptr) { return; } @@ -1776,7 +1781,7 @@ gb_internal void add_type_and_value(CheckerContext *ctx, Ast *expr, AddressingMo mutex = &ctx->pkg->type_and_value_mutex; } - mutex_lock(mutex); + if (use_mutex) mutex_lock(mutex); Ast *prev_expr = nullptr; while (prev_expr != expr) { prev_expr = expr; @@ -1801,7 +1806,7 @@ gb_internal void add_type_and_value(CheckerContext *ctx, Ast *expr, AddressingMo break; }; } - mutex_unlock(mutex); + if (use_mutex) mutex_unlock(mutex); } gb_internal void add_entity_definition(CheckerInfo *i, Ast *identifier, Entity *entity) { @@ -2345,11 +2350,9 @@ gb_internal void check_procedure_later(Checker *c, ProcInfo *info) { } if (DEBUG_CHECK_ALL_PROCEDURES) { - MUTEX_GUARD_BLOCK(&c->info.all_procedures_mutex) { - GB_ASSERT(info != nullptr); - GB_ASSERT(info->decl != nullptr); - array_add(&c->info.all_procedures, info); - } + GB_ASSERT(info != nullptr); + GB_ASSERT(info->decl != nullptr); + mpsc_enqueue(&c->info.all_procedures_queue, info); } } @@ -3195,19 +3198,17 @@ gb_internal Type *find_type_in_pkg(CheckerInfo *info, String const &pkg, String return e->type; } -gb_internal CheckerTypePath *new_checker_type_path() { - gbAllocator a = heap_allocator(); - auto *tp = gb_alloc_item(a, CheckerTypePath); - array_init(tp, a, 0, 16); +gb_internal CheckerTypePath *new_checker_type_path(gbAllocator allocator) { + auto *tp = gb_alloc_item(allocator, CheckerTypePath); + array_init(tp, allocator, 0, 16); return tp; } -gb_internal void destroy_checker_type_path(CheckerTypePath *tp) { +gb_internal void destroy_checker_type_path(CheckerTypePath *tp, gbAllocator allocator) { array_free(tp); - gb_free(heap_allocator(), tp); + gb_free(allocator, tp); } - gb_internal void check_type_path_push(CheckerContext *c, Entity *e) { GB_ASSERT(c->type_path != nullptr); GB_ASSERT(e != nullptr); @@ -5283,9 +5284,10 @@ gb_internal void check_add_import_decl(CheckerContext *ctx, Ast *decl) { GB_ASSERT(scope->flags&ScopeFlag_Pkg); - if (ptr_set_update(&parent_scope->imported, scope)) { - // error(token, "Multiple import of the same file within this scope"); - } + ptr_set_add(&parent_scope->imported, scope); + // if (ptr_set_update(&parent_scope->imported, scope)) { + // // error(token, "Multiple import of the same file within this scope"); + // } String import_name = path_to_entity_name(id->import_name.string, id->fullpath, false); if (is_blank_ident(import_name)) { @@ -6041,10 +6043,10 @@ gb_internal void calculate_global_init_order(Checker *c) { CheckerInfo *info = &c->info; TIME_SECTION("calculate_global_init_order: generate entity dependency graph"); - Arena *arena = get_arena(ThreadArena_Temporary); - ArenaTempGuard arena_guard(arena); + Arena *temporary_arena = get_arena(ThreadArena_Temporary); + ArenaTempGuard temporary_arena_guard(temporary_arena); - Array dep_graph = generate_entity_dependency_graph(info, arena); + Array dep_graph = generate_entity_dependency_graph(info, temporary_arena); TIME_SECTION("calculate_global_init_order: priority queue create"); // NOTE(bill): Priority queue @@ -6321,8 +6323,9 @@ gb_internal void check_safety_all_procedures_for_unchecked(Checker *c) { defer (map_destroy(&untyped)); - for_array(i, c->info.all_procedures) { - ProcInfo *pi = c->info.all_procedures[i]; + array_reserve(&c->info.all_procedures, c->info.all_procedures_queue.count.load()); + + for (ProcInfo *pi = nullptr; mpsc_dequeue(&c->info.all_procedures_queue, &pi); /**/) { GB_ASSERT(pi != nullptr); GB_ASSERT(pi->decl != nullptr); Entity *e = pi->decl->entity; @@ -6337,6 +6340,8 @@ gb_internal void check_safety_all_procedures_for_unchecked(Checker *c) { consume_proc_info(c, pi, &untyped); } } + + array_add(&c->info.all_procedures, pi); } } @@ -7412,7 +7417,6 @@ gb_internal void check_parsed_files(Checker *c) { TIME_SECTION("add untyped expression values"); - // Add untyped expression values for (UntypedExprInfo u = {}; mpsc_dequeue(&c->global_untyped_queue, &u); /**/) { GB_ASSERT(u.expr != nullptr && u.info != nullptr); if (is_type_typed(u.info->type)) { diff --git a/src/checker.hpp b/src/checker.hpp index 8b4d61ee2..5a40b10a0 100644 --- a/src/checker.hpp +++ b/src/checker.hpp @@ -309,11 +309,12 @@ struct EntityGraphNode; typedef PtrSet EntityGraphNodeSet; struct EntityGraphNode { - Entity * entity; // Procedure, Variable, Constant + Entity *entity; // Procedure, Variable, Constant + EntityGraphNodeSet pred; EntityGraphNodeSet succ; - isize index; // Index in array/queue - isize dep_count; + isize index; // Index in array/queue + isize dep_count; }; @@ -516,7 +517,7 @@ struct CheckerInfo { BlockingMutex load_file_mutex; StringMap load_file_cache; - BlockingMutex all_procedures_mutex; + MPSCQueue all_procedures_queue; Array all_procedures; BlockingMutex instrumentation_mutex; @@ -629,7 +630,7 @@ gb_internal void scope_lookup_parent (Scope *s, String const &name, Scope **s gb_internal Entity *scope_insert (Scope *s, Entity *entity); -gb_internal void add_type_and_value (CheckerContext *c, Ast *expression, AddressingMode mode, Type *type, ExactValue const &value); +gb_internal void add_type_and_value (CheckerContext *c, Ast *expression, AddressingMode mode, Type *type, ExactValue const &value, bool use_mutex=true); gb_internal ExprInfo *check_get_expr_info (CheckerContext *c, Ast *expr); gb_internal void add_untyped (CheckerContext *c, Ast *expression, AddressingMode mode, Type *basic_type, ExactValue const &value); gb_internal void add_entity_use (CheckerContext *c, Ast *identifier, Entity *entity); @@ -650,8 +651,8 @@ gb_internal void check_collect_entities(CheckerContext *c, Slice const &n gb_internal void check_collect_entities_from_when_stmt(CheckerContext *c, AstWhenStmt *ws); gb_internal void check_delayed_file_import_entity(CheckerContext *c, Ast *decl); -gb_internal CheckerTypePath *new_checker_type_path(); -gb_internal void destroy_checker_type_path(CheckerTypePath *tp); +gb_internal CheckerTypePath *new_checker_type_path(gbAllocator allocator); +gb_internal void destroy_checker_type_path(CheckerTypePath *tp, gbAllocator allocator); gb_internal void check_type_path_push(CheckerContext *c, Entity *e); gb_internal Entity *check_type_path_pop (CheckerContext *c); diff --git a/src/entity.cpp b/src/entity.cpp index 5ca3fa916..e07e882f3 100644 --- a/src/entity.cpp +++ b/src/entity.cpp @@ -348,7 +348,7 @@ gb_internal Entity *alloc_entity(EntityKind kind, Scope *scope, Token token, Typ entity->type = type; entity->id = 1 + global_entity_id.fetch_add(1); if (token.pos.file_id) { - entity->file = thread_safe_get_ast_file_from_id(token.pos.file_id); + entity->file = thread_unsafe_get_ast_file_from_id(token.pos.file_id); } return entity; } diff --git a/src/error.cpp b/src/error.cpp index 10bf1caf5..53bc01654 100644 --- a/src/error.cpp +++ b/src/error.cpp @@ -153,6 +153,17 @@ gb_internal AstFile *thread_safe_get_ast_file_from_id(i32 index) { } +// use AFTER PARSER +gb_internal AstFile *thread_unsafe_get_ast_file_from_id(i32 index) { + GB_ASSERT(index >= 0); + AstFile *file = nullptr; + if (index < global_files.count) { + file = global_files[index]; + } + return file; +} + + // NOTE: defined in build_settings.cpp gb_internal bool global_warnings_as_errors(void); diff --git a/src/threading.cpp b/src/threading.cpp index f1d9264e3..a35176ce6 100644 --- a/src/threading.cpp +++ b/src/threading.cpp @@ -448,9 +448,9 @@ gb_internal void semaphore_wait(Semaphore *s) { } #endif -static const int RWLOCK_WRITER = 1; -static const int RWLOCK_UPGRADED = 2; -static const int RWLOCK_READER = 4; +static const int RWLOCK_WRITER = 1<<0; +static const int RWLOCK_UPGRADED = 1<<1; +static const int RWLOCK_READER = 1<<2; struct RWSpinLock { Futex bits; }; @@ -467,7 +467,7 @@ bool rwlock_try_acquire_upgrade(RWSpinLock *l) { void rwlock_acquire_upgrade(RWSpinLock *l) { while (!rwlock_try_acquire_upgrade(l)) { - futex_wait(&l->bits, RWLOCK_UPGRADED); + futex_wait(&l->bits, RWLOCK_UPGRADED | RWLOCK_WRITER); } } void rwlock_release_upgrade(RWSpinLock *l) { @@ -481,7 +481,7 @@ bool rwlock_try_release_upgrade_and_acquire_write(RWSpinLock *l) { void rwlock_release_upgrade_and_acquire_write(RWSpinLock *l) { while (!rwlock_try_release_upgrade_and_acquire_write(l)) { - futex_wait(&l->bits, RWLOCK_WRITER); + futex_wait(&l->bits, RWLOCK_UPGRADED); } } -- cgit v1.2.3 From 5ea2e1fe60872c5d2b20e180a1514a082b6513e4 Mon Sep 17 00:00:00 2001 From: gingerBill Date: Wed, 10 Sep 2025 21:41:52 +0100 Subject: Minimize mutex usage when in single threaded mode. --- src/check_decl.cpp | 2 +- src/check_expr.cpp | 10 +++++----- src/check_stmt.cpp | 12 +++++++----- src/checker.cpp | 39 +++++++++++++++++++++++++-------------- src/checker.hpp | 4 ++-- src/parser.cpp | 1 + src/parser.hpp | 1 + 7 files changed, 42 insertions(+), 27 deletions(-) (limited to 'src/check_stmt.cpp') diff --git a/src/check_decl.cpp b/src/check_decl.cpp index 58e4f0120..8fbcb5e40 100644 --- a/src/check_decl.cpp +++ b/src/check_decl.cpp @@ -948,7 +948,7 @@ gb_internal Entity *init_entity_foreign_library(CheckerContext *ctx, Entity *e) error(ident, "foreign library names must be an identifier"); } else { String name = ident->Ident.token.string; - Entity *found = scope_lookup(ctx->scope, name); + Entity *found = scope_lookup(ctx->scope, name, ident->Ident.hash); if (found == nullptr) { if (is_blank_ident(name)) { diff --git a/src/check_expr.cpp b/src/check_expr.cpp index abfd11485..d2505c047 100644 --- a/src/check_expr.cpp +++ b/src/check_expr.cpp @@ -1738,7 +1738,7 @@ gb_internal Entity *check_ident(CheckerContext *c, Operand *o, Ast *n, Type *nam o->expr = n; String name = n->Ident.token.string; - Entity *e = scope_lookup(c->scope, name); + Entity *e = scope_lookup(c->scope, name, n->Ident.hash); if (e == nullptr) { if (is_blank_ident(name)) { error(n, "'_' cannot be used as a value"); @@ -5361,7 +5361,7 @@ gb_internal Entity *check_entity_from_ident_or_selector(CheckerContext *c, Ast * } } else */if (node->kind == Ast_Ident) { String name = node->Ident.token.string; - return scope_lookup(c->scope, name); + return scope_lookup(c->scope, name, node->Ident.hash); } else if (!ident_only) if (node->kind == Ast_SelectorExpr) { ast_node(se, SelectorExpr, node); if (se->token.kind == Token_ArrowRight) { @@ -5383,7 +5383,7 @@ gb_internal Entity *check_entity_from_ident_or_selector(CheckerContext *c, Ast * if (op_expr->kind == Ast_Ident) { String op_name = op_expr->Ident.token.string; - Entity *e = scope_lookup(c->scope, op_name); + Entity *e = scope_lookup(c->scope, op_name, op_expr->Ident.hash); if (e == nullptr) { return nullptr; } @@ -5480,7 +5480,7 @@ gb_internal Entity *check_selector(CheckerContext *c, Operand *operand, Ast *nod if (op_expr->kind == Ast_Ident) { String op_name = op_expr->Ident.token.string; - Entity *e = scope_lookup(c->scope, op_name); + Entity *e = scope_lookup(c->scope, op_name, op_expr->Ident.hash); add_entity_use(c, op_expr, e); expr_entity = e; @@ -5903,7 +5903,7 @@ gb_internal bool check_identifier_exists(Scope *s, Ast *node, bool nested = fals return true; } } else { - Entity *e = scope_lookup(s, name); + Entity *e = scope_lookup(s, name, i->hash); if (e != nullptr) { if (out_scope) *out_scope = e->scope; return true; diff --git a/src/check_stmt.cpp b/src/check_stmt.cpp index ba9c08fed..62cfc256a 100644 --- a/src/check_stmt.cpp +++ b/src/check_stmt.cpp @@ -484,7 +484,7 @@ gb_internal Type *check_assignment_variable(CheckerContext *ctx, Operand *lhs, O } if (ident_node != nullptr) { ast_node(i, Ident, ident_node); - e = scope_lookup(ctx->scope, i->token.string); + e = scope_lookup(ctx->scope, i->token.string, i->hash); if (e != nullptr && e->kind == Entity_Variable) { used = (e->flags & EntityFlag_Used) != 0; // NOTE(bill): Make backup just in case } @@ -1812,8 +1812,9 @@ gb_internal void check_range_stmt(CheckerContext *ctx, Ast *node, u32 mod_flags) error(node, "Iteration over a bit_set of an enum is not allowed runtime type information (RTTI) has been disallowed"); } if (rs->vals.count == 1 && rs->vals[0] && rs->vals[0]->kind == Ast_Ident) { - String name = rs->vals[0]->Ident.token.string; - Entity *found = scope_lookup(ctx->scope, name); + AstIdent *ident = &rs->vals[0]->Ident; + String name = ident->token.string; + Entity *found = scope_lookup(ctx->scope, name, ident->hash); if (found && are_types_identical(found->type, t->BitSet.elem)) { ERROR_BLOCK(); gbString s = expr_to_string(expr); @@ -1857,8 +1858,9 @@ gb_internal void check_range_stmt(CheckerContext *ctx, Ast *node, u32 mod_flags) error(node, "#reverse for is not supported for map types, as maps are unordered"); } if (rs->vals.count == 1 && rs->vals[0] && rs->vals[0]->kind == Ast_Ident) { - String name = rs->vals[0]->Ident.token.string; - Entity *found = scope_lookup(ctx->scope, name); + AstIdent *ident = &rs->vals[0]->Ident; + String name = ident->token.string; + Entity *found = scope_lookup(ctx->scope, name, ident->hash); if (found && are_types_identical(found->type, t->Map.key)) { ERROR_BLOCK(); gbString s = expr_to_string(expr); diff --git a/src/checker.cpp b/src/checker.cpp index eb334e147..7da3948dc 100644 --- a/src/checker.cpp +++ b/src/checker.cpp @@ -380,16 +380,25 @@ gb_internal Entity *scope_lookup_current(Scope *s, String const &name) { } -gb_internal void scope_lookup_parent(Scope *scope, String const &name, Scope **scope_, Entity **entity_) { +gb_global std::atomic in_single_threaded_checker_stage; + +gb_internal void scope_lookup_parent(Scope *scope, String const &name, Scope **scope_, Entity **entity_, u32 hash) { + bool is_single_threaded = in_single_threaded_checker_stage.load(std::memory_order_relaxed); if (scope != nullptr) { bool gone_thru_proc = false; bool gone_thru_package = false; - StringHashKey key = string_hash_string(name); + StringHashKey key = {}; + if (hash) { + key.hash = hash; + key.string = name; + } else { + key = string_hash_string(name); + } for (Scope *s = scope; s != nullptr; s = s->parent) { Entity **found = nullptr; - rw_mutex_shared_lock(&s->mutex); + if (!is_single_threaded) rw_mutex_shared_lock(&s->mutex); found = string_map_get(&s->elements, key); - rw_mutex_shared_unlock(&s->mutex); + if (!is_single_threaded) rw_mutex_shared_unlock(&s->mutex); if (found) { Entity *e = *found; if (gone_thru_proc) { @@ -424,9 +433,9 @@ gb_internal void scope_lookup_parent(Scope *scope, String const &name, Scope **s if (scope_) *scope_ = nullptr; } -gb_internal Entity *scope_lookup(Scope *s, String const &name) { +gb_internal Entity *scope_lookup(Scope *s, String const &name, u32 hash) { Entity *entity = nullptr; - scope_lookup_parent(s, name, nullptr, &entity); + scope_lookup_parent(s, name, nullptr, &entity, hash); return entity; } @@ -507,11 +516,9 @@ end:; return result; } -gb_global bool in_single_threaded_checker_stage = false; - gb_internal Entity *scope_insert(Scope *s, Entity *entity) { String name = entity->token.string; - if (in_single_threaded_checker_stage) { + if (in_single_threaded_checker_stage.load(std::memory_order_relaxed)) { return scope_insert_with_name_no_mutex(s, name, entity); } else { return scope_insert_with_name(s, name, entity); @@ -853,9 +860,13 @@ gb_internal void check_scope_usage(Checker *c, Scope *scope, u64 vet_flags) { gb_internal void add_dependency(CheckerInfo *info, DeclInfo *d, Entity *e) { - rw_mutex_lock(&d->deps_mutex); - ptr_set_add(&d->deps, e); - rw_mutex_unlock(&d->deps_mutex); + if (in_single_threaded_checker_stage.load(std::memory_order_relaxed)) { + ptr_set_add(&d->deps, e); + } else { + rw_mutex_lock(&d->deps_mutex); + ptr_set_add(&d->deps, e); + rw_mutex_unlock(&d->deps_mutex); + } } gb_internal void add_type_info_dependency(CheckerInfo *info, DeclInfo *d, Type *type) { if (d == nullptr || type == nullptr) { @@ -4958,7 +4969,7 @@ gb_internal void check_single_global_entity(Checker *c, Entity *e, DeclInfo *d) } gb_internal void check_all_global_entities(Checker *c) { - in_single_threaded_checker_stage = true; + in_single_threaded_checker_stage.store(true, std::memory_order_relaxed); // NOTE(bill): This must be single threaded // Don't bother trying @@ -4980,7 +4991,7 @@ gb_internal void check_all_global_entities(Checker *c) { } } - in_single_threaded_checker_stage = false; + in_single_threaded_checker_stage.store(false, std::memory_order_relaxed); } diff --git a/src/checker.hpp b/src/checker.hpp index 968988962..68779c280 100644 --- a/src/checker.hpp +++ b/src/checker.hpp @@ -623,8 +623,8 @@ gb_internal Entity *entity_of_node(Ast *expr); gb_internal Entity *scope_lookup_current(Scope *s, String const &name); -gb_internal Entity *scope_lookup (Scope *s, String const &name); -gb_internal void scope_lookup_parent (Scope *s, String const &name, Scope **scope_, Entity **entity_); +gb_internal Entity *scope_lookup (Scope *s, String const &name, u32 hash=0); +gb_internal void scope_lookup_parent (Scope *s, String const &name, Scope **scope_, Entity **entity_, u32 hash=0); gb_internal Entity *scope_insert (Scope *s, Entity *entity); diff --git a/src/parser.cpp b/src/parser.cpp index a7006dd39..a32494c05 100644 --- a/src/parser.cpp +++ b/src/parser.cpp @@ -750,6 +750,7 @@ gb_internal Ast *ast_matrix_index_expr(AstFile *f, Ast *expr, Token open, Token gb_internal Ast *ast_ident(AstFile *f, Token token) { Ast *result = alloc_ast_node(f, Ast_Ident); result->Ident.token = token; + result->Ident.hash = string_hash(token.string); return result; } diff --git a/src/parser.hpp b/src/parser.hpp index d2dd22667..56447df43 100644 --- a/src/parser.hpp +++ b/src/parser.hpp @@ -396,6 +396,7 @@ struct AstSplitArgs { AST_KIND(Ident, "identifier", struct { \ Token token; \ Entity *entity; \ + u32 hash; \ }) \ AST_KIND(Implicit, "implicit", Token) \ AST_KIND(Uninit, "uninitialized value", Token) \ -- cgit v1.2.3 From 4165e8e8888f4894c039c0a55cf67c24fae83af0 Mon Sep 17 00:00:00 2001 From: Jwaxy <98110672+jwaxy@users.noreply.github.com> Date: Sat, 27 Sep 2025 13:53:04 +0300 Subject: Prevent returning struct containing compound literal slice --- src/check_stmt.cpp | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'src/check_stmt.cpp') diff --git a/src/check_stmt.cpp b/src/check_stmt.cpp index 62cfc256a..f509267fb 100644 --- a/src/check_stmt.cpp +++ b/src/check_stmt.cpp @@ -2671,6 +2671,20 @@ gb_internal void check_return_stmt(CheckerContext *ctx, Ast *node) { ERROR_BLOCK(); unsafe_return_error(o, "a compound literal of a slice"); error_line("\tNote: A constant slice value will use the memory of the current stack frame\n"); + } else if (expr->kind == Ast_CompoundLit && is_type_struct(o.type)) { + ast_node(cl, CompoundLit, expr); + for (Ast *elem : cl->elems) { + if (elem->kind == Ast_FieldValue) { + ast_node(fv, FieldValue, elem); + if (fv->value->kind == Ast_CompoundLit && is_type_slice(entity_of_node(fv->field)->type)) { + ast_node(sl, CompoundLit, fv->value); + if (sl->elems.count == 0) { + continue; + } + unsafe_return_error(o, "a compound literal containing a slice"); + } + } + } } } -- cgit v1.2.3 From 15fe0bfe599cd4300d3651b8e74b171cb50599f0 Mon Sep 17 00:00:00 2001 From: Jwaxy <98110672+jwaxy@users.noreply.github.com> Date: Sat, 27 Sep 2025 14:45:46 +0300 Subject: Make return struct with slice check recursive --- src/check_stmt.cpp | 135 ++++++++++++++++++++++++++--------------------------- 1 file changed, 66 insertions(+), 69 deletions(-) (limited to 'src/check_stmt.cpp') diff --git a/src/check_stmt.cpp b/src/check_stmt.cpp index f509267fb..f6e201011 100644 --- a/src/check_stmt.cpp +++ b/src/check_stmt.cpp @@ -2541,6 +2541,71 @@ gb_internal void check_if_stmt(CheckerContext *ctx, Ast *node, u32 mod_flags) { check_close_scope(ctx); } +// NOTE(bill): This is very basic escape analysis +// This needs to be improved tremendously, and a lot of it done during the +// middle-end (or LLVM side) to improve checks and error messages +void check_unsafe_return(Operand const &o, Type *type, Ast *expr) { + auto unsafe_return_error = [](Operand const &o, char const *msg, Type *extra_type=nullptr) { + gbString s = expr_to_string(o.expr); + if (extra_type) { + gbString t = type_to_string(extra_type); + error(o.expr, "It is unsafe to return %s ('%s') of type ('%s') from a procedure, as it uses the current stack frame's memory", msg, s, t); + gb_string_free(t); + } else { + error(o.expr, "It is unsafe to return %s ('%s') from a procedure, as it uses the current stack frame's memory", msg, s); + } + gb_string_free(s); + }; + + if (expr->kind == Ast_CompoundLit && is_type_slice(type)) { + ast_node(cl, CompoundLit, expr); + if (cl->elems.count == 0) { + return; + } + unsafe_return_error(o, "a compound literal of a slice"); + } else if (expr->kind == Ast_UnaryExpr && expr->UnaryExpr.op.kind == Token_And) { + Ast *x = unparen_expr(expr->UnaryExpr.expr); + Entity *e = entity_of_node(x); + if (is_entity_local_variable(e)) { + unsafe_return_error(o, "the address of a local variable"); + } else if (x->kind == Ast_CompoundLit) { + unsafe_return_error(o, "the address of a compound literal"); + } else if (x->kind == Ast_IndexExpr) { + Entity *f = entity_of_node(x->IndexExpr.expr); + if (f && (is_type_array_like(f->type) || is_type_matrix(f->type))) { + if (is_entity_local_variable(f)) { + unsafe_return_error(o, "the address of an indexed variable", f->type); + } + } + } else if (x->kind == Ast_MatrixIndexExpr) { + Entity *f = entity_of_node(x->MatrixIndexExpr.expr); + if (f && (is_type_matrix(f->type) && is_entity_local_variable(f))) { + unsafe_return_error(o, "the address of an indexed variable", f->type); + } + } + } else if (expr->kind == Ast_SliceExpr) { + Ast *x = unparen_expr(expr->SliceExpr.expr); + Entity *e = entity_of_node(x); + if (is_entity_local_variable(e) && is_type_array(e->type)) { + unsafe_return_error(o, "a slice of a local variable"); + } else if (x->kind == Ast_CompoundLit) { + unsafe_return_error(o, "a slice of a compound literal"); + } + } else if (o.mode == Addressing_Constant && is_type_slice(type)) { + ERROR_BLOCK(); + unsafe_return_error(o, "a compound literal of a slice"); + error_line("\tNote: A constant slice value will use the memory of the current stack frame\n"); + } else if (expr->kind == Ast_CompoundLit) { + ast_node(cl, CompoundLit, expr); + for (Ast *elem : cl->elems) { + if (elem->kind == Ast_FieldValue) { + ast_node(fv, FieldValue, elem); + check_unsafe_return(o, entity_of_node(fv->field)->type, fv->value); + } + } + } +} + gb_internal void check_return_stmt(CheckerContext *ctx, Ast *node) { ast_node(rs, ReturnStmt, node); @@ -2617,75 +2682,7 @@ gb_internal void check_return_stmt(CheckerContext *ctx, Ast *node) { expr = unparen_expr(arg); } - auto unsafe_return_error = [](Operand const &o, char const *msg, Type *extra_type=nullptr) { - gbString s = expr_to_string(o.expr); - if (extra_type) { - gbString t = type_to_string(extra_type); - error(o.expr, "It is unsafe to return %s ('%s') of type ('%s') from a procedure, as it uses the current stack frame's memory", msg, s, t); - gb_string_free(t); - } else { - error(o.expr, "It is unsafe to return %s ('%s') from a procedure, as it uses the current stack frame's memory", msg, s); - } - gb_string_free(s); - }; - - - // NOTE(bill): This is very basic escape analysis - // This needs to be improved tremendously, and a lot of it done during the - // middle-end (or LLVM side) to improve checks and error messages - if (expr->kind == Ast_CompoundLit && is_type_slice(o.type)) { - ast_node(cl, CompoundLit, expr); - if (cl->elems.count == 0) { - continue; - } - unsafe_return_error(o, "a compound literal of a slice"); - } else if (expr->kind == Ast_UnaryExpr && expr->UnaryExpr.op.kind == Token_And) { - Ast *x = unparen_expr(expr->UnaryExpr.expr); - Entity *e = entity_of_node(x); - if (is_entity_local_variable(e)) { - unsafe_return_error(o, "the address of a local variable"); - } else if (x->kind == Ast_CompoundLit) { - unsafe_return_error(o, "the address of a compound literal"); - } else if (x->kind == Ast_IndexExpr) { - Entity *f = entity_of_node(x->IndexExpr.expr); - if (f && (is_type_array_like(f->type) || is_type_matrix(f->type))) { - if (is_entity_local_variable(f)) { - unsafe_return_error(o, "the address of an indexed variable", f->type); - } - } - } else if (x->kind == Ast_MatrixIndexExpr) { - Entity *f = entity_of_node(x->MatrixIndexExpr.expr); - if (f && (is_type_matrix(f->type) && is_entity_local_variable(f))) { - unsafe_return_error(o, "the address of an indexed variable", f->type); - } - } - } else if (expr->kind == Ast_SliceExpr) { - Ast *x = unparen_expr(expr->SliceExpr.expr); - Entity *e = entity_of_node(x); - if (is_entity_local_variable(e) && is_type_array(e->type)) { - unsafe_return_error(o, "a slice of a local variable"); - } else if (x->kind == Ast_CompoundLit) { - unsafe_return_error(o, "a slice of a compound literal"); - } - } else if (o.mode == Addressing_Constant && is_type_slice(o.type)) { - ERROR_BLOCK(); - unsafe_return_error(o, "a compound literal of a slice"); - error_line("\tNote: A constant slice value will use the memory of the current stack frame\n"); - } else if (expr->kind == Ast_CompoundLit && is_type_struct(o.type)) { - ast_node(cl, CompoundLit, expr); - for (Ast *elem : cl->elems) { - if (elem->kind == Ast_FieldValue) { - ast_node(fv, FieldValue, elem); - if (fv->value->kind == Ast_CompoundLit && is_type_slice(entity_of_node(fv->field)->type)) { - ast_node(sl, CompoundLit, fv->value); - if (sl->elems.count == 0) { - continue; - } - unsafe_return_error(o, "a compound literal containing a slice"); - } - } - } - } + check_unsafe_return(o, o.type, expr); } } -- cgit v1.2.3 From af83c30b6f5303dfddf8e38019c0431658b6e176 Mon Sep 17 00:00:00 2001 From: gingerBill Date: Sat, 27 Sep 2025 14:13:16 +0100 Subject: And extra safety checks --- src/check_stmt.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) (limited to 'src/check_stmt.cpp') diff --git a/src/check_stmt.cpp b/src/check_stmt.cpp index f6e201011..5e7a8e323 100644 --- a/src/check_stmt.cpp +++ b/src/check_stmt.cpp @@ -2545,7 +2545,7 @@ gb_internal void check_if_stmt(CheckerContext *ctx, Ast *node, u32 mod_flags) { // This needs to be improved tremendously, and a lot of it done during the // middle-end (or LLVM side) to improve checks and error messages void check_unsafe_return(Operand const &o, Type *type, Ast *expr) { - auto unsafe_return_error = [](Operand const &o, char const *msg, Type *extra_type=nullptr) { + auto const unsafe_return_error = [](Operand const &o, char const *msg, Type *extra_type=nullptr) { gbString s = expr_to_string(o.expr); if (extra_type) { gbString t = type_to_string(extra_type); @@ -2557,6 +2557,10 @@ void check_unsafe_return(Operand const &o, Type *type, Ast *expr) { gb_string_free(s); }; + if (type == nullptr || expr == nullptr) { + return; + } + if (expr->kind == Ast_CompoundLit && is_type_slice(type)) { ast_node(cl, CompoundLit, expr); if (cl->elems.count == 0) { @@ -2600,7 +2604,10 @@ void check_unsafe_return(Operand const &o, Type *type, Ast *expr) { for (Ast *elem : cl->elems) { if (elem->kind == Ast_FieldValue) { ast_node(fv, FieldValue, elem); - check_unsafe_return(o, entity_of_node(fv->field)->type, fv->value); + Entity *e = entity_of_node(fv->field); + if (e != nullptr) { + check_unsafe_return(o, e->type, fv->value); + } } } } -- cgit v1.2.3 From 7e9ceaa281f52488398c42fb803fb14bcd8f76bb Mon Sep 17 00:00:00 2001 From: Jeroen van Rijn Date: Mon, 1 Dec 2025 22:16:37 +0100 Subject: Fix #5982 Add escape analysis check for #load directive to fix #5982. --- src/check_stmt.cpp | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'src/check_stmt.cpp') diff --git a/src/check_stmt.cpp b/src/check_stmt.cpp index 5e7a8e323..835f0162a 100644 --- a/src/check_stmt.cpp +++ b/src/check_stmt.cpp @@ -2596,6 +2596,10 @@ void check_unsafe_return(Operand const &o, Type *type, Ast *expr) { unsafe_return_error(o, "a slice of a compound literal"); } } else if (o.mode == Addressing_Constant && is_type_slice(type)) { + if (is_load_directive_call(o.expr)) { + return; + } + ERROR_BLOCK(); unsafe_return_error(o, "a compound literal of a slice"); error_line("\tNote: A constant slice value will use the memory of the current stack frame\n"); -- cgit v1.2.3