From 1009182f7b35e38e0fba375ad830fc609a7be831 Mon Sep 17 00:00:00 2001 From: gingerBill Date: Tue, 26 Mar 2024 14:13:55 +0000 Subject: [PATCH] Fix another #soa race condition bug --- src/check_expr.cpp | 5 ++++ src/check_type.cpp | 60 +++++++++++++++++++++++++-------------- src/checker.cpp | 31 ++++++++++++++++++++ src/checker.hpp | 1 + src/llvm_backend_type.cpp | 2 +- src/types.cpp | 2 ++ 6 files changed, 78 insertions(+), 23 deletions(-) diff --git a/src/check_expr.cpp b/src/check_expr.cpp index a34b425c2..3a275729f 100644 --- a/src/check_expr.cpp +++ b/src/check_expr.cpp @@ -122,6 +122,8 @@ gb_internal bool is_expr_inferred_fixed_array(Ast *type_expr); gb_internal Entity *find_polymorphic_record_entity(GenTypesData *found_gen_types, isize param_count, Array const &ordered_operands); +gb_internal bool complete_soa_type(Checker *checker, Type *t, bool wait_to_finish); + enum LoadDirectiveResult { LoadDirective_Success = 0, LoadDirective_Error = 1, @@ -5031,6 +5033,9 @@ gb_internal Entity *check_selector(CheckerContext *c, Operand *operand, Ast *nod } } + if (operand->type && is_type_soa_struct(type_deref(operand->type))) { + complete_soa_type(c->checker, type_deref(operand->type), false); + } if (entity == nullptr && selector->kind == Ast_Ident) { String field_name = selector->Ident.token.string; diff --git a/src/check_type.cpp b/src/check_type.cpp index d26d9b660..40a7ec947 100644 --- a/src/check_type.cpp +++ b/src/check_type.cpp @@ -2690,17 +2690,19 @@ struct SoaTypeWorkerData { }; -gb_internal void complete_soa_type(CheckerContext *ctx, Type *t, bool wait_to_finish) { +gb_internal bool complete_soa_type(Checker *checker, Type *t, bool wait_to_finish) { Type *original_type = t; + gb_unused(original_type); + t = base_type(t); if (t == nullptr || !is_type_soa_struct(t)) { - return; + return true; } MUTEX_GUARD(&t->Struct.soa_mutex); if (t->Struct.fields_wait_signal.futex.load()) { - return; + return true; } isize field_count = 0; @@ -2711,8 +2713,6 @@ gb_internal void complete_soa_type(CheckerContext *ctx, Type *t, bool wait_to_fi case StructSoa_Dynamic: extra_field_count = 3; break; } - - Ast *node = t->Struct.node; Scope *scope = t->Struct.scope; i64 soa_count = t->Struct.soa_count; Type *elem = t->Struct.soa_elem; @@ -2721,18 +2721,26 @@ gb_internal void complete_soa_type(CheckerContext *ctx, Type *t, bool wait_to_fi if (wait_to_finish) { wait_signal_until_available(&old_struct->Struct.fields_wait_signal); + } else { + GB_ASSERT(old_struct->Struct.fields_wait_signal.futex.load()); } - field_count = old_struct->Struct.fields.count; t->Struct.fields = slice_make(permanent_allocator(), field_count+extra_field_count); t->Struct.tags = gb_alloc_array(permanent_allocator(), String, field_count+extra_field_count); - if (soa_count > I32_MAX) { - soa_count = I32_MAX; - error(node, "Array count too large for an #soa struct, got %lld", cast(long long)soa_count); - } - t->Struct.soa_count = cast(i32)soa_count; + + + auto const &add_entity = [](Scope *scope, Entity *entity) { + String name = entity->token.string; + if (!is_blank_ident(name)) { + Entity *ie = scope_insert(scope, entity); + if (ie != nullptr) { + redeclaration_error(name, entity, ie); + } + } + }; + for_array(i, old_struct->Struct.fields) { Entity *old_field = old_struct->Struct.fields[i]; @@ -2746,8 +2754,8 @@ gb_internal void complete_soa_type(CheckerContext *ctx, Type *t, bool wait_to_fi } Entity *new_field = alloc_entity_field(scope, old_field->token, field_type, false, old_field->Variable.field_index); t->Struct.fields[i] = new_field; - add_entity(ctx, scope, nullptr, new_field); - add_entity_use(ctx, nullptr, new_field); + add_entity(scope, new_field); + new_field->flags |= EntityFlag_Used; } else { t->Struct.fields[i] = old_field; } @@ -2758,29 +2766,32 @@ gb_internal void complete_soa_type(CheckerContext *ctx, Type *t, bool wait_to_fi if (t->Struct.soa_kind != StructSoa_Fixed) { Entity *len_field = alloc_entity_field(scope, make_token_ident("__$len"), t_int, false, cast(i32)field_count+0); t->Struct.fields[field_count+0] = len_field; - add_entity(ctx, scope, nullptr, len_field); - add_entity_use(ctx, nullptr, len_field); + add_entity(scope, len_field); + len_field->flags |= EntityFlag_Used; if (t->Struct.soa_kind == StructSoa_Dynamic) { Entity *cap_field = alloc_entity_field(scope, make_token_ident("__$cap"), t_int, false, cast(i32)field_count+1); t->Struct.fields[field_count+1] = cap_field; - add_entity(ctx, scope, nullptr, cap_field); - add_entity_use(ctx, nullptr, cap_field); + add_entity(scope, cap_field); + cap_field->flags |= EntityFlag_Used; - init_mem_allocator(ctx->checker); + init_mem_allocator(checker); Entity *allocator_field = alloc_entity_field(scope, make_token_ident("allocator"), t_allocator, false, cast(i32)field_count+2); t->Struct.fields[field_count+2] = allocator_field; - add_entity(ctx, scope, nullptr, allocator_field); - add_entity_use(ctx, nullptr, allocator_field); + add_entity(scope, allocator_field); + allocator_field->flags |= EntityFlag_Used; } } - add_type_info_type(ctx, original_type); + // add_type_info_type(ctx, original_type); + + wait_signal_set(&t->Struct.fields_wait_signal); + return true; } gb_internal WORKER_TASK_PROC(complete_soa_type_worker) { SoaTypeWorkerData *wd = cast(SoaTypeWorkerData *)data; - complete_soa_type(&wd->ctx, wd->type, wd->wait_to_finish); + complete_soa_type(wd->ctx.checker, wd->type, wd->wait_to_finish); return 0; } @@ -2825,6 +2836,9 @@ gb_internal Type *make_soa_struct_internal(CheckerContext *ctx, Ast *array_typ_e scope = create_scope(ctx->info, ctx->scope); soa_struct->Struct.scope = scope; + if (elem && elem->kind == Type_Named) { + add_declaration_dependency(ctx, elem->Named.type_name); + } if (is_polymorphic) { field_count = 0; @@ -2938,6 +2952,8 @@ gb_internal Type *make_soa_struct_internal(CheckerContext *ctx, Ast *array_typ_e wd->ctx = *ctx; wd->type = soa_struct; wd->wait_to_finish = true; + + mpsc_enqueue(&ctx->checker->soa_types_to_complete, soa_struct); thread_pool_add_task(complete_soa_type_worker, wd); } diff --git a/src/checker.cpp b/src/checker.cpp index e7d0ad9cb..ccda31a4f 100644 --- a/src/checker.cpp +++ b/src/checker.cpp @@ -1371,6 +1371,7 @@ gb_internal void init_checker(Checker *c) { array_init(&c->nested_proc_lits, heap_allocator(), 0, 1<<20); mpsc_init(&c->global_untyped_queue, a); // , 1<<20); + mpsc_init(&c->soa_types_to_complete, a); // , 1<<20); c->builtin_ctx = make_checker_context(c); } @@ -1383,6 +1384,7 @@ gb_internal void destroy_checker(Checker *c) { array_free(&c->nested_proc_lits); array_free(&c->procs_to_check); mpsc_destroy(&c->global_untyped_queue); + mpsc_destroy(&c->soa_types_to_complete); } @@ -1682,6 +1684,26 @@ gb_internal bool add_entity_with_name(CheckerContext *c, Scope *scope, Ast *iden } return true; } + +gb_internal bool add_entity_with_name(CheckerInfo *info, Scope *scope, Ast *identifier, Entity *entity, String name) { + if (scope == nullptr) { + return false; + } + + + if (!is_blank_ident(name)) { + Entity *ie = scope_insert(scope, entity); + if (ie != nullptr) { + return redeclaration_error(name, entity, ie); + } + } + if (identifier != nullptr) { + GB_ASSERT(entity->file != nullptr); + add_entity_definition(info, identifier, entity); + } + return true; +} + gb_internal bool add_entity(CheckerContext *c, Scope *scope, Ast *identifier, Entity *entity) { return add_entity_with_name(c, scope, identifier, entity, entity->token.string); } @@ -4440,6 +4462,10 @@ gb_internal void check_all_global_entities(Checker *c) { DeclInfo *d = e->decl_info; check_single_global_entity(c, e, d); if (e->type != nullptr && is_type_typed(e->type)) { + for (Type *t = nullptr; mpsc_dequeue(&c->soa_types_to_complete, &t); /**/) { + complete_soa_type(c, t, false); + } + (void)type_size_of(e->type); (void)type_align_of(e->type); } @@ -6108,6 +6134,9 @@ gb_internal void check_add_definitions_from_queues(Checker *c) { } gb_internal void check_merge_queues_into_arrays(Checker *c) { + for (Type *t = nullptr; mpsc_dequeue(&c->soa_types_to_complete, &t); /**/) { + complete_soa_type(c, t, false); + } check_add_entities_from_queues(c); check_add_definitions_from_queues(c); } @@ -6318,6 +6347,8 @@ gb_internal void check_parsed_files(Checker *c) { TIME_SECTION("check bodies have all been checked"); check_unchecked_bodies(c); + TIME_SECTION("check #soa types"); + check_merge_queues_into_arrays(c); thread_pool_wait(); diff --git a/src/checker.hpp b/src/checker.hpp index e0dc54a87..1701da58d 100644 --- a/src/checker.hpp +++ b/src/checker.hpp @@ -500,6 +500,7 @@ struct Checker { MPSCQueue global_untyped_queue; + MPSCQueue soa_types_to_complete; }; diff --git a/src/llvm_backend_type.cpp b/src/llvm_backend_type.cpp index de83f5058..20e4991e7 100644 --- a/src/llvm_backend_type.cpp +++ b/src/llvm_backend_type.cpp @@ -860,7 +860,7 @@ gb_internal void lb_setup_type_info_data_giant_array(lbModule *m, i64 global_typ Entity *f = t->Struct.fields[source_index]; i64 foffset = 0; if (!t->Struct.is_raw_union) { - GB_ASSERT(t->Struct.offsets != nullptr); + GB_ASSERT_MSG(t->Struct.offsets != nullptr, "%s", type_to_string(t)); GB_ASSERT(0 <= f->Variable.field_index && f->Variable.field_index < count); foffset = t->Struct.offsets[source_index]; } diff --git a/src/types.cpp b/src/types.cpp index 6e63f56ed..ebe6271f2 100644 --- a/src/types.cpp +++ b/src/types.cpp @@ -3772,6 +3772,8 @@ gb_internal i64 type_align_of_internal(Type *t, TypePath *path) { return 1; } + type_set_offsets(t); + i64 max = 1; for_array(i, t->Struct.fields) { Type *field_type = t->Struct.fields[i]->type;