Skip to content

Fix memory leak of function attribute hash table #8070

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
wants to merge 4 commits into from
Closed

Fix memory leak of function attribute hash table #8070

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3225066
Fix freeing of internal attribute arguments
bwoebi Mar 7, 2022
53a687d
Copy function attributes in function_copy_ctor()
TimWolla Mar 7, 2022
ec1cb37
Fix memory leak of function attribute hash table
TimWolla Mar 7, 2022
fb7b2cf
Add tests for function parameter attributes to ext/zend_test
TimWolla Feb 9, 2022
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
16 changes: 16 additions & 0 deletions Zend/zend.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -665,6 +665,22 @@ static void function_copy_ctor(zval *zv) /* {{{ */
}
func->common.arg_info = new_arg_info + 1;
}
if (old_func->common.attributes) {
zend_attribute *old_attr;

func->common.attributes = NULL;

ZEND_HASH_PACKED_FOREACH_PTR(old_func->common.attributes, old_attr) {
uint32_t i;
zend_attribute *attr;

attr = zend_add_attribute(&func->common.attributes, old_attr->name, old_attr->argc, old_attr->flags, old_attr->offset, old_attr->lineno);

for (i = 0 ; i < old_attr->argc; i++) {
ZVAL_DUP(&attr->args[i].value, &old_attr->args[i].value);
}
} ZEND_HASH_FOREACH_END();
}
}
/* }}} */

Expand Down
9 changes: 7 additions & 2 deletions Zend/zend_attributes.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ ZEND_API bool zend_is_attribute_repeated(HashTable *attributes, zend_attribute *
static void attr_free(zval *v)
{
zend_attribute *attr = Z_PTR_P(v);
bool persistent = attr->flags & ZEND_ATTRIBUTE_PERSISTENT;

zend_string_release(attr->name);
zend_string_release(attr->lcname);
Expand All @@ -213,10 +214,14 @@ static void attr_free(zval *v)
if (attr->args[i].name) {
zend_string_release(attr->args[i].name);
}
zval_ptr_dtor(&attr->args[i].value);
if (persistent) {
zval_internal_ptr_dtor(&attr->args[i].value);
} else {
zval_ptr_dtor(&attr->args[i].value);
}
}

pefree(attr, attr->flags & ZEND_ATTRIBUTE_PERSISTENT);
pefree(attr, persistent);
}

ZEND_API zend_attribute *zend_add_attribute(HashTable **attributes, zend_string *name, uint32_t argc, uint32_t flags, uint32_t offset, uint32_t lineno)
Expand Down
19 changes: 15 additions & 4 deletions Zend/zend_opcode.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,11 @@ ZEND_API void zend_function_dtor(zval *zv)
/* For methods this will be called explicitly. */
if (!function->common.scope) {
zend_free_internal_arg_info(&function->internal_function);

if (function->common.attributes) {
zend_hash_release(function->common.attributes);
function->common.attributes = NULL;
}
}

if (!(function->common.fn_flags & ZEND_ACC_ARENA_ALLOCATED)) {
Expand Down Expand Up @@ -433,11 +438,17 @@ ZEND_API void destroy_zend_class(zval *zv)
zend_hash_destroy(&ce->properties_info);
zend_string_release_ex(ce->name, 1);

/* TODO: eliminate this loop for classes without functions with arg_info */
/* TODO: eliminate this loop for classes without functions with arg_info / attributes */
ZEND_HASH_MAP_FOREACH_PTR(&ce->function_table, fn) {
if ((fn->common.fn_flags & (ZEND_ACC_HAS_RETURN_TYPE|ZEND_ACC_HAS_TYPE_HINTS)) &&
fn->common.scope == ce) {
zend_free_internal_arg_info(&fn->internal_function);
if (fn->common.scope == ce) {
if (fn->common.fn_flags & (ZEND_ACC_HAS_RETURN_TYPE|ZEND_ACC_HAS_TYPE_HINTS)) {
zend_free_internal_arg_info(&fn->internal_function);
}

if (fn->common.attributes) {
zend_hash_release(fn->common.attributes);
fn->common.attributes = NULL;
}
}
} ZEND_HASH_FOREACH_END();

Expand Down
113 changes: 113 additions & 0 deletions ext/zend_test/test.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ static zend_class_entry *zend_test_class;
static zend_class_entry *zend_test_child_class;
static zend_class_entry *zend_test_trait;
static zend_class_entry *zend_test_attribute;
static zend_class_entry *zend_test_parameter_attribute;
static zend_class_entry *zend_test_class_with_method_with_parameter_attribute;
static zend_class_entry *zend_test_child_class_with_method_with_parameter_attribute;
static zend_class_entry *zend_test_ns_foo_class;
static zend_class_entry *zend_test_ns2_foo_class;
static zend_class_entry *zend_test_ns2_ns_foo_class;
Expand Down Expand Up @@ -313,6 +316,17 @@ static ZEND_FUNCTION(namespaced_func)
RETURN_TRUE;
}

static ZEND_FUNCTION(zend_test_parameter_with_attribute)
{
zend_string *parameter;

ZEND_PARSE_PARAMETERS_START(1, 1)
Z_PARAM_STR(parameter)
ZEND_PARSE_PARAMETERS_END();

RETURN_LONG(1);
}

static zend_object *zend_test_class_new(zend_class_entry *class_type)
{
zend_object *obj = zend_objects_new(class_type);
Expand Down Expand Up @@ -425,6 +439,50 @@ static ZEND_METHOD(ZendTestNS2_ZendSubNS_Foo, method)
ZEND_PARSE_PARAMETERS_NONE();
}

static ZEND_METHOD(ZendTestParameterAttribute, __construct)
{
zend_string *parameter;

ZEND_PARSE_PARAMETERS_START(1, 1)
Z_PARAM_STR(parameter)
ZEND_PARSE_PARAMETERS_END();

ZVAL_STR_COPY(OBJ_PROP_NUM(Z_OBJ_P(ZEND_THIS), 0), parameter);
}

static ZEND_METHOD(ZendTestClassWithMethodWithParameterAttribute, no_override)
{
zend_string *parameter;

ZEND_PARSE_PARAMETERS_START(1, 1)
Z_PARAM_STR(parameter)
ZEND_PARSE_PARAMETERS_END();

RETURN_LONG(2);
}

static ZEND_METHOD(ZendTestClassWithMethodWithParameterAttribute, override)
{
zend_string *parameter;

ZEND_PARSE_PARAMETERS_START(1, 1)
Z_PARAM_STR(parameter)
ZEND_PARSE_PARAMETERS_END();

RETURN_LONG(3);
}

static ZEND_METHOD(ZendTestChildClassWithMethodWithParameterAttribute, override)
{
zend_string *parameter;

ZEND_PARSE_PARAMETERS_START(1, 1)
Z_PARAM_STR(parameter)
ZEND_PARSE_PARAMETERS_END();

RETURN_LONG(4);
}

PHP_INI_BEGIN()
STD_PHP_INI_BOOLEAN("zend_test.replace_zend_execute_ex", "0", PHP_INI_SYSTEM, OnUpdateBool, replace_zend_execute_ex, zend_zend_test_globals, zend_test_globals)
STD_PHP_INI_BOOLEAN("zend_test.register_passes", "0", PHP_INI_SYSTEM, OnUpdateBool, register_passes, zend_zend_test_globals, zend_test_globals)
Expand Down Expand Up @@ -460,6 +518,61 @@ PHP_MINIT_FUNCTION(zend_test)
attr->validator = zend_attribute_validate_zendtestattribute;
}

zend_test_parameter_attribute = register_class_ZendTestParameterAttribute();
zend_internal_attribute_register(zend_test_parameter_attribute, ZEND_ATTRIBUTE_TARGET_PARAMETER);

{
zend_attribute *attr;

attr = zend_add_parameter_attribute(
zend_hash_str_find_ptr(CG(function_table), "zend_test_parameter_with_attribute", sizeof("zend_test_parameter_with_attribute") - 1),
0,
zend_test_parameter_attribute->name,
1
);

ZVAL_PSTRING(&attr->args[0].value, "value1");
}

zend_test_class_with_method_with_parameter_attribute = register_class_ZendTestClassWithMethodWithParameterAttribute();

{
zend_attribute *attr;

attr = zend_add_parameter_attribute(
zend_hash_str_find_ptr(&zend_test_class_with_method_with_parameter_attribute->function_table, "no_override", sizeof("no_override") - 1),
0,
zend_test_parameter_attribute->name,
1
);

ZVAL_PSTRING(&attr->args[0].value, "value2");

attr = zend_add_parameter_attribute(
zend_hash_str_find_ptr(&zend_test_class_with_method_with_parameter_attribute->function_table, "override", sizeof("override") - 1),
0,
zend_test_parameter_attribute->name,
1
);

ZVAL_PSTRING(&attr->args[0].value, "value3");
}

zend_test_child_class_with_method_with_parameter_attribute = register_class_ZendTestChildClassWithMethodWithParameterAttribute(zend_test_class_with_method_with_parameter_attribute);

{
zend_attribute *attr;

attr = zend_add_parameter_attribute(
zend_hash_str_find_ptr(&zend_test_child_class_with_method_with_parameter_attribute->function_table, "override", sizeof("override") - 1),
0,
zend_test_parameter_attribute->name,
1
);

ZVAL_PSTRING(&attr->args[0].value, "value4");
}

zend_test_ns_foo_class = register_class_ZendTestNS_Foo();
zend_test_ns2_foo_class = register_class_ZendTestNS2_Foo();
zend_test_ns2_ns_foo_class = register_class_ZendTestNS2_ZendSubNS_Foo();
Expand Down
17 changes: 17 additions & 0 deletions ext/zend_test/test.stub.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,21 @@ final class ZendTestAttribute {

}

final class ZendTestParameterAttribute {
public string $parameter;

public function __construct(string $parameter) {}
}

class ZendTestClassWithMethodWithParameterAttribute {
final public function no_override(string $parameter): int {}
public function override(string $parameter): int {}
}

class ZendTestChildClassWithMethodWithParameterAttribute extends ZendTestClassWithMethodWithParameterAttribute {
public function override(string $parameter): int {}
}

enum ZendTestUnitEnum {
case Foo;
case Bar;
Expand Down Expand Up @@ -92,6 +107,8 @@ function zend_weakmap_remove(object $object): bool {}
function zend_weakmap_dump(): array {}

function zend_get_unit_enum(): ZendTestUnitEnum {}

function zend_test_parameter_with_attribute(#[ZendTestParameterAttribute] string $parameter): int {}
}

namespace ZendTestNS {
Expand Down
78 changes: 77 additions & 1 deletion ext/zend_test/test_arginfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* This is a generated file, edit the .stub.php file instead.
* Stub hash: 7326163f8ce5340c12e74af72d47a8926eb39786 */
* Stub hash: af5d698b35753ac9f852688644d6844ba0914b2b */

ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_zend_test_array_return, 0, 0, IS_ARRAY, 0)
ZEND_END_ARG_INFO()
Expand Down Expand Up @@ -71,6 +71,10 @@ ZEND_END_ARG_INFO()
ZEND_BEGIN_ARG_WITH_RETURN_OBJ_INFO_EX(arginfo_zend_get_unit_enum, 0, 0, ZendTestUnitEnum, 0)
ZEND_END_ARG_INFO()

ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_zend_test_parameter_with_attribute, 0, 1, IS_LONG, 0)
ZEND_ARG_TYPE_INFO(0, parameter, IS_STRING, 0)
ZEND_END_ARG_INFO()

ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_ZendTestNS2_ZendSubNS_namespaced_func, 0, 0, _IS_BOOL, 0)
ZEND_END_ARG_INFO()

Expand All @@ -91,6 +95,16 @@ ZEND_END_ARG_INFO()

#define arginfo_class__ZendTestTrait_testMethod arginfo_ZendTestNS2_ZendSubNS_namespaced_func

ZEND_BEGIN_ARG_INFO_EX(arginfo_class_ZendTestParameterAttribute___construct, 0, 0, 1)
ZEND_ARG_TYPE_INFO(0, parameter, IS_STRING, 0)
ZEND_END_ARG_INFO()

#define arginfo_class_ZendTestClassWithMethodWithParameterAttribute_no_override arginfo_zend_test_parameter_with_attribute

#define arginfo_class_ZendTestClassWithMethodWithParameterAttribute_override arginfo_zend_test_parameter_with_attribute

#define arginfo_class_ZendTestChildClassWithMethodWithParameterAttribute_override arginfo_zend_test_parameter_with_attribute

#define arginfo_class_ZendTestNS_Foo_method arginfo_zend_test_void_return

#define arginfo_class_ZendTestNS2_Foo_method arginfo_zend_test_void_return
Expand All @@ -116,13 +130,18 @@ static ZEND_FUNCTION(zend_weakmap_attach);
static ZEND_FUNCTION(zend_weakmap_remove);
static ZEND_FUNCTION(zend_weakmap_dump);
static ZEND_FUNCTION(zend_get_unit_enum);
static ZEND_FUNCTION(zend_test_parameter_with_attribute);
static ZEND_FUNCTION(namespaced_func);
static ZEND_METHOD(_ZendTestClass, is_object);
static ZEND_METHOD(_ZendTestClass, __toString);
static ZEND_METHOD(_ZendTestClass, returnsStatic);
static ZEND_METHOD(_ZendTestClass, returnsThrowable);
static ZEND_METHOD(_ZendTestChildClass, returnsThrowable);
static ZEND_METHOD(_ZendTestTrait, testMethod);
static ZEND_METHOD(ZendTestParameterAttribute, __construct);
static ZEND_METHOD(ZendTestClassWithMethodWithParameterAttribute, no_override);
static ZEND_METHOD(ZendTestClassWithMethodWithParameterAttribute, override);
static ZEND_METHOD(ZendTestChildClassWithMethodWithParameterAttribute, override);
static ZEND_METHOD(ZendTestNS_Foo, method);
static ZEND_METHOD(ZendTestNS2_Foo, method);
static ZEND_METHOD(ZendTestNS2_ZendSubNS_Foo, method);
Expand All @@ -147,6 +166,7 @@ static const zend_function_entry ext_functions[] = {
ZEND_FE(zend_weakmap_remove, arginfo_zend_weakmap_remove)
ZEND_FE(zend_weakmap_dump, arginfo_zend_weakmap_dump)
ZEND_FE(zend_get_unit_enum, arginfo_zend_get_unit_enum)
ZEND_FE(zend_test_parameter_with_attribute, arginfo_zend_test_parameter_with_attribute)
ZEND_NS_FE("ZendTestNS2\\ZendSubNS", namespaced_func, arginfo_ZendTestNS2_ZendSubNS_namespaced_func)
ZEND_FE_END
};
Expand Down Expand Up @@ -183,6 +203,25 @@ static const zend_function_entry class_ZendTestAttribute_methods[] = {
};


static const zend_function_entry class_ZendTestParameterAttribute_methods[] = {
ZEND_ME(ZendTestParameterAttribute, __construct, arginfo_class_ZendTestParameterAttribute___construct, ZEND_ACC_PUBLIC)
ZEND_FE_END
};


static const zend_function_entry class_ZendTestClassWithMethodWithParameterAttribute_methods[] = {
ZEND_ME(ZendTestClassWithMethodWithParameterAttribute, no_override, arginfo_class_ZendTestClassWithMethodWithParameterAttribute_no_override, ZEND_ACC_PUBLIC|ZEND_ACC_FINAL)
ZEND_ME(ZendTestClassWithMethodWithParameterAttribute, override, arginfo_class_ZendTestClassWithMethodWithParameterAttribute_override, ZEND_ACC_PUBLIC)
ZEND_FE_END
};


static const zend_function_entry class_ZendTestChildClassWithMethodWithParameterAttribute_methods[] = {
ZEND_ME(ZendTestChildClassWithMethodWithParameterAttribute, override, arginfo_class_ZendTestChildClassWithMethodWithParameterAttribute_override, ZEND_ACC_PUBLIC)
ZEND_FE_END
};


static const zend_function_entry class_ZendTestUnitEnum_methods[] = {
ZEND_FE_END
};
Expand Down Expand Up @@ -314,6 +353,43 @@ static zend_class_entry *register_class_ZendTestAttribute(void)
return class_entry;
}

static zend_class_entry *register_class_ZendTestParameterAttribute(void)
{
zend_class_entry ce, *class_entry;

INIT_CLASS_ENTRY(ce, "ZendTestParameterAttribute", class_ZendTestParameterAttribute_methods);
class_entry = zend_register_internal_class_ex(&ce, NULL);
class_entry->ce_flags |= ZEND_ACC_FINAL;

zval property_parameter_default_value;
ZVAL_UNDEF(&property_parameter_default_value);
zend_string *property_parameter_name = zend_string_init("parameter", sizeof("parameter") - 1, 1);
zend_declare_typed_property(class_entry, property_parameter_name, &property_parameter_default_value, ZEND_ACC_PUBLIC, NULL, (zend_type) ZEND_TYPE_INIT_MASK(MAY_BE_STRING));
zend_string_release(property_parameter_name);

return class_entry;
}

static zend_class_entry *register_class_ZendTestClassWithMethodWithParameterAttribute(void)
{
zend_class_entry ce, *class_entry;

INIT_CLASS_ENTRY(ce, "ZendTestClassWithMethodWithParameterAttribute", class_ZendTestClassWithMethodWithParameterAttribute_methods);
class_entry = zend_register_internal_class_ex(&ce, NULL);

return class_entry;
}

static zend_class_entry *register_class_ZendTestChildClassWithMethodWithParameterAttribute(zend_class_entry *class_entry_ZendTestClassWithMethodWithParameterAttribute)
{
zend_class_entry ce, *class_entry;

INIT_CLASS_ENTRY(ce, "ZendTestChildClassWithMethodWithParameterAttribute", class_ZendTestChildClassWithMethodWithParameterAttribute_methods);
class_entry = zend_register_internal_class_ex(&ce, class_entry_ZendTestClassWithMethodWithParameterAttribute);

return class_entry;
}

static zend_class_entry *register_class_ZendTestUnitEnum(void)
{
zend_class_entry *class_entry = zend_register_internal_enum("ZendTestUnitEnum", IS_UNDEF, class_ZendTestUnitEnum_methods);
Expand Down
Loading