Skip to content

Commit b9f64f7

Browse files
authored
Improve ZPP weak parsing by returning value directly when possible
These changes are based on a previous PR by @ndossche to reduce codebloat: #18436 For zend_parse_arg_str_weak() we can return a `zend_string*` directly, as errors can be indicated by a NULL pointer return. For zend_parse_arg_double_weak() we can return a `double` directly, as we can represent ZPP errors via `NAN` as booleans and integers never coerce to NAN. One might think that the string `'NAN'` can be coerced to double `NAN`, however PHP doesn't allow this coercion. And an explicit cast to double via `(float)` of such a string results in 0. For zend_parse_arg_bool_weak() we instead create a new enum zpp_parse_bool_status that represents a tri-state which we return. Allowing us to return the boolean value via the return type instead of using an out pointer.
1 parent d71eadc commit b9f64f7

6 files changed

Lines changed: 93 additions & 76 deletions

File tree

Zend/zend_API.c

Lines changed: 41 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -509,35 +509,33 @@ static ZEND_COLD bool zend_null_arg_deprecated(const char *fallback_type, uint32
509509
return !EG(exception);
510510
}
511511

512-
ZEND_API bool ZEND_FASTCALL zend_parse_arg_bool_weak(const zval *arg, bool *dest, uint32_t arg_num) /* {{{ */
512+
ZEND_API zpp_parse_bool_status ZEND_FASTCALL zend_parse_arg_bool_weak(const zval *arg, uint32_t arg_num) /* {{{ */
513513
{
514514
if (EXPECTED(Z_TYPE_P(arg) <= IS_STRING)) {
515515
if (UNEXPECTED(Z_TYPE_P(arg) == IS_NULL) && !zend_null_arg_deprecated("bool", arg_num)) {
516-
return 0;
516+
return ZPP_PARSE_BOOL_STATUS_ERROR;
517517
}
518-
*dest = zend_is_true(arg);
519-
} else {
520-
return 0;
518+
return zend_is_true(arg);
521519
}
522-
return 1;
520+
return ZPP_PARSE_BOOL_STATUS_ERROR;
523521
}
524522
/* }}} */
525523

526-
ZEND_API bool ZEND_FASTCALL zend_parse_arg_bool_slow(const zval *arg, bool *dest, uint32_t arg_num) /* {{{ */
524+
ZEND_API zpp_parse_bool_status ZEND_FASTCALL zend_parse_arg_bool_slow(const zval *arg, uint32_t arg_num) /* {{{ */
527525
{
528526
if (UNEXPECTED(ZEND_ARG_USES_STRICT_TYPES())) {
529-
return 0;
527+
return ZPP_PARSE_BOOL_STATUS_ERROR;
530528
}
531-
return zend_parse_arg_bool_weak(arg, dest, arg_num);
529+
return zend_parse_arg_bool_weak(arg, arg_num);
532530
}
533531
/* }}} */
534532

535-
ZEND_API bool ZEND_FASTCALL zend_flf_parse_arg_bool_slow(const zval *arg, bool *dest, uint32_t arg_num)
533+
ZEND_API zpp_parse_bool_status ZEND_FASTCALL zend_flf_parse_arg_bool_slow(const zval *arg, uint32_t arg_num)
536534
{
537535
if (UNEXPECTED(ZEND_FLF_ARG_USES_STRICT_TYPES())) {
538-
return 0;
536+
return ZPP_PARSE_BOOL_STATUS_ERROR;
539537
}
540-
return zend_parse_arg_bool_weak(arg, dest, arg_num);
538+
return zend_parse_arg_bool_weak(arg, arg_num);
541539
}
542540

543541
ZEND_API bool ZEND_FASTCALL zend_parse_arg_long_weak(const zval *arg, zend_long *dest, uint32_t arg_num) /* {{{ */
@@ -624,44 +622,46 @@ ZEND_API bool ZEND_FASTCALL zend_flf_parse_arg_long_slow(const zval *arg, zend_l
624622
return zend_parse_arg_long_weak(arg, dest, arg_num);
625623
}
626624

627-
ZEND_API bool ZEND_FASTCALL zend_parse_arg_double_weak(const zval *arg, double *dest, uint32_t arg_num) /* {{{ */
625+
ZEND_API double ZEND_FASTCALL zend_parse_arg_double_weak(const zval *arg, uint32_t arg_num) /* {{{ */
628626
{
629627
if (EXPECTED(Z_TYPE_P(arg) == IS_LONG)) {
630-
*dest = (double)Z_LVAL_P(arg);
628+
return (double)Z_LVAL_P(arg);
631629
} else if (EXPECTED(Z_TYPE_P(arg) == IS_STRING)) {
632630
zend_long l;
631+
double dval;
633632
uint8_t type;
634633

635-
if (UNEXPECTED((type = is_numeric_str_function(Z_STR_P(arg), &l, dest)) != IS_DOUBLE)) {
634+
if (UNEXPECTED((type = is_numeric_str_function(Z_STR_P(arg), &l, &dval)) != IS_DOUBLE)) {
636635
if (EXPECTED(type != 0)) {
637-
*dest = (double)(l);
636+
return (double)(l);
638637
} else {
639-
return 0;
638+
return NAN;
640639
}
640+
} else {
641+
return dval;
641642
}
642643
} else if (EXPECTED(Z_TYPE_P(arg) < IS_TRUE)) {
643644
if (UNEXPECTED(Z_TYPE_P(arg) == IS_NULL) && !zend_null_arg_deprecated("float", arg_num)) {
644-
return 0;
645+
return NAN;
645646
}
646-
*dest = 0.0;
647+
return 0.0;
647648
} else if (EXPECTED(Z_TYPE_P(arg) == IS_TRUE)) {
648-
*dest = 1.0;
649+
return 1.0;
649650
} else {
650-
return 0;
651+
return NAN;
651652
}
652-
return 1;
653653
}
654654
/* }}} */
655655

656-
ZEND_API bool ZEND_FASTCALL zend_parse_arg_double_slow(const zval *arg, double *dest, uint32_t arg_num) /* {{{ */
656+
ZEND_API double ZEND_FASTCALL zend_parse_arg_double_slow(const zval *arg, uint32_t arg_num) /* {{{ */
657657
{
658658
if (EXPECTED(Z_TYPE_P(arg) == IS_LONG)) {
659659
/* SSTH Exception: IS_LONG may be accepted instead as IS_DOUBLE */
660-
*dest = (double)Z_LVAL_P(arg);
660+
return (double)Z_LVAL_P(arg);
661661
} else if (UNEXPECTED(ZEND_ARG_USES_STRICT_TYPES())) {
662-
return 0;
662+
return NAN;
663663
}
664-
return zend_parse_arg_double_weak(arg, dest, arg_num);
664+
return zend_parse_arg_double_weak(arg, arg_num);
665665
}
666666
/* }}} */
667667

@@ -728,58 +728,58 @@ ZEND_API bool ZEND_FASTCALL zend_parse_arg_number_or_str_slow(zval *arg, zval **
728728
return true;
729729
}
730730

731-
ZEND_API bool ZEND_FASTCALL zend_parse_arg_str_weak(zval *arg, zend_string **dest, uint32_t arg_num) /* {{{ */
731+
ZEND_API zend_string* ZEND_FASTCALL zend_parse_arg_str_weak(zval *arg, uint32_t arg_num) /* {{{ */
732732
{
733733
if (EXPECTED(Z_TYPE_P(arg) < IS_STRING)) {
734734
if (UNEXPECTED(Z_TYPE_P(arg) == IS_NULL) && !zend_null_arg_deprecated("string", arg_num)) {
735-
return 0;
735+
return NULL;
736736
}
737737
convert_to_string(arg);
738-
*dest = Z_STR_P(arg);
738+
return Z_STR_P(arg);
739739
} else if (UNEXPECTED(Z_TYPE_P(arg) == IS_OBJECT)) {
740740
zend_object *zobj = Z_OBJ_P(arg);
741741
zval obj;
742742
if (zobj->handlers->cast_object(zobj, &obj, IS_STRING) == SUCCESS) {
743743
OBJ_RELEASE(zobj);
744744
ZVAL_COPY_VALUE(arg, &obj);
745-
*dest = Z_STR_P(arg);
746-
return 1;
745+
return Z_STR_P(arg);
747746
}
748-
return 0;
747+
return NULL;
749748
} else {
750-
return 0;
749+
return NULL;
751750
}
752-
return 1;
753751
}
754752
/* }}} */
755753

756-
ZEND_API bool ZEND_FASTCALL zend_parse_arg_str_slow(zval *arg, zend_string **dest, uint32_t arg_num) /* {{{ */
754+
ZEND_API zend_string* ZEND_FASTCALL zend_parse_arg_str_slow(zval *arg, uint32_t arg_num) /* {{{ */
757755
{
758756
if (UNEXPECTED(ZEND_ARG_USES_STRICT_TYPES())) {
759-
return 0;
757+
return NULL;
760758
}
761-
return zend_parse_arg_str_weak(arg, dest, arg_num);
759+
return zend_parse_arg_str_weak(arg, arg_num);
762760
}
763761
/* }}} */
764762

765-
ZEND_API bool ZEND_FASTCALL zend_flf_parse_arg_str_slow(zval *arg, zend_string **dest, uint32_t arg_num)
763+
ZEND_API zend_string* ZEND_FASTCALL zend_flf_parse_arg_str_slow(zval *arg, uint32_t arg_num)
766764
{
767765
if (UNEXPECTED(ZEND_FLF_ARG_USES_STRICT_TYPES())) {
768-
return 0;
766+
return NULL;
769767
}
770-
return zend_parse_arg_str_weak(arg, dest, arg_num);
768+
return zend_parse_arg_str_weak(arg, arg_num);
771769
}
772770

773771
ZEND_API bool ZEND_FASTCALL zend_parse_arg_str_or_long_slow(zval *arg, zend_string **dest_str, zend_long *dest_long, uint32_t arg_num) /* {{{ */
774772
{
773+
zend_string *str;
775774
if (UNEXPECTED(ZEND_ARG_USES_STRICT_TYPES())) {
776775
return 0;
777776
}
778777
if (zend_parse_arg_long_weak(arg, dest_long, arg_num)) {
779778
*dest_str = NULL;
780779
return 1;
781-
} else if (zend_parse_arg_str_weak(arg, dest_str, arg_num)) {
780+
} else if ((str = zend_parse_arg_str_weak(arg, arg_num)) != NULL) {
782781
*dest_long = 0;
782+
*dest_str = str;
783783
return 1;
784784
} else {
785785
return 0;

Zend/zend_API.h

Lines changed: 29 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2179,21 +2179,27 @@ ZEND_API ZEND_COLD void zend_class_redeclaration_error_ex(int type, zend_string
21792179

21802180
/* Inlined implementations shared by new and old parameter parsing APIs */
21812181

2182+
typedef enum zpp_parse_bool_status {
2183+
ZPP_PARSE_BOOL_STATUS_FALSE = 0,
2184+
ZPP_PARSE_BOOL_STATUS_TRUE = 1,
2185+
ZPP_PARSE_BOOL_STATUS_ERROR = 2,
2186+
} zpp_parse_bool_status;
2187+
21822188
ZEND_API bool ZEND_FASTCALL zend_parse_arg_class(zval *arg, zend_class_entry **pce, uint32_t num, bool check_null);
2183-
ZEND_API bool ZEND_FASTCALL zend_parse_arg_bool_slow(const zval *arg, bool *dest, uint32_t arg_num);
2184-
ZEND_API bool ZEND_FASTCALL zend_parse_arg_bool_weak(const zval *arg, bool *dest, uint32_t arg_num);
2189+
ZEND_API zpp_parse_bool_status ZEND_FASTCALL zend_parse_arg_bool_slow(const zval *arg, uint32_t arg_num);
2190+
ZEND_API zpp_parse_bool_status ZEND_FASTCALL zend_parse_arg_bool_weak(const zval *arg, uint32_t arg_num);
21852191
ZEND_API bool ZEND_FASTCALL zend_parse_arg_long_slow(const zval *arg, zend_long *dest, uint32_t arg_num);
21862192
ZEND_API bool ZEND_FASTCALL zend_parse_arg_long_weak(const zval *arg, zend_long *dest, uint32_t arg_num);
2187-
ZEND_API bool ZEND_FASTCALL zend_parse_arg_double_slow(const zval *arg, double *dest, uint32_t arg_num);
2188-
ZEND_API bool ZEND_FASTCALL zend_parse_arg_double_weak(const zval *arg, double *dest, uint32_t arg_num);
2189-
ZEND_API bool ZEND_FASTCALL zend_parse_arg_str_slow(zval *arg, zend_string **dest, uint32_t arg_num);
2190-
ZEND_API bool ZEND_FASTCALL zend_parse_arg_str_weak(zval *arg, zend_string **dest, uint32_t arg_num);
2193+
ZEND_API double ZEND_FASTCALL zend_parse_arg_double_slow(const zval *arg, uint32_t arg_num);
2194+
ZEND_API double ZEND_FASTCALL zend_parse_arg_double_weak(const zval *arg, uint32_t arg_num);
2195+
ZEND_API zend_string* ZEND_FASTCALL zend_parse_arg_str_slow(zval *arg, uint32_t arg_num);
2196+
ZEND_API zend_string* ZEND_FASTCALL zend_parse_arg_str_weak(zval *arg, uint32_t arg_num);
21912197
ZEND_API bool ZEND_FASTCALL zend_parse_arg_number_slow(zval *arg, zval **dest, uint32_t arg_num);
21922198
ZEND_API bool ZEND_FASTCALL zend_parse_arg_number_or_str_slow(zval *arg, zval **dest, uint32_t arg_num);
21932199
ZEND_API bool ZEND_FASTCALL zend_parse_arg_str_or_long_slow(zval *arg, zend_string **dest_str, zend_long *dest_long, uint32_t arg_num);
21942200

2195-
ZEND_API bool ZEND_FASTCALL zend_flf_parse_arg_bool_slow(const zval *arg, bool *dest, uint32_t arg_num);
2196-
ZEND_API bool ZEND_FASTCALL zend_flf_parse_arg_str_slow(zval *arg, zend_string **dest, uint32_t arg_num);
2201+
ZEND_API zpp_parse_bool_status ZEND_FASTCALL zend_flf_parse_arg_bool_slow(const zval *arg, uint32_t arg_num);
2202+
ZEND_API zend_string* ZEND_FASTCALL zend_flf_parse_arg_str_slow(zval *arg, uint32_t arg_num);
21972203
ZEND_API bool ZEND_FASTCALL zend_flf_parse_arg_long_slow(const zval *arg, zend_long *dest, uint32_t arg_num);
21982204

21992205
static zend_always_inline bool zend_parse_arg_bool_ex(const zval *arg, bool *dest, bool *is_null, bool check_null, uint32_t arg_num, bool frameless)
@@ -2209,11 +2215,16 @@ static zend_always_inline bool zend_parse_arg_bool_ex(const zval *arg, bool *des
22092215
*is_null = 1;
22102216
*dest = 0;
22112217
} else {
2218+
zpp_parse_bool_status result;
22122219
if (frameless) {
2213-
return zend_flf_parse_arg_bool_slow(arg, dest, arg_num);
2220+
result = zend_flf_parse_arg_bool_slow(arg, arg_num);
22142221
} else {
2215-
return zend_parse_arg_bool_slow(arg, dest, arg_num);
2222+
result = zend_parse_arg_bool_slow(arg, arg_num);
2223+
}
2224+
if (UNEXPECTED(result == ZPP_PARSE_BOOL_STATUS_ERROR)) {
2225+
return false;
22162226
}
2227+
*dest = result;
22172228
}
22182229
return 1;
22192230
}
@@ -2259,7 +2270,8 @@ static zend_always_inline bool zend_parse_arg_double(const zval *arg, double *de
22592270
*is_null = 1;
22602271
*dest = 0.0;
22612272
} else {
2262-
return zend_parse_arg_double_slow(arg, dest, arg_num);
2273+
*dest = zend_parse_arg_double_slow(arg, arg_num);
2274+
return !zend_isnan(*dest);
22632275
}
22642276
return 1;
22652277
}
@@ -2296,12 +2308,13 @@ static zend_always_inline bool zend_parse_arg_str_ex(zval *arg, zend_string **de
22962308
*dest = NULL;
22972309
} else {
22982310
if (frameless) {
2299-
return zend_flf_parse_arg_str_slow(arg, dest, arg_num);
2311+
*dest = zend_flf_parse_arg_str_slow(arg, arg_num);
23002312
} else {
2301-
return zend_parse_arg_str_slow(arg, dest, arg_num);
2313+
*dest = zend_parse_arg_str_slow(arg, arg_num);
23022314
}
2315+
return *dest != NULL;
23032316
}
2304-
return 1;
2317+
return true;
23052318
}
23062319

23072320
static zend_always_inline bool zend_parse_arg_str(zval *arg, zend_string **dest, bool check_null, uint32_t arg_num)
@@ -2532,7 +2545,8 @@ static zend_always_inline bool zend_parse_arg_array_ht_or_str(
25322545
*dest_str = NULL;
25332546
} else {
25342547
*dest_ht = NULL;
2535-
return zend_parse_arg_str_slow(arg, dest_str, arg_num);
2548+
*dest_str = zend_parse_arg_str_slow(arg, arg_num);
2549+
return *dest_str != NULL;
25362550
}
25372551
return 1;
25382552
}

Zend/zend_execute.c

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -733,8 +733,6 @@ static bool zend_verify_weak_scalar_type_hint(uint32_t type_mask, zval *arg)
733733
{
734734
zend_long lval;
735735
double dval;
736-
zend_string *str;
737-
bool bval;
738736

739737
/* Type preference order: int -> float -> string -> bool */
740738
if (type_mask & MAY_BE_LONG) {
@@ -760,16 +758,23 @@ static bool zend_verify_weak_scalar_type_hint(uint32_t type_mask, zval *arg)
760758
return false;
761759
}
762760
}
763-
if ((type_mask & MAY_BE_DOUBLE) && zend_parse_arg_double_weak(arg, &dval, 0)) {
764-
zval_ptr_dtor(arg);
765-
ZVAL_DOUBLE(arg, dval);
766-
return true;
761+
if (type_mask & MAY_BE_DOUBLE) {
762+
dval = zend_parse_arg_double_weak(arg, 0);
763+
if (EXPECTED(!zend_isnan(dval))) {
764+
zval_ptr_dtor(arg);
765+
ZVAL_DOUBLE(arg, dval);
766+
return true;
767+
}
767768
}
768-
if ((type_mask & MAY_BE_STRING) && zend_parse_arg_str_weak(arg, &str, 0)) {
769+
if ((type_mask & MAY_BE_STRING) && zend_parse_arg_str_weak(arg, 0)) {
769770
/* on success "arg" is converted to IS_STRING */
770771
return true;
771772
}
772-
if ((type_mask & MAY_BE_BOOL) == MAY_BE_BOOL && zend_parse_arg_bool_weak(arg, &bval, 0)) {
773+
if ((type_mask & MAY_BE_BOOL) == MAY_BE_BOOL) {
774+
zpp_parse_bool_status bval = zend_parse_arg_bool_weak(arg, 0);
775+
if (UNEXPECTED(bval == ZPP_PARSE_BOOL_STATUS_ERROR)) {
776+
return false;
777+
}
773778
zval_ptr_dtor(arg);
774779
ZVAL_BOOL(arg, bval);
775780
return true;
@@ -793,21 +798,19 @@ static bool can_convert_to_string(const zval *zv) {
793798
static bool zend_verify_weak_scalar_type_hint_no_sideeffect(uint32_t type_mask, const zval *arg)
794799
{
795800
zend_long lval;
796-
double dval;
797-
bool bval;
798801

799802
/* Pass (uint32_t)-1 as arg_num to indicate to ZPP not to emit any deprecation notice,
800803
* this is needed because the version with side effects also uses 0 (e.g. for typed properties) */
801804
if ((type_mask & MAY_BE_LONG) && zend_parse_arg_long_weak(arg, &lval, (uint32_t)-1)) {
802805
return true;
803806
}
804-
if ((type_mask & MAY_BE_DOUBLE) && zend_parse_arg_double_weak(arg, &dval, (uint32_t)-1)) {
807+
if ((type_mask & MAY_BE_DOUBLE) && !zend_isnan(zend_parse_arg_double_weak(arg, (uint32_t)-1))) {
805808
return true;
806809
}
807810
if ((type_mask & MAY_BE_STRING) && can_convert_to_string(arg)) {
808811
return true;
809812
}
810-
if ((type_mask & MAY_BE_BOOL) == MAY_BE_BOOL && zend_parse_arg_bool_weak(arg, &bval, (uint32_t)-1)) {
813+
if ((type_mask & MAY_BE_BOOL) == MAY_BE_BOOL && zend_parse_arg_bool_weak(arg, (uint32_t)-1) != ZPP_PARSE_BOOL_STATUS_ERROR) {
811814
return true;
812815
}
813816
return false;

Zend/zend_frameless_function.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@
6464
dest_ht = NULL; \
6565
ZVAL_COPY(&str_tmp, arg ## arg_num); \
6666
arg ## arg_num = &str_tmp; \
67-
if (!zend_flf_parse_arg_str_slow(arg ## arg_num, &dest_str, arg_num)) { \
67+
if (!(dest_str = zend_flf_parse_arg_str_slow(arg ## arg_num, arg_num))) { \
6868
zend_wrong_parameter_type_error(arg_num, Z_EXPECTED_ARRAY_OR_STRING, arg ## arg_num); \
6969
goto flf_clean; \
7070
} \

Zend/zend_vm_def.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8843,7 +8843,7 @@ ZEND_VM_COLD_CONST_HANDLER(121, ZEND_STRLEN, CONST|TMP|CV, ANY)
88438843
}
88448844

88458845
ZVAL_COPY(&tmp, value);
8846-
if (zend_parse_arg_str_weak(&tmp, &str, 1)) {
8846+
if ((str = zend_parse_arg_str_weak(&tmp, 1)) != NULL) {
88478847
ZVAL_LONG(EX_VAR(opline->result.var), ZSTR_LEN(str));
88488848
zval_ptr_dtor(&tmp);
88498849
break;

0 commit comments

Comments
 (0)