New type-based pool allocator code miscompiled due to aliasing issue?

[Richard Biener]

On Wed, Jun 17, 2015 at 1:22 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Wed, Jun 17, 2015 at 11:13:58AM +0200, Martin Jambor wrote:
>> > >> Do you mean Richard following changes:
>> > >>
>> > >> alloc-pool.h (allocate):
>> > >> ...
>> > >> +  /* Placement new contructor.  */
>> > >> +  inline void *operator new (size_t, elt_loc_list *&ptr)
>> > >> +  {
>> > >> +    return ptr;
>> > >> +  }
>> > >
>> > > That should be there with including <new>
>> > >
>> > >> and e.g. cselib.h:
>> > >>
>> > >> struct cselib_val
>> > >> {
>> > >>   /* Pool allocation new operator.  */
>> > >>   inline void *operator new (size_t)
>> > >>   {
>> > >>     return pool.allocate ();
>> > >>   }
>> > >>
>> > >>   /* Placement new contructor.  */
>> > >>   inline void *operator new (size_t, char *&ptr)
>> > >>   {
>> > >>     return ptr;
>> > >>   }
>> > >
>> > > Yes, though I wonder whether cselib_val should really have undefined
>> > > contents after
>> > > allocating it?  (or does the pool allocator zero the memory?)
>
> IMHO if you want to put placement new into allocate, then you probably need
> two different methods, one that will return you just void * to the
> allocated memory chunk that you should use
> inline void *operator new (size_t) { return pool.allocate (); } on,
> and another method that will use that and additionally invoke a placement
> new on it and return you the constructed pointer, which you'd use elsewhere.
> Otherwise you'd construct the object twice...

Note that in the case of cselib_val a

  new cselib_val;

will already invoke the constructor.  I think it's just too much
(partly) C++-ification here.
Either all alloc-pool users have to work that way or none.

Richard.

>         Jakub