On July 17, 2015 3:11:51 PM GMT+02:00, Ulrich Weigand <uweig...@de.ibm.com> wrote: >On 07/09/2015 11:43 PM, Martin Liška wrote: > >> This final version which I agreed with Richard Sandiford. >> Hope this can be finally installed to trunk? >> >> Patch can bootstrap and survive regression tests on x86_64-linux-gnu. > >Unfortunately, this still crashes on my SPU toolchain build machine, >for pretty much the same reason outlined here: >https://gcc.gnu.org/ml/gcc-patches/2015-06/msg00868.html > >However, the host compiler no longer miscompiles these lines: > > empty_shared_hash = new shared_hash_def; > empty_shared_hash->refcount = 1; > >But that is simply because that "new" now goes to the default >heap-based allocator from the standard library. There is a >"shared_hash_def_pool", but that's apparently not being used >for anything -- this probably was not intended? > > >But now the following lines are miscompiled: > > elt_list *el = elt_list_pool.allocate (); > el->next = next; > el->elt = elt; > >from new_elt_list in cselib.c. Again, the "allocate" call ends >simply with a cast: > > header = m_returned_free_list; > m_returned_free_list = header->next; > > return (void *)(header); > >and type-based aliasing now states the access to "header->next" >in allocate must not alias the access to "el->next" in new_elt_list, >but clearly it does. > >(Since there is no C++ operator new involved at all anymore, >this clearly violates even the C aliasing rules ...) > >I really think the allocate routine needs to be more careful to >avoid violating aliasing, e.g. by using memcpy or union-based >type-punning to access its free list info.
As far as I understand the object allocator delegates construction to callers and thus in the above case cselib Would be responsible for calling placement new on the return value from Allocate. Richard. >Bye, >Ulrich
