From 80315edd580ceb2fd7cf21aef2130c4c265acc90 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Sat, 10 Sep 2022 11:03:10 +0100 Subject: [PATCH 1/3] Introduce PROGRESS_CACHE_SLOT() macro --- Zend/zend_execute.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c index 785626adfe7..97da6efb805 100644 --- a/Zend/zend_execute.c +++ b/Zend/zend_execute.c @@ -1001,6 +1001,8 @@ static zend_always_inline bool zend_value_instanceof_static(zval *zv) { # define HAVE_CACHE_SLOT 1 #endif +#define PROGRESS_CACHE_SLOT() if (HAVE_CACHE_SLOT) {cache_slot++;} + static zend_always_inline zend_class_entry *zend_fetch_ce_from_cache_slot( void **cache_slot, zend_type *type) { @@ -1065,9 +1067,7 @@ static zend_always_inline bool zend_check_type_slow( if (!ce || !instanceof_function(Z_OBJCE_P(arg), ce)) { return false; } - if (HAVE_CACHE_SLOT) { - cache_slot++; - } + PROGRESS_CACHE_SLOT(); } ZEND_TYPE_LIST_FOREACH_END(); return true; } else { @@ -1085,9 +1085,7 @@ static zend_always_inline bool zend_check_type_slow( } } - if (HAVE_CACHE_SLOT) { - cache_slot++; - } + PROGRESS_CACHE_SLOT(); } ZEND_TYPE_LIST_FOREACH_END(); } } else { From 9286101da4732d82345cacd618ce0eab78b7b729 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Sat, 10 Sep 2022 10:59:57 +0100 Subject: [PATCH 2/3] Fix GH-9516: (A&B)|D as a param should allow AB or D. Not just A. The issue was that we didn't compute enough cache slots for DNF types. Nor progressed throught the CE's in the cache slot, meaning we were only checking if the value passed satisfied the first type of the nested intersection type. --- .../type_declarations/dnf_types/gh9516.phpt | 157 ++++++++++++++++++ Zend/zend_compile.c | 18 +- Zend/zend_execute.c | 18 +- 3 files changed, 186 insertions(+), 7 deletions(-) create mode 100644 Zend/tests/type_declarations/dnf_types/gh9516.phpt diff --git a/Zend/tests/type_declarations/dnf_types/gh9516.phpt b/Zend/tests/type_declarations/dnf_types/gh9516.phpt new file mode 100644 index 00000000000..3d91932ebd4 --- /dev/null +++ b/Zend/tests/type_declarations/dnf_types/gh9516.phpt @@ -0,0 +1,157 @@ +--TEST-- +GH-9516: (A&B)|D as a param should allow AB or D. Not just A. +--FILE-- +method1(new A_); + echo 'Fail', \PHP_EOL; +} catch (\Throwable $throwable) { + echo $throwable->getMessage(), \PHP_EOL; +} + +try { + $t->method1(new B_); + echo 'Fail', \PHP_EOL; +} catch (\Throwable $throwable) { + echo $throwable->getMessage(), \PHP_EOL; +} + +try { + $t->method1(new AB_); + echo 'Pass', \PHP_EOL; +} catch (\Throwable $throwable) { + echo $throwable->getMessage(), \PHP_EOL; +} + +try { + $t->method1(new D_); + echo 'Pass', \PHP_EOL; +} catch (\Throwable $throwable) { + echo $throwable->getMessage(), \PHP_EOL; +} + +// Lets try in reverse? +try { + $t->method2(new A_); + echo 'Fail', \PHP_EOL; +} catch (\Throwable $throwable) { + echo $throwable->getMessage(), \PHP_EOL; +} + +try { + $t->method2(new B_); + echo 'Fail', \PHP_EOL; +} catch (\Throwable $throwable) { + echo $throwable->getMessage(), \PHP_EOL; +} + +try { + $t->method2(new AB_); + echo 'Pass', \PHP_EOL; +} catch (\Throwable $throwable) { + echo $throwable->getMessage(), \PHP_EOL; +} + +try { + $t->method2(new D_); + echo 'Pass', \PHP_EOL; +} catch (\Throwable $throwable) { + echo $throwable->getMessage(), \PHP_EOL; +} + +/* Single before intersection */ +try { + $t->method3(new A_); + echo 'Fail', \PHP_EOL; +} catch (\Throwable $throwable) { + echo $throwable->getMessage(), \PHP_EOL; +} + +try { + $t->method3(new B_); + echo 'Fail', \PHP_EOL; +} catch (\Throwable $throwable) { + echo $throwable->getMessage(), \PHP_EOL; +} + +try { + $t->method3(new AB_); + echo 'Pass', \PHP_EOL; +} catch (\Throwable $throwable) { + echo $throwable->getMessage(), \PHP_EOL; +} + +try { + $t->method3(new D_); + echo 'Pass', \PHP_EOL; +} catch (\Throwable $throwable) { + echo $throwable->getMessage(), \PHP_EOL; +} + +// Lets try in reverse? +try { + $t->method4(new A_); + echo 'Fail', \PHP_EOL; +} catch (\Throwable $throwable) { + echo $throwable->getMessage(), \PHP_EOL; +} + +try { + $t->method4(new B_); + echo 'Fail', \PHP_EOL; +} catch (\Throwable $throwable) { + echo $throwable->getMessage(), \PHP_EOL; +} + +try { + $t->method4(new AB_); + echo 'Pass', \PHP_EOL; +} catch (\Throwable $throwable) { + echo $throwable->getMessage(), \PHP_EOL; +} + +try { + $t->method4(new D_); + echo 'Pass', \PHP_EOL; +} catch (\Throwable $throwable) { + echo $throwable->getMessage(), \PHP_EOL; +} + + +?> +--EXPECTF-- +T::method1(): Argument #1 ($arg) must be of type (A&B)|D, A_ given, called in %s on line %d +T::method1(): Argument #1 ($arg) must be of type (A&B)|D, B_ given, called in %s on line %d +Pass +Pass +T::method2(): Argument #1 ($arg) must be of type (B&A)|D, A_ given, called in %s on line %d +T::method2(): Argument #1 ($arg) must be of type (B&A)|D, B_ given, called in %s on line %d +Pass +Pass +T::method3(): Argument #1 ($arg) must be of type D|(A&B), A_ given, called in %s on line %d +T::method3(): Argument #1 ($arg) must be of type D|(A&B), B_ given, called in %s on line %d +Pass +Pass +T::method4(): Argument #1 ($arg) must be of type D|(B&A), A_ given, called in %s on line %d +T::method4(): Argument #1 ($arg) must be of type D|(B&A), B_ given, called in %s on line %d +Pass +Pass diff --git a/Zend/zend_compile.c b/Zend/zend_compile.c index 19ff1b08862..b0aebed5818 100644 --- a/Zend/zend_compile.c +++ b/Zend/zend_compile.c @@ -2380,7 +2380,23 @@ static size_t zend_type_get_num_classes(zend_type type) { return 0; } if (ZEND_TYPE_HAS_LIST(type)) { - return ZEND_TYPE_LIST(type)->num_types; + /* Intersection types cannot have nested list types */ + if (ZEND_TYPE_IS_INTERSECTION(type)) { + return ZEND_TYPE_LIST(type)->num_types; + } + ZEND_ASSERT(ZEND_TYPE_IS_UNION(type)); + size_t count = 0; + zend_type *list_type; + + ZEND_TYPE_LIST_FOREACH(ZEND_TYPE_LIST(type), list_type) { + if (ZEND_TYPE_IS_INTERSECTION(*list_type)) { + count += ZEND_TYPE_LIST(*list_type)->num_types; + } else { + ZEND_ASSERT(!ZEND_TYPE_HAS_LIST(*list_type)); + count += 1; + } + } ZEND_TYPE_LIST_FOREACH_END(); + return count; } return 1; } diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c index 97da6efb805..657b453b838 100644 --- a/Zend/zend_execute.c +++ b/Zend/zend_execute.c @@ -1035,19 +1035,25 @@ static zend_always_inline zend_class_entry *zend_fetch_ce_from_cache_slot( } static bool zend_check_intersection_type_from_cache_slot(zend_type_list *intersection_type_list, - zend_class_entry *arg_ce, void **cache_slot) + zend_class_entry *arg_ce, void ***cache_slot_ptr) { + void **cache_slot = *cache_slot_ptr; zend_class_entry *ce; zend_type *list_type; + bool status = true; ZEND_TYPE_LIST_FOREACH(intersection_type_list, list_type) { ce = zend_fetch_ce_from_cache_slot(cache_slot, list_type); /* If type is not an instance of one of the types taking part in the * intersection it cannot be a valid instance of the whole intersection type. */ if (!ce || !instanceof_function(arg_ce, ce)) { - return false; + status = false; } + PROGRESS_CACHE_SLOT(); } ZEND_TYPE_LIST_FOREACH_END(); - return true; + if (HAVE_CACHE_SLOT) { + *cache_slot_ptr = cache_slot; + } + return status; } static zend_always_inline bool zend_check_type_slow( @@ -1073,9 +1079,10 @@ static zend_always_inline bool zend_check_type_slow( } else { ZEND_TYPE_LIST_FOREACH(ZEND_TYPE_LIST(*type), list_type) { if (ZEND_TYPE_IS_INTERSECTION(*list_type)) { - if (zend_check_intersection_type_from_cache_slot(ZEND_TYPE_LIST(*list_type), Z_OBJCE_P(arg), cache_slot)) { + if (zend_check_intersection_type_from_cache_slot(ZEND_TYPE_LIST(*list_type), Z_OBJCE_P(arg), &cache_slot)) { return true; } + /* The cache_slot is progressed in zend_check_intersection_type_from_cache_slot() */ } else { ZEND_ASSERT(!ZEND_TYPE_HAS_LIST(*list_type)); ce = zend_fetch_ce_from_cache_slot(cache_slot, list_type); @@ -1083,9 +1090,8 @@ static zend_always_inline bool zend_check_type_slow( if (ce && instanceof_function(Z_OBJCE_P(arg), ce)) { return true; } + PROGRESS_CACHE_SLOT(); } - - PROGRESS_CACHE_SLOT(); } ZEND_TYPE_LIST_FOREACH_END(); } } else { From c70a8281e375acd0d0d315836ac7bf511067276e Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Sat, 10 Sep 2022 11:39:01 +0100 Subject: [PATCH 3/3] Use DNF intersection type check also for simple intersection types --- Zend/zend_execute.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/Zend/zend_execute.c b/Zend/zend_execute.c index 657b453b838..35f2f3c6151 100644 --- a/Zend/zend_execute.c +++ b/Zend/zend_execute.c @@ -1066,16 +1066,7 @@ static zend_always_inline bool zend_check_type_slow( if (UNEXPECTED(ZEND_TYPE_HAS_LIST(*type))) { zend_type *list_type; if (ZEND_TYPE_IS_INTERSECTION(*type)) { - ZEND_TYPE_LIST_FOREACH(ZEND_TYPE_LIST(*type), list_type) { - ce = zend_fetch_ce_from_cache_slot(cache_slot, list_type); - /* If type is not an instance of one of the types taking part in the - * intersection it cannot be a valid instance of the whole intersection type. */ - if (!ce || !instanceof_function(Z_OBJCE_P(arg), ce)) { - return false; - } - PROGRESS_CACHE_SLOT(); - } ZEND_TYPE_LIST_FOREACH_END(); - return true; + return zend_check_intersection_type_from_cache_slot(ZEND_TYPE_LIST(*type), Z_OBJCE_P(arg), &cache_slot); } else { ZEND_TYPE_LIST_FOREACH(ZEND_TYPE_LIST(*type), list_type) { if (ZEND_TYPE_IS_INTERSECTION(*list_type)) {