Skip to content

Commit 96d11a3

Browse files
committed
Fix GH-19065: Long match statement can segfault compiler during recursive SSA renaming
On some systems, like Alpine, the thread stack size is small by default. The last step of SSA construction involves variable renaming that is recursive, and also makes copies of their version of the renamed variables on the stack. This combination causes a stack overflow during compilation on Alpine. Triggerable for example with very long match statements. A stop-gap solution would be to use heap allocated arrays for the renamed variable list, but that would only delay the error as increasing the number of match arms increases the depth of the dominator tree, and will eventually run into the same issue. This patch transforms the algorithm into an iterative one. There are two states stored in a worklist stack: positive numbers indicate that the block still needs to undergo variable renaming. Negative numbers indicate that the block and its dominated children are already renamed. Because 0 is also a valid block number, we bias the block numbers by adding 1. To restore to the right variant when backtracking the "recursive" step, we index into an array pointing to the different variable renaming variants.
1 parent faa7831 commit 96d11a3

File tree

1 file changed

+81
-20
lines changed

1 file changed

+81
-20
lines changed

Zend/Optimizer/zend_ssa.c

Lines changed: 81 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include "zend_ssa.h"
2323
#include "zend_dump.h"
2424
#include "zend_inference.h"
25+
#include "zend_worklist.h"
2526
#include "Optimizer/zend_optimizer_internal.h"
2627

2728
static bool dominates(const zend_basic_block *blocks, int a, int b) {
@@ -787,23 +788,14 @@ ZEND_API int zend_ssa_rename_op(const zend_op_array *op_array, const zend_op *op
787788
}
788789
/* }}} */
789790

790-
static zend_result zend_ssa_rename(const zend_op_array *op_array, uint32_t build_flags, zend_ssa *ssa, int *var, int n) /* {{{ */
791+
static void zend_ssa_rename_in_block(const zend_op_array *op_array, uint32_t build_flags, zend_ssa *ssa, int *var, int n) /* {{{ */
791792
{
792793
zend_basic_block *blocks = ssa->cfg.blocks;
793794
zend_ssa_block *ssa_blocks = ssa->blocks;
794795
zend_ssa_op *ssa_ops = ssa->ops;
795796
int ssa_vars_count = ssa->vars_count;
796797
int i, j;
797798
zend_op *opline, *end;
798-
int *tmp = NULL;
799-
ALLOCA_FLAG(use_heap = 0);
800-
801-
// FIXME: Can we optimize this copying out in some cases?
802-
if (blocks[n].next_child >= 0) {
803-
tmp = do_alloca(sizeof(int) * (op_array->last_var + op_array->T), use_heap);
804-
memcpy(tmp, var, sizeof(int) * (op_array->last_var + op_array->T));
805-
var = tmp;
806-
}
807799

808800
if (ssa_blocks[n].phis) {
809801
zend_ssa_phi *phi = ssa_blocks[n].phis;
@@ -887,22 +879,91 @@ static zend_result zend_ssa_rename(const zend_op_array *op_array, uint32_t build
887879
}
888880

889881
ssa->vars_count = ssa_vars_count;
882+
}
883+
/* }}} */
890884

891-
j = blocks[n].children;
892-
while (j >= 0) {
893-
// FIXME: Tail call optimization?
894-
if (zend_ssa_rename(op_array, build_flags, ssa, var, j) == FAILURE)
895-
return FAILURE;
896-
j = blocks[j].next_child;
897-
}
885+
static zend_result zend_ssa_rename(const zend_op_array *op_array, uint32_t build_flags, zend_ssa *ssa, int *var, int n)
886+
{
887+
/* The worklist contains block numbers, encoded as positive or negative value.
888+
* Positive values indicate that the variable rename still needs to happen for the block.
889+
* Negative values indicate the variable rename was done and all children were handled too.
890+
* In that case, we will clean up.
891+
* Because block 0 is valid, we bias the block numbers by adding 1 such that we can distinguish
892+
* positive and negative values in all cases. */
893+
zend_worklist_stack work;
894+
ALLOCA_FLAG(work_use_heap);
895+
ZEND_WORKLIST_STACK_ALLOCA(&work, ssa->cfg.blocks_count, work_use_heap);
896+
zend_worklist_stack_push(&work, n + 1);
897+
898+
/* This is used together with `save_vars` to backtrack the right version of the renamed variables to use. */
899+
ALLOCA_FLAG(save_positions_use_heap);
900+
ALLOCA_FLAG(save_vars_use_heap);
901+
unsigned int save_vars_top = 0;
902+
unsigned int *save_positions = do_alloca(sizeof(unsigned int) * ssa->cfg.blocks_count, save_positions_use_heap);
903+
int **save_vars = do_alloca(sizeof(int *) * (ssa->cfg.blocks_count + 1), save_vars_use_heap);
904+
save_vars[0] = var;
905+
906+
while (work.len) {
907+
n = zend_worklist_stack_pop(&work);
908+
909+
/* Enter state: perform SSA variable rename */
910+
if (n > 0) {
911+
n--;
912+
913+
/* Push backtrack state */
914+
zend_worklist_stack_push(&work, -(n + 1));
915+
916+
// FIXME: Can we optimize this copying out in some cases?
917+
int *new_var;
918+
if (ssa->cfg.blocks[n].next_child >= 0) {
919+
new_var = emalloc(sizeof(int) * (op_array->last_var + op_array->T));
920+
memcpy(new_var, save_vars[save_vars_top], sizeof(int) * (op_array->last_var + op_array->T));
921+
save_positions[n] = save_vars_top++;
922+
save_vars[save_vars_top] = new_var;
923+
} else {
924+
new_var = save_vars[save_vars_top];
925+
save_positions[n] = save_vars_top;
926+
}
927+
928+
zend_ssa_rename_in_block(op_array, build_flags, ssa, new_var, n);
898929

899-
if (tmp) {
900-
free_alloca(tmp, use_heap);
930+
/* Push children in enter state */
931+
unsigned int child_count = 0;
932+
int len_prior = work.len;
933+
int j = ssa->cfg.blocks[n].children;
934+
while (j >= 0) {
935+
zend_worklist_stack_push(&work, j + 1);
936+
j = ssa->cfg.blocks[j].next_child;
937+
child_count++;
938+
}
939+
940+
/* Reverse block order to maintain SSA variable number order given in previous PHP versions,
941+
* but the data structure doesn't allow reverse dominator tree traversal. */
942+
for (unsigned int i = 0; i < child_count / 2; i++) {
943+
int tmp = work.buf[len_prior + i];
944+
work.buf[len_prior + i] = work.buf[work.len - 1 - i];
945+
work.buf[work.len - 1 - i] = tmp;
946+
}
947+
}
948+
/* Leave state: backtrack */
949+
else {
950+
n = -n;
951+
n--;
952+
953+
for (unsigned int i = save_vars_top, p = save_positions[n]; i > p; i--) {
954+
efree(save_vars[i]);
955+
}
956+
957+
save_vars_top = save_positions[n];
958+
}
901959
}
902960

961+
free_alloca(save_vars, save_vars_use_heap);
962+
free_alloca(save_positions, save_positions_use_heap);
963+
ZEND_WORKLIST_STACK_FREE_ALLOCA(&work, work_use_heap);
964+
903965
return SUCCESS;
904966
}
905-
/* }}} */
906967

907968
ZEND_API zend_result zend_build_ssa(zend_arena **arena, const zend_script *script, const zend_op_array *op_array, uint32_t build_flags, zend_ssa *ssa) /* {{{ */
908969
{

0 commit comments

Comments
 (0)