Subject: fix for Emacs pseudovector incompatibility with GCC 4.6.0

From: Paul Eggert <[email-masked]>
Date: Mon, 25 Apr 2011 00:41:58 -0700

Emacs's pseudovector implementation occasionally runs afoul of
optimizations introduced by version 4.6.0 of GCC, and this can break
Emacs.  Luckily there's a fix, which I would like to install after some more
testing.  I'm publishing it here now for a heads-up.

Another possible fix would be to disable the GCC optimizations.
However, this would no doubt make Emacs slower overall, and anyway the
optimizations are valid according to the rules of C which means that
non-GCC compilers may well be doing them too.  It's better to alter
Emacs to avoid the problem, since this is not too much trouble.

Here are more details about the problem.

Building Emacs 23.3 (as well as the Emacs trunk) on Ubuntu 10.10 x86
and GCC 4.6.0, with Emacs configured by "configure
--enable-checking=all", fails with the following symptoms:

  `/bin/pwd`/temacs --batch --load loadup bootstrap
  
  /home/eggert/junk/emacs-23.3/src/buffer.c:5177: Emacs fatal error: assertion failed: (XVECTOR (Vbuffer_defaults)->size & (PSEUDOVECTOR_FLAG | PVEC_TYPE_MASK)) == (PSEUDOVECTOR_FLAG | (PVEC_BUFFER))
  Aborted

I tracked this down to an incompatibility between Emacs and GCC 4.6.0
on the x86 with -O2 optimization.  GCC does a type-based aliasing
optimization, and reorders stores and loads to locations that cannot
possibly be the same location if the types are as the program says
they are.  Unfortunately, Emacs's pseudovector implementation dissembles
about the types and therefore runs afoul of the optimization.
Possibly there are subtle bugs induced by this, even when
--enable-checking is not used, but --enable-checking makes the problem
obvious.

Here's the source code, in buffer.c's init_buffer_once:

  reset_buffer_local_variables (&buffer_local_symbols, 1);
  /* Prevent GC from getting confused.  */
  buffer_defaults.text = &buffer_defaults.own_text;
  buffer_local_symbols.text = &buffer_local_symbols.own_text;
  BUF_INTERVALS (&buffer_defaults) = 0;
  BUF_INTERVALS (&buffer_local_symbols) = 0;
  XSETPVECTYPE (&buffer_defaults, PVEC_BUFFER);
  XSETBUFFER (Vbuffer_defaults, &buffer_defaults);

Here are the relevant definitions in lisp.h:

  #define XSETPVECTYPE(v,code) ((v)->size |= PSEUDOVECTOR_FLAG | (code))
  #define XSETBUFFER(a, b) (XSETPSEUDOVECTOR (a, b, PVEC_BUFFER))
  #define XSETPSEUDOVECTOR(a, b, code) \
    (XSETVECTOR (a, b),							\
     eassert ((XVECTOR (a)->size & (PSEUDOVECTOR_FLAG | PVEC_TYPE_MASK))	\
	      == (PSEUDOVECTOR_FLAG | (code))))

Here's the generated x86 code, with the problem highlighted:

		movl	$buffer_local_symbols, %eax
		movl	$1, %edx
		call	reset_buffer_local_variables
		movl	$buffer_defaults, %eax
		orl	$5, %eax
		movl	%eax, Vbuffer_defaults
		andl	$-8, %eax
	0=>	movl	(%eax), %eax
	1=>	orl	$1073872896, buffer_defaults
		movl	$buffer_defaults+8, buffer_defaults+76
		andl	$1082129920, %eax
	2=>	cmpl	$1073872896, %eax
		movl	$buffer_local_symbols+8, buffer_local_symbols+76
		movl	$0, buffer_defaults+64
		movl	$0, buffer_local_symbols+64
		je	.L2396
		movl	suppress_checking, %eax
		testl	%eax, %eax
		je	.L2398
	.L2396:

The code marked (1) implements the expansion of XSETPVECTYPE, and sets
buffer_defaults.size to 0x40020000, the mark for a buffer.  The code
marked (2) is part of the expansion of the eassert, and it checks that
XVECTOR (Vbuffer_defaults)->size has the proper flag and code.  But
(2) is relying on a *cached* copy of the size, which was loaded in (0),
and (0) precedes (1).  So, the assertion fails.

The patch is attached.  It's against my copy of Emacs, which has a few
other fixes that I haven't had time to merge to the trunk yet.  But it
should give a good feel for what's involved.