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]

[tree-ssa] Don't consider const pointer dereferences clobbered [patch]


This fixes the recently reported compile failures when compiling
gengtype.c on Darwin.  The bug is rather convoluted to trigger,
but it boiled down to the compiler allowing readonly
INDIRECT_REFs to be considered call clobbered.

Consider the following code fragment.  Ignore the fact that the
code is meaningless.  It has the control/data flow structure that
reproduces the bug.

-----------------------------------------------------------------------------
typedef	unsigned int size_t;
size_t strlength (const char * const);
char foo();

static const char * const str = "mingo";

bar()
{
  size_t c;
  char *x;

  c = strlength (str);
  while (c < 10)
    {
      if (c > 5)
	*x = foo ();
      if (*x < 'a')
	break;
    }

  return *x == '3';
}
-----------------------------------------------------------------------------

When compiled with -pedantic, the compiler will ICE trying to
change the value for *str from CONSTANT to UNDEFINED.  This
happens because '*str' and '*x' are considered aliases and since
*str is entered first in the aliases table, it is used as the
alias leader for the alias set.

When CCP is evaluating the PHI node at the end of the while, it
finds that none of the edges are executable and so it tries to
set its value to UNDEFINED.  But the default value of *str is a
constant.  This is not a valid lattice transition and the
compiler aborts.

The problem is that *str is not a store alias (it's a read only
variable), so it can never be considered modified by the
optimizers.

It's interesting that this only happens with -pedantic.  The
reason is that the gimplifier produces different code without
-pedantic.  Namely, the initial call to strlength() is gimplified
to 

  T.1 = "mingo";
  T.2 = (char *)T.1;
  T.3 = (const char * const)T.2;
  c = strlength (T.3);

instead of

  c = strlength (str);

This is enough to paper over the problem because now *T.3 will be
the alias leader instead of *str.  This patch fixes the problem
and it should also fix the treelang bootstrap problem that
Andreas reported (I haven't tried it yet).


Diego.



	* tree-dfa.c (get_expr_operands): Do not clobber readonly operands
	in CALL_EXPRs.
	(find_vars_r): Likewise.
	(add_indirect_ref_var): When creating new INDIRECT_REF variables,
	copy the readonly attribute from the variable's type.

testsuite/ChangeLog.tree-ssa:

	* gcc.c-torture/compile/20030405-1.[cx]: New test.

Index: tree-dfa.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/Attic/tree-dfa.c,v
retrieving revision 1.1.4.93
diff -d -u -p -r1.1.4.93 tree-dfa.c
--- tree-dfa.c	1 Apr 2003 15:21:32 -0000	1.1.4.93
+++ tree-dfa.c	6 Apr 2003 01:53:30 -0000
@@ -92,6 +92,8 @@ struct walk_state
 
 
 /* Flags to describe operand properties in get_stmt_operands and helpers.  */
+
+/* By default, operands are loaded.  */
 static const int opf_none	= 0;
 
 /* Operand is the target of an assignment expression.  */
@@ -432,9 +434,6 @@ get_expr_operands (stmt, expr_p, flags, 
 	 To address this problem, we add a VUSE<*p> at the call site of
 	 foo().  */
       flags = opf_force_vop | opf_ignore_bp;
-      if (may_clobber)
-	flags |= opf_is_def;
-
       for (op = TREE_OPERAND (expr, 1); op; op = TREE_CHAIN (op))
 	{
 	  tree arg = TREE_VALUE (op);
@@ -446,13 +445,14 @@ get_expr_operands (stmt, expr_p, flags, 
 	  if (SSA_DECL_P (arg)
 	      && POINTER_TYPE_P (TREE_TYPE (arg)))
 	    {
+	      int clobber_arg = (may_clobber && !TREE_READONLY (arg));
 	      tree deref = indirect_ref (arg);
 
 	      /* By default, adding a reference to an INDIRECT_REF
 		 variable, adds a VUSE of the base pointer.  Since we have
 		 already added a real USE for the pointer, we don't need to
 		 add a VUSE for it as well.  */
-	      add_stmt_operand (&deref, stmt, flags, prev_vops);
+	      add_stmt_operand (&deref, stmt, flags | clobber_arg, prev_vops);
 	    }
 	}
 
@@ -2231,21 +2231,25 @@ find_vars_r (tp, walk_subtrees, data)
       tree op;
       bool may_clobber = call_may_clobber (*tp);
 
-      if (may_clobber)
-	walk_state->is_store = 1;
-
       for (op = TREE_OPERAND (*tp, 1); op; op = TREE_CHAIN (op))
 	{
 	  tree arg = TREE_VALUE (op);
 	  if (SSA_DECL_P (arg)
 	      && POINTER_TYPE_P (TREE_TYPE (arg)))
-	    add_indirect_ref_var (arg, walk_state);
+	    {
+	      if (may_clobber && !TREE_READONLY (arg))
+		walk_state->is_store = 1;
+
+	      add_indirect_ref_var (arg, walk_state);
+	      walk_state->is_store = saved_is_store;
+	    }
 	}
 
       /* If the function may clobber globals and addressable locals, add a
 	 reference to GLOBAL_VAR.  */
       if (may_clobber)
 	{
+	  walk_state->is_store = 1;
 	  if (global_var == NULL_TREE)
 	    create_global_var ();
 	  add_referenced_var (global_var, global_var, walk_state);
@@ -2369,7 +2373,9 @@ add_indirect_ref_var (ptr, walk_state)
   tree deref = indirect_ref (ptr);
   if (deref == NULL_TREE)
     {
-      deref = build1 (INDIRECT_REF, TREE_TYPE (TREE_TYPE (ptr)), ptr);
+      tree type = TREE_TYPE (TREE_TYPE (ptr));
+      deref = build1 (INDIRECT_REF, type, ptr);
+      TREE_READONLY (deref) = TYPE_READONLY (type);
       set_indirect_ref (ptr, deref);
     }
 
Index: testsuite/gcc.c-torture/compile/20030405-1.c
===================================================================
RCS file: testsuite/gcc.c-torture/compile/20030405-1.c
diff -N testsuite/gcc.c-torture/compile/20030405-1.c
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gcc.c-torture/compile/20030405-1.c	6 Apr 2003 01:53:33 -0000
@@ -0,0 +1,30 @@
+/* When compiled with -pedantic, this program will cause an ICE when the
+   constant propagator tries to set the value of *str to UNDEFINED.
+   
+   This happens because *str is erroneously considered as a store alias.
+   The aliasing code is then making *str an alias leader for its alias set
+   and when the PHI node at the end of the while() is visited the first
+   time, CCP will try to assign it a value of UNDEFINED, but the default
+   value for *str is a constant.  */
+typedef	unsigned int size_t;
+size_t strlength (const char * const);
+char foo();
+
+static const char * const str = "mingo";
+
+bar()
+{
+  size_t c;
+  char *x;
+
+  c = strlength (str);
+  while (c < 10)
+    {
+      if (c > 5)
+	*x = foo ();
+      if (*x < 'a')
+	break;
+    }
+
+  return *x == '3';
+}
Index: testsuite/gcc.c-torture/compile/20030405-1.x
===================================================================
RCS file: testsuite/gcc.c-torture/compile/20030405-1.x
diff -N testsuite/gcc.c-torture/compile/20030405-1.x
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ testsuite/gcc.c-torture/compile/20030405-1.x	6 Apr 2003 01:53:33 -0000
@@ -0,0 +1,3 @@
+# This test was found to fail only when -pedantic is used.
+set options "-pedantic"
+return 0


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