Skip to content

Commit e90b328

Browse files
realFlowControlrealFlowControl
authored
convert ZEND_INIT_FCALL to ZEND_INIT_FCALL_BY_NAME (#356)
* convert `ZEND_INIT_FCALL` to `ZEND_INIT_DYNAMIC_CALL` * instead, switch to `ZEND_INIT_FCALL_BY_NAME` and update literals table * Adding tests * small fixes
1 parent 573121d commit e90b328

File tree

6 files changed

+189
-27
lines changed

6 files changed

+189
-27
lines changed

‎src/cache.c‎

Lines changed: 100 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ static zend_always_inline void php_parallel_cache_type(zend_type *type) { /* {{{
106106
if (ZEND_TYPE_USES_ARENA(*type)) {
107107
ZEND_TYPE_FULL_MASK(*type) &= ~_ZEND_TYPE_ARENA_BIT;
108108
}
109-
109+
110110
ZEND_TYPE_SET_PTR(*type, list);
111111
}
112112

@@ -125,6 +125,9 @@ static zend_always_inline void php_parallel_cache_type(zend_type *type) { /* {{{
125125
/* {{{ */
126126
static zend_op_array* php_parallel_cache_create(const zend_function *source, bool statics) {
127127
zend_op_array *cached = php_parallel_cache_copy_mem((void*) source, sizeof(zend_op_array));
128+
uint32_t *literal_map = NULL;
129+
uint32_t *offset_map = NULL;
130+
uint32_t new_last_literal = cached->last_literal;
128131

129132
cached->fn_flags |= ZEND_ACC_IMMUTABLE;
130133

@@ -144,13 +147,13 @@ static zend_op_array* php_parallel_cache_create(const zend_function *source, boo
144147
#if PHP_VERSION_ID >= 80100
145148
if (cached->num_dynamic_func_defs) {
146149
uint32_t it = 0;
147-
150+
148151
cached->dynamic_func_defs = php_parallel_cache_copy_mem(
149152
cached->dynamic_func_defs,
150153
sizeof(zend_op_array*) * cached->num_dynamic_func_defs);
151-
154+
152155
while (it < cached->num_dynamic_func_defs) {
153-
cached->dynamic_func_defs[it] =
156+
cached->dynamic_func_defs[it] =
154157
(zend_op_array*) php_parallel_cache_create(
155158
(zend_function*) cached->dynamic_func_defs[it], statics);
156159
it++;
@@ -165,15 +168,51 @@ static zend_op_array* php_parallel_cache_create(const zend_function *source, boo
165168
cached->refcount = NULL;
166169

167170
if (cached->last_literal) {
168-
zval *literal = cached->literals,
169-
*end = literal + cached->last_literal;
170-
zval *slot = php_parallel_cache_copy_mem(
171-
cached->literals,
172-
sizeof(zval) * cached->last_literal);
171+
zend_op *src_opline = source->op_array.opcodes;
172+
zend_op *src_end = src_opline + source->op_array.last;
173+
174+
// A map to keep track of which literals are referenced by the
175+
// `ZEND_INIT_FCALL` opcodes we found so that we can expand those later
176+
literal_map = emalloc(sizeof(uint32_t) * cached->last_literal);
177+
memset(literal_map, 0, sizeof(uint32_t) * cached->last_literal);
178+
179+
// Search for `ZEND_INIT_FCALL` opcodes and remember the indexes for the
180+
// literals, as we are rewriting them later to `ZEND_INIT_FCALL_BY_NAME`
181+
// which requires a second, lower cased literal just in the next literal
182+
// slot.
183+
while (src_opline < src_end) {
184+
if (src_opline->opcode == ZEND_INIT_FCALL && src_opline->op2_type == IS_CONST) {
185+
uint32_t idx;
186+
#if ZEND_USE_ABS_CONST_ADDR
187+
idx = (zval*)src_opline->op2.zv - source->op_array.literals;
188+
#else
189+
idx = ((zval*)((char*)src_opline + src_opline->op2.constant) - source->op_array.literals);
190+
#endif
191+
if (idx < cached->last_literal) {
192+
if (literal_map[idx] == 0) {
193+
literal_map[idx] = 1;
194+
new_last_literal++;
195+
}
196+
}
197+
}
198+
src_opline++;
199+
}
200+
}
201+
202+
if (new_last_literal) {
203+
zval *literal = source->op_array.literals;
204+
zval *slot = php_parallel_cache_alloc(sizeof(zval) * new_last_literal);
205+
uint32_t idx = 0;
206+
207+
offset_map = emalloc(sizeof(uint32_t) * cached->last_literal);
173208

174209
cached->literals = slot;
175210

176-
while (literal < end) {
211+
for (uint32_t i = 0; i < cached->last_literal; i++) {
212+
/* Record the mapping from old literal index (i) to new literal index (idx)
213+
so we can update opcode operands later. */
214+
offset_map[i] = idx;
215+
177216
if (Z_TYPE_P(literal) == IS_ARRAY) {
178217
ZVAL_ARR(slot,
179218
php_parallel_copy_hash_persistent(
@@ -183,12 +222,27 @@ static zend_op_array* php_parallel_cache_create(const zend_function *source, boo
183222
} else if (Z_TYPE_P(literal) == IS_STRING) {
184223
ZVAL_STR(slot,
185224
php_parallel_copy_string_interned(Z_STR_P(literal)));
225+
} else {
226+
*slot = *literal;
227+
}
228+
229+
Z_TYPE_FLAGS_P(slot) &= ~(IS_TYPE_REFCOUNTED|IS_TYPE_COLLECTABLE);
230+
231+
/* If this literal was used by INIT_FCALL, insert its lowercased version next. */
232+
if (literal_map[i]) {
233+
zend_string *lower = zend_string_tolower(Z_STR_P(slot));
234+
slot++;
235+
idx++;
236+
ZVAL_STR(slot, php_parallel_copy_string_interned(lower));
237+
zend_string_release(lower);
238+
Z_TYPE_FLAGS_P(slot) &= ~(IS_TYPE_REFCOUNTED|IS_TYPE_COLLECTABLE);
186239
}
187240

188-
Z_TYPE_FLAGS_P(slot) &= ~(IS_TYPE_REFCOUNTED|IS_TYPE_COLLECTABLE);
189241
literal++;
190242
slot++;
243+
idx++;
191244
}
245+
cached->last_literal = new_last_literal;
192246
}
193247

194248
if (cached->last_var) {
@@ -211,15 +265,27 @@ static zend_op_array* php_parallel_cache_create(const zend_function *source, boo
211265
*end = opline + cached->last;
212266

213267
while (opline < end) {
268+
/* Replace ZEND_INIT_FCALL with ZEND_INIT_FCALL_BY_NAME.
269+
We must clear op1_type (IS_UNUSED) and op1.var (0) to invalidate the
270+
original thread's cache slot. */
271+
if (opline->opcode == ZEND_INIT_FCALL) {
272+
opline->opcode = ZEND_INIT_FCALL_BY_NAME;
273+
opline->op1_type = IS_UNUSED;
274+
opline->op1.var = 0;
275+
ZEND_VM_SET_OPCODE_HANDLER(opline);
276+
}
277+
278+
/* Remap IS_CONST operands to their new locations in the expanded literal table
279+
using the offset_map we built earlier. */
214280
if (opline->op1_type == IS_CONST) {
281+
uint32_t idx;
282+
zend_op *src_opline = source->op_array.opcodes + (opline - opcodes);
215283
#if ZEND_USE_ABS_CONST_ADDR
216-
opline->op1.zv = (zval*)((char*)opline->op1.zv + ((char*)cached->literals - (char*)source->op_array.literals));
284+
idx = (zval*)src_opline->op1.zv - source->op_array.literals;
285+
opline->op1.zv = &cached->literals[offset_map[idx]];
217286
#else
218-
opline->op1.constant =
219-
(char*)(cached->literals +
220-
((zval*)((char*)(source->op_array.opcodes + (opline - opcodes)) +
221-
(int32_t)opline->op1.constant) - source->op_array.literals)) -
222-
(char*)opline;
287+
idx = ((zval*)((char*)src_opline + src_opline->op1.constant) - source->op_array.literals);
288+
opline->op1.constant = (char*)&cached->literals[offset_map[idx]] - (char*)opline;
223289
#endif
224290
if (opline->opcode == ZEND_SEND_VAL
225291
|| opline->opcode == ZEND_SEND_VAL_EX
@@ -228,14 +294,14 @@ static zend_op_array* php_parallel_cache_create(const zend_function *source, boo
228294
}
229295
}
230296
if (opline->op2_type == IS_CONST) {
297+
uint32_t idx;
298+
zend_op *src_opline = source->op_array.opcodes + (opline - opcodes);
231299
#if ZEND_USE_ABS_CONST_ADDR
232-
opline->op2.zv = (zval*)((char*)opline->op2.zv + ((char*)cached->literals - (char*)source->op_array.literals));
300+
idx = (zval*)src_opline->op2.zv - source->op_array.literals;
301+
opline->op2.zv = &cached->literals[offset_map[idx]];
233302
#else
234-
opline->op2.constant =
235-
(char*)(cached->literals +
236-
((zval*)((char*)(source->op_array.opcodes + (opline - opcodes)) +
237-
(int32_t)opline->op2.constant) - source->op_array.literals)) -
238-
(char*)opline;
303+
idx = ((zval*)((char*)src_opline + src_opline->op2.constant) - source->op_array.literals);
304+
opline->op2.constant = (char*)&cached->literals[offset_map[idx]] - (char*)opline;
239305
#endif
240306
}
241307
#if ZEND_USE_ABS_JMP_ADDR
@@ -272,6 +338,14 @@ static zend_op_array* php_parallel_cache_create(const zend_function *source, boo
272338
cached->opcodes = opcodes;
273339
}
274340

341+
if (literal_map) {
342+
efree(literal_map);
343+
}
344+
345+
if (offset_map) {
346+
efree(offset_map);
347+
}
348+
275349
if (cached->arg_info) {
276350
zend_arg_info *it = cached->arg_info,
277351
*end = it + cached->num_args,
@@ -291,7 +365,7 @@ static zend_op_array* php_parallel_cache_create(const zend_function *source, boo
291365
info->name =
292366
php_parallel_copy_string_interned(it->name);
293367
}
294-
368+
295369
php_parallel_cache_type(&info->type);
296370

297371
info++;
@@ -335,7 +409,7 @@ static zend_op_array* php_parallel_cache_create(const zend_function *source, boo
335409
/* {{{ */
336410
static zend_always_inline zend_function* php_parallel_cache_function_ex(const zend_function *source, bool statics) {
337411
zend_op_array *cached;
338-
412+
339413
pthread_mutex_lock(&PCG(mutex));
340414

341415
if ((cached = zend_hash_index_find_ptr(&PCG(table), (zend_ulong) source->op_array.opcodes))) {
@@ -346,7 +420,7 @@ static zend_always_inline zend_function* php_parallel_cache_function_ex(const ze
346420

347421
zend_hash_index_add_ptr(
348422
&PCG(table),
349-
(zend_ulong) source->op_array.opcodes,
423+
(zend_ulong) source->op_array.opcodes,
350424
cached);
351425

352426
_php_parallel_cached_function_return:

‎src/copy.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -987,7 +987,7 @@ static void php_parallel_copy_zval_persistent(
987987

988988
zend_function* php_parallel_copy_function(const zend_function *function, bool persistent) {
989989
if (persistent) {
990-
function =
990+
function =
991991
php_parallel_cache_function(function);
992992

993993
php_parallel_dependencies_store(function);

‎tests/base/init_fcall_fix_001.phpt‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
--TEST--
2+
Check INIT_FCALL fix with \parallel\run() (undefined function)
3+
--SKIPIF--
4+
<?php
5+
if (!extension_loaded('parallel')) {
6+
echo 'skip';
7+
}
8+
if (ini_get("opcache.enable_cli")) {
9+
die("skip opcache must not be loaded");
10+
}
11+
?>
12+
--FILE--
13+
<?php
14+
function dummy_func() { return "FOO"; }
15+
\parallel\run(function(){
16+
try {
17+
// This will be compiled as INIT_FCALL but should be converted to INIT_FCALL_BY_NAME
18+
// and fail gracefully because it doesn't exist in the thread.
19+
return dummy_func();
20+
} catch (Error $e) {
21+
echo "Caught: " . $e->getMessage();
22+
}
23+
})->value();
24+
?>
25+
--EXPECT--
26+
Caught: Call to undefined function dummy_func()

‎tests/base/init_fcall_fix_002.phpt‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
--TEST--
2+
Check INIT_FCALL fix with Runtime::run() (undefined function)
3+
--SKIPIF--
4+
<?php
5+
if (!extension_loaded('parallel')) {
6+
echo 'skip';
7+
}
8+
if (ini_get("opcache.enable_cli")) {
9+
die("skip opcache must not be loaded");
10+
}
11+
?>
12+
--FILE--
13+
<?php
14+
function dummy_func() { return "FOO"; }
15+
$runtime = new \parallel\Runtime();
16+
$runtime->run(function(){
17+
try {
18+
// This will be compiled as INIT_FCALL but should be converted to INIT_FCALL_BY_NAME
19+
// and fail gracefully because it doesn't exist in the thread.
20+
return dummy_func();
21+
} catch (Error $e) {
22+
echo "Caught: " . $e->getMessage();
23+
}
24+
})->value();
25+
?>
26+
--EXPECT--
27+
Caught: Call to undefined function dummy_func()

‎tests/base/init_fcall_fix_003.phpt‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
--TEST--
2+
Check INIT_FCALL fix with Runtime::run() (undefined function)
3+
--SKIPIF--
4+
<?php
5+
if (!extension_loaded('parallel')) {
6+
echo 'skip';
7+
}
8+
if (ini_get("opcache.enable_cli")) {
9+
die("skip opcache must not be loaded");
10+
}
11+
?>
12+
--FILE--
13+
<?php
14+
function dummy_func() { return "FOO"; }
15+
$runtime = new \parallel\Runtime(__DIR__ . DIRECTORY_SEPARATOR . 'init_fcall_fix_003_bootstrap.php');
16+
$runtime->run(function(){
17+
try {
18+
$s = existing();
19+
// This will be compiled as INIT_FCALL but should be converted to INIT_FCALL_BY_NAME
20+
// and fail gracefully because it doesn't exist in the thread.
21+
$s .= dummy_func();
22+
$s .= dummy_func();
23+
return $s;
24+
} catch (Error $e) {
25+
echo "Caught: " . $e->getMessage();
26+
}
27+
})->value();
28+
?>
29+
--EXPECT--
30+
Caught: Call to undefined function dummy_func()

‎tests/base/init_fcall_fix_003_bootstrap.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
<?php
2+
3+
function existing() {
4+
return "BAR";
5+
}

0 commit comments

Comments
 (0)