Hi!
Looking into bug #69295 I've noticed the following problem with hashes
on master: if we try to create very large hashtable, on 64-bit build
(e.g. one with 1126626666 elements) then _zend_hash_init would set the
following:
ht->nTableSize = 0x80000000
ht->nTableMask = 0xfffffffe
(I'm using hex so that the numbers would be more clear). Then when we
come to zend_hash_real_init_ex, we do allocation of HT_SIZE(ht). But
then after this line:
HT_SET_DATA_ADDR(ht, pemalloc(HT_SIZE(ht), (ht)->u.flags &
HASH_FLAG_PERSISTENT));
I see this:
(gdb) p ht->arData
$37 = (Bucket *) 0xffffffff0a000000
Looks like what happened is that whet HT_HASH_SIZE was calculated,
(-(int32_t)(ht)->nTableMask) was calculated as 0x80000000 as signed it,
and then promoted to size_t to multiply it with sizeof, which produces
0xffffffff80000000. Of course, adding this to a pointer wouldn't do much
good. So the next line,
HT_HASH_RESET(ht);
crashes because the pointer arData is broken. I think this:
#define HT_HASH_SIZE(ht)
((uint32_t)(-(int32_t)(ht)->nTableMask) * sizeof(uint32_t))
Should fix it but would like second pair of eyes on this.
There's also another problem in the code in zend_hash_real_init_ex - the
initialized flag is set before the address is actually set, so if the
allocation fails, the dtor may get broken array marked as initialized.
This however seems to be easily fixed if flags are moved to be after
allocation.
Stas Malyshev
smalyshev@gmail.com
Hi Stas,
This should be fixed now.
As I understood you already fixed the second problem.
Thanks for catching this.
Dmitry.
On Mon, Jun 1, 2015 at 5:38 AM, Stanislav Malyshev smalyshev@gmail.com
wrote:
Hi!
Looking into bug #69295 I've noticed the following problem with hashes
on master: if we try to create very large hashtable, on 64-bit build
(e.g. one with 1126626666 elements) then _zend_hash_init would set the
following:ht->nTableSize = 0x80000000
ht->nTableMask = 0xfffffffe
(I'm using hex so that the numbers would be more clear). Then when we
come to zend_hash_real_init_ex, we do allocation of HT_SIZE(ht). But
then after this line:HT_SET_DATA_ADDR(ht, pemalloc(HT_SIZE(ht), (ht)->u.flags &
HASH_FLAG_PERSISTENT));I see this:
(gdb) p ht->arData
$37 = (Bucket *) 0xffffffff0a000000Looks like what happened is that whet HT_HASH_SIZE was calculated,
(-(int32_t)(ht)->nTableMask) was calculated as 0x80000000 as signed it,
and then promoted to size_t to multiply it with sizeof, which produces
0xffffffff80000000. Of course, adding this to a pointer wouldn't do much
good. So the next line,
HT_HASH_RESET(ht);
crashes because the pointer arData is broken. I think this:#define HT_HASH_SIZE(ht)
((uint32_t)(-(int32_t)(ht)->nTableMask) * sizeof(uint32_t))Should fix it but would like second pair of eyes on this.
There's also another problem in the code in zend_hash_real_init_ex - the
initialized flag is set before the address is actually set, so if the
allocation fails, the dtor may get broken array marked as initialized.
This however seems to be easily fixed if flags are moved to be after
allocation.Stas Malyshev
smalyshev@gmail.com