Simplify task function validation

Author: realFlowControl

README.md

@@ -53,9 +53,9 @@ Task Dependencies
 
 A task receives its opcodes, captured values, and arguments. It does not inherit the submitting runtime's user-defined functions, classes, or constants.
 
-Load the same function source used by the submitting runtime through the runtime bootstrap, or pass a `Closure` created with `Closure::fromCallable()` as a task argument. Direct function calls are validated against the destination runtime before the task starts. A missing or different function prevents all task code from executing and is reported by `Future::value()`; a `try/catch` inside the task cannot catch this task-contract error.
+Statically named function calls in a task are checked against the destination runtime before the task starts. A missing function prevents all task code from executing and is reported by `Future::value()`; a `try/catch` inside the task cannot catch this error.
 
-Do not rely on an `include` inside a task to provide a direct function dependency. If the function existed when the task was compiled, validation happens before the `include` executes. Use the bootstrap for ambient functions and task arguments for explicit dependencies.
+Define required functions in the runtime bootstrap, or pass a `Closure` created with `Closure::fromCallable()` as a task argument. Do not rely on an `include` inside the task because validation runs before it. Calls whose names are only known at runtime cannot be checked before execution.
 
 Development
 ===========

src/cache.c

@@ -23,7 +23,6 @@
 static struct {
 	pthread_mutex_t mutex;
 	HashTable       functions;
-	HashTable       identities;
 #if PHP_VERSION_ID < 80200
 	HashTable statics;
 #endif
@@ -57,15 +56,6 @@ typedef struct _php_parallel_cache_entry_t {
 #endif
 } php_parallel_cache_entry_t;
 
-typedef struct _php_parallel_cache_identity_t {
-	const zend_op *opcodes;
-	zend_string   *filename;
-	zif_handler    handler;
-	uint32_t       line_start;
-	uint32_t       line_end;
-	uint8_t        type;
-} php_parallel_cache_identity_t;
-
 /* {{{ */
 static zend_always_inline void *php_parallel_cache_alloc(size_t size)
 {
@@ -150,73 +140,6 @@ static zend_always_inline HashTable *php_parallel_cache_statics(HashTable *stati
 } /* }}} */
 #endif
 
-static zend_always_inline void php_parallel_cache_identities(const zend_function *source, const zend_function *cached)
-{ /* {{{ */
-	const zend_op *source_opline = source->op_array.opcodes;
-	const zend_op *cached_opline = cached->op_array.opcodes;
-	const zend_op *end = source_opline + source->op_array.last;
-
-	while (source_opline < end) {
-		if (source_opline->opcode == ZEND_INIT_FCALL) {
-			zend_function *function = zend_fetch_function(Z_STR_P(RT_CONSTANT(source_opline, source_opline->op2)));
-
-			if (function) {
-				php_parallel_cache_identity_t *identity = php_parallel_cache_alloc(sizeof(*identity));
-
-				memset(identity, 0, sizeof(*identity));
-				identity->type = function->type;
-
-				if (ZEND_USER_CODE(function->type)) {
-					identity->opcodes = function->op_array.opcodes;
-					identity->filename = function->op_array.filename
-					                         ? php_parallel_copy_string_interned(function->op_array.filename)
-					                         : NULL;
-					identity->line_start = function->op_array.line_start;
-					identity->line_end = function->op_array.line_end;
-				} else {
-					identity->handler = function->internal_function.handler;
-				}
-
-				zend_hash_index_update_ptr(&PCG(identities), (zend_ulong)cached_opline, identity);
-			}
-		}
-
-		source_opline++;
-		cached_opline++;
-	}
-} /* }}} */
-
-bool php_parallel_cache_function_matches(const zend_op *opline, const zend_function *function,
-                                         php_parallel_cache_function_source_t *source)
-{ /* {{{ */
-	php_parallel_cache_identity_t *identity;
-	bool                           matches = false;
-
-	pthread_mutex_lock(&PCG(mutex));
-	identity = zend_hash_index_find_ptr(&PCG(identities), (zend_ulong)opline);
-	pthread_mutex_unlock(&PCG(mutex));
-
-	if (!identity) {
-		return false;
-	}
-
-	source->filename = identity->filename;
-	source->line = identity->line_start;
-
-	if (ZEND_USER_CODE(identity->type)) {
-		matches =
-		    ZEND_USER_CODE(function->type) && (identity->opcodes == function->op_array.opcodes ||
-		                                       (identity->filename && function->op_array.filename &&
-		                                        zend_string_equals(identity->filename, function->op_array.filename) &&
-		                                        identity->line_start == function->op_array.line_start &&
-		                                        identity->line_end == function->op_array.line_end));
-	} else {
-		matches = function->type == identity->type && function->internal_function.handler == identity->handler;
-	}
-
-	return matches;
-} /* }}} */
-
 static zend_always_inline void php_parallel_cache_type(zend_type *type)
 { /* {{{ */
 	zend_type *single;
@@ -252,7 +175,9 @@ static zend_always_inline void php_parallel_cache_type(zend_type *type)
 static zend_op_array *php_parallel_cache_create(const zend_function *source PARALLEL_CACHE_STATICS_PARAM)
 {
 	zend_op_array *cached = php_parallel_cache_copy_mem((void *)source, sizeof(zend_op_array));
-	bool           shared = cached->refcount == NULL;
+	uint32_t      *literal_map = NULL;
+	uint32_t      *offset_map = NULL;
+	uint32_t       new_last_literal = cached->last_literal;
 
 	cached->fn_flags |= ZEND_ACC_IMMUTABLE;
 
@@ -285,20 +210,58 @@ static zend_op_array *php_parallel_cache_create(const zend_function *source PARA
 	}
 #endif
 
-	if (shared) {
-		php_parallel_cache_identities(source, (zend_function *)cached);
+	if (!cached->refcount) {
 		goto _php_parallel_cached_function_return;
 	}
 
 	cached->refcount = NULL;
 
 	if (cached->last_literal) {
-		zval *literal = source->op_array.literals, *end = literal + cached->last_literal;
-		zval *slot = php_parallel_cache_alloc(sizeof(zval) * cached->last_literal);
+		zend_op *src_opline = source->op_array.opcodes;
+		zend_op *src_end = src_opline + source->op_array.last;
+
+		// A map to keep track of which literals are referenced by the
+		// `ZEND_INIT_FCALL` opcodes we found so that we can expand those later
+		literal_map = emalloc(sizeof(uint32_t) * cached->last_literal);
+		memset(literal_map, 0, sizeof(uint32_t) * cached->last_literal);
+
+		// Search for `ZEND_INIT_FCALL` opcodes and remember the indexes for the
+		// literals, as we are rewriting them later to `ZEND_INIT_FCALL_BY_NAME`
+		// which requires a second, lower cased literal just in the next literal
+		// slot.
+		while (src_opline < src_end) {
+			if (src_opline->opcode == ZEND_INIT_FCALL && src_opline->op2_type == IS_CONST) {
+				uint32_t idx;
+#if ZEND_USE_ABS_CONST_ADDR
+				idx = (zval *)src_opline->op2.zv - source->op_array.literals;
+#else
+				idx = ((zval *)((char *)src_opline + src_opline->op2.constant) - source->op_array.literals);
+#endif
+				if (idx < cached->last_literal) {
+					if (literal_map[idx] == 0) {
+						literal_map[idx] = 1;
+						new_last_literal++;
+					}
+				}
+			}
+			src_opline++;
+		}
+	}
+
+	if (new_last_literal) {
+		zval    *literal = source->op_array.literals;
+		zval    *slot = php_parallel_cache_alloc(sizeof(zval) * new_last_literal);
+		uint32_t idx = 0;
+
+		offset_map = emalloc(sizeof(uint32_t) * cached->last_literal);
 
 		cached->literals = slot;
 
-		while (literal < end) {
+		for (uint32_t i = 0; i < cached->last_literal; i++) {
+			/* Record the mapping from old literal index (i) to new literal index (idx)
+			   so we can update opcode operands later. */
+			offset_map[i] = idx;
+
 			if (Z_TYPE_P(literal) == IS_ARRAY) {
 				ZVAL_ARR(slot, php_parallel_copy_hash_persistent(Z_ARRVAL_P(literal), php_parallel_copy_string_interned,
 				                                                 php_parallel_cache_copy_mem,
@@ -310,25 +273,62 @@ static zend_op_array *php_parallel_cache_create(const zend_function *source PARA
 			}
 
 			Z_TYPE_FLAGS_P(slot) &= ~(IS_TYPE_REFCOUNTED | IS_TYPE_COLLECTABLE);
+
+			/* If this literal was used by INIT_FCALL, insert its lowercased version next. */
+			if (literal_map[i]) {
+				zend_string *lower = zend_string_tolower(Z_STR_P(slot));
+				slot++;
+				idx++;
+				ZVAL_STR(slot, php_parallel_copy_string_interned(lower));
+				zend_string_release(lower);
+				Z_TYPE_FLAGS_P(slot) &= ~(IS_TYPE_REFCOUNTED | IS_TYPE_COLLECTABLE);
+			}
+
 			literal++;
 			slot++;
+			idx++;
+		}
+		cached->last_literal = new_last_literal;
+	}
+
+	if (cached->last_var) {
+		zend_string **vars = cached->vars;
+		uint32_t      it = 0, end = cached->last_var;
+		zend_string **heap = php_parallel_cache_alloc(cached->last_var * sizeof(zend_string *));
+
+		while (it < end) {
+			heap[it] = php_parallel_copy_string_interned(vars[it]);
+			it++;
 		}
+		cached->vars = heap;
 	}
 
 	if (cached->last) {
 		zend_op *opcodes = php_parallel_cache_copy_mem(cached->opcodes, sizeof(zend_op) * cached->last);
 		zend_op *opline = opcodes, *end = opline + cached->last;
 
 		while (opline < end) {
+			/* Replace ZEND_INIT_FCALL with ZEND_INIT_FCALL_BY_NAME.
+			   We must clear op1_type (IS_UNUSED) and op1.var (0) to invalidate the
+			   original thread's cache slot. */
+			if (opline->opcode == ZEND_INIT_FCALL) {
+				opline->opcode = ZEND_INIT_FCALL_BY_NAME;
+				opline->op1_type = IS_UNUSED;
+				opline->op1.var = 0;
+				ZEND_VM_SET_OPCODE_HANDLER(opline);
+			}
+
+			/* Remap IS_CONST operands to their new locations in the expanded literal table
+			   using the offset_map we built earlier. */
 			if (opline->op1_type == IS_CONST) {
 				uint32_t idx;
 				zend_op *src_opline = source->op_array.opcodes + (opline - opcodes);
 #if ZEND_USE_ABS_CONST_ADDR
 				idx = (zval *)src_opline->op1.zv - source->op_array.literals;
-				opline->op1.zv = &cached->literals[idx];
+				opline->op1.zv = &cached->literals[offset_map[idx]];
 #else
 				idx = ((zval *)((char *)src_opline + src_opline->op1.constant) - source->op_array.literals);
-				opline->op1.constant = (char *)&cached->literals[idx] - (char *)opline;
+				opline->op1.constant = (char *)&cached->literals[offset_map[idx]] - (char *)opline;
 #endif
 				if (opline->opcode == ZEND_SEND_VAL || opline->opcode == ZEND_SEND_VAL_EX ||
 				    opline->opcode == ZEND_QM_ASSIGN) {
@@ -340,17 +340,12 @@ static zend_op_array *php_parallel_cache_create(const zend_function *source PARA
 				zend_op *src_opline = source->op_array.opcodes + (opline - opcodes);
 #if ZEND_USE_ABS_CONST_ADDR
 				idx = (zval *)src_opline->op2.zv - source->op_array.literals;
-				opline->op2.zv = &cached->literals[idx];
+				opline->op2.zv = &cached->literals[offset_map[idx]];
 #else
 				idx = ((zval *)((char *)src_opline + src_opline->op2.constant) - source->op_array.literals);
-				opline->op2.constant = (char *)&cached->literals[idx] - (char *)opline;
+				opline->op2.constant = (char *)&cached->literals[offset_map[idx]] - (char *)opline;
 #endif
 			}
-
-			if (opline->opcode == ZEND_INIT_FCALL) {
-				Z_EXTRA_P(RT_CONSTANT(opline, opline->op2)) = 0;
-				ZEND_VM_SET_OPCODE_HANDLER(opline);
-			}
 #if ZEND_USE_ABS_JMP_ADDR
 			switch (opline->opcode) {
 			case ZEND_JMP:
@@ -385,18 +380,12 @@ static zend_op_array *php_parallel_cache_create(const zend_function *source PARA
 		cached->opcodes = opcodes;
 	}
 
-	php_parallel_cache_identities(source, (zend_function *)cached);
-
-	if (cached->last_var) {
-		zend_string **vars = cached->vars;
-		uint32_t      it = 0, end = cached->last_var;
-		zend_string **heap = php_parallel_cache_alloc(cached->last_var * sizeof(zend_string *));
+	if (literal_map) {
+		efree(literal_map);
+	}
 
-		while (it < end) {
-			heap[it] = php_parallel_copy_string_interned(vars[it]);
-			it++;
-		}
-		cached->vars = heap;
+	if (offset_map) {
+		efree(offset_map);
 	}
 
 	if (cached->arg_info) {
@@ -547,7 +536,6 @@ zend_function *php_parallel_cache_function(const zend_function *source)
 PHP_MINIT_FUNCTION(PARALLEL_CACHE)
 {
 	zend_hash_init(&PCG(functions), 32, NULL, NULL, 1);
-	zend_hash_init(&PCG(identities), 32, NULL, NULL, 1);
 #if PHP_VERSION_ID < 80200
 	zend_hash_init(&PCG(statics), 32, NULL, NULL, 1);
 #endif
@@ -565,7 +553,6 @@ PHP_MINIT_FUNCTION(PARALLEL_CACHE)
 PHP_MSHUTDOWN_FUNCTION(PARALLEL_CACHE)
 {
 	zend_hash_destroy(&PCG(functions));
-	zend_hash_destroy(&PCG(identities));
 #if PHP_VERSION_ID < 80200
 	zend_hash_destroy(&PCG(statics));
 #endif

src/cache.h

@@ -38,14 +38,6 @@ zend_function *php_parallel_cache_function(const zend_function *source);
  */
 zend_function *php_parallel_cache_closure(const zend_function *source, zend_function *destination);
 
-typedef struct _php_parallel_cache_function_source_t {
-	zend_string *filename;
-	uint32_t     line;
-} php_parallel_cache_function_source_t;
-
-bool php_parallel_cache_function_matches(const zend_op *opline, const zend_function *function,
-                                         php_parallel_cache_function_source_t *source);
-
 PHP_MINIT_FUNCTION(PARALLEL_CACHE);
 PHP_MSHUTDOWN_FUNCTION(PARALLEL_CACHE);
 #endif