Skip to content

Commit 6e473e7

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 6e473e7

File tree

6 files changed

+80
-23
lines changed

6 files changed

+80
-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: 34 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -24,17 +24,23 @@
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+
#ifdef ZEND_CHECK_STACK_LIMIT
32+
if (UNEXPECTED(zend_call_stack_overflowed(EG(stack_limit)))) {
33+
return FAILURE;
34+
}
35+
#endif
36+
3137
while (1) {
3238
int i;
3339

3440
b->flags |= ZEND_BB_REACHABLE;
3541
if (b->successors_count == 0) {
3642
b->flags |= ZEND_BB_EXIT;
37-
return;
43+
return SUCCESS;
3844
}
3945

4046
for (i = 0; i < b->successors_count; i++) {
@@ -89,28 +95,32 @@ static void zend_mark_reachable(zend_op *opcodes, zend_cfg *cfg, zend_basic_bloc
8995
if (i == b->successors_count - 1) {
9096
/* Tail call optimization */
9197
if (succ->flags & ZEND_BB_REACHABLE) {
92-
return;
98+
return SUCCESS;
9399
}
94100

95101
b = succ;
96102
break;
97103
} else {
98104
/* Recursively check reachability */
99105
if (!(succ->flags & ZEND_BB_REACHABLE)) {
100-
zend_mark_reachable(opcodes, cfg, succ);
106+
if (zend_mark_reachable(opcodes, cfg, succ) != SUCCESS) {
107+
return FAILURE;
108+
}
101109
}
102110
}
103111
}
104112
}
105113
}
106114
/* }}} */
107115

108-
static void zend_mark_reachable_blocks(const zend_op_array *op_array, zend_cfg *cfg, int start) /* {{{ */
116+
static zend_result zend_mark_reachable_blocks(const zend_op_array *op_array, zend_cfg *cfg, int start) /* {{{ */
109117
{
110118
zend_basic_block *blocks = cfg->blocks;
111119

112120
blocks[start].flags = ZEND_BB_START;
113-
zend_mark_reachable(op_array->opcodes, cfg, blocks + start);
121+
if (zend_mark_reachable(op_array->opcodes, cfg, blocks + start) != SUCCESS) {
122+
return FAILURE;
123+
}
114124

115125
if (op_array->last_try_catch) {
116126
zend_basic_block *b;
@@ -146,7 +156,9 @@ static void zend_mark_reachable_blocks(const zend_op_array *op_array, zend_cfg *
146156
if (b->flags & ZEND_BB_REACHABLE) {
147157
op_array->try_catch_array[j].try_op = op_array->try_catch_array[j].catch_op;
148158
changed = 1;
149-
zend_mark_reachable(op_array->opcodes, cfg, blocks + block_map[op_array->try_catch_array[j].try_op]);
159+
if (zend_mark_reachable(op_array->opcodes, cfg, blocks + block_map[op_array->try_catch_array[j].try_op]) != SUCCESS) {
160+
return FAILURE;
161+
}
150162
break;
151163
}
152164
b++;
@@ -163,23 +175,29 @@ static void zend_mark_reachable_blocks(const zend_op_array *op_array, zend_cfg *
163175
b->flags |= ZEND_BB_CATCH;
164176
if (!(b->flags & ZEND_BB_REACHABLE)) {
165177
changed = 1;
166-
zend_mark_reachable(op_array->opcodes, cfg, b);
178+
if (zend_mark_reachable(op_array->opcodes, cfg, b) != SUCCESS) {
179+
return FAILURE;
180+
}
167181
}
168182
}
169183
if (op_array->try_catch_array[j].finally_op) {
170184
b = blocks + block_map[op_array->try_catch_array[j].finally_op];
171185
b->flags |= ZEND_BB_FINALLY;
172186
if (!(b->flags & ZEND_BB_REACHABLE)) {
173187
changed = 1;
174-
zend_mark_reachable(op_array->opcodes, cfg, b);
188+
if (zend_mark_reachable(op_array->opcodes, cfg, b) != SUCCESS) {
189+
return FAILURE;
190+
}
175191
}
176192
}
177193
if (op_array->try_catch_array[j].finally_end) {
178194
b = blocks + block_map[op_array->try_catch_array[j].finally_end];
179195
b->flags |= ZEND_BB_FINALLY_END;
180196
if (!(b->flags & ZEND_BB_REACHABLE)) {
181197
changed = 1;
182-
zend_mark_reachable(op_array->opcodes, cfg, b);
198+
if (zend_mark_reachable(op_array->opcodes, cfg, b) != SUCCESS) {
199+
return FAILURE;
200+
}
183201
}
184202
}
185203
} else {
@@ -223,10 +241,12 @@ static void zend_mark_reachable_blocks(const zend_op_array *op_array, zend_cfg *
223241
}
224242
}
225243
}
244+
245+
return SUCCESS;
226246
}
227247
/* }}} */
228248

229-
void zend_cfg_remark_reachable_blocks(const zend_op_array *op_array, zend_cfg *cfg) /* {{{ */
249+
zend_result zend_cfg_remark_reachable_blocks(const zend_op_array *op_array, zend_cfg *cfg) /* {{{ */
230250
{
231251
zend_basic_block *blocks = cfg->blocks;
232252
int i;
@@ -245,7 +265,7 @@ void zend_cfg_remark_reachable_blocks(const zend_op_array *op_array, zend_cfg *c
245265
blocks[i].flags = 0;
246266
}
247267

248-
zend_mark_reachable_blocks(op_array, cfg, start);
268+
return zend_mark_reachable_blocks(op_array, cfg, start);
249269
}
250270
/* }}} */
251271

@@ -267,7 +287,7 @@ static void initialize_block(zend_basic_block *block) {
267287
block_map[i]++; \
268288
} while (0)
269289

270-
ZEND_API void zend_build_cfg(zend_arena **arena, const zend_op_array *op_array, uint32_t build_flags, zend_cfg *cfg) /* {{{ */
290+
ZEND_API zend_result zend_build_cfg(zend_arena **arena, const zend_op_array *op_array, uint32_t build_flags, zend_cfg *cfg) /* {{{ */
271291
{
272292
uint32_t flags = 0;
273293
uint32_t i;
@@ -587,7 +607,7 @@ ZEND_API void zend_build_cfg(zend_arena **arena, const zend_op_array *op_array,
587607

588608
/* Build CFG, Step 4, Mark Reachable Basic Blocks */
589609
cfg->flags |= flags;
590-
zend_mark_reachable_blocks(op_array, cfg, 0);
610+
return zend_mark_reachable_blocks(op_array, cfg, 0);
591611
}
592612
/* }}} */
593613

‎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)