Skip to content

Commit

Permalink
Initialize SplFixedArray elements to NULL instead of UNDEF
Browse files Browse the repository at this point in the history
The SplFixedArray API treats all elements as NULL, even if they
have not been explicitly initialized. Rather than initializing
to UNDEF an treating that specially in various circumstances,
directly initialize elements to NULL.

This also fixes an assertion failure in the attached test case.
  • Loading branch information
nikic committed Jan 30, 2020
1 parent b33697d commit ca8657a
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 31 deletions.
50 changes: 19 additions & 31 deletions ext/spl/spl_fixedarray.c
Expand Up @@ -76,12 +76,19 @@ static inline spl_fixedarray_object *spl_fixed_array_from_obj(zend_object *obj)

#define Z_SPLFIXEDARRAY_P(zv) spl_fixed_array_from_obj(Z_OBJ_P((zv)))

static inline void spl_fixedarray_init_elems(spl_fixedarray *array, size_t from, size_t to) {
for (size_t i = from; i < to; i++) {
ZVAL_NULL(&array->elements[i]);
}
}

static void spl_fixedarray_init(spl_fixedarray *array, zend_long size) /* {{{ */
{
if (size > 0) {
array->size = 0; /* reset size in case ecalloc() fails */
array->elements = ecalloc(size, sizeof(zval));
array->elements = safe_emalloc(size, sizeof(zval), 0);
array->size = size;
spl_fixedarray_init_elems(array, 0, size);
} else {
array->elements = NULL;
array->size = 0;
Expand Down Expand Up @@ -116,7 +123,7 @@ static void spl_fixedarray_resize(spl_fixedarray *array, zend_long size) /* {{{
}
} else if (size > array->size) {
array->elements = safe_erealloc(array->elements, size, sizeof(zval), 0);
memset(array->elements + array->size, '\0', sizeof(zval) * (size - array->size));
spl_fixedarray_init_elems(array, array->size, size);
} else { /* size < array->size */
zend_long i;

Expand Down Expand Up @@ -161,12 +168,8 @@ static HashTable* spl_fixedarray_object_get_properties(zend_object *obj) /* {{{{
zend_long j = zend_hash_num_elements(ht);

for (i = 0; i < intern->array.size; i++) {
if (!Z_ISUNDEF(intern->array.elements[i])) {
zend_hash_index_update(ht, i, &intern->array.elements[i]);
Z_TRY_ADDREF(intern->array.elements[i]);
} else {
zend_hash_index_update(ht, i, &EG(uninitialized_zval));
}
zend_hash_index_update(ht, i, &intern->array.elements[i]);
Z_TRY_ADDREF(intern->array.elements[i]);
}
if (j > intern->array.size) {
for (i = intern->array.size; i < j; ++i) {
Expand Down Expand Up @@ -323,8 +326,6 @@ static inline zval *spl_fixedarray_object_read_dimension_helper(spl_fixedarray_o
if (index < 0 || index >= intern->array.size) {
zend_throw_exception(spl_ce_RuntimeException, "Index invalid or out of range", 0);
return NULL;
} else if (Z_ISUNDEF(intern->array.elements[index])) {
return NULL;
} else {
return &intern->array.elements[index];
}
Expand Down Expand Up @@ -392,9 +393,7 @@ static inline void spl_fixedarray_object_write_dimension_helper(spl_fixedarray_o
zend_throw_exception(spl_ce_RuntimeException, "Index invalid or out of range", 0);
return;
} else {
if (!Z_ISUNDEF(intern->array.elements[index])) {
zval_ptr_dtor(&(intern->array.elements[index]));
}
zval_ptr_dtor(&(intern->array.elements[index]));
ZVAL_COPY_DEREF(&intern->array.elements[index], value);
}
}
Expand Down Expand Up @@ -440,7 +439,7 @@ static inline void spl_fixedarray_object_unset_dimension_helper(spl_fixedarray_o
return;
} else {
zval_ptr_dtor(&(intern->array.elements[index]));
ZVAL_UNDEF(&intern->array.elements[index]);
ZVAL_NULL(&intern->array.elements[index]);
}
}
/* }}} */
Expand All @@ -459,7 +458,6 @@ static void spl_fixedarray_object_unset_dimension(zend_object *object, zval *off
}

spl_fixedarray_object_unset_dimension_helper(intern, offset);

}
/* }}} */

Expand All @@ -477,16 +475,10 @@ static inline int spl_fixedarray_object_has_dimension_helper(spl_fixedarray_obje
if (index < 0 || index >= intern->array.size) {
retval = 0;
} else {
if (Z_ISUNDEF(intern->array.elements[index])) {
retval = 0;
} else if (check_empty) {
if (zend_is_true(&intern->array.elements[index])) {
retval = 1;
} else {
retval = 0;
}
} else { /* != NULL and !check_empty */
retval = 1;
if (check_empty) {
retval = zend_is_true(&intern->array.elements[index]);
} else {
retval = Z_TYPE(intern->array.elements[index]) != IS_NULL;
}
}

Expand Down Expand Up @@ -628,12 +620,8 @@ SPL_METHOD(SplFixedArray, toArray)

array_init(return_value);
for (; i < intern->array.size; i++) {
if (!Z_ISUNDEF(intern->array.elements[i])) {
zend_hash_index_update(Z_ARRVAL_P(return_value), i, &intern->array.elements[i]);
Z_TRY_ADDREF(intern->array.elements[i]);
} else {
zend_hash_index_update(Z_ARRVAL_P(return_value), i, &EG(uninitialized_zval));
}
zend_hash_index_update(Z_ARRVAL_P(return_value), i, &intern->array.elements[i]);
Z_TRY_ADDREF(intern->array.elements[i]);
}
} else {
RETURN_EMPTY_ARRAY();
Expand Down
14 changes: 14 additions & 0 deletions ext/spl/tests/SplFixedArray_indirect_modification.phpt
@@ -0,0 +1,14 @@
--TEST--
SplFixedArray indirect modification notice
--FILE--
<?php
$a = new SplFixedArray(1);
$a[0][] = 3;
var_dump($a);
?>
--EXPECTF--
Notice: Indirect modification of overloaded element of SplFixedArray has no effect in %s on line %d
object(SplFixedArray)#1 (1) {
[0]=>
NULL
}

0 comments on commit ca8657a

Please sign in to comment.