[PATCH] Avoid setting MEM_READONLY_P on decls that are not really read-only (PR regression/19813)

[Jakub Jelinek <jakub at redhat dot com>]

Hi!

g++ -O2 -dj test.C on:

template <int X> struct A { A(); A(int x) : i(x) {}; int i; };

const static A<1> a;
const static A<2> b(0);
const static A<3> c(4);

int foo (void)
{
  const static A<4> d;
  return a.i + b.i + c.i + d.i;
}

int bar (void)
{
  const static A<5> e(5);
  return e.i;
}

yields:
grep -C1 /u test.C.03.jump | grep -v '^--\|^$'
(insn 43 42 44 5 (set (reg:SI 67 [ a.i ])
        (mem/s/u:SI (symbol_ref:DI ("a") [flags 0x2] <var_decl 0x2a97cb3b60 a>) [3 a.i+0 S4 A32])) -1 (nil)
    (nil))
(insn 44 43 45 5 (set (reg:SI 68 [ b.i ])
        (mem/s/u:SI (symbol_ref:DI ("b") [flags 0x2] <var_decl 0x2a97cb7d00 b>) [3 b.i+0 S4 A32])) -1 (nil)
    (nil))
(insn 46 45 47 5 (set (reg:SI 70 [ c.i ])
        (mem/s/u:SI (symbol_ref:DI ("c") [flags 0x2] <var_decl 0x2a97cbb270 c>) [3 c.i+0 S4 A32])) -1 (nil)
    (nil))
(insn 48 47 49 5 (set (reg:SI 72 [ d.i ])
        (mem/s/u:SI (symbol_ref:DI ("_ZZ3foovE1d") [flags 0x2] <var_decl 0x2a97cbfa90 d>) [3 d.i+0 S4 A32])) -1 (nil)
    (nil))
(insn 22 20 24 2 (set (mem/s/u:SI (symbol_ref:DI ("_ZZ3barvE1e") [flags 0x2] <var_decl 0x2a97cc6b60 e>) [3 e.i+0 S4 A32])
        (const_int 5 [0x5])) -1 (nil)
(insn 28 27 29 3 (set (reg:SI 63 [ e.i ])
        (mem/s/u:SI (symbol_ref:DI ("_ZZ3barvE1e") [flags 0x2] <var_decl 0x2a97cc6b60 e>) [3 e.i+0 S4 A32])) -1 (nil)
    (nil))
(insn 22 20 23 1 (set (mem/s/u:SI (symbol_ref:DI ("b") [flags 0x2] <var_decl 0x2a97cb7d00 b>) [3 b.i+0 S4 A32])
        (const_int 0 [0x0])) -1 (nil)
(insn 23 22 26 1 (set (mem/s/u:SI (symbol_ref:DI ("c") [flags 0x2] <var_decl 0x2a97cbb270 c>) [3 c.i+0 S4 A32])
        (const_int 4 [0x4])) -1 (nil)

As you can see from the instructions, most of the MEM/u's are written into
during the lifetime of the program (and for those that aren't in this grep they
are likely written into in A<>::A()).
On larger testcase as in the PR this leads to miscompilation.

c-common.c (c_apply_type_quals_to_decl) already doesn't set TREE_ONLY
for variables that need constructing:
  if (((type_quals & TYPE_QUAL_CONST)
       || (type && TREE_CODE (type) == REFERENCE_TYPE))
      /* An object declared 'const' is only readonly after it is
         initialized.  We don't have any way of expressing this currently,
         so we need to be conservative and unset TREE_READONLY for types
         with constructors.  Otherwise aliasing code will ignore stores in
         an inline constructor.  */
      && !(type && TYPE_NEEDS_CONSTRUCTING (type)))
    TREE_READONLY (decl) = 1;
If in the testcase above the template is changed into normal class, this
makes sure readonly is not set and therefore neither /u.

Unfortunately, for templates when c_apply_type_quals_to_decl is called,
the type doesn't have TYPE_NEEDS_CONSTRUCTING set yet.  It gets set later
on from complete_type's instantiation (particularly finish_struct_bits),
but at that point TREE_READONLY is already set and nothing clears it.

The following patch fixes it.  In addition to clearing the readonly
flag in start_decl_1 I have added a safety net in set_mem_attributes_minus_bitpos
as well (alternatively it could get an assert).

Ok to commit?

2005-02-16  Jakub Jelinek  <jakub@redhat.com>

	PR regression/19813
	* emit-rtl.c (set_mem_attributes_minus_bitpos): Don't set
	MEM_READONLY_P on bases that needs constructing.

	* decl.c (start_decl_1): Clear TREE_READONLY (decl) if
	TYPE_NEEDS_CONSTRUCTING (type).

--- gcc/emit-rtl.c.jj	2005-01-04 15:41:28.000000000 +0100
+++ gcc/emit-rtl.c	2005-02-16 12:44:53.777089745 +0100
@@ -1532,7 +1532,9 @@ set_mem_attributes_minus_bitpos (rtx ref
       tree base = get_base_address (t);
       if (base && DECL_P (base)
 	  && TREE_READONLY (base)
-	  && (TREE_STATIC (base) || DECL_EXTERNAL (base)))
+	  && (TREE_STATIC (base) || DECL_EXTERNAL (base))
+	  && !(TREE_TYPE (base)
+	       && TYPE_NEEDS_CONSTRUCTING (TREE_TYPE (base))))
 	MEM_READONLY_P (ref) = 1;
 
       if (TREE_THIS_VOLATILE (t))
--- gcc/cp/decl.c.jj	2005-02-16 09:24:13.000000000 +0100
+++ gcc/cp/decl.c	2005-02-16 12:42:55.666217794 +0100
@@ -3823,6 +3823,14 @@ start_decl_1 (tree decl)
      instantiation has occurred that TYPE_HAS_NONTRIVIAL_DESTRUCTOR
      will be set correctly.  */
   maybe_push_cleanup_level (type);
+
+  /* An object declared 'const' is only readonly after it is
+     initialized.  We don't have any way of expressing this currently,
+     so we need to be conservative and unset TREE_READONLY for types
+     with constructors.  Otherwise aliasing code will ignore stores in
+     an inline constructor.  */
+   if (TYPE_NEEDS_CONSTRUCTING (type))
+     TREE_READONLY (decl) = 0;
 }
 
 /* Handle initialization of references.  DECL, TYPE, and INIT have the

	Jakub