bug-gnu-emacs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#8546: fix for Emacs pseudovector incompatibility with GCC 4.6.0


From: Paul Eggert
Subject: bug#8546: fix for Emacs pseudovector incompatibility with GCC 4.6.0
Date: Mon, 25 Apr 2011 00:41:58 -0700
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.14) Gecko/20110223 Thunderbird/3.1.8

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.

Attachment: pseudovec.txt.gz
Description: GNU Zip compressed data


reply via email to

[Prev in Thread] Current Thread [Next in Thread]