Browse Source

Enforce success failure pairings of `compare_exchange_*_explicit` at compile time

gingerBill 3 years ago
parent
commit
6bc0c611ab
3 changed files with 97 additions and 10 deletions
  1. 1 1
      core/sync/primitives_atomic.odin
  2. 85 7
      src/check_builtin.cpp
  3. 11 2
      src/types.cpp

+ 1 - 1
core/sync/primitives_atomic.odin

@@ -77,7 +77,7 @@ atomic_mutex_unlock :: proc(m: ^Atomic_Mutex) {
 
 
 // atomic_mutex_try_lock tries to lock m, will return true on success, and false on failure
 // atomic_mutex_try_lock tries to lock m, will return true on success, and false on failure
 atomic_mutex_try_lock :: proc(m: ^Atomic_Mutex) -> bool {
 atomic_mutex_try_lock :: proc(m: ^Atomic_Mutex) -> bool {
-	_, ok := atomic_compare_exchange_strong_explicit(&m.state, .Unlocked, .Locked, .acquire, .acquire)
+	_, ok := atomic_compare_exchange_strong_explicit(&m.state, .Unlocked, .Locked, .acquire, .consume)
 	return ok
 	return ok
 }
 }
 
 

+ 85 - 7
src/check_builtin.cpp

@@ -379,7 +379,7 @@ bool check_builtin_objc_procedure(CheckerContext *c, Operand *operand, Ast *call
 	}
 	}
 }
 }
 
 
-bool check_atomic_memory_order_argument(CheckerContext *c, Ast *expr, String const &builtin_name, char const *extra_message = nullptr) {
+bool check_atomic_memory_order_argument(CheckerContext *c, Ast *expr, String const &builtin_name, OdinAtomicMemoryOrder *memory_order_, char const *extra_message = nullptr) {
 	Operand x = {};
 	Operand x = {};
 	check_expr_with_type_hint(c, &x, expr, t_atomic_memory_order);
 	check_expr_with_type_hint(c, &x, expr, t_atomic_memory_order);
 	if (x.mode == Addressing_Invalid) {
 	if (x.mode == Addressing_Invalid) {
@@ -400,6 +400,9 @@ bool check_atomic_memory_order_argument(CheckerContext *c, Ast *expr, String con
 		error(x.expr, "Illegal Atomic_Memory_Order value, got %lld", cast(long long)value);
 		error(x.expr, "Illegal Atomic_Memory_Order value, got %lld", cast(long long)value);
 		return false;
 		return false;
 	}
 	}
+	if (memory_order_) {
+		*memory_order_ = cast(OdinAtomicMemoryOrder)value;
+	}
 
 
 	return true;
 	return true;
 
 
@@ -3232,7 +3235,7 @@ bool check_builtin_procedure(CheckerContext *c, Operand *operand, Ast *call, i32
 
 
 	case BuiltinProc_atomic_thread_fence:
 	case BuiltinProc_atomic_thread_fence:
 	case BuiltinProc_atomic_signal_fence:
 	case BuiltinProc_atomic_signal_fence:
-		if (!check_atomic_memory_order_argument(c, ce->args[0], builtin_name)) {
+		if (!check_atomic_memory_order_argument(c, ce->args[0], builtin_name, nullptr)) {
 			return false;
 			return false;
 		}
 		}
 		operand->mode = Addressing_NoValue;
 		operand->mode = Addressing_NoValue;
@@ -3269,9 +3272,17 @@ bool check_builtin_procedure(CheckerContext *c, Operand *operand, Ast *call, i32
 			check_expr_with_type_hint(c, &x, ce->args[1], elem);
 			check_expr_with_type_hint(c, &x, ce->args[1], elem);
 			check_assignment(c, &x, elem, builtin_name);
 			check_assignment(c, &x, elem, builtin_name);
 
 
-			if (!check_atomic_memory_order_argument(c, ce->args[2], builtin_name)) {
+			OdinAtomicMemoryOrder memory_order = {};
+			if (!check_atomic_memory_order_argument(c, ce->args[2], builtin_name, &memory_order)) {
 				return false;
 				return false;
 			}
 			}
+			switch (memory_order) {
+			case OdinAtomicMemoryOrder_consume:
+			case OdinAtomicMemoryOrder_acquire:
+			case OdinAtomicMemoryOrder_acq_rel:
+				error(ce->args[2], "Illegal memory order .%s for %.*s", OdinAtomicMemoryOrder_strings[memory_order], LIT(builtin_name));
+				break;
+			}
 
 
 			operand->type = nullptr;
 			operand->type = nullptr;
 			operand->mode = Addressing_NoValue;
 			operand->mode = Addressing_NoValue;
@@ -3303,10 +3314,18 @@ bool check_builtin_procedure(CheckerContext *c, Operand *operand, Ast *call, i32
 				return false;
 				return false;
 			}
 			}
 
 
-			if (!check_atomic_memory_order_argument(c, ce->args[1], builtin_name)) {
+			OdinAtomicMemoryOrder memory_order = {};
+			if (!check_atomic_memory_order_argument(c, ce->args[1], builtin_name, &memory_order)) {
 				return false;
 				return false;
 			}
 			}
 
 
+			switch (memory_order) {
+			case OdinAtomicMemoryOrder_release:
+			case OdinAtomicMemoryOrder_acq_rel:
+				error(ce->args[1], "Illegal memory order .%s for %.*s", OdinAtomicMemoryOrder_strings[memory_order], LIT(builtin_name));
+				break;
+			}
+
 			operand->type = elem;
 			operand->type = elem;
 			operand->mode = Addressing_Value;
 			operand->mode = Addressing_Value;
 			break;
 			break;
@@ -3352,7 +3371,7 @@ bool check_builtin_procedure(CheckerContext *c, Operand *operand, Ast *call, i32
 			check_assignment(c, &x, elem, builtin_name);
 			check_assignment(c, &x, elem, builtin_name);
 
 
 
 
-			if (!check_atomic_memory_order_argument(c, ce->args[2], builtin_name)) {
+			if (!check_atomic_memory_order_argument(c, ce->args[2], builtin_name, nullptr)) {
 				return false;
 				return false;
 			}
 			}
 
 
@@ -3396,13 +3415,72 @@ bool check_builtin_procedure(CheckerContext *c, Operand *operand, Ast *call, i32
 			check_assignment(c, &x, elem, builtin_name);
 			check_assignment(c, &x, elem, builtin_name);
 			check_assignment(c, &y, elem, builtin_name);
 			check_assignment(c, &y, elem, builtin_name);
 
 
-			if (!check_atomic_memory_order_argument(c, ce->args[3], builtin_name, "success ordering")) {
+			OdinAtomicMemoryOrder success_memory_order = {};
+			OdinAtomicMemoryOrder failure_memory_order = {};
+			if (!check_atomic_memory_order_argument(c, ce->args[3], builtin_name, &success_memory_order, "success ordering")) {
 				return false;
 				return false;
 			}
 			}
-			if (!check_atomic_memory_order_argument(c, ce->args[4], builtin_name, "failure ordering")) {
+			if (!check_atomic_memory_order_argument(c, ce->args[4], builtin_name, &failure_memory_order, "failure ordering")) {
 				return false;
 				return false;
 			}
 			}
 
 
+			bool invalid_combination = false;
+
+			switch (success_memory_order) {
+			case OdinAtomicMemoryOrder_relaxed:
+			case OdinAtomicMemoryOrder_release:
+				if (failure_memory_order != OdinAtomicMemoryOrder_relaxed) {
+					invalid_combination = true;
+				}
+				break;
+			case OdinAtomicMemoryOrder_consume:
+				switch (failure_memory_order) {
+				case OdinAtomicMemoryOrder_relaxed:
+				case OdinAtomicMemoryOrder_consume:
+					break;
+				default:
+					invalid_combination = true;
+					break;
+				}
+				break;
+			case OdinAtomicMemoryOrder_acquire:
+			case OdinAtomicMemoryOrder_acq_rel:
+				switch (failure_memory_order) {
+				case OdinAtomicMemoryOrder_relaxed:
+				case OdinAtomicMemoryOrder_consume:
+				case OdinAtomicMemoryOrder_acquire:
+					break;
+				default:
+					invalid_combination = true;
+					break;
+				}
+				break;
+			case OdinAtomicMemoryOrder_seq_cst:
+				switch (failure_memory_order) {
+				case OdinAtomicMemoryOrder_relaxed:
+				case OdinAtomicMemoryOrder_consume:
+				case OdinAtomicMemoryOrder_acquire:
+				case OdinAtomicMemoryOrder_seq_cst:
+					break;
+				default:
+					invalid_combination = true;
+					break;
+				}
+				break;
+			default:
+				invalid_combination = true;
+				break;
+			}
+
+
+			if (invalid_combination) {
+				error(ce->args[3], "Illegal memory order pairing for %.*s, success = .%s, failure = .%s",
+					LIT(builtin_name),
+					OdinAtomicMemoryOrder_strings[success_memory_order],
+					OdinAtomicMemoryOrder_strings[failure_memory_order]
+				);
+			}
+
 			operand->mode = Addressing_OptionalOk;
 			operand->mode = Addressing_OptionalOk;
 			operand->type = elem;
 			operand->type = elem;
 			break;
 			break;

+ 11 - 2
src/types.cpp

@@ -693,8 +693,8 @@ gb_global Type *t_objc_SEL   = nullptr;
 gb_global Type *t_objc_Class = nullptr;
 gb_global Type *t_objc_Class = nullptr;
 
 
 enum OdinAtomicMemoryOrder : i32 {
 enum OdinAtomicMemoryOrder : i32 {
-	OdinAtomicMemoryOrder_relaxed = 0,
-	OdinAtomicMemoryOrder_consume = 1,
+	OdinAtomicMemoryOrder_relaxed = 0, // unordered
+	OdinAtomicMemoryOrder_consume = 1, // monotonic
 	OdinAtomicMemoryOrder_acquire = 2,
 	OdinAtomicMemoryOrder_acquire = 2,
 	OdinAtomicMemoryOrder_release = 3,
 	OdinAtomicMemoryOrder_release = 3,
 	OdinAtomicMemoryOrder_acq_rel = 4,
 	OdinAtomicMemoryOrder_acq_rel = 4,
@@ -702,6 +702,15 @@ enum OdinAtomicMemoryOrder : i32 {
 	OdinAtomicMemoryOrder_COUNT,
 	OdinAtomicMemoryOrder_COUNT,
 };
 };
 
 
+char const *OdinAtomicMemoryOrder_strings[OdinAtomicMemoryOrder_COUNT] = {
+	"relaxed",
+	"consume",
+	"acquire",
+	"release",
+	"acq_rel",
+	"seq_cst",
+};
+
 gb_global Type *t_atomic_memory_order = nullptr;
 gb_global Type *t_atomic_memory_order = nullptr;