From 3d0885f9e54ba46bbb082df4c9c32f0be98ee423 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Wed, 10 Jul 2024 14:52:38 +0200 Subject: [PATCH 1/4] Fix GH-14873: PHP 8.4 min function fails on typed integer The problem is that this line in the VM: `ZVAL_NULL(result);` changes the type of arg1 as well, because after the DFA pass the result and input both use CV0($result). We should not contract assignments with CVs in frameless calls with arguments. An older attempt is found at GH-14876 that tried to modify the VM/JIT. --- Zend/Optimizer/dfa_pass.c | 5 ++ ext/opcache/tests/opt/gh14873.phpt | 114 +++++++++++++++++++++++++++++ 2 files changed, 119 insertions(+) create mode 100644 ext/opcache/tests/opt/gh14873.phpt diff --git a/Zend/Optimizer/dfa_pass.c b/Zend/Optimizer/dfa_pass.c index e281a26f5084c..b3fc7beb85ab5 100644 --- a/Zend/Optimizer/dfa_pass.c +++ b/Zend/Optimizer/dfa_pass.c @@ -325,6 +325,11 @@ static bool opline_supports_assign_contraction( return 0; } + if (opline->opcode >= ZEND_FRAMELESS_ICALL_1 && opline->opcode <= ZEND_FRAMELESS_ICALL_3) { + /* Frameless calls override the return value, but the return value may overlap with the arguments. */ + return opline->op1_type != IS_CV || opline->op1.var != cv_var; + } + if (opline->opcode == ZEND_DO_ICALL || opline->opcode == ZEND_DO_UCALL || opline->opcode == ZEND_DO_FCALL || opline->opcode == ZEND_DO_FCALL_BY_NAME) { /* Function calls may dtor the return value after it has already been written -- allow diff --git a/ext/opcache/tests/opt/gh14873.phpt b/ext/opcache/tests/opt/gh14873.phpt new file mode 100644 index 0000000000000..977a5f4ee1c69 --- /dev/null +++ b/ext/opcache/tests/opt/gh14873.phpt @@ -0,0 +1,114 @@ +--TEST-- +GH-14873 (PHP 8.4 min function fails on typed integer) +--EXTENSIONS-- +opcache +--INI-- +opcache.enable=1 +opcache.enable_cli=1 +opcache.optimization_level=0x7FFEBFFF +opcache.opt_debug_level=0x20000 +--FILE-- + +--EXPECTF-- +$_main: + ; (lines=25, args=0, vars=0, tmps=%d) + ; (after optimizer) + ; %s +0000 INIT_FCALL 1 %d string("var_dump") +0001 INIT_FCALL 1 %d string("testtrim1") +0002 SEND_VAL string(" boo ") 1 +0003 V0 = DO_UCALL +0004 SEND_VAR V0 1 +0005 DO_ICALL +0006 INIT_FCALL 1 %d string("var_dump") +0007 INIT_FCALL 1 %d string("testmin2") +0008 SEND_VAL int(5) 1 +0009 V0 = DO_UCALL +0010 SEND_VAR V0 1 +0011 DO_ICALL +0012 INIT_FCALL 1 %d string("var_dump") +0013 INIT_FCALL 1 %d string("testmin2_tmp") +0014 SEND_VAL int(5) 1 +0015 V0 = DO_UCALL +0016 SEND_VAR V0 1 +0017 DO_ICALL +0018 INIT_FCALL 1 %d string("var_dump") +0019 INIT_FCALL 1 %d string("teststrstr3") +0020 SEND_VAL string("needles") 1 +0021 V0 = DO_UCALL +0022 SEND_VAR V0 1 +0023 DO_ICALL +0024 RETURN int(1) + +testTrim1: + ; (lines=4, args=1, vars=1, tmps=%d) + ; (after optimizer) + ; %s +0000 CV0($value) = RECV 1 +0001 T1 = FRAMELESS_ICALL_1(trim) CV0($value) +0002 ASSIGN CV0($value) T1 +0003 RETURN CV0($value) + +testMin2: + ; (lines=5, args=1, vars=1, tmps=%d) + ; (after optimizer) + ; %s +0000 CV0($value) = RECV 1 +0001 T1 = FRAMELESS_ICALL_2(min) CV0($value) int(100) +0002 CV0($value) = QM_ASSIGN T1 +0003 VERIFY_RETURN_TYPE CV0($value) +0004 RETURN CV0($value) + +testMin2_TMP: + ; (lines=5, args=1, vars=1, tmps=%d) + ; (after optimizer) + ; %s +0000 CV0($value) = RECV 1 +0001 T1 = ADD CV0($value) int(1) +0002 CV0($value) = FRAMELESS_ICALL_2(min) T1 int(100) +0003 VERIFY_RETURN_TYPE CV0($value) +0004 RETURN CV0($value) + +testStrstr3: + ; (lines=6, args=1, vars=1, tmps=%d) + ; (after optimizer) + ; %s +0000 CV0($value) = RECV 1 +0001 T1 = FRAMELESS_ICALL_3(strstr) CV0($value) string("needle") +0002 OP_DATA bool(false) +0003 ASSIGN CV0($value) T1 +0004 VERIFY_RETURN_TYPE CV0($value) +0005 RETURN CV0($value) +LIVE RANGES: + 1: 0002 - 0003 (tmp/var) +string(3) "boo" +int(5) +int(6) +string(7) "needles" From 0cf179e534651e453b6a8af1e8ca81f570499fab Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Wed, 10 Jul 2024 20:09:42 +0200 Subject: [PATCH 2/4] Address review comments --- Zend/Optimizer/dfa_pass.c | 14 ++++- ext/opcache/tests/opt/gh14873.phpt | 99 ++++++++++++++++++++++++++---- 2 files changed, 98 insertions(+), 15 deletions(-) diff --git a/Zend/Optimizer/dfa_pass.c b/Zend/Optimizer/dfa_pass.c index b3fc7beb85ab5..cfbfcec6b25c2 100644 --- a/Zend/Optimizer/dfa_pass.c +++ b/Zend/Optimizer/dfa_pass.c @@ -325,9 +325,17 @@ static bool opline_supports_assign_contraction( return 0; } - if (opline->opcode >= ZEND_FRAMELESS_ICALL_1 && opline->opcode <= ZEND_FRAMELESS_ICALL_3) { - /* Frameless calls override the return value, but the return value may overlap with the arguments. */ - return opline->op1_type != IS_CV || opline->op1.var != cv_var; + /* Frameless calls override the return value, but the return value may overlap with the arguments. */ + switch (opline->opcode) { + case ZEND_FRAMELESS_ICALL_3: + if ((opline + 1)->op1_type == IS_CV && (opline + 1)->op1.var == cv_var) return 0; + ZEND_FALLTHROUGH; + case ZEND_FRAMELESS_ICALL_2: + if (opline->op2_type == IS_CV && opline->op2.var == cv_var) return 0; + ZEND_FALLTHROUGH; + case ZEND_FRAMELESS_ICALL_1: + if (opline->op1_type == IS_CV && opline->op1.var == cv_var) return 0; + break; } if (opline->opcode == ZEND_DO_ICALL || opline->opcode == ZEND_DO_UCALL diff --git a/ext/opcache/tests/opt/gh14873.phpt b/ext/opcache/tests/opt/gh14873.phpt index 977a5f4ee1c69..82bc9aa2bbd30 100644 --- a/ext/opcache/tests/opt/gh14873.phpt +++ b/ext/opcache/tests/opt/gh14873.phpt @@ -15,30 +15,48 @@ function testTrim1(string $value): string { return $value; } -function testMin2(int $value): int { +function testMin2First(int $value): int { $value = min($value, 100); return $value; } +function testMin2Second(int $value): int { + $value = min(100, $value); + return $value; +} + function testMin2_TMP(int $value): int { $value = min($value + 1, 100); return $value; } -function testStrstr3(string $value): string { +function testStrstr3First(string $value): string { $value = strstr($value, "needle", false); return $value; } +function testStrstr3Second(string $value): string { + $value = strstr("needles", $value, false); + return $value; +} + +function testStrstr3Third(bool $value): string { + $value = strstr("needles", "needle", $value); + return $value; +} + var_dump(testTrim1(" boo ")); -var_dump(testMin2(5)); +var_dump(testMin2First(5)); +var_dump(testMin2Second(5)); var_dump(testMin2_TMP(5)); -var_dump(testStrstr3("needles")); +var_dump(testStrstr3First("needles")); +var_dump(testStrstr3Second("needle")); +var_dump(testStrstr3Third(false)); ?> --EXPECTF-- $_main: - ; (lines=25, args=0, vars=0, tmps=%d) + ; (lines=43, args=0, vars=0, tmps=%d) ; (after optimizer) ; %s 0000 INIT_FCALL 1 %d string("var_dump") @@ -48,24 +66,42 @@ $_main: 0004 SEND_VAR V0 1 0005 DO_ICALL 0006 INIT_FCALL 1 %d string("var_dump") -0007 INIT_FCALL 1 %d string("testmin2") +0007 INIT_FCALL 1 112 string("testmin2first") 0008 SEND_VAL int(5) 1 0009 V0 = DO_UCALL 0010 SEND_VAR V0 1 0011 DO_ICALL 0012 INIT_FCALL 1 %d string("var_dump") -0013 INIT_FCALL 1 %d string("testmin2_tmp") +0013 INIT_FCALL 1 112 string("testmin2second") 0014 SEND_VAL int(5) 1 0015 V0 = DO_UCALL 0016 SEND_VAR V0 1 0017 DO_ICALL 0018 INIT_FCALL 1 %d string("var_dump") -0019 INIT_FCALL 1 %d string("teststrstr3") -0020 SEND_VAL string("needles") 1 +0019 INIT_FCALL 1 112 string("testmin2_tmp") +0020 SEND_VAL int(5) 1 0021 V0 = DO_UCALL 0022 SEND_VAR V0 1 0023 DO_ICALL -0024 RETURN int(1) +0024 INIT_FCALL 1 96 string("var_dump") +0025 INIT_FCALL 1 112 string("teststrstr3first") +0026 SEND_VAL string("needles") 1 +0027 V0 = DO_UCALL +0028 SEND_VAR V0 1 +0029 DO_ICALL +0030 INIT_FCALL 1 96 string("var_dump") +0031 INIT_FCALL 1 112 string("teststrstr3second") +0032 SEND_VAL string("needle") 1 +0033 V0 = DO_UCALL +0034 SEND_VAR V0 1 +0035 DO_ICALL +0036 INIT_FCALL 1 96 string("var_dump") +0037 INIT_FCALL 1 112 string("teststrstr3third") +0038 SEND_VAL bool(false) 1 +0039 V0 = DO_UCALL +0040 SEND_VAR V0 1 +0041 DO_ICALL +0042 RETURN int(1) testTrim1: ; (lines=4, args=1, vars=1, tmps=%d) @@ -76,7 +112,7 @@ testTrim1: 0002 ASSIGN CV0($value) T1 0003 RETURN CV0($value) -testMin2: +testMin2First: ; (lines=5, args=1, vars=1, tmps=%d) ; (after optimizer) ; %s @@ -86,6 +122,16 @@ testMin2: 0003 VERIFY_RETURN_TYPE CV0($value) 0004 RETURN CV0($value) +testMin2Second: + ; (lines=5, args=1, vars=1, tmps=%d) + ; (after optimizer) + ; %s +0000 CV0($value) = RECV 1 +0001 T1 = FRAMELESS_ICALL_2(min) int(100) CV0($value) +0002 CV0($value) = QM_ASSIGN T1 +0003 VERIFY_RETURN_TYPE CV0($value) +0004 RETURN CV0($value) + testMin2_TMP: ; (lines=5, args=1, vars=1, tmps=%d) ; (after optimizer) @@ -96,7 +142,7 @@ testMin2_TMP: 0003 VERIFY_RETURN_TYPE CV0($value) 0004 RETURN CV0($value) -testStrstr3: +testStrstr3First: ; (lines=6, args=1, vars=1, tmps=%d) ; (after optimizer) ; %s @@ -106,9 +152,38 @@ testStrstr3: 0003 ASSIGN CV0($value) T1 0004 VERIFY_RETURN_TYPE CV0($value) 0005 RETURN CV0($value) +LIVE RANGES: + 1: 0002 - 0003 (tmp/var) + +testStrstr3Second: + ; (lines=6, args=1, vars=1, tmps=%d) + ; (after optimizer) + ; %s +0000 CV0($value) = RECV 1 +0001 T1 = FRAMELESS_ICALL_3(strstr) string("needles") CV0($value) +0002 OP_DATA bool(false) +0003 ASSIGN CV0($value) T1 +0004 VERIFY_RETURN_TYPE CV0($value) +0005 RETURN CV0($value) +LIVE RANGES: + 1: 0002 - 0003 (tmp/var) + +testStrstr3Third: + ; (lines=6, args=1, vars=1, tmps=%d) + ; (after optimizer) + ; %s +0000 CV0($value) = RECV 1 +0001 T1 = FRAMELESS_ICALL_3(strstr) string("needles") string("needle") +0002 OP_DATA CV0($value) +0003 CV0($value) = QM_ASSIGN T1 +0004 VERIFY_RETURN_TYPE CV0($value) +0005 RETURN CV0($value) LIVE RANGES: 1: 0002 - 0003 (tmp/var) string(3) "boo" int(5) +int(5) int(6) string(7) "needles" +string(7) "needles" +string(7) "needles" From cb6de21c3fe6b437f0ba3c2feee75df61a67e765 Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Wed, 10 Jul 2024 20:29:18 +0200 Subject: [PATCH 3/4] Fix stack size %d placeholder --- ext/opcache/tests/opt/gh14873.phpt | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/ext/opcache/tests/opt/gh14873.phpt b/ext/opcache/tests/opt/gh14873.phpt index 82bc9aa2bbd30..d442128d023c8 100644 --- a/ext/opcache/tests/opt/gh14873.phpt +++ b/ext/opcache/tests/opt/gh14873.phpt @@ -66,37 +66,37 @@ $_main: 0004 SEND_VAR V0 1 0005 DO_ICALL 0006 INIT_FCALL 1 %d string("var_dump") -0007 INIT_FCALL 1 112 string("testmin2first") +0007 INIT_FCALL 1 %d string("testmin2first") 0008 SEND_VAL int(5) 1 0009 V0 = DO_UCALL 0010 SEND_VAR V0 1 0011 DO_ICALL 0012 INIT_FCALL 1 %d string("var_dump") -0013 INIT_FCALL 1 112 string("testmin2second") +0013 INIT_FCALL 1 %d string("testmin2second") 0014 SEND_VAL int(5) 1 0015 V0 = DO_UCALL 0016 SEND_VAR V0 1 0017 DO_ICALL 0018 INIT_FCALL 1 %d string("var_dump") -0019 INIT_FCALL 1 112 string("testmin2_tmp") +0019 INIT_FCALL 1 %d string("testmin2_tmp") 0020 SEND_VAL int(5) 1 0021 V0 = DO_UCALL 0022 SEND_VAR V0 1 0023 DO_ICALL -0024 INIT_FCALL 1 96 string("var_dump") -0025 INIT_FCALL 1 112 string("teststrstr3first") +0024 INIT_FCALL 1 %d string("var_dump") +0025 INIT_FCALL 1 %d string("teststrstr3first") 0026 SEND_VAL string("needles") 1 0027 V0 = DO_UCALL 0028 SEND_VAR V0 1 0029 DO_ICALL -0030 INIT_FCALL 1 96 string("var_dump") -0031 INIT_FCALL 1 112 string("teststrstr3second") +0030 INIT_FCALL 1 %d string("var_dump") +0031 INIT_FCALL 1 %d string("teststrstr3second") 0032 SEND_VAL string("needle") 1 0033 V0 = DO_UCALL 0034 SEND_VAR V0 1 0035 DO_ICALL -0036 INIT_FCALL 1 96 string("var_dump") -0037 INIT_FCALL 1 112 string("teststrstr3third") +0036 INIT_FCALL 1 %d string("var_dump") +0037 INIT_FCALL 1 %d string("teststrstr3third") 0038 SEND_VAL bool(false) 1 0039 V0 = DO_UCALL 0040 SEND_VAR V0 1 From 1d7ca61b4ef29a5af3747f6c359ea479c3dbc1bd Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Wed, 10 Jul 2024 20:42:16 +0200 Subject: [PATCH 4/4] Return 1 directly instead of break --- Zend/Optimizer/dfa_pass.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Zend/Optimizer/dfa_pass.c b/Zend/Optimizer/dfa_pass.c index cfbfcec6b25c2..ce2a19f2e8fe9 100644 --- a/Zend/Optimizer/dfa_pass.c +++ b/Zend/Optimizer/dfa_pass.c @@ -335,7 +335,7 @@ static bool opline_supports_assign_contraction( ZEND_FALLTHROUGH; case ZEND_FRAMELESS_ICALL_1: if (opline->op1_type == IS_CV && opline->op1.var == cv_var) return 0; - break; + return 1; } if (opline->opcode == ZEND_DO_ICALL || opline->opcode == ZEND_DO_UCALL