Browse Source

Fix a race condition when produced anonymous procedure literals with `-use-separate-modules`

gingerBill 2 years ago
parent
commit
581eebb197
5 changed files with 99 additions and 55 deletions
  1. 3 0
      src/checker.hpp
  2. 1 52
      src/llvm_backend.cpp
  3. 3 1
      src/llvm_backend.hpp
  4. 1 0
      src/llvm_backend_const.cpp
  5. 91 2
      src/llvm_backend_general.cpp

+ 3 - 0
src/checker.hpp

@@ -199,6 +199,9 @@ struct DeclInfo {
 	BlockingMutex type_and_value_mutex;
 	BlockingMutex type_and_value_mutex;
 
 
 	Array<BlockLabel> labels;
 	Array<BlockLabel> labels;
+
+	// NOTE(bill): this is to prevent a race condition since these procedure literals can be created anywhere at any time
+	struct lbModule *code_gen_module;
 };
 };
 
 
 // ProcInfo stores the information needed for checking a procedure
 // ProcInfo stores the information needed for checking a procedure

+ 1 - 52
src/llvm_backend.cpp

@@ -772,56 +772,6 @@ gb_internal lbValue lb_map_set_proc_for_type(lbModule *m, Type *type) {
 	return {p->value, p->type};
 	return {p->value, p->type};
 }
 }
 
 
-
-gb_internal lbValue lb_generate_anonymous_proc_lit(lbModule *m, String const &prefix_name, Ast *expr, lbProcedure *parent) {
-	mutex_lock(&m->gen->anonymous_proc_lits_mutex);
-	defer (mutex_unlock(&m->gen->anonymous_proc_lits_mutex));
-
-	lbProcedure **found = map_get(&m->gen->anonymous_proc_lits, expr);
-	if (found) {
-		return lb_find_procedure_value_from_entity(m, (*found)->entity);
-	}
-
-	ast_node(pl, ProcLit, expr);
-
-	// NOTE(bill): Generate a new name
-	// parent$count
-	isize name_len = prefix_name.len + 6 + 11;
-	char *name_text = gb_alloc_array(permanent_allocator(), char, name_len);
-	static std::atomic<i32> name_id;
-	name_len = gb_snprintf(name_text, name_len, "%.*s$anon-%d", LIT(prefix_name), 1+name_id.fetch_add(1));
-	String name = make_string((u8 *)name_text, name_len-1);
-
-	Type *type = type_of_expr(expr);
-
-	Token token = {};
-	token.pos = ast_token(expr).pos;
-	token.kind = Token_Ident;
-	token.string = name;
-	Entity *e = alloc_entity_procedure(nullptr, token, type, pl->tags);
-	e->file = expr->file();
-	e->decl_info = pl->decl;
-	e->code_gen_module = m;
-	e->flags |= EntityFlag_ProcBodyChecked;
-	lbProcedure *p = lb_create_procedure(m, e);
-
-	lbValue value = {};
-	value.value = p->value;
-	value.type = p->type;
-
-	array_add(&m->procedures_to_generate, p);
-	if (parent != nullptr) {
-		array_add(&parent->children, p);
-	} else {
-		string_map_set(&m->members, name, value);
-	}
-
-	map_set(&m->gen->anonymous_proc_lits, expr, p);
-
-	return value;
-}
-
-
 gb_internal lbValue lb_gen_map_cell_info_ptr(lbModule *m, Type *type) {
 gb_internal lbValue lb_gen_map_cell_info_ptr(lbModule *m, Type *type) {
 	lbAddr *found = map_get(&m->map_cell_info_map, type);
 	lbAddr *found = map_get(&m->map_cell_info_map, type);
 	if (found) {
 	if (found) {
@@ -1513,7 +1463,7 @@ gb_internal WORKER_TASK_PROC(lb_generate_missing_procedures_to_check_worker_proc
 	lbModule *m = cast(lbModule *)data;
 	lbModule *m = cast(lbModule *)data;
 	for (isize i = 0; i < m->missing_procedures_to_check.count; i++) {
 	for (isize i = 0; i < m->missing_procedures_to_check.count; i++) {
 		lbProcedure *p = m->missing_procedures_to_check[i];
 		lbProcedure *p = m->missing_procedures_to_check[i];
-		debugf("Generate missing procedure: %.*s\n", LIT(p->name));
+		debugf("Generate missing procedure: %.*s module %p\n", LIT(p->name), m);
 		lb_generate_procedure(m, p);
 		lb_generate_procedure(m, p);
 	}
 	}
 	return 0;
 	return 0;
@@ -1577,7 +1527,6 @@ gb_internal void lb_llvm_module_passes(lbGenerator *gen, bool do_threading) {
 	thread_pool_wait();
 	thread_pool_wait();
 }
 }
 
 
-
 gb_internal String lb_filepath_ll_for_module(lbModule *m) {
 gb_internal String lb_filepath_ll_for_module(lbModule *m) {
 	String path = concatenate3_strings(permanent_allocator(),
 	String path = concatenate3_strings(permanent_allocator(),
 		build_context.build_paths[BuildPath_Output].basename,
 		build_context.build_paths[BuildPath_Output].basename,

+ 3 - 1
src/llvm_backend.hpp

@@ -201,7 +201,7 @@ struct lbGenerator {
 	PtrMap<LLVMContextRef, lbModule *> modules_through_ctx; 
 	PtrMap<LLVMContextRef, lbModule *> modules_through_ctx; 
 	lbModule default_module;
 	lbModule default_module;
 
 
-	BlockingMutex anonymous_proc_lits_mutex;
+	RecursiveMutex anonymous_proc_lits_mutex;
 	PtrMap<Ast *, lbProcedure *> anonymous_proc_lits; 
 	PtrMap<Ast *, lbProcedure *> anonymous_proc_lits; 
 
 
 	BlockingMutex foreign_mutex;
 	BlockingMutex foreign_mutex;
@@ -545,6 +545,8 @@ gb_internal gb_inline i64 lb_max_zero_init_size(void) {
 gb_internal LLVMTypeRef OdinLLVMGetArrayElementType(LLVMTypeRef type);
 gb_internal LLVMTypeRef OdinLLVMGetArrayElementType(LLVMTypeRef type);
 gb_internal LLVMTypeRef OdinLLVMGetVectorElementType(LLVMTypeRef type);
 gb_internal LLVMTypeRef OdinLLVMGetVectorElementType(LLVMTypeRef type);
 
 
+gb_internal String lb_filepath_ll_for_module(lbModule *m);
+
 #define LB_STARTUP_RUNTIME_PROC_NAME   "__$startup_runtime"
 #define LB_STARTUP_RUNTIME_PROC_NAME   "__$startup_runtime"
 #define LB_CLEANUP_RUNTIME_PROC_NAME   "__$cleanup_runtime"
 #define LB_CLEANUP_RUNTIME_PROC_NAME   "__$cleanup_runtime"
 #define LB_STARTUP_TYPE_INFO_PROC_NAME "__$startup_type_info"
 #define LB_STARTUP_TYPE_INFO_PROC_NAME "__$startup_type_info"

+ 1 - 0
src/llvm_backend_const.cpp

@@ -473,6 +473,7 @@ gb_internal lbValue lb_const_value(lbModule *m, Type *type, ExactValue value, bo
 	if (value.kind == ExactValue_Procedure) {
 	if (value.kind == ExactValue_Procedure) {
 		lbValue res = {};
 		lbValue res = {};
 		Ast *expr = unparen_expr(value.value_procedure);
 		Ast *expr = unparen_expr(value.value_procedure);
+		GB_ASSERT(expr != nullptr);
 		if (expr->kind == Ast_ProcLit) {
 		if (expr->kind == Ast_ProcLit) {
 			res = lb_generate_anonymous_proc_lit(m, str_lit("_proclit"), expr);
 			res = lb_generate_anonymous_proc_lit(m, str_lit("_proclit"), expr);
 		} else {
 		} else {

+ 91 - 2
src/llvm_backend_general.cpp

@@ -334,10 +334,35 @@ gb_internal bool lb_is_instr_terminating(LLVMValueRef instr) {
 	return false;
 	return false;
 }
 }
 
 
+gb_internal lbModule *lb_module_of_expr(lbGenerator *gen, Ast *expr) {
+	GB_ASSERT(expr != nullptr);
+	lbModule **found = nullptr;
+	AstFile *file = expr->file();
+	if (file) {
+		found = map_get(&gen->modules, cast(void *)file);
+		if (found) {
+			return *found;
+		}
+
+		if (file->pkg) {
+			found = map_get(&gen->modules, cast(void *)file->pkg);
+			if (found) {
+				return *found;
+			}
+		}
+	}
+
+	return &gen->default_module;
+}
 
 
 gb_internal lbModule *lb_module_of_entity(lbGenerator *gen, Entity *e) {
 gb_internal lbModule *lb_module_of_entity(lbGenerator *gen, Entity *e) {
 	GB_ASSERT(e != nullptr);
 	GB_ASSERT(e != nullptr);
 	lbModule **found = nullptr;
 	lbModule **found = nullptr;
+	if (e->kind == Entity_Procedure &&
+	    e->decl_info &&
+	    e->decl_info->code_gen_module) {
+		return e->decl_info->code_gen_module;
+	}
 	if (e->file) {
 	if (e->file) {
 		found = map_get(&gen->modules, cast(void *)e->file);
 		found = map_get(&gen->modules, cast(void *)e->file);
 		if (found) {
 		if (found) {
@@ -2661,9 +2686,12 @@ gb_internal lbValue lb_find_ident(lbProcedure *p, lbModule *m, Entity *e, Ast *e
 
 
 
 
 gb_internal lbValue lb_find_procedure_value_from_entity(lbModule *m, Entity *e) {
 gb_internal lbValue lb_find_procedure_value_from_entity(lbModule *m, Entity *e) {
+	lbGenerator *gen = m->gen;
+
 	GB_ASSERT(is_type_proc(e->type));
 	GB_ASSERT(is_type_proc(e->type));
 	e = strip_entity_wrapping(e);
 	e = strip_entity_wrapping(e);
 	GB_ASSERT(e != nullptr);
 	GB_ASSERT(e != nullptr);
+	GB_ASSERT(e->kind == Entity_Procedure);
 
 
 	lbValue *found = nullptr;
 	lbValue *found = nullptr;
 	rw_mutex_shared_lock(&m->values_mutex);
 	rw_mutex_shared_lock(&m->values_mutex);
@@ -2677,20 +2705,24 @@ gb_internal lbValue lb_find_procedure_value_from_entity(lbModule *m, Entity *e)
 
 
 	lbModule *other_module = m;
 	lbModule *other_module = m;
 	if (USE_SEPARATE_MODULES) {
 	if (USE_SEPARATE_MODULES) {
-		other_module = lb_module_of_entity(m->gen, e);
+		other_module = lb_module_of_entity(gen, e);
 	}
 	}
 	if (other_module == m) {
 	if (other_module == m) {
-		debugf("Missing Procedure (lb_find_procedure_value_from_entity): %.*s\n", LIT(e->token.string));
+		debugf("Missing Procedure (lb_find_procedure_value_from_entity): %.*s module %p\n", LIT(e->token.string), m);
 	}
 	}
 	ignore_body = other_module != m;
 	ignore_body = other_module != m;
 
 
 	lbProcedure *missing_proc = lb_create_procedure(m, e, ignore_body);
 	lbProcedure *missing_proc = lb_create_procedure(m, e, ignore_body);
 	if (ignore_body) {
 	if (ignore_body) {
+		mutex_lock(&gen->anonymous_proc_lits_mutex);
+		defer (mutex_unlock(&gen->anonymous_proc_lits_mutex));
+
 		GB_ASSERT(other_module != nullptr);
 		GB_ASSERT(other_module != nullptr);
 		rw_mutex_shared_lock(&other_module->values_mutex);
 		rw_mutex_shared_lock(&other_module->values_mutex);
 		auto *found = map_get(&other_module->values, e);
 		auto *found = map_get(&other_module->values, e);
 		rw_mutex_shared_unlock(&other_module->values_mutex);
 		rw_mutex_shared_unlock(&other_module->values_mutex);
 		if (found == nullptr) {
 		if (found == nullptr) {
+			// THIS IS THE RACE CONDITION
 			lbProcedure *missing_proc_in_other_module = lb_create_procedure(other_module, e, false);
 			lbProcedure *missing_proc_in_other_module = lb_create_procedure(other_module, e, false);
 			array_add(&other_module->missing_procedures_to_check, missing_proc_in_other_module);
 			array_add(&other_module->missing_procedures_to_check, missing_proc_in_other_module);
 		}
 		}
@@ -2707,6 +2739,63 @@ gb_internal lbValue lb_find_procedure_value_from_entity(lbModule *m, Entity *e)
 }
 }
 
 
 
 
+
+gb_internal lbValue lb_generate_anonymous_proc_lit(lbModule *m, String const &prefix_name, Ast *expr, lbProcedure *parent) {
+	lbGenerator *gen = m->gen;
+
+	mutex_lock(&gen->anonymous_proc_lits_mutex);
+	defer (mutex_unlock(&gen->anonymous_proc_lits_mutex));
+
+	TokenPos pos = ast_token(expr).pos;
+	lbProcedure **found = map_get(&gen->anonymous_proc_lits, expr);
+	if (found) {
+		return lb_find_procedure_value_from_entity(m, (*found)->entity);
+	}
+
+	ast_node(pl, ProcLit, expr);
+
+	// NOTE(bill): Generate a new name
+	// parent$count
+	isize name_len = prefix_name.len + 6 + 11;
+	char *name_text = gb_alloc_array(permanent_allocator(), char, name_len);
+	static std::atomic<i32> name_id;
+	name_len = gb_snprintf(name_text, name_len, "%.*s$anon-%d", LIT(prefix_name), 1+name_id.fetch_add(1));
+	String name = make_string((u8 *)name_text, name_len-1);
+
+	Type *type = type_of_expr(expr);
+
+	GB_ASSERT(pl->decl->entity == nullptr);
+	Token token = {};
+	token.pos = ast_token(expr).pos;
+	token.kind = Token_Ident;
+	token.string = name;
+	Entity *e = alloc_entity_procedure(nullptr, token, type, pl->tags);
+	e->file = expr->file();
+
+	// NOTE(bill): this is to prevent a race condition since these procedure literals can be created anywhere at any time
+	pl->decl->code_gen_module = m;
+	e->decl_info = pl->decl;
+	pl->decl->entity = e;
+	e->flags |= EntityFlag_ProcBodyChecked;
+
+	lbProcedure *p = lb_create_procedure(m, e);
+	GB_ASSERT(e->code_gen_module == m);
+
+	lbValue value = {};
+	value.value = p->value;
+	value.type = p->type;
+
+	map_set(&gen->anonymous_proc_lits, expr, p);
+	array_add(&m->procedures_to_generate, p);
+	if (parent != nullptr) {
+		array_add(&parent->children, p);
+	} else {
+		string_map_set(&m->members, name, value);
+	}
+	return value;
+}
+
+
 gb_internal lbAddr lb_add_global_generated(lbModule *m, Type *type, lbValue value, Entity **entity_) {
 gb_internal lbAddr lb_add_global_generated(lbModule *m, Type *type, lbValue value, Entity **entity_) {
 	GB_ASSERT(type != nullptr);
 	GB_ASSERT(type != nullptr);
 	type = default_type(type);
 	type = default_type(type);