Skip to content

Commit 71cb9d3

Browse files
nielsdosnielsdos
committed
Fix GH-14361: Deep recursion in zend_cfg.c causes segfault instead of error
Building the CFG for JIT or optimizations can run into deep recursion, and depending on the stack limit cause a segfault. This patch uses the stack limit check to fail out of those cases. This prevents a segfault. We use this check elsewhere in the compiler as well. However, in this case we just make optimizations or JIT fail without aborting the application such that code can still execute. The attached test case will succeed both without and with this patch, it is mainly intended to test if propagating failure doesn't cause crashes. To reproduce the issue, set a low stack limit using `ulimit -s` and run the original test case from #14361
1 parent 4fca8a6 commit 71cb9d3

File tree

6 files changed

+78
-23
lines changed

6 files changed

+78
-23
lines changed

‎Zend/Optimizer/block_pass.c

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1686,9 +1686,9 @@ void zend_optimize_cfg(zend_op_array *op_array, zend_optimizer_ctx *ctx)
16861686

16871687
/* Build CFG */
16881688
checkpoint = zend_arena_checkpoint(ctx->arena);
1689-
zend_build_cfg(&ctx->arena, op_array, 0, &cfg);
16901689

1691-
if (cfg.blocks_count * (op_array->last_var + op_array->T) > 64 * 1024 * 1024) {
1690+
if (zend_build_cfg(&ctx->arena, op_array, 0, &cfg) != SUCCESS
1691+
|| cfg.blocks_count * (op_array->last_var + op_array->T) > 64 * 1024 * 1024) {
16921692
zend_arena_release(&ctx->arena, checkpoint);
16931693
return;
16941694
}
@@ -1751,7 +1751,9 @@ void zend_optimize_cfg(zend_op_array *op_array, zend_optimizer_ctx *ctx)
17511751
}
17521752

17531753
/* Eliminate unreachable basic blocks */
1754-
zend_cfg_remark_reachable_blocks(op_array, &cfg);
1754+
if (zend_cfg_remark_reachable_blocks(op_array, &cfg) != SUCCESS) {
1755+
break;
1756+
}
17551757

17561758
/* Merge Blocks */
17571759
zend_merge_blocks(op_array, &cfg, &opt_count);

‎Zend/Optimizer/dfa_pass.c

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,9 @@ zend_result zend_dfa_analyze_op_array(zend_op_array *op_array, zend_optimizer_ct
5050
/* Build SSA */
5151
memset(ssa, 0, sizeof(zend_ssa));
5252

53-
zend_build_cfg(&ctx->arena, op_array, ZEND_CFG_NO_ENTRY_PREDECESSORS, &ssa->cfg);
53+
if (zend_build_cfg(&ctx->arena, op_array, ZEND_CFG_NO_ENTRY_PREDECESSORS, &ssa->cfg) != SUCCESS) {
54+
return FAILURE;
55+
}
5456

5557
if ((ssa->cfg.flags & ZEND_FUNC_INDIRECT_VAR_ACCESS)) {
5658
/* TODO: we can't analyze functions with indirect variable access ??? */

‎Zend/Optimizer/zend_cfg.c

Lines changed: 32 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -24,17 +24,21 @@
2424
#include "zend_optimizer_internal.h"
2525
#include "zend_sort.h"
2626

27-
static void zend_mark_reachable(zend_op *opcodes, zend_cfg *cfg, zend_basic_block *b) /* {{{ */
27+
static zend_result zend_mark_reachable(zend_op *opcodes, zend_cfg *cfg, zend_basic_block *b) /* {{{ */
2828
{
2929
zend_basic_block *blocks = cfg->blocks;
3030

31+
if (UNEXPECTED(zend_call_stack_overflowed(EG(stack_limit)))) {
32+
return FAILURE;
33+
}
34+
3135
while (1) {
3236
int i;
3337

3438
b->flags |= ZEND_BB_REACHABLE;
3539
if (b->successors_count == 0) {
3640
b->flags |= ZEND_BB_EXIT;
37-
return;
41+
return SUCCESS;
3842
}
3943

4044
for (i = 0; i < b->successors_count; i++) {
@@ -89,28 +93,32 @@ static void zend_mark_reachable(zend_op *opcodes, zend_cfg *cfg, zend_basic_bloc
8993
if (i == b->successors_count - 1) {
9094
/* Tail call optimization */
9195
if (succ->flags & ZEND_BB_REACHABLE) {
92-
return;
96+
return SUCCESS;
9397
}
9498

9599
b = succ;
96100
break;
97101
} else {
98102
/* Recursively check reachability */
99103
if (!(succ->flags & ZEND_BB_REACHABLE)) {
100-
zend_mark_reachable(opcodes, cfg, succ);
104+
if (zend_mark_reachable(opcodes, cfg, succ) != SUCCESS) {
105+
return FAILURE;
106+
}
101107
}
102108
}
103109
}
104110
}
105111
}
106112
/* }}} */
107113

108-
static void zend_mark_reachable_blocks(const zend_op_array *op_array, zend_cfg *cfg, int start) /* {{{ */
114+
static zend_result zend_mark_reachable_blocks(const zend_op_array *op_array, zend_cfg *cfg, int start) /* {{{ */
109115
{
110116
zend_basic_block *blocks = cfg->blocks;
111117

112118
blocks[start].flags = ZEND_BB_START;
113-
zend_mark_reachable(op_array->opcodes, cfg, blocks + start);
119+
if (zend_mark_reachable(op_array->opcodes, cfg, blocks + start) != SUCCESS) {
120+
return FAILURE;
121+
}
114122

115123
if (op_array->last_try_catch) {
116124
zend_basic_block *b;
@@ -146,7 +154,9 @@ static void zend_mark_reachable_blocks(const zend_op_array *op_array, zend_cfg *
146154
if (b->flags & ZEND_BB_REACHABLE) {
147155
op_array->try_catch_array[j].try_op = op_array->try_catch_array[j].catch_op;
148156
changed = 1;
149-
zend_mark_reachable(op_array->opcodes, cfg, blocks + block_map[op_array->try_catch_array[j].try_op]);
157+
if (zend_mark_reachable(op_array->opcodes, cfg, blocks + block_map[op_array->try_catch_array[j].try_op]) != SUCCESS) {
158+
return FAILURE;
159+
}
150160
break;
151161
}
152162
b++;
@@ -163,23 +173,29 @@ static void zend_mark_reachable_blocks(const zend_op_array *op_array, zend_cfg *
163173
b->flags |= ZEND_BB_CATCH;
164174
if (!(b->flags & ZEND_BB_REACHABLE)) {
165175
changed = 1;
166-
zend_mark_reachable(op_array->opcodes, cfg, b);
176+
if (zend_mark_reachable(op_array->opcodes, cfg, b) != SUCCESS) {
177+
return FAILURE;
178+
}
167179
}
168180
}
169181
if (op_array->try_catch_array[j].finally_op) {
170182
b = blocks + block_map[op_array->try_catch_array[j].finally_op];
171183
b->flags |= ZEND_BB_FINALLY;
172184
if (!(b->flags & ZEND_BB_REACHABLE)) {
173185
changed = 1;
174-
zend_mark_reachable(op_array->opcodes, cfg, b);
186+
if (zend_mark_reachable(op_array->opcodes, cfg, b) != SUCCESS) {
187+
return FAILURE;
188+
}
175189
}
176190
}
177191
if (op_array->try_catch_array[j].finally_end) {
178192
b = blocks + block_map[op_array->try_catch_array[j].finally_end];
179193
b->flags |= ZEND_BB_FINALLY_END;
180194
if (!(b->flags & ZEND_BB_REACHABLE)) {
181195
changed = 1;
182-
zend_mark_reachable(op_array->opcodes, cfg, b);
196+
if (zend_mark_reachable(op_array->opcodes, cfg, b) != SUCCESS) {
197+
return FAILURE;
198+
}
183199
}
184200
}
185201
} else {
@@ -223,10 +239,12 @@ static void zend_mark_reachable_blocks(const zend_op_array *op_array, zend_cfg *
223239
}
224240
}
225241
}
242+
243+
return SUCCESS;
226244
}
227245
/* }}} */
228246

229-
void zend_cfg_remark_reachable_blocks(const zend_op_array *op_array, zend_cfg *cfg) /* {{{ */
247+
zend_result zend_cfg_remark_reachable_blocks(const zend_op_array *op_array, zend_cfg *cfg) /* {{{ */
230248
{
231249
zend_basic_block *blocks = cfg->blocks;
232250
int i;
@@ -245,7 +263,7 @@ void zend_cfg_remark_reachable_blocks(const zend_op_array *op_array, zend_cfg *c
245263
blocks[i].flags = 0;
246264
}
247265

248-
zend_mark_reachable_blocks(op_array, cfg, start);
266+
return zend_mark_reachable_blocks(op_array, cfg, start);
249267
}
250268
/* }}} */
251269

@@ -267,7 +285,7 @@ static void initialize_block(zend_basic_block *block) {
267285
block_map[i]++; \
268286
} while (0)
269287

270-
ZEND_API void zend_build_cfg(zend_arena **arena, const zend_op_array *op_array, uint32_t build_flags, zend_cfg *cfg) /* {{{ */
288+
ZEND_API zend_result zend_build_cfg(zend_arena **arena, const zend_op_array *op_array, uint32_t build_flags, zend_cfg *cfg) /* {{{ */
271289
{
272290
uint32_t flags = 0;
273291
uint32_t i;
@@ -587,7 +605,7 @@ ZEND_API void zend_build_cfg(zend_arena **arena, const zend_op_array *op_array,
587605

588606
/* Build CFG, Step 4, Mark Reachable Basic Blocks */
589607
cfg->flags |= flags;
590-
zend_mark_reachable_blocks(op_array, cfg, 0);
608+
return zend_mark_reachable_blocks(op_array, cfg, 0);
591609
}
592610
/* }}} */
593611

‎Zend/Optimizer/zend_cfg.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,8 +115,8 @@ typedef struct _zend_cfg {
115115

116116
BEGIN_EXTERN_C()
117117

118-
ZEND_API void zend_build_cfg(zend_arena **arena, const zend_op_array *op_array, uint32_t build_flags, zend_cfg *cfg);
119-
void zend_cfg_remark_reachable_blocks(const zend_op_array *op_array, zend_cfg *cfg);
118+
ZEND_API zend_result zend_build_cfg(zend_arena **arena, const zend_op_array *op_array, uint32_t build_flags, zend_cfg *cfg);
119+
zend_result zend_cfg_remark_reachable_blocks(const zend_op_array *op_array, zend_cfg *cfg);
120120
ZEND_API void zend_cfg_build_predecessors(zend_arena **arena, zend_cfg *cfg);
121121
ZEND_API void zend_cfg_compute_dominators_tree(const zend_op_array *op_array, zend_cfg *cfg);
122122
ZEND_API void zend_cfg_identify_loops(const zend_op_array *op_array, zend_cfg *cfg);

‎ext/opcache/jit/zend_jit.c

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -921,12 +921,10 @@ static int zend_jit_build_cfg(const zend_op_array *op_array, zend_cfg *cfg)
921921

922922
flags = ZEND_CFG_STACKLESS | ZEND_CFG_NO_ENTRY_PREDECESSORS | ZEND_SSA_RC_INFERENCE_FLAG | ZEND_SSA_USE_CV_RESULTS | ZEND_CFG_RECV_ENTRY;
923923

924-
zend_build_cfg(&CG(arena), op_array, flags, cfg);
925-
926924
/* Don't JIT huge functions. Apart from likely being detrimental due to the amount of
927925
* generated code, some of our analysis is recursive and will stack overflow with many
928926
* blocks. */
929-
if (cfg->blocks_count > 100000) {
927+
if (zend_build_cfg(&CG(arena), op_array, flags, cfg) != SUCCESS || cfg->blocks_count > 100000) {
930928
return FAILURE;
931929
}
932930

‎ext/opcache/tests/jit/gh14361.phpt

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
--TEST--
2+
GH-14361 (Deep recursion in zend_cfg.c causes segfault instead of error)
3+
--SKIPIF--
4+
<?php
5+
if (!function_exists('zend_test_zend_call_stack_get')) die("skip zend_test_zend_call_stack_get() is not available");
6+
?>
7+
--EXTENSIONS--
8+
zend_test
9+
opcache
10+
--INI--
11+
zend.max_allowed_stack_size=128K
12+
opcache.enable=1
13+
opcache.enable_cli=1
14+
opcache.jit=tracing
15+
opcache.jit_buffer_size=64M
16+
opcache.optimization_level=0x000000a0
17+
--FILE--
18+
<?php
19+
$tmp = fopen(__DIR__."/gh14361_tmp.php", "w");
20+
fwrite($tmp, '<?php ');
21+
for ($i = 0; $i < 70000; $i++) {
22+
fwrite($tmp, 'if (@((0) == (0)) !== (true)) { $f++; }');
23+
}
24+
fclose($tmp);
25+
26+
include __DIR__."/gh14361_tmp.php";
27+
echo "Done\n";
28+
?>
29+
--CLEAN--
30+
<?php
31+
@unlink(__DIR__."/gh14361_tmp.php");
32+
?>
33+
--EXPECT--
34+
Done
35+

0 commit comments

Comments
 (0)