Re: Big hashes problem in master

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 *) 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 >