Re: [PATCH] Restore mempools functionality for MemBlob

On 03/30/2011 04:46 PM, Amos Jeffries wrote:
> On Wed, 30 Mar 2011 15:46:04 -0600, Alex Rousskov wrote:
>> On 03/30/2011 02:59 PM, Kinkie wrote:
>>
>>>> * If pools are indeed not initialized (and they would not be during
>>>> some
>>>> static allocations), we should not access StrPoolsAttrs at all! There
>>>> are different initialization stages here, and you have to handle the
>>>> all
>>>> of them for the code to work reliably.
>>
>>> Why? StrPoolAttrs is statically initialized in the same translation
>>> unit as memAllocString. If memAllocString can be called, it has
>>> already been initlaized.
>>
>> The last bit is incorrect, AFAIK.
>>
>> StrPoolAttrs being static (I actually missed that part due to the
>> terrible syntax used for its declaration!) guarantees that it will be
>> either all zeros or initialized when memAllocString() is called. The
>> code calling memAllocString() does not know anything about StrPoolAttrs
>> and StrPoolAttrs is not local to memAllocString(), so there is no
>> guarantee that StrPoolAttrs will be initialized before the first call.
>>
>> I could not find the exact reference quickly, but the following FAQ
>> describes the same "static initialization order fiasco" problem, albeit
>> indirectly:
>> http://www.parashift.com/c++-faq-lite/ctors.html#faq-10.14
>>
>
> The fiasco is a peculiarity of methods. Which need valid *this.

Not really. The initialization order dependency can bite in many places.
I just failed to find a better link that clearly documents all possible
cases. Here is one more case that does not involve methods:
http://www.parashift.com/c++-faq-lite/ctors.html#faq-10.18

> We are dealing with static constant array. I would be surprised if the
> compiler did not optimize that down to a hard-coded piece of
> pre-initialized memory on the heap.

Sure, but this is not guaranteed, compilers differ, and some of us often
run with optimizations disabled.

> Hmm, is the a real dependable guarantee of it being zero?
> or do we actually have to deal with the random values situation?

Yes, there is such a guarantee. The static array will be filled with
zeros if we got there before "initialization with the constant
expression" stage took place. Here is a quote from the C++ standard:

> Objects with static storage duration (3.7.1) shall be
> zero-initialized (8.5) before any other initialization takes place.

Without this guarantee, the Construct On First Use Idiom would not work:
http://www.parashift.com/c++-faq-lite/ctors.html#faq-10.15

Here is more from the same source, but do not let it confuse you:

> Zero-initialization and initialization with a constant expression are
> collectively called static initialization; all other initialization
> is dynamic initialization. Objects of POD types (3.9) with static
> storage duration initialized with constant expressions (5.19) shall
> be initialized before any dynamic initialization takes place.

> If it *is* guaranteed zero we are safe, any size will be <=
> StrPoolAttrs[0].obj_size.
> And also the i < count iterator abort will cut in preventing any further
> array checks quite optimally.
>
> Problems remain if the array count global is initialized but not the
> array, or if either is set to random values before initialization.

IIRC, the array count is currently a #define... Yes, making it a static
integer would make the code better as we would prevent us from iterating
zero-initialized (i.e., essentially invalid) arrays.

> We could avoid the counter/array disconnect problem by terminating the
> array with an entry of zeros, avoiding the need for an array counter
> entirely.

Sure, there are many solutions here and some may not even require a lot
of code changes, but we need to be clear about the guarantees and the
cases we need to cover.

Please note that my initial comment was based on the wrong assumption
that the array is not static (which would mean random values rather than
zeros). Some versions of the patch may get lucky since the array is
actually static, but it is clear from the discussion that it would be
pure luck (which can be easily changed with more modifications under the
wrong assumptions about the initialization guarantees).

Cheers,

Alex.

>>> In any case, I failed to convince you of the viability of this
>>> approach. As promised, I'll then implement your suggestion of
>>> assigning max(pool_size)+1 to early allocations;
>>
>> I do not insist on the M+1 hack. If others think that the check is not
>> worth the extra KBs for early allocations, it can go. I am just
>> presenting what I consider is a viable option so that we can make an
>> informed decision rather than just removing a useful check as "not
>> needed".
>>
>> Thank you,
>>
>> Alex.
Received on Wed Mar 30 2011 - 23:45:33 MDT