Skip to content

hacks #27

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ndossche wants to merge 7 commits into from
Closed

hacks #27

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6817df8
hax
ndossche Jun 27, 2023
d3dcdd5
Fix mistake
ndossche Jun 27, 2023
14eefe0
undo oopsie
ndossche Jun 27, 2023
69cd3b9
more fixing
ndossche Jun 27, 2023
0c675bf
fix
ndossche Jun 28, 2023
ade9b5c
Start trying to move everything to a _twice variant instead of _ex
ndossche Jun 28, 2023
a00f5ae
Move more to _twice, maybe a bit extreme but let's see what the perfo…
ndossche Jun 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 40 additions & 6 deletions Zend/zend_execute.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3624,6 +3624,7 @@ static zend_always_inline void i_zval_ptr_dtor_noref(zval *zval_ptr) {
}
}

// TODO: try to get rid of this copy by trying to merge everything to the _twice variant
ZEND_API zval* zend_assign_to_typed_ref_ex(zval *variable_ptr, zval *orig_value, uint8_t value_type, bool strict, zend_refcounted **garbage_ptr)
{
bool ret;
Expand Down Expand Up @@ -3659,14 +3660,47 @@ ZEND_API zval* zend_assign_to_typed_ref_ex(zval *variable_ptr, zval *orig_value,
return variable_ptr;
}

ZEND_API zval* zend_assign_to_typed_ref(zval *variable_ptr, zval *orig_value, uint8_t value_type, bool strict)
ZEND_API zval* zend_assign_to_typed_ref_twice(zval *variable_ptr, zval *orig_value, uint8_t value_type, bool strict, zval *extra_copy)
{
zend_refcounted *garbage = NULL;
zval *result = zend_assign_to_typed_ref_ex(variable_ptr, orig_value, value_type, strict, &garbage);
if (garbage) {
GC_DTOR_NO_REF(garbage);
bool ret;
zval value;
zend_refcounted *ref = NULL;

if (Z_ISREF_P(orig_value)) {
ref = Z_COUNTED_P(orig_value);
orig_value = Z_REFVAL_P(orig_value);
}
return result;

ZVAL_COPY(&value, orig_value);
ret = zend_verify_ref_assignable_zval(Z_REF_P(variable_ptr), &value, strict);
variable_ptr = Z_REFVAL_P(variable_ptr);
if (EXPECTED(ret)) {
if (UNEXPECTED(extra_copy)) {
ZVAL_COPY(extra_copy, &value);
}
if (Z_REFCOUNTED_P(variable_ptr)) {
GC_DTOR(Z_COUNTED_P(variable_ptr));
}
ZVAL_COPY_VALUE(variable_ptr, &value);
} else {
zval_ptr_dtor_nogc(&value);
}
if (value_type & (IS_VAR|IS_TMP_VAR)) {
if (UNEXPECTED(ref)) {
if (UNEXPECTED(GC_DELREF(ref) == 0)) {
zval_ptr_dtor(orig_value);
efree_size(ref, sizeof(zend_reference));
}
} else {
i_zval_ptr_dtor_noref(orig_value);
}
}
return variable_ptr;
}

ZEND_API zval* zend_assign_to_typed_ref(zval *variable_ptr, zval *orig_value, uint8_t value_type, bool strict)
{
return zend_assign_to_typed_ref_twice(variable_ptr, orig_value, value_type, strict, NULL);
}

ZEND_API bool ZEND_FASTCALL zend_verify_prop_assignable_by_ref_ex(const zend_property_info *prop_info, zval *orig_val, bool strict, zend_verify_prop_assignable_by_ref_context context) {
Expand Down
28 changes: 28 additions & 0 deletions Zend/zend_execute.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ ZEND_API void ZEND_FASTCALL zend_ref_del_type_source(zend_property_info_source_l

ZEND_API zval* zend_assign_to_typed_ref(zval *variable_ptr, zval *value, uint8_t value_type, bool strict);
ZEND_API zval* zend_assign_to_typed_ref_ex(zval *variable_ptr, zval *value, uint8_t value_type, bool strict, zend_refcounted **garbage_ptr);
ZEND_API zval* zend_assign_to_typed_ref_twice(zval *variable_ptr, zval *value, uint8_t value_type, bool strict, zval *extra_copy);

static zend_always_inline void zend_copy_to_variable(zval *variable_ptr, zval *value, uint8_t value_type)
{
Expand Down Expand Up @@ -179,6 +180,33 @@ static zend_always_inline zval* zend_assign_to_variable(zval *variable_ptr, zval
return variable_ptr;
}

static zend_always_inline zval* zend_assign_to_variable_twice(zval *variable_ptr, zval *second_variable_ptr, zval *value, uint8_t value_type, bool strict)
{
do {
if (UNEXPECTED(Z_REFCOUNTED_P(variable_ptr))) {
if (Z_ISREF_P(variable_ptr)) {
if (UNEXPECTED(ZEND_REF_HAS_TYPE_SOURCES(Z_REF_P(variable_ptr)))) {
return zend_assign_to_typed_ref_twice(variable_ptr, value, value_type, strict, second_variable_ptr);
}

variable_ptr = Z_REFVAL_P(variable_ptr);
if (EXPECTED(!Z_REFCOUNTED_P(variable_ptr))) {
break;
}
}
zend_refcounted *garbage = Z_COUNTED_P(variable_ptr);
zend_copy_to_variable(variable_ptr, value, value_type);
ZVAL_COPY(second_variable_ptr, variable_ptr);
Copy link

@iluuu1994 iluuu1994 Jun 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't the whole point of this to assign to second_variable_ptr before calling GC_DTOR_NO_REF(garbage), as that may reassign the variable, freeing variable_ptr before being assigned again? I think that's the difference in the tests you're seeing, this should assign value directly, preferably, and before GC_DTOR_NO_REF (variable_ptr and value should be the same at that point).

Copy link

@iluuu1994 iluuu1994 Jun 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, now I'm wondering. Could we have just used variable_ptr instead of value and avoided this whole issue? It's different behavior, but we don't really care about that. It just shouldn't crash. Let me try.

GC_DTOR_NO_REF(garbage);
return variable_ptr;
}
} while (0);

zend_copy_to_variable(variable_ptr, value, value_type);
ZVAL_COPY(second_variable_ptr, variable_ptr);
return variable_ptr;
}

static zend_always_inline zval* zend_assign_to_variable_ex(zval *variable_ptr, zval *value, zend_uchar value_type, bool strict, zend_refcounted **garbage_ptr)
{
do {
Expand Down
67 changes: 35 additions & 32 deletions Zend/zend_vm_def.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2381,7 +2381,6 @@ ZEND_VM_HANDLER(24, ZEND_ASSIGN_OBJ, VAR|UNUSED|THIS|CV, CONST|TMPVAR|CV, CACHE_
zval *object, *value, tmp;
zend_object *zobj;
zend_string *name, *tmp_name;
zend_refcounted *garbage = NULL;

SAVE_OPLINE();
object = GET_OP1_OBJ_ZVAL_PTR_PTR_UNDEF(BP_VAR_W);
Expand Down Expand Up @@ -2411,13 +2410,21 @@ ZEND_VM_C_LABEL(assign_object):
zend_property_info *prop_info = (zend_property_info*) CACHED_PTR_EX(cache_slot + 2);

if (UNEXPECTED(prop_info != NULL)) {
zend_refcounted *garbage = NULL;
value = zend_assign_to_typed_prop(prop_info, property_val, value, &garbage EXECUTE_DATA_CC);
ZEND_VM_C_GOTO(free_and_exit_assign_obj);
if (UNEXPECTED(RETURN_VALUE_USED(opline)) && value) {
ZVAL_COPY_DEREF(EX_VAR(opline->result.var), value);
}
if (garbage) {
GC_DTOR_NO_REF(garbage);
}
ZEND_VM_C_GOTO(free_op_data_and_exit_assign_obj);
} else {
ZEND_VM_C_LABEL(fast_assign_obj):
value = zend_assign_to_variable_ex(property_val, value, OP_DATA_TYPE, EX_USES_STRICT_TYPES(), &garbage);
if (UNEXPECTED(RETURN_VALUE_USED(opline))) {
ZVAL_COPY(EX_VAR(opline->result.var), value);
value = zend_assign_to_variable_twice(property_val, EX_VAR(opline->result.var), value, OP_DATA_TYPE, EX_USES_STRICT_TYPES());
} else {
value = zend_assign_to_variable(property_val, value, OP_DATA_TYPE, EX_USES_STRICT_TYPES());
}
ZEND_VM_C_GOTO(exit_assign_obj);
}
Expand Down Expand Up @@ -2497,11 +2504,9 @@ ZEND_VM_C_LABEL(free_and_exit_assign_obj):
if (UNEXPECTED(RETURN_VALUE_USED(opline)) && value) {
ZVAL_COPY_DEREF(EX_VAR(opline->result.var), value);
}
ZEND_VM_C_LABEL(free_op_data_and_exit_assign_obj):
FREE_OP_DATA();
ZEND_VM_C_LABEL(exit_assign_obj):
if (garbage) {
GC_DTOR_NO_REF(garbage);
}
FREE_OP2();
FREE_OP1();
/* assign_obj has two opcodes! */
Expand All @@ -2514,7 +2519,6 @@ ZEND_VM_HANDLER(25, ZEND_ASSIGN_STATIC_PROP, ANY, ANY, CACHE_SLOT, SPEC(OP_DATA=
USE_OPLINE
zval *prop, *value;
zend_property_info *prop_info;
zend_refcounted *garbage = NULL;

SAVE_OPLINE();

Expand All @@ -2527,18 +2531,21 @@ ZEND_VM_HANDLER(25, ZEND_ASSIGN_STATIC_PROP, ANY, ANY, CACHE_SLOT, SPEC(OP_DATA=
value = GET_OP_DATA_ZVAL_PTR(BP_VAR_R);

if (UNEXPECTED(ZEND_TYPE_IS_SET(prop_info->type))) {
zend_refcounted *garbage = NULL;
value = zend_assign_to_typed_prop(prop_info, prop, value, &garbage EXECUTE_DATA_CC);
FREE_OP_DATA();
if (UNEXPECTED(RETURN_VALUE_USED(opline))) {
ZVAL_COPY(EX_VAR(opline->result.var), value);
}
if (garbage) {
GC_DTOR_NO_REF(garbage);
}
} else {
value = zend_assign_to_variable_ex(prop, value, OP_DATA_TYPE, EX_USES_STRICT_TYPES(), &garbage);
}

if (UNEXPECTED(RETURN_VALUE_USED(opline))) {
ZVAL_COPY(EX_VAR(opline->result.var), value);
}

if (garbage) {
GC_DTOR_NO_REF(garbage);
if (UNEXPECTED(RETURN_VALUE_USED(opline))) {
value = zend_assign_to_variable_twice(prop, EX_VAR(opline->result.var), value, OP_DATA_TYPE, EX_USES_STRICT_TYPES());
} else {
value = zend_assign_to_variable(prop, value, OP_DATA_TYPE, EX_USES_STRICT_TYPES());
}
}

/* assign_static_prop has two opcodes! */
Expand All @@ -2552,7 +2559,6 @@ ZEND_VM_HANDLER(23, ZEND_ASSIGN_DIM, VAR|CV, CONST|TMPVAR|UNUSED|NEXT|CV, SPEC(O
zval *value;
zval *variable_ptr;
zval *dim;
zend_refcounted *garbage = NULL;

SAVE_OPLINE();
orig_object_ptr = object_ptr = GET_OP1_ZVAL_PTR_PTR_UNDEF(BP_VAR_W);
Expand Down Expand Up @@ -2597,6 +2603,9 @@ ZEND_VM_C_LABEL(try_assign_dim_array):
Z_ADDREF_P(value);
}
}
if (UNEXPECTED(RETURN_VALUE_USED(opline))) {
ZVAL_COPY(EX_VAR(opline->result.var), value);
}
} else {
dim = GET_OP2_ZVAL_PTR_UNDEF(BP_VAR_R);
if (OP2_TYPE == IS_CONST) {
Expand All @@ -2608,13 +2617,11 @@ ZEND_VM_C_LABEL(try_assign_dim_array):
ZEND_VM_C_GOTO(assign_dim_error);
}
value = GET_OP_DATA_ZVAL_PTR(BP_VAR_R);
value = zend_assign_to_variable_ex(variable_ptr, value, OP_DATA_TYPE, EX_USES_STRICT_TYPES(), &garbage);
}
if (UNEXPECTED(RETURN_VALUE_USED(opline))) {
ZVAL_COPY(EX_VAR(opline->result.var), value);
}
if (garbage) {
GC_DTOR_NO_REF(garbage);
if (UNEXPECTED(RETURN_VALUE_USED(opline))) {
value = zend_assign_to_variable_twice(variable_ptr, EX_VAR(opline->result.var), value, OP_DATA_TYPE, EX_USES_STRICT_TYPES());
} else {
value = zend_assign_to_variable(variable_ptr, value, OP_DATA_TYPE, EX_USES_STRICT_TYPES());
}
}
} else {
if (EXPECTED(Z_ISREF_P(object_ptr))) {
Expand Down Expand Up @@ -2709,14 +2716,10 @@ ZEND_VM_HANDLER(22, ZEND_ASSIGN, VAR|CV, CONST|TMP|VAR|CV, SPEC(RETVAL))
variable_ptr = GET_OP1_ZVAL_PTR_PTR_UNDEF(BP_VAR_W);

if (!ZEND_VM_SPEC || UNEXPECTED(RETURN_VALUE_USED(opline))) {
zend_refcounted *garbage = NULL;

value = zend_assign_to_variable_ex(variable_ptr, value, OP2_TYPE, EX_USES_STRICT_TYPES(), &garbage);
if (UNEXPECTED(RETURN_VALUE_USED(opline))) {
ZVAL_COPY(EX_VAR(opline->result.var), value);
}
if (garbage) {
GC_DTOR_NO_REF(garbage);
value = zend_assign_to_variable_twice(variable_ptr, EX_VAR(opline->result.var), value, OP2_TYPE, EX_USES_STRICT_TYPES());
} else {
value = zend_assign_to_variable(variable_ptr, value, OP2_TYPE, EX_USES_STRICT_TYPES());
}
} else {
value = zend_assign_to_variable(variable_ptr, value, OP2_TYPE, EX_USES_STRICT_TYPES());
Expand Down
Loading