From 69d263e2a1cd1078365e41568bb9e72d97a5f244 Mon Sep 17 00:00:00 2001 From: Arnaud Le Blanc Date: Fri, 27 May 2022 13:15:15 +0200 Subject: [PATCH] Add JIT guards for INIT_METHOD_CALL when the method may be modified (#8600) Non-polymorphic methods can be modified from one request to an other due to recompilation or conditional declaration. Fixes GH-8591 Co-authored-by: Oleg Stepanischev --- ext/opcache/jit/zend_jit_arm64.dasc | 4 +-- ext/opcache/jit/zend_jit_internal.h | 20 +++++++++++ ext/opcache/jit/zend_jit_trace.c | 20 ----------- ext/opcache/jit/zend_jit_x86.dasc | 4 +-- ext/opcache/tests/jit/gh8591-001.inc | 7 ++++ ext/opcache/tests/jit/gh8591-001.phpt | 49 +++++++++++++++++++++++++ ext/opcache/tests/jit/gh8591-002.inc | 7 ++++ ext/opcache/tests/jit/gh8591-002.phpt | 52 +++++++++++++++++++++++++++ ext/opcache/tests/jit/gh8591-003.phpt | 45 +++++++++++++++++++++++ ext/opcache/tests/jit/gh8591-004.inc | 7 ++++ ext/opcache/tests/jit/gh8591-004.phpt | 51 ++++++++++++++++++++++++++ ext/opcache/tests/jit/gh8591-005.inc | 12 +++++++ ext/opcache/tests/jit/gh8591-005.phpt | 41 +++++++++++++++++++++ ext/opcache/tests/jit/gh8591-006.inc | 12 +++++++ ext/opcache/tests/jit/gh8591-006.phpt | 38 ++++++++++++++++++++ 15 files changed, 345 insertions(+), 24 deletions(-) create mode 100644 ext/opcache/tests/jit/gh8591-001.inc create mode 100644 ext/opcache/tests/jit/gh8591-001.phpt create mode 100644 ext/opcache/tests/jit/gh8591-002.inc create mode 100644 ext/opcache/tests/jit/gh8591-002.phpt create mode 100644 ext/opcache/tests/jit/gh8591-003.phpt create mode 100644 ext/opcache/tests/jit/gh8591-004.inc create mode 100644 ext/opcache/tests/jit/gh8591-004.phpt create mode 100644 ext/opcache/tests/jit/gh8591-005.inc create mode 100644 ext/opcache/tests/jit/gh8591-005.phpt create mode 100644 ext/opcache/tests/jit/gh8591-006.inc create mode 100644 ext/opcache/tests/jit/gh8591-006.phpt diff --git a/ext/opcache/jit/zend_jit_arm64.dasc b/ext/opcache/jit/zend_jit_arm64.dasc index 0694733217b..4f78e2e3806 100644 --- a/ext/opcache/jit/zend_jit_arm64.dasc +++ b/ext/opcache/jit/zend_jit_arm64.dasc @@ -8998,7 +8998,7 @@ static int zend_jit_init_method_call(dasm_State **Dst, |2: } - if (!func + if ((!func || zend_jit_may_be_modified(func, op_array)) && trace && trace->op == ZEND_JIT_TRACE_INIT_CALL && trace->func @@ -9006,7 +9006,7 @@ static int zend_jit_init_method_call(dasm_State **Dst, int32_t exit_point; const void *exit_addr; - exit_point = zend_jit_trace_get_exit_point(opline, ZEND_JIT_EXIT_METHOD_CALL); + exit_point = zend_jit_trace_get_exit_point(opline, func ? ZEND_JIT_EXIT_INVALIDATE : ZEND_JIT_EXIT_METHOD_CALL); exit_addr = zend_jit_trace_get_exit_addr(exit_point); if (!exit_addr) { return 0; diff --git a/ext/opcache/jit/zend_jit_internal.h b/ext/opcache/jit/zend_jit_internal.h index 4190b224672..a1b8b98baa7 100644 --- a/ext/opcache/jit/zend_jit_internal.h +++ b/ext/opcache/jit/zend_jit_internal.h @@ -719,6 +719,26 @@ static zend_always_inline const zend_op* zend_jit_trace_get_exit_opline(zend_jit return NULL; } +static inline bool zend_jit_may_be_modified(const zend_function *func, const zend_op_array *called_from) +{ + if (func->type == ZEND_INTERNAL_FUNCTION) { +#ifdef _WIN32 + /* ASLR */ + return 1; +#else + return 0; +#endif + } else if (func->type == ZEND_USER_FUNCTION) { + if (func->common.fn_flags & ZEND_ACC_PRELOADED) { + return 0; + } + if (func->op_array.filename == called_from->filename && !func->op_array.scope) { + return 0; + } + } + return 1; +} + static zend_always_inline bool zend_jit_may_be_polymorphic_call(const zend_op *opline) { if (opline->opcode == ZEND_INIT_FCALL diff --git a/ext/opcache/jit/zend_jit_trace.c b/ext/opcache/jit/zend_jit_trace.c index a7c5b13240d..7c473ad78b1 100644 --- a/ext/opcache/jit/zend_jit_trace.c +++ b/ext/opcache/jit/zend_jit_trace.c @@ -366,26 +366,6 @@ static int zend_jit_trace_may_exit(const zend_op_array *op_array, const zend_op return 0; } -static bool zend_jit_may_be_modified(const zend_function *func, const zend_op_array *called_from) -{ - if (func->type == ZEND_INTERNAL_FUNCTION) { -#ifdef _WIN32 - /* ASLR */ - return 1; -#else - return 0; -#endif - } else if (func->type == ZEND_USER_FUNCTION) { - if (func->common.fn_flags & ZEND_ACC_PRELOADED) { - return 0; - } - if (func->op_array.filename == called_from->filename && !func->op_array.scope) { - return 0; - } - } - return 1; -} - static zend_always_inline uint32_t zend_jit_trace_type_to_info_ex(zend_uchar type, uint32_t info) { if (type == IS_UNKNOWN) { diff --git a/ext/opcache/jit/zend_jit_x86.dasc b/ext/opcache/jit/zend_jit_x86.dasc index 1515c32e211..e2613f96db2 100644 --- a/ext/opcache/jit/zend_jit_x86.dasc +++ b/ext/opcache/jit/zend_jit_x86.dasc @@ -9630,7 +9630,7 @@ static int zend_jit_init_method_call(dasm_State **Dst, |2: } - if (!func + if ((!func || zend_jit_may_be_modified(func, op_array)) && trace && trace->op == ZEND_JIT_TRACE_INIT_CALL && trace->func @@ -9641,7 +9641,7 @@ static int zend_jit_init_method_call(dasm_State **Dst, int32_t exit_point; const void *exit_addr; - exit_point = zend_jit_trace_get_exit_point(opline, ZEND_JIT_EXIT_METHOD_CALL); + exit_point = zend_jit_trace_get_exit_point(opline, func ? ZEND_JIT_EXIT_INVALIDATE : ZEND_JIT_EXIT_METHOD_CALL); exit_addr = zend_jit_trace_get_exit_addr(exit_point); if (!exit_addr) { return 0; diff --git a/ext/opcache/tests/jit/gh8591-001.inc b/ext/opcache/tests/jit/gh8591-001.inc new file mode 100644 index 00000000000..720c4101534 --- /dev/null +++ b/ext/opcache/tests/jit/gh8591-001.inc @@ -0,0 +1,7 @@ +cast(); + } + } + + private function cast() + { + global $x; + $x = static::$field; + } +} + +new Model(); + +// mark the file as changed (important) +touch(__DIR__ . '/gh8591-001.inc'); + +var_dump($x); + +print "OK"; +--EXPECT-- +int(1) +OK diff --git a/ext/opcache/tests/jit/gh8591-002.inc b/ext/opcache/tests/jit/gh8591-002.inc new file mode 100644 index 00000000000..720c4101534 --- /dev/null +++ b/ext/opcache/tests/jit/gh8591-002.inc @@ -0,0 +1,7 @@ +cast(); + } + } + + private function cast() + { + global $x; + $x = static::$field; + } +} + +new Model(); + +var_dump($x); + +print "OK"; +--EXPECT-- +int(1) +OK diff --git a/ext/opcache/tests/jit/gh8591-003.phpt b/ext/opcache/tests/jit/gh8591-003.phpt new file mode 100644 index 00000000000..dc71e17dddf --- /dev/null +++ b/ext/opcache/tests/jit/gh8591-003.phpt @@ -0,0 +1,45 @@ +--TEST-- +Bug GH-8591 003 (JIT does not account for class re-compile) +--EXTENSIONS-- +opcache +--INI-- +opcache.enable=1 +opcache.enable_cli=1 +opcache.jit_buffer_size=1M +opcache.jit=1255 +opcache.file_update_protection=0 +opcache.revalidate_freq=0 +opcache.protect_memory=1 +--FILE-- +cast(); + } + } + + private function cast() + { + global $x; + $x = static::$field; + } +} + +new Model(); + +var_dump($x); + +print "OK"; +--EXPECT-- +int(1) +OK diff --git a/ext/opcache/tests/jit/gh8591-004.inc b/ext/opcache/tests/jit/gh8591-004.inc new file mode 100644 index 00000000000..ae5be814d9b --- /dev/null +++ b/ext/opcache/tests/jit/gh8591-004.inc @@ -0,0 +1,7 @@ +cast(); + } + } + + private function cast() + { + global $x; + $x = static::$field; + } +} + +new Model(); + +// mark the file as changed (important) +touch(__DIR__ . '/gh8591-004.inc'); + +var_dump($x); + +print "OK"; +--EXPECT-- +int(1) +OK diff --git a/ext/opcache/tests/jit/gh8591-005.inc b/ext/opcache/tests/jit/gh8591-005.inc new file mode 100644 index 00000000000..5eb8a84eee7 --- /dev/null +++ b/ext/opcache/tests/jit/gh8591-005.inc @@ -0,0 +1,12 @@ +cast(); + } + } +} + +new Model(); + +// mark the file as changed (important) +touch(__DIR__ . '/gh8591-005.inc'); + +var_dump($x); + +print "OK"; +--EXPECT-- +int(1) +OK diff --git a/ext/opcache/tests/jit/gh8591-006.inc b/ext/opcache/tests/jit/gh8591-006.inc new file mode 100644 index 00000000000..5eb8a84eee7 --- /dev/null +++ b/ext/opcache/tests/jit/gh8591-006.inc @@ -0,0 +1,12 @@ + +--FILE-- +cast(); + } + } +} + +new Model(); + +var_dump($x); + +print "OK"; +--EXPECT-- +int(1) +OK