[Bug middle-end/43085] Make profiledbootstrap fails with cc1plus catching SIGSEGV

["uweigand at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=43085

Ulrich Weigand <uweigand at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |uweigand at gcc dot gnu.org
          Component|c++                         |middle-end

--- Comment #22 from Ulrich Weigand <uweigand at gcc dot gnu.org> 2011-04-27 16:14:49 UTC ---
I've confirmed the problem with the current 4.5 branch.  Note that the problem
disappears when building with --enable-checking.

A minimal testcase showing the internal compiler error in cc1plus when built
with profile-directed feedback:

class __pool_alloc_base
{
  void _M_round_up () { }
  void _M_get_free_list () { }
  void _M_get_mutex () { }
  void _M_refill () { }
  void _M_allocate_chunk () { }
};

template <typename _Tp> class __pool_alloc : __pool_alloc_base
{
  __pool_alloc () { }
  ~__pool_alloc () { }
};

template class __pool_alloc<char>;

The crash occurs because build_new_method_call is unable to find the base_ctor
method in the __pool_alloc_base class, and then crashes when accessing a NULL
method pointer.  The pointer is not checked for NULL as that routine must be
there since it was added by the compiler itself automatically.

However, due to a mis-compilation of the add_method routine, the method vector
data structure was not actually updated.  The code in add_method does:

      bool reallocated;

      /* We only expect to add few methods in the COMPLETE_P case, so
         just make room for one more method in that case.  */
      if (complete_p)
        reallocated = VEC_reserve_exact (tree, gc, method_vec, 1);
      else
        reallocated = VEC_reserve (tree, gc, method_vec, 1);
      if (reallocated)
        CLASSTYPE_METHOD_VEC (type) = method_vec;
      if (slot == VEC_length (tree, method_vec))
        VEC_quick_push (tree, method_vec, overload);
      else
        VEC_quick_insert (tree, method_vec, slot, overload);

But due to the mis-compile, the 
  CLASSTYPE_METHOD_VEC (type) = method_vec;
line is not executed in this particular case, even though "reallocated" is set.

The vec.h header file routines inlined into add_method here are as follows:

static inline int VEC_OP (T,base,space)                                   \
     (VEC(T,base) *vec_, int alloc_ VEC_CHECK_DECL)                       \
{                                                                         \
  VEC_ASSERT (alloc_ >= 0, "space", T, base);                             \
  return vec_ ? vec_->alloc - vec_->num >= (unsigned)alloc_ : !alloc_;    \
}                                                                         \

static inline int VEC_OP (T,A,reserve)                                    \
     (VEC(T,A) **vec_, int alloc_ VEC_CHECK_DECL MEM_STAT_DECL)           \
{                                                                         \
  int extend = !VEC_OP (T,base,space) (VEC_BASE(*vec_), alloc_            \
                                       VEC_CHECK_PASS);                   \
                                                                          \
  if (extend)                                                             \
    *vec_ = (VEC(T,A) *) vec_##A##_p_reserve (*vec_, alloc_ PASS_MEM_STAT); \
                                                                          \
  return extend;                                                          \
}                                                                         \

static inline int VEC_OP (T,A,reserve_exact)                              \
     (VEC(T,A) **vec_, int alloc_ VEC_CHECK_DECL MEM_STAT_DECL)           \
{                                                                         \
  int extend = !VEC_OP (T,base,space) (VEC_BASE(*vec_), alloc_            \
                                       VEC_CHECK_PASS);                   \
                                                                          \
  if (extend)                                                             \
    *vec_ = (VEC(T,A) *) vec_##A##_p_reserve_exact (*vec_, alloc_         \
                                                    PASS_MEM_STAT);       \
                                                                          \
  return extend;                                                          \
}                                                                         \

and the mis-compile happens when computing the "space" predicate.  Due to the
particular circumstances, the effect is that the "reserve_exact" routine is
actually called, but nevertheless in effect "false" is returned to the caller
of the inlined routine.

In the generated assembler code, the effect can be seen most easily by
comparing the code for 
reallocated = VEC_reserve_exact (tree, gc, method_vec, 1);
with the one generated for
reallocated = VEC_reserve (tree, gc, method_vec, 1);

In the latter case, we get the following snippet for computing "space":

 82ec7de:       85 db                   test   %ebx,%ebx
 82ec7e0:       0f 84 d9 00 00 00       je     82ec8bf <add_method+0x90f>
 82ec7e6:       8b 33                   mov    (%ebx),%esi
 82ec7e8:       31 c9                   xor    %ecx,%ecx
 82ec7ea:       39 73 04                cmp    %esi,0x4(%ebx)
 82ec7ed:       0f 94 c1                sete   %cl
 82ec7f0:       89 ce                   mov    %ecx,%esi
 82ec7f2:       0f 84 cc 00 00 00       je     82ec8c4 <add_method+0x914>

Note the "sete", which in effect sets the returned boolean value.

However, in the same snippet for the "reserve_exact" branch, we have instead:

 82ec0d9:       85 db                   test   %ebx,%ebx
 82ec0db:       0f 84 4c 07 00 00       je     A1: 82ec82d <add_method+0x87d> 
 82ec0e1:       8b 2b                   mov    (%ebx),%ebp
 82ec0e3:       31 c0                   xor    %eax,%eax
 82ec0e5:       39 6b 04                cmp    %ebp,0x4(%ebx)
 82ec0e8:       89 c6                   mov    %eax,%esi
 82ec0ea:       0f 84 42 07 00 00       je     A2: 82ec832 <add_method+0x882>

Note how the "sete" is missing, and "esi" is always set to zero.


This bug is introduced at the if-conversion stage ("ce3" pass).  Before that,
we have the following RTL:

(insn 1908 1810 1909 177 xxx.i:13972 (parallel [
            (set (reg/v:SI 0 ax [orig:195 extend ] [195])
                (const_int 0 [0x0]))
            (clobber (reg:CC 17 flags))
        ]) -1 (nil))

(insn 1909 1908 1910 177 xxx.i:13972 (set (reg:CCZ 17 flags)
        (compare:CCZ (mem/s:SI (plus:SI (reg/v/f:SI 3 bx [orig:94 method_vec ]
[94])
                    (const_int 4 [0x4])) [15 method_vec_511->base.alloc+0 S4
A32])
            (reg:SI 2 cx))) -1 (nil))

(insn 1910 1909 1114 177 xxx.i:13972 (set (strict_low_part (reg:QI 0 ax
[orig:195 extend ] [195]))
        (eq:QI (reg:CCZ 17 flags)
            (const_int 0 [0x0]))) -1 (nil))

(jump_insn 1114 1910 1115 177 xxx.i:13972 (set (pc)
        (if_then_else (ne (reg:CCZ 17 flags)
                (const_int 0 [0x0]))
            (label_ref:SI 1173)
            (pc))) 464 {*jcc_1} (expr_list:REG_DEAD (reg:CCZ 17 flags)
        (expr_list:REG_BR_PROB (const_int 8371 [0x20b3])
            (nil)))
 -> 1173)

(note 1115 1114 1116 178 [bb 178] NOTE_INSN_BASIC_BLOCK)

(insn 1116 1115 1575 178 xxx.i:13972 (set (reg/v:QI 4 si [orig:62 reallocated ]
[62])
        (reg:QI 0 ax [orig:195 extend ] [195])) 53 {*movqi_1}
(expr_list:REG_DEAD (reg:QI 0 ax [orig:195 extend ] [195])
        (nil)))

(jump_insn 1575 1116 1576 178 (set (pc)
        (label_ref 1117)) 477 {jump} (nil)
 -> 1117)

where insn 1910 is what should result in the "sete" instruction.

However, after if-conversion, we get:

(insn 1908 1810 1116 177 xxx.i:13972 (parallel [
            (set (reg/v:SI 0 ax [orig:195 extend ] [195])
                (const_int 0 [0x0]))
            (clobber (reg:CC 17 flags))
        ]) -1 (nil))

(insn 1116 1908 1909 177 xxx.i:13972 (set (reg/v:QI 4 si [orig:62 reallocated ]
[62])
        (reg:QI 0 ax [orig:195 extend ] [195])) 53 {*movqi_1}
(expr_list:REG_DEAD (reg:QI 0 ax [orig:195 extend ] [195])
        (nil)))

(insn 1909 1116 1910 177 xxx.i:13972 (set (reg:CCZ 17 flags)
        (compare:CCZ (mem/s:SI (plus:SI (reg/v/f:SI 3 bx [orig:94 method_vec ]
[94])
                    (const_int 4 [0x4])) [15 method_vec_511->base.alloc+0 S4
A32])
            (reg:SI 2 cx))) -1 (nil))

(insn 1910 1909 1114 177 xxx.i:13972 (set (strict_low_part (reg:QI 0 ax
[orig:195 extend ] [195]))
        (eq:QI (reg:CCZ 17 flags)
            (const_int 0 [0x0]))) -1 (nil))

(jump_insn 1114 1910 1924 177 xxx.i:13972 (set (pc)
        (if_then_else (eq (reg:CCZ 17 flags)
                (const_int 0 [0x0]))
            (label_ref:SI 1117)
            (pc))) 464 {*jcc_1} (expr_list:REG_DEAD (reg:CCZ 17 flags)
        (expr_list:REG_BR_PROB (const_int 1629 [0x65d])
            (nil)))
 -> 1117)

(note 1924 1114 1925 178 [bb 178] NOTE_INSN_BASIC_BLOCK)

(jump_insn 1925 1924 1926 178 (set (pc)
        (label_ref 1173)) -1 (nil)
 -> 1173)


Note how insn 1116 has now been moved to *before* insn 1910, so that it uses an
incorrect value of "ax", and insn 1910 is effectively dead code (which gets
deleted later).

The reason for this is apparently a bug in the data-flow analysis done by the
if-conversion pass when trying to determine whether an instruction can be moved
to a different location.

This bug, it seems, has been detected independently in a different situation
and fixed (in time for 4.6) by Bernd Schmidt by this series of patches:

http://gcc.gnu.org/ml/gcc-cvs/2010-04/msg00745.html
http://gcc.gnu.org/ml/gcc-cvs/2010-04/msg00857.html
http://gcc.gnu.org/ml/gcc-cvs/2010-04/msg01005.html

See the discussion around those patches for details on the data-flow bug.

I've backported these three patches to the 4.5 branch, and this fixes the
problem for me (profiled-bootstrap works again).