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]

[PATCH] Fix tree node sharing problem


Hi,

Ada is very sensitive to tree node sharing because it deals with a lot of 
variable-sized types.  When the variable components are gimplified via 
gimplify_type_sizes, they cause temporaries to be created and these are tied 
to a given subprogram; hence the gimplification cannot occur on the original 
expression attached to the type but on a copy that has previously been 
created when sharing was detected.  This is the mark/copy-if-shared/unmark 
mechanism in the gimplifier.

The current mechanism fails on the following testcase:

package q is
  type ADAPTER_SIZE_TYPE is new INTEGER range 0 .. 65536;
  function ADAPTER_MAX_COMPS return ADAPTER_SIZE_TYPE;
end q;

with q; use q;

package p is
  type HALF_INTEGER is range -32768 .. 32767;
  subtype HALF_NATURAL is HALF_INTEGER range 0 .. 32767;

  MAX_COMPS : constant HALF_NATURAL := HALF_NATURAL(ADAPTER_MAX_COMPS);
  subtype COMP_POINTER_TYPE is HALF_NATURAL range 0 .. MAX_COMPS;
  subtype BUNDLE_POINTER_TYPE is HALF_NATURAL range 0 .. 1;
  subtype CSLIT_POINTER_TYPE is HALF_NATURAL range 0 .. 1;

  procedure BUNDLE_NAM(BP : in BUNDLE_POINTER_TYPE);
  procedure SEQUENCE_NAM(BP : in BUNDLE_POINTER_TYPE);
end p;

package body p is

  function COMPONENT_DAT(BP : in BUNDLE_POINTER_TYPE; CP : in 
COMP_POINTER_TYPE) return HALF_INTEGER is
    type CP_TYPE is access COMP_POINTER_TYPE;
    type CD_TYPE is access HALF_INTEGER;
    CD : CD_TYPE;
  begin
    return CD.all;
  end;

  procedure BUNDLE_NAM(BP : in BUNDLE_POINTER_TYPE) is
    N0 : CSLIT_POINTER_TYPE := COMPONENT_DAT(BP, 4);
  begin
    null;
  end;

  procedure SEQUENCE_NAM(BP : in BUNDLE_POINTER_TYPE) is
    N0 : CSLIT_POINTER_TYPE := COMPONENT_DAT(BP, 4);
  begin
    null;
  end;

end p;

A local variable created in BUNDLE_NAM is expanded in SEQUENCE_NAM.

The problem is the declaration of pointer type CP_TYPE that references 
COMP_POINTER_TYPE in COMPONENT_DAT:

    type CP_TYPE is access COMP_POINTER_TYPE;

One of its subtrees references ADAPTER_MAX_COMPS via the upper bound of 
COMP_POINTER_TYPE.  The sequence of events is fairly intricate and I'm not 
sure giving it will not further complicate the matter; the bottom line is 
that the mere declaration of a pointer to a type should not cause the 
expressions of the designated type to be "marked".

The underlying problem is the discrepancy between gimplify_type_sizes and the 
TYPE_DECL case of walk_tree (Richard Kenner put the ??? note to underline the 
weak specification of the whole thing):

tree.c:walk_tree
[...]
    case DECL_EXPR:
      /* Walk into various fields of the type that it's defining.  We only
	 want to walk into these fields of a type in this case.  Note that
	 decls get walked as part of the processing of a BIND_EXPR.

	 ??? Precisely which fields of types that we are supposed to walk in
	 this case vs. the normal case aren't well defined.  */
      if (TREE_CODE (DECL_EXPR_DECL (*tp)) == TYPE_DECL
	  && TREE_CODE (TREE_TYPE (DECL_EXPR_DECL (*tp))) != ERROR_MARK)
	{
	  tree *type_p = &TREE_TYPE (DECL_EXPR_DECL (*tp));

More precisely, I'm now convinced (I've been working on this problem for quite 
some time) that they should be in keeping in the sense that the fields walked 
in the TYPE_DECL case of walk_tree must be the fields that can be 
expressions, and those are the fields that are directly gimplified in 
gimplify_type_sizes (not through a recursive invocation via another type).

The problem for the testcase is that TYPE_MAX_VALUE of an integer type can be 
an expression in Ada, is directly gimplified in gimplify_type_sizes but is 
walked by walk_tree on a mere invocation of a pointer to the integer type.

Hence the proposed patch that brings TYPE_MIN_VALUE/TYPE_MAX_VALUE on par with 
other potentially variable-sized fields of types.  There is one problem 
though: the C++ front-end uses the tree walking machinery to detect unused 
template paramaters in template specialization, by running the machinery on 
the "formal" expression resulting from the specialization.  The patch has an 
adverse effect on partial specialization of array templates:

+FAIL: g++.dg/template/partial3.C (test for excess errors)
+FAIL: g++.old-deja/g++.pt/partial2.C (test for excess errors)
+FAIL: g++.old-deja/g++.pt/spec21.C  (test for errors, line 16)

because there is no TYPE_DECL for the index type in the "formal" expression.
That's easily fixed though by specifically teaching for_each_template_parm_r 
about TYPE_MIN_VALUE/TYPE_MAX_VALUE.

Bootstrapped/regtested on x86_64-suse-linux.  OK for mainline?


2006-03-21  Eric Botcazou  <ebotcazou@adacore.com>

	* tree.c (walk_type_fields): Do not handle TYPE_MIN_VALUE and
	TYPE_MAX_VALUE for scalar types here but...
	(walk_tree): ...there instead.  Return NULL_TREE if the TYPE_DECL
	is attached an error mark, and the return value of the callback
	if it is not NULL_TREE.
	* cp/pt.c (for_each_template_parm_r) <INTEGER_TYPE>: New case.
	Call for_each_template_parm on TYPE_MIN_VALUE and TYPE_MAX_VALUE.


-- 
Eric Botcazou
Index: tree.c
===================================================================
--- tree.c	(revision 112130)
+++ tree.c	(working copy)
@@ -7247,14 +7248,6 @@ walk_type_fields (tree type, walk_tree_f
       WALK_SUBTREE (TYPE_DOMAIN (type));
       break;
 
-    case BOOLEAN_TYPE:
-    case ENUMERAL_TYPE:
-    case INTEGER_TYPE:
-    case REAL_TYPE:
-      WALK_SUBTREE (TYPE_MIN_VALUE (type));
-      WALK_SUBTREE (TYPE_MAX_VALUE (type));
-      break;
-
     case OFFSET_TYPE:
       WALK_SUBTREE (TREE_TYPE (type));
       WALK_SUBTREE (TYPE_OFFSET_BASETYPE (type));
@@ -7453,23 +7446,29 @@ walk_tree (tree *tp, walk_tree_fn func, 
       }
 
     case DECL_EXPR:
-      /* Walk into various fields of the type that it's defining.  We only
-	 want to walk into these fields of a type in this case.  Note that
-	 decls get walked as part of the processing of a BIND_EXPR.
-
-	 ??? Precisely which fields of types that we are supposed to walk in
-	 this case vs. the normal case aren't well defined.  */
-      if (TREE_CODE (DECL_EXPR_DECL (*tp)) == TYPE_DECL
-	  && TREE_CODE (TREE_TYPE (DECL_EXPR_DECL (*tp))) != ERROR_MARK)
+      /* If this is a TYPE_DECL, walk into various fields of the type that it's
+	 defining.  We only want to walk into these fields of a type in this
+	 case and not in the general case of a mere reference to the type.
+
+	 The criterion is as follows: if the field can be an expression, it
+	 must be walked only here.  That should be in keeping with the fields
+	 that are directly gimplified in gimplify_type_sizes in order for the
+	 mark/copy-if-shared/unmark machinery of the gimplifier to work with
+	 variable-sized types.
+  
+	 Note that DECLs get walked as part of the processing of a BIND_EXPR.  */
+      if (TREE_CODE (DECL_EXPR_DECL (*tp)) == TYPE_DECL)
 	{
 	  tree *type_p = &TREE_TYPE (DECL_EXPR_DECL (*tp));
+	  if (*type_p == ERROR_MARK)
+	    return NULL_TREE;
 
 	  /* Call the function for the type.  See if it returns anything or
 	     doesn't want us to continue.  If we are to continue, walk both
 	     the normal fields and those for the declaration case.  */
 	  result = (*func) (type_p, &walk_subtrees, data);
 	  if (result || !walk_subtrees)
-	    return NULL_TREE;
+	    return result;
 
 	  result = walk_type_fields (*type_p, func, data, pset);
 	  if (result)
@@ -7500,6 +7499,16 @@ walk_tree (tree *tp, walk_tree_fn func, 
 		}
 	    }
 
+	  /* Same for scalar types.  */
+	  else if (TREE_CODE (*type_p) == BOOLEAN_TYPE
+		   || TREE_CODE (*type_p) == ENUMERAL_TYPE
+		   || TREE_CODE (*type_p) == INTEGER_TYPE
+		   || TREE_CODE (*type_p) == REAL_TYPE)
+	    {
+	      WALK_SUBTREE (TYPE_MIN_VALUE (*type_p));
+	      WALK_SUBTREE (TYPE_MAX_VALUE (*type_p));
+	    }
+
 	  WALK_SUBTREE (TYPE_SIZE (*type_p));
 	  WALK_SUBTREE_TAIL (TYPE_SIZE_UNIT (*type_p));
 	}
Index: cp/pt.c
===================================================================
--- cp/pt.c	(revision 112130)
+++ cp/pt.c	(working copy)
@@ -4806,6 +4806,14 @@ for_each_template_parm_r (tree *tp, int 
 	return error_mark_node;
       break;
 
+    case INTEGER_TYPE:
+      if (for_each_template_parm (TYPE_MIN_VALUE (t),
+				  fn, data, pfd->visited)
+	  || for_each_template_parm (TYPE_MAX_VALUE (t),
+				     fn, data, pfd->visited))
+	return error_mark_node;
+      break;
+
     case METHOD_TYPE:
       /* Since we're not going to walk subtrees, we have to do this
 	 explicitly here.  */

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