Browse Source

Fix another #soa race condition bug

gingerBill 1 year ago
parent
commit
1009182f7b
6 changed files with 78 additions and 23 deletions
  1. 5 0
      src/check_expr.cpp
  2. 38 22
      src/check_type.cpp
  3. 31 0
      src/checker.cpp
  4. 1 0
      src/checker.hpp
  5. 1 1
      src/llvm_backend_type.cpp
  6. 2 0
      src/types.cpp

+ 5 - 0
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<Operand> const &ordered_operands);
 gb_internal Entity *find_polymorphic_record_entity(GenTypesData *found_gen_types, isize param_count, Array<Operand> const &ordered_operands);
 
 
+gb_internal bool complete_soa_type(Checker *checker, Type *t, bool wait_to_finish);
+
 enum LoadDirectiveResult {
 enum LoadDirectiveResult {
 	LoadDirective_Success  = 0,
 	LoadDirective_Success  = 0,
 	LoadDirective_Error    = 1,
 	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) {
 	if (entity == nullptr && selector->kind == Ast_Ident) {
 		String field_name = selector->Ident.token.string;
 		String field_name = selector->Ident.token.string;

+ 38 - 22
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;
 	Type *original_type = t;
+	gb_unused(original_type);
+
 	t = base_type(t);
 	t = base_type(t);
 	if (t == nullptr || !is_type_soa_struct(t)) {
 	if (t == nullptr || !is_type_soa_struct(t)) {
-		return;
+		return true;
 	}
 	}
 
 
 	MUTEX_GUARD(&t->Struct.soa_mutex);
 	MUTEX_GUARD(&t->Struct.soa_mutex);
 
 
 	if (t->Struct.fields_wait_signal.futex.load()) {
 	if (t->Struct.fields_wait_signal.futex.load()) {
-		return;
+		return true;
 	}
 	}
 
 
 	isize field_count = 0;
 	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;
 	case StructSoa_Dynamic:	extra_field_count = 3; break;
 	}
 	}
 
 
-
-	Ast *node = t->Struct.node;
 	Scope *scope = t->Struct.scope;
 	Scope *scope = t->Struct.scope;
 	i64 soa_count = t->Struct.soa_count;
 	i64 soa_count = t->Struct.soa_count;
 	Type *elem = t->Struct.soa_elem;
 	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) {
 	if (wait_to_finish) {
 		wait_signal_until_available(&old_struct->Struct.fields_wait_signal);
 		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;
 	field_count = old_struct->Struct.fields.count;
 
 
 	t->Struct.fields = slice_make<Entity *>(permanent_allocator(), field_count+extra_field_count);
 	t->Struct.fields = slice_make<Entity *>(permanent_allocator(), field_count+extra_field_count);
 	t->Struct.tags = gb_alloc_array(permanent_allocator(), String, 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) {
 	for_array(i, old_struct->Struct.fields) {
 		Entity *old_field = old_struct->Struct.fields[i];
 		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);
 			Entity *new_field = alloc_entity_field(scope, old_field->token, field_type, false, old_field->Variable.field_index);
 			t->Struct.fields[i] = new_field;
 			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 {
 		} else {
 			t->Struct.fields[i] = old_field;
 			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) {
 	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);
 		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;
 		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) {
 		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);
 			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;
 			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);
 			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;
 			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) {
 gb_internal WORKER_TASK_PROC(complete_soa_type_worker) {
 	SoaTypeWorkerData *wd = cast(SoaTypeWorkerData *)data;
 	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;
 	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);
 	scope = create_scope(ctx->info, ctx->scope);
 	soa_struct->Struct.scope = scope;
 	soa_struct->Struct.scope = scope;
 
 
+	if (elem && elem->kind == Type_Named) {
+		add_declaration_dependency(ctx, elem->Named.type_name);
+	}
 
 
 	if (is_polymorphic) {
 	if (is_polymorphic) {
 		field_count = 0;
 		field_count = 0;
@@ -2938,6 +2952,8 @@ gb_internal Type *make_soa_struct_internal(CheckerContext *ctx, Ast *array_typ_e
 		wd->ctx = *ctx;
 		wd->ctx = *ctx;
 		wd->type = soa_struct;
 		wd->type = soa_struct;
 		wd->wait_to_finish = true;
 		wd->wait_to_finish = true;
+
+		mpsc_enqueue(&ctx->checker->soa_types_to_complete, soa_struct);
 		thread_pool_add_task(complete_soa_type_worker, wd);
 		thread_pool_add_task(complete_soa_type_worker, wd);
 	}
 	}
 
 

+ 31 - 0
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);
 	array_init(&c->nested_proc_lits, heap_allocator(), 0, 1<<20);
 
 
 	mpsc_init(&c->global_untyped_queue, a); // , 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);
 	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->nested_proc_lits);
 	array_free(&c->procs_to_check);
 	array_free(&c->procs_to_check);
 	mpsc_destroy(&c->global_untyped_queue);
 	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;
 	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) {
 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);
 	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;
 		DeclInfo *d = e->decl_info;
 		check_single_global_entity(c, e, d);
 		check_single_global_entity(c, e, d);
 		if (e->type != nullptr && is_type_typed(e->type)) {
 		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_size_of(e->type);
 			(void)type_align_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) {
 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_entities_from_queues(c);
 	check_add_definitions_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");
 	TIME_SECTION("check bodies have all been checked");
 	check_unchecked_bodies(c);
 	check_unchecked_bodies(c);
 
 
+	TIME_SECTION("check #soa types");
+
 	check_merge_queues_into_arrays(c);
 	check_merge_queues_into_arrays(c);
 	thread_pool_wait();
 	thread_pool_wait();
 
 

+ 1 - 0
src/checker.hpp

@@ -500,6 +500,7 @@ struct Checker {
 
 
 
 
 	MPSCQueue<UntypedExprInfo> global_untyped_queue;
 	MPSCQueue<UntypedExprInfo> global_untyped_queue;
+	MPSCQueue<Type *> soa_types_to_complete;
 };
 };
 
 
 
 

+ 1 - 1
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];
 					Entity *f = t->Struct.fields[source_index];
 					i64 foffset = 0;
 					i64 foffset = 0;
 					if (!t->Struct.is_raw_union) {
 					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);
 						GB_ASSERT(0 <= f->Variable.field_index && f->Variable.field_index < count);
 						foffset = t->Struct.offsets[source_index];
 						foffset = t->Struct.offsets[source_index];
 					}
 					}

+ 2 - 0
src/types.cpp

@@ -3772,6 +3772,8 @@ gb_internal i64 type_align_of_internal(Type *t, TypePath *path) {
 			return 1;
 			return 1;
 		}
 		}
 
 
+		type_set_offsets(t);
+
 		i64 max = 1;
 		i64 max = 1;
 		for_array(i, t->Struct.fields) {
 		for_array(i, t->Struct.fields) {
 			Type *field_type = t->Struct.fields[i]->type;
 			Type *field_type = t->Struct.fields[i]->type;