From e3615944a8441e9d261d5ff07ab6bb0be2e7e82d Mon Sep 17 00:00:00 2001 From: Dmitry Stogov Date: Tue, 28 Nov 2023 19:55:34 +0300 Subject: [PATCH 1/2] Fixed GH-12812: Integer string in variable used as offset produces wrong undefined array key warning --- ext/opcache/jit/zend_jit_arm64.dasc | 75 +++---------------- ext/opcache/jit/zend_jit_disasm.c | 2 + ext/opcache/jit/zend_jit_internal.h | 2 + ext/opcache/jit/zend_jit_vm_helpers.c | 37 ++++++++++ ext/opcache/jit/zend_jit_x86.dasc | 100 +++----------------------- ext/opcache/tests/jit/gh12812.phpt | 29 ++++++++ 6 files changed, 86 insertions(+), 159 deletions(-) create mode 100644 ext/opcache/tests/jit/gh12812.phpt diff --git a/ext/opcache/jit/zend_jit_arm64.dasc b/ext/opcache/jit/zend_jit_arm64.dasc index d40aaf904727e..53282d510dd8f 100644 --- a/ext/opcache/jit/zend_jit_arm64.dasc +++ b/ext/opcache/jit/zend_jit_arm64.dasc @@ -2023,39 +2023,10 @@ static int zend_jit_undefined_offset_ex_stub(dasm_State **Dst) static int zend_jit_undefined_offset_stub(dasm_State **Dst) { |->undefined_offset: -#ifdef __APPLE__ - | stp x29, x30, [sp, # -16]! - | mov x29, sp -#endif - | //sub r4, 8 - | ldr REG0, EX->opline - | ldr REG1w, OP:REG0->result.var - | add REG1, REG1, FP - | SET_Z_TYPE_INFO REG1, IS_NULL, TMP1w - | ldrb REG1w, OP:REG0->op2_type - | cmp REG1w, #IS_CONST - | bne >2 - | ldrsw REG1, OP:REG0->op2.constant - | add REG0, REG0, REG1 - | b >3 - |2: - | ldr REG0w, OP:REG0->op2.var - | add REG0, REG0, FP - |3: - | mov CARG1, #E_WARNING - | LOAD_ADDR CARG2, "Undefined array key " ZEND_LONG_FMT - | ldr CARG3, [REG0] -#ifdef __APPLE__ - | str CARG3, [sp, #-16]! - | EXT_CALL zend_error, REG0 - | add sp, sp, #16 - | ldp x29, x30, [sp], #16 - | ret -#else - | EXT_JMP zend_error, REG0 // tail call - | //add r4, 8 // stack alignment - | //ret -#endif + || if (!GCC_GLOBAL_REGS) { + | mov FCARG1x, FP + || } + | EXT_JMP zend_jit_undefined_long_key, R0 return 1; } @@ -2072,40 +2043,10 @@ static int zend_jit_undefined_index_ex_stub(dasm_State **Dst) static int zend_jit_undefined_index_stub(dasm_State **Dst) { |->undefined_index: -#ifdef __APPLE__ - | stp x29, x30, [sp, # -16]! - | mov x29, sp -#endif - | //sub r4, 8 - | ldr REG0, EX->opline - | ldr REG1w, OP:REG0->result.var - | add REG1, REG1, FP - | SET_Z_TYPE_INFO REG1, IS_NULL, TMP1w - | ldrb REG1w, OP:REG0->op2_type - | cmp REG1w, #IS_CONST - | bne >2 - | ldrsw REG1, OP:REG0->op2.constant - | add REG0, REG0, REG1 - | b >3 - |2: - | ldr REG0w, OP:REG0->op2.var - | add REG0, REG0, FP - |3: - | mov CARG1, #E_WARNING - | LOAD_ADDR CARG2, "Undefined array key \"%s\"" - | ldr CARG3, [REG0] - | add CARG3, CARG3, #offsetof(zend_string, val) -#ifdef __APPLE__ - | str CARG3, [sp, #-16]! - | EXT_CALL zend_error, REG0 - | add sp, sp, #16 - | ldp x29, x30, [sp], #16 - | ret -#else - | EXT_JMP zend_error, REG0 // tail call - | //add r4, 8 - | //ret -#endif + || if (!GCC_GLOBAL_REGS) { + | mov FCARG1x, FP + || } + | EXT_JMP zend_jit_undefined_string_key, R0 return 1; } diff --git a/ext/opcache/jit/zend_jit_disasm.c b/ext/opcache/jit/zend_jit_disasm.c index f470cb9a96855..c49a0df863aed 100644 --- a/ext/opcache/jit/zend_jit_disasm.c +++ b/ext/opcache/jit/zend_jit_disasm.c @@ -655,6 +655,8 @@ static int zend_jit_disasm_init(void) REGISTER_HELPER(zend_jit_vm_stack_free_args_helper); REGISTER_HELPER(zend_jit_copy_extra_args_helper); REGISTER_HELPER(zend_jit_deprecated_helper); + REGISTER_HELPER(zend_jit_undefined_long_key); + REGISTER_HELPER(zend_jit_undefined_string_key); REGISTER_HELPER(zend_jit_assign_const_to_typed_ref); REGISTER_HELPER(zend_jit_assign_tmp_to_typed_ref); REGISTER_HELPER(zend_jit_assign_var_to_typed_ref); diff --git a/ext/opcache/jit/zend_jit_internal.h b/ext/opcache/jit/zend_jit_internal.h index 544bcdd1f245b..92f6fb7b11dc5 100644 --- a/ext/opcache/jit/zend_jit_internal.h +++ b/ext/opcache/jit/zend_jit_internal.h @@ -328,6 +328,8 @@ ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL zend_jit_loop_counter_helper(ZEND_OPCODE_H void ZEND_FASTCALL zend_jit_copy_extra_args_helper(EXECUTE_DATA_D); bool ZEND_FASTCALL zend_jit_deprecated_helper(OPLINE_D); +void ZEND_FASTCALL zend_jit_undefined_long_key(EXECUTE_DATA_D); +void ZEND_FASTCALL zend_jit_undefined_string_key(EXECUTE_DATA_D); zend_constant* ZEND_FASTCALL zend_jit_get_constant(const zval *key, uint32_t flags); zend_constant* ZEND_FASTCALL zend_jit_check_constant(const zval *key); diff --git a/ext/opcache/jit/zend_jit_vm_helpers.c b/ext/opcache/jit/zend_jit_vm_helpers.c index 5342731fc81b7..ff7fbd87546eb 100644 --- a/ext/opcache/jit/zend_jit_vm_helpers.c +++ b/ext/opcache/jit/zend_jit_vm_helpers.c @@ -199,6 +199,43 @@ bool ZEND_FASTCALL zend_jit_deprecated_helper(OPLINE_D) return 1; } +void ZEND_FASTCALL zend_jit_undefined_long_key(EXECUTE_DATA_D) +{ + const zend_op *opline = EX(opline); + zval *result = EX_VAR(opline->result.var); + zval *dim; + + ZVAL_NULL(result); + if (opline->op2_type == IS_CONST) { + dim = RT_CONSTANT(opline, opline->op2); + } else { + dim = EX_VAR(opline->op2.var); + } + ZEND_ASSERT(Z_TYPE_P(dim) == IS_LONG); + zend_error(E_WARNING, "Undefined array key " ZEND_LONG_FMT, Z_LVAL_P(dim)); +} + +void ZEND_FASTCALL zend_jit_undefined_string_key(EXECUTE_DATA_D) +{ + const zend_op *opline = EX(opline); + zval *result = EX_VAR(opline->result.var); + zval *dim; + zend_ulong lval; + + ZVAL_NULL(result); + if (opline->op2_type == IS_CONST) { + dim = RT_CONSTANT(opline, opline->op2); + } else { + dim = EX_VAR(opline->op2.var); + } + ZEND_ASSERT(Z_TYPE_P(dim) == IS_STRING); + if (ZEND_HANDLE_NUMERIC(Z_STR_P(dim), lval)) { + zend_error(E_WARNING, "Undefined array key " ZEND_LONG_FMT, lval); + } else { + zend_error(E_WARNING, "Undefined array key \"%s\"", Z_STRVAL_P(dim)); + } +} + ZEND_OPCODE_HANDLER_RET ZEND_FASTCALL zend_jit_profile_helper(ZEND_OPCODE_HANDLER_ARGS) { zend_op_array *op_array = (zend_op_array*)EX(func); diff --git a/ext/opcache/jit/zend_jit_x86.dasc b/ext/opcache/jit/zend_jit_x86.dasc index b441b3dc966f1..d881b466dd962 100644 --- a/ext/opcache/jit/zend_jit_x86.dasc +++ b/ext/opcache/jit/zend_jit_x86.dasc @@ -1966,50 +1966,10 @@ static int zend_jit_undefined_offset_ex_stub(dasm_State **Dst) static int zend_jit_undefined_offset_stub(dasm_State **Dst) { |->undefined_offset: - |.if X64WIN - | sub r4, 0x28 - |.elif X64 - | sub r4, 8 - |.else - | sub r4, 12 - |.endif - | mov r0, EX->opline - | mov ecx, dword OP:r0->result.var - | cmp byte OP:r0->op2_type, IS_CONST - | SET_Z_TYPE_INFO FP + r1, IS_NULL - | jne >2 - |.if X64 - | movsxd r1, dword OP:r0->op2.constant - | add r0, r1 - |.else - | mov r0, aword OP:r0->op2.zv - |.endif - | jmp >3 - |2: - | mov eax, dword OP:r0->op2.var - | add r0, FP - |3: - |.if X64WIN - | mov CARG1, E_WARNING - | LOAD_ADDR CARG2, "Undefined array key " ZEND_LONG_FMT - | mov CARG3, aword [r0] - | EXT_CALL zend_error, r0 - | add r4, 0x28 // stack alignment - |.elif X64 - | mov CARG1, E_WARNING - | LOAD_ADDR CARG2, "Undefined array key " ZEND_LONG_FMT - | mov CARG3, aword [r0] - | EXT_CALL zend_error, r0 - | add r4, 8 // stack alignment - |.else - | sub r4, 4 - | push aword [r0] - | push "Undefined array key " ZEND_LONG_FMT - | push E_WARNING - | EXT_CALL zend_error, r0 - | add r4, 28 - |.endif - | ret + || if (!GCC_GLOBAL_REGS) { + | mov FCARG1a, FP + || } + | EXT_JMP zend_jit_undefined_long_key, r0 return 1; } @@ -2026,54 +1986,10 @@ static int zend_jit_undefined_index_ex_stub(dasm_State **Dst) static int zend_jit_undefined_index_stub(dasm_State **Dst) { |->undefined_index: - |.if X64WIN - | sub r4, 0x28 - |.elif X64 - | sub r4, 8 - |.else - | sub r4, 12 - |.endif - | mov r0, EX->opline - | mov ecx, dword OP:r0->result.var - | cmp byte OP:r0->op2_type, IS_CONST - | SET_Z_TYPE_INFO FP + r1, IS_NULL - | jne >2 - |.if X64 - | movsxd r1, dword OP:r0->op2.constant - | add r0, r1 - |.else - | mov r0, aword OP:r0->op2.zv - |.endif - | jmp >3 - |2: - | mov eax, dword OP:r0->op2.var - | add r0, FP - |3: - |.if X64WIN - | mov CARG1, E_WARNING - | LOAD_ADDR CARG2, "Undefined array key \"%s\"" - | mov CARG3, aword [r0] - | add CARG3, offsetof(zend_string, val) - | EXT_CALL zend_error, r0 - | add r4, 0x28 - |.elif X64 - | mov CARG1, E_WARNING - | LOAD_ADDR CARG2, "Undefined array key \"%s\"" - | mov CARG3, aword [r0] - | add CARG3, offsetof(zend_string, val) - | EXT_CALL zend_error, r0 - | add r4, 8 - |.else - | sub r4, 4 - | mov r0, aword [r0] - | add r0, offsetof(zend_string, val) - | push r0 - | push "Undefined array key \"%s\"" - | push E_WARNING - | EXT_CALL zend_error, r0 - | add r4, 28 - |.endif - | ret + || if (!GCC_GLOBAL_REGS) { + | mov FCARG1a, FP + || } + | EXT_JMP zend_jit_undefined_string_key, r0 return 1; } diff --git a/ext/opcache/tests/jit/gh12812.phpt b/ext/opcache/tests/jit/gh12812.phpt new file mode 100644 index 0000000000000..267abf2aae064 --- /dev/null +++ b/ext/opcache/tests/jit/gh12812.phpt @@ -0,0 +1,29 @@ +--TEST-- +GH-12812: JIT: Integer string in variable used as offset produces wrong undefined array key warning +--INI-- +opcache.enable=1 +opcache.enable_cli=1 +--FILE-- +getMessage(), "\n"; +} +try { + var_dump($container[$dimension]); +} catch (\Throwable $e) { + echo $e->getMessage(), "\n"; +} + +?> +--EXPECTF-- +Warning: Undefined array key 7 in %s on line %d +NULL + +Warning: Undefined array key 7 in %s on line %d +NULL From 69e9e8d29b72ec5c5395c16d9f841458caf2ade5 Mon Sep 17 00:00:00 2001 From: Dmitry Stogov Date: Tue, 28 Nov 2023 20:36:26 +0300 Subject: [PATCH 2/2] Fixed register names --- ext/opcache/jit/zend_jit_arm64.dasc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/opcache/jit/zend_jit_arm64.dasc b/ext/opcache/jit/zend_jit_arm64.dasc index 53282d510dd8f..d4e67b6c5a4a6 100644 --- a/ext/opcache/jit/zend_jit_arm64.dasc +++ b/ext/opcache/jit/zend_jit_arm64.dasc @@ -2026,7 +2026,7 @@ static int zend_jit_undefined_offset_stub(dasm_State **Dst) || if (!GCC_GLOBAL_REGS) { | mov FCARG1x, FP || } - | EXT_JMP zend_jit_undefined_long_key, R0 + | EXT_JMP zend_jit_undefined_long_key, REG0 return 1; } @@ -2046,7 +2046,7 @@ static int zend_jit_undefined_index_stub(dasm_State **Dst) || if (!GCC_GLOBAL_REGS) { | mov FCARG1x, FP || } - | EXT_JMP zend_jit_undefined_string_key, R0 + | EXT_JMP zend_jit_undefined_string_key, REG0 return 1; }