This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[RFA, RFC PATCH, rtl-optimization]: Assert that crtl->emit.regno_pointer_align_length is non-zero when calling gen_reg_rtx


Hello!

As discussed in PR 57896, calling gen_reg_rtx without populated crtl
was the cause of severe internal data corruption. The code following

  /* Make sure regno_pointer_align, and regno_reg_rtx are large
     enough to have an element for this pseudo reg number.  */

comment is simply not prepared to handle case where
crtl->emit.regno_pointer_align_length (and reg_rtx_no) is zero. Even
when resizing the array, the allocated array size stays zero (new size
is calculated as crtl->emit.regno_pointer_align_length * 2), and
consequently

  val = gen_raw_REG (mode, reg_rtx_no);
  regno_reg_rtx[reg_rtx_no++] = val;
  return val;

at the end of the function writes well outside the array.

IMO, the consequences of this data corruption warrant assert in
gen_reg_rtx that crtl->emit.regno_pointer_align_length is non-zero. In
case of invalid use, the compiler would ICE in a predictable way, with
a clear message where and what went wrong, instead of ICEing in random
place due to various unpredictable effects of nearby data corruption
(please see debugging troubles in the PR 57896).

I would like to propose the patch for all release branches. The
benefit of gcc_assert instead of gcc_checking_assert is in consistent
ICE (and consequently consistent bugreports) instead of a data
corruption even for the non-checking compiler. I am aware that this
assert *could* trigger some ICEs on other targets, but these will be
triggered _instead_ of internal data corruption.

2014-02-20  Uros Bizjak  <ubizjak@gmail.com>

    * emit-rtl.c (gen_reg_rtx): Assert that
    crtl->emit.regno_pointer_align_length is non-zero.

The patch was bootstrapped and regression tested on
x86_64-pc-linux-gnu {,-m32} on 4.8 and 4.9 branch.

OK for mainline and release branches?

Uros.

Index: emit-rtl.c
===================================================================
--- emit-rtl.c  (revision 207965)
+++ emit-rtl.c  (working copy)
@@ -896,6 +896,9 @@ gen_reg_rtx (enum machine_mode mode)
       return gen_rtx_CONCAT (mode, realpart, imagpart);
     }

+  /* Do not call gen_reg_rtx with uninitialized crtl.  */
+  gcc_assert (crtl->emit.regno_pointer_align_length);
+
   /* Make sure regno_pointer_align, and regno_reg_rtx are large
      enough to have an element for this pseudo reg number.  */


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]