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

["Ulrich Weigand" <uweigand at de dot ibm dot com>]

Hello,

since the new type-based pool allocator was merged, the SPU toolchain
automated build / regression test has been failing due to crashes of
the compiled GCC due to random memory corruption.

Debugging this, it seems this is caused by GCC being miscompiled by
the host compiler.  As one example, var-tracking.c contains this
code in vt_initialize:

  empty_shared_hash = new shared_hash_def;
  empty_shared_hash->refcount = 1;

Via operator new, these bits of alloc-pool.h are inlined:

template <typename T>
inline T *
pool_allocator<T>::allocate ()
{
[...]
  /* Pull the first free element from the free list, and return it.  */
  header = m_returned_free_list;
  VALGRIND_DISCARD (VALGRIND_MAKE_MEM_DEFINED (header, sizeof (*header)));
  m_returned_free_list = header->next;
  m_elts_free--;
[...]
  /* Call default constructor.  */
  return (T *)(header);
}

Note that the read from "header->next" accesses the same memory location
as the write to "empty_shared_hash->refcount" above.

Type-based aliasing now assumes that those accesses cannot alias, since
they use incompatible types.  The scheduler then moves the write to
before the read, which causes m_returned_free_list to be set to 1,
causing a crash later on.

Now -- I'm only seeing this effect on my SPU toolchain regression test
machine, which uses a RHEL 5 distribution with its default GCC 4.1.2
system compiler.  (Using -fno-strict-aliasing fixes the symptom.)

Maybe this old compiler simply has a bug in type-based aliasing.  But
on the other hand, I'm not completely sure whether this *is* a bug --
is the code above supposed to be valid C++, or does it in fact violate
the aliasing rules after all?

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com