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]

RFC: PATCH to pointer-to-function alias set handling


For a while now, the C++ front end has declared that a class with virtual
bases can alias anything.  This is unnecessarily conservative; a pointer to
a class can only be used to reference objects of its member or base types,
regardless of any calculations to find those subobjects.  Changing that 

I also wanted to make PMFs work, which opened up a much larger can of
worms.  g++.dg/opt/ptrmem1.C is an example of a case where two pointers to
the same member function type produce different pmf types, because our
internal representation of the types differs.  The pmf produced by the
address expression has a default argument, whereas the typedef does not, so
the method_types are different.  This causes an abort in set_mem_alias_set
as called from store_field (on targets where a pmf doesn't fit in a DImode
register).

I fixed this problem by setting the TYPE_MAIN_VARIANT of a FUNCTION_TYPE or
METHOD_TYPE to a version with no default args.  However, the bug resurfaces
with a minor adjustment to the testcase:

  typedef bool bb;

  class QWidget
  {
    public: void repaint( bb* erase );
  };
  void f (void)
  {
    typedef void (QWidget::*A)(bool*);
    A b = &QWidget::repaint;
  }

Here, bb* is not the same tree as bool*, so the method_types are different,
so the pmfs are different, so we get the same abort.  To fix this, the
TYPE_MAIN_VARIANT of the FUNCTION_TYPE would have to strip all typedef
names from the parameter types.  Here's a C testcase that shows the
problem:

  typedef int foo;
  int f (foo *p) { return 0; }
  int g (int *p) { return 1; }

  void h (int (***pp2)(int *), int (**pp1)(foo *)) { *pp2 = pp1; }

  int main()
  {
    int (*p1)(foo *);
    int (**p2)(int *);

    h (&p2, &p1);
    p1 = f;
    *p2 = g;
    return p1(0);
  }

This fails at -O2 because the typedef fools gcc into thinking that *p2
can't alias p1.

The following patch fixes that failure; does this seem like a reasonable
approach to other folks?

*** ./cp/cp-lang.c.~1~	2004-01-05 15:07:19.000000000 -0500
--- ./cp/cp-lang.c	2004-01-09 17:40:25.000000000 -0500
*************** Boston, MA 02111-1307, USA.  */
*** 35,46 ****
  enum c_language_kind c_language = clk_cxx;
  
  static HOST_WIDE_INT cxx_get_alias_set (tree);
- static bool ok_to_generate_alias_set_for_type (tree);
  static bool cxx_warn_unused_global_decl (tree);
  static tree cp_expr_size (tree);
  static size_t cp_tree_size (enum tree_code);
  static bool cp_var_mod_type_p (tree);
  static void cxx_initialize_diagnostics (diagnostic_context *);
  
  #undef LANG_HOOKS_NAME
  #define LANG_HOOKS_NAME "GNU C++"
--- 35,46 ----
  enum c_language_kind c_language = clk_cxx;
  
  static HOST_WIDE_INT cxx_get_alias_set (tree);
  static bool cxx_warn_unused_global_decl (tree);
  static tree cp_expr_size (tree);
  static size_t cp_tree_size (enum tree_code);
  static bool cp_var_mod_type_p (tree);
  static void cxx_initialize_diagnostics (diagnostic_context *);
+ static tree cp_canonical_type (tree);
  
  #undef LANG_HOOKS_NAME
  #define LANG_HOOKS_NAME "GNU C++"
*************** static void cxx_initialize_diagnostics (
*** 188,193 ****
--- 188,195 ----
  #define LANG_HOOKS_TYPE_PROMOTES_TO cxx_type_promotes_to
  #undef LANG_HOOKS_REGISTER_BUILTIN_TYPE
  #define LANG_HOOKS_REGISTER_BUILTIN_TYPE c_register_builtin_type
+ #undef LANG_HOOKS_CANONICAL_TYPE
+ #define LANG_HOOKS_CANONICAL_TYPE cp_canonical_type
  
  /* Each front end provides its own hooks, for toplev.c.  */
  const struct lang_hooks lang_hooks = LANG_HOOKS_INITIALIZER;
*************** const char *const tree_code_name[] = {
*** 233,292 ****
  };
  #undef DEFTREECODE
  
- /* Check if a C++ type is safe for aliasing.
-    Return TRUE if T safe for aliasing FALSE otherwise.  */
- 
- static bool
- ok_to_generate_alias_set_for_type (tree t)
- {
-   if (TYPE_PTRMEMFUNC_P (t))
-     return true;
-   if (AGGREGATE_TYPE_P (t))
-     {
-       if ((TREE_CODE (t) == RECORD_TYPE) || (TREE_CODE (t) == UNION_TYPE))
- 	{
- 	  tree fields;
- 	  /* Backend-created structs are safe.  */
- 	  if (! CLASS_TYPE_P (t))
- 	    return true;
- 	  /* PODs are safe.  */
- 	  if (! CLASSTYPE_NON_POD_P(t))
- 	    return true;
- 	  /* Classes with virtual baseclasses are not.  */
- 	  if (TYPE_USES_VIRTUAL_BASECLASSES (t))
- 	    return false;
- 	  /* Recursively check the base classes.  */
- 	  if (TYPE_BINFO (t) != NULL && TYPE_BINFO_BASETYPES (t) != NULL)
- 	    {
- 	      int i;
- 	      for (i = 0; i < TREE_VEC_LENGTH (TYPE_BINFO_BASETYPES (t)); i++)
- 		{
- 		  tree binfo = TREE_VEC_ELT (TYPE_BINFO_BASETYPES (t), i);
- 		  if (!ok_to_generate_alias_set_for_type (BINFO_TYPE (binfo)))
- 		    return false;
- 		}
- 	    }
- 	  /* Check all the fields.  */
- 	  for (fields = TYPE_FIELDS (t); fields; fields = TREE_CHAIN (fields))
- 	    {
- 	      if (TREE_CODE (fields) != FIELD_DECL)
- 		continue;
- 	      if (! ok_to_generate_alias_set_for_type (TREE_TYPE (fields)))
- 		return false;
- 	    }
- 	  return true;
- 	}
-       else if (TREE_CODE (t) == ARRAY_TYPE)
- 	return ok_to_generate_alias_set_for_type (TREE_TYPE (t));
-       else
- 	/* This should never happen, we dealt with all the aggregate
- 	   types that can appear in C++ above.  */
- 	abort ();
-     }
-   else
-     return true;
- }
- 
  /* Special routine to get the alias set for C++.  */
  
  static HOST_WIDE_INT
--- 235,240 ----
*************** cxx_get_alias_set (tree t)
*** 299,316 ****
         complete type.  */
      return get_alias_set (TYPE_CONTEXT (t));
    
!   if (/* It's not yet safe to use alias sets for some classes in C++.  */
!       !ok_to_generate_alias_set_for_type (t)
!       /* Nor is it safe to use alias sets for pointers-to-member
! 	 functions, due to the fact that there may be more than one
! 	 RECORD_TYPE type corresponding to the same pointer-to-member
! 	 type.  */
!       || TYPE_PTRMEMFUNC_P (t))
!     return 0;
  
    return c_common_get_alias_set (t);
  }
  
  /* Called from check_global_declarations.  */
  
  static bool
--- 247,283 ----
         complete type.  */
      return get_alias_set (TYPE_CONTEXT (t));
    
!   /* Extract the canonical pmf type.  */
!   if (TYPE_PTRMEMFUNC_P (t))
!     {
!       tree t2 = canonical_type (t);
!       if (t != t2)
! 	return get_alias_set (t2);
!     }
  
    return c_common_get_alias_set (t);
  }
  
+ /* This routine is called from canonical_type.  Returns the canonical
+    version of a language-dependent type, or NULL_TREE for no
+    language-dependent handling.  */
+ 
+ static tree
+ cp_canonical_type (tree type)
+ {
+   if (TYPE_PTRMEMFUNC_P (type))
+     {
+       tree oldp = TYPE_PTRMEMFUNC_FN_TYPE (type);
+       tree ptr = canonical_type (oldp);
+       if (ptr != oldp)
+ 	return build_ptrmemfunc_type (ptr);
+       else
+ 	return type;
+     }
+ 
+   return NULL_TREE;
+ }
+ 
  /* Called from check_global_declarations.  */
  
  static bool
*** ./tree.c.~1~	2003-12-19 00:16:30.000000000 -0500
--- ./tree.c	2004-01-09 17:36:53.000000000 -0500
*************** build_type_no_quals (tree t)
*** 3702,3707 ****
--- 3702,3710 ----
        return build_pointer_type (build_type_no_quals (TREE_TYPE (t)));
      case REFERENCE_TYPE:
        return build_reference_type (build_type_no_quals (TREE_TYPE (t)));
+     case RECORD_TYPE:
+       /* Handle C++ pointers to member functions.  */
+       t = canonical_type (t);
      default:
        return TYPE_MAIN_VARIANT (t);
      }
*************** get_inner_array_type (tree array)
*** 3830,3835 ****
--- 3833,3908 ----
    return type;
  }
  
+ /* Returns the canonical version of TYPE -- one without attributes or
+    typedefs.  */
+ 
+ tree
+ canonical_type (tree type)
+ {
+   tree sub, new;
+ 
+   new = (*lang_hooks.types.canonical_type) (type);
+   if (new != NULL_TREE)
+     return new;
+ 
+   if (POINTER_TYPE_P (type))
+     {
+       sub = canonical_type (TREE_TYPE (type));
+ 
+       if (sub == TREE_TYPE (type))
+ 	return type;
+ 
+       if (TREE_CODE (type) == POINTER_TYPE)
+ 	new = build_pointer_type (sub);
+       else if (TREE_CODE (type) == REFERENCE_TYPE)
+ 	new = build_reference_type (sub);
+       else
+ 	abort ();
+     }
+   else if (TREE_CODE (type) == OFFSET_TYPE)
+     {
+       sub = canonical_type (TREE_TYPE (type));
+ 
+       if (sub == TREE_TYPE (type))
+ 	return type;
+ 
+       new = build_offset_type (TYPE_OFFSET_BASETYPE (type), sub);
+     }
+   else if (TREE_CODE (type) == ARRAY_TYPE)
+     {
+       sub = canonical_type (TREE_TYPE (type));
+ 
+       if (sub == TREE_TYPE (type))
+ 	return type;
+ 
+       new = build_array_type (sub, TYPE_DOMAIN (type));
+     }
+   else
+     new = TYPE_MAIN_VARIANT (type);
+ 
+   return build_qualified_type (new, TYPE_QUALS (type));
+ }
+ 
+ /* ARGTYPES is the TYPE_ARG_TYPES for a FUNCTION_TYPE.  If it has no
+    default arguments, return it; otherwise, return an equivalent list with
+    no default arguments.  */
+ 
+ static tree
+ canonical_argtypes (tree argtypes)
+ {
+   tree chain;
+   tree type;
+   if (argtypes == NULL_TREE)
+     return NULL_TREE;
+   chain = canonical_argtypes (TREE_CHAIN (argtypes));
+   type = canonical_type (TREE_VALUE (argtypes));
+   if (chain == TREE_CHAIN (argtypes)
+       && type == TREE_VALUE (argtypes)
+       && TREE_PURPOSE (argtypes) == NULL_TREE)
+     return argtypes;
+   return tree_cons (NULL_TREE, type, chain);
+ }
+ 
  /* Construct, lay out and return
     the type of functions returning type VALUE_TYPE
     given arguments of types ARG_TYPES.
*************** get_inner_array_type (tree array)
*** 3840,3846 ****
  tree
  build_function_type (tree value_type, tree arg_types)
  {
!   tree t;
    unsigned int hashcode;
  
    if (TREE_CODE (value_type) == FUNCTION_TYPE)
--- 3913,3919 ----
  tree
  build_function_type (tree value_type, tree arg_types)
  {
!   tree t, oldt;
    unsigned int hashcode;
  
    if (TREE_CODE (value_type) == FUNCTION_TYPE)
*************** build_function_type (tree value_type, tr
*** 3850,3856 ****
      }
  
    /* Make a node of the sort we want.  */
!   t = make_node (FUNCTION_TYPE);
    TREE_TYPE (t) = value_type;
    TYPE_ARG_TYPES (t) = arg_types;
  
--- 3923,3929 ----
      }
  
    /* Make a node of the sort we want.  */
!   oldt = t = make_node (FUNCTION_TYPE);
    TREE_TYPE (t) = value_type;
    TYPE_ARG_TYPES (t) = arg_types;
  
*************** build_function_type (tree value_type, tr
*** 3858,3863 ****
--- 3931,3949 ----
    hashcode = TYPE_HASH (value_type) + type_hash_list (arg_types);
    t = type_hash_canon (hashcode, t);
  
+   /* Set TYPE_MAIN_VARIANT to a canonicalized version of the type.  */
+   if (t == oldt)
+     {
+       tree canon_args = canonical_argtypes (arg_types);
+       if (canon_args != arg_types)
+ 	{
+ 	  tree m = build_function_type (value_type, canon_args);
+ 	  TYPE_NEXT_VARIANT (t) = TYPE_NEXT_VARIANT (m);
+ 	  TYPE_NEXT_VARIANT (m) = t;
+ 	  TYPE_MAIN_VARIANT (t) = m;
+ 	}
+     }
+ 
    if (!COMPLETE_TYPE_P (t))
      layout_type (t);
    return t;
*************** build_method_type_directly (tree basetyp
*** 3899,3910 ****
  			    tree rettype,
  			    tree argtypes)
  {
!   tree t;
    tree ptype;
    int hashcode;
  
    /* Make a node of the sort we want.  */
!   t = make_node (METHOD_TYPE);
  
    TYPE_METHOD_BASETYPE (t) = TYPE_MAIN_VARIANT (basetype);
    TREE_TYPE (t) = rettype;
--- 3985,3996 ----
  			    tree rettype,
  			    tree argtypes)
  {
!   tree t, oldt;
    tree ptype;
    int hashcode;
  
    /* Make a node of the sort we want.  */
!   oldt = t = make_node (METHOD_TYPE);
  
    TYPE_METHOD_BASETYPE (t) = TYPE_MAIN_VARIANT (basetype);
    TREE_TYPE (t) = rettype;
*************** build_method_type_directly (tree basetyp
*** 3922,3930 ****
  
    t = type_hash_canon (hashcode, t);
  
    if (!COMPLETE_TYPE_P (t))
      layout_type (t);
! 
    return t;
  }
  
--- 4008,4029 ----
  
    t = type_hash_canon (hashcode, t);
  
+   /* Set TYPE_MAIN_VARIANT to a canonicalized version of the type.  */
+   if (t == oldt)
+     {
+       tree canon_args = canonical_argtypes (TREE_CHAIN (argtypes));
+       if (canon_args != TREE_CHAIN (argtypes))
+ 	{
+ 	  tree m = build_method_type_directly (basetype, rettype, canon_args);
+ 	  TYPE_NEXT_VARIANT (t) = TYPE_NEXT_VARIANT (m);
+ 	  TYPE_NEXT_VARIANT (m) = t;
+ 	  TYPE_MAIN_VARIANT (t) = m;
+ 	}
+     }
+ 
    if (!COMPLETE_TYPE_P (t))
      layout_type (t);
!       
    return t;
  }
  
*** ./c-common.c.~1~	2004-01-05 15:06:37.000000000 -0500
--- ./c-common.c	2004-01-09 09:33:20.000000000 -0500
*************** c_common_get_alias_set (tree t)
*** 2916,2922 ****
  
              int *ip;
              int **ipp = &ip;
!             const int* const* cipp = &ipp;
  
           And, it doesn't make sense for that to be legal unless you
  	 can dereference IPP and CIPP.  So, we ignore cv-qualifiers on
--- 2916,2922 ----
  
              int *ip;
              int **ipp = &ip;
!             const int* const* cipp = ipp;
  
           And, it doesn't make sense for that to be legal unless you
  	 can dereference IPP and CIPP.  So, we ignore cv-qualifiers on
*** ./langhooks.c.~1~	2004-01-05 15:06:43.000000000 -0500
--- ./langhooks.c	2004-01-09 16:20:38.000000000 -0500
*************** lhd_incomplete_type_error (tree value AT
*** 243,248 ****
--- 243,256 ----
    abort ();
  }
  
+ /* This routine is called from canonical_type to get the canonical version
+    of a language-dependent type.  */
+ tree
+ lhd_canonical_type (tree type ATTRIBUTE_UNUSED)
+ {
+   return NULL_TREE;
+ }
+ 
  /* Provide a default routine for alias sets that always returns -1.  This
     is used by languages that don't need to do anything special.  */
  
*** ./langhooks.h.~1~	2003-11-14 16:04:15.000000000 -0500
--- ./langhooks.h	2004-01-09 16:32:57.000000000 -0500
*************** struct lang_hooks_for_types
*** 151,156 ****
--- 151,161 ----
       was used (or 0 if that isn't known) and TYPE is the type that was
       invalid.  */
    void (*incomplete_type_error) (tree value, tree type);
+ 
+   /* This routine is called from canonical_type.  Returns the canonical
+      version of a language-dependent type, or NULL_TREE for no
+      language-dependent handling.  */
+   tree (*canonical_type) (tree type);
  };
  
  /* Language hooks related to decls and the symbol table.  */
*** ./tree.h.~1~	2004-01-05 15:06:45.000000000 -0500
--- ./tree.h	2004-01-09 17:31:47.000000000 -0500
*************** extern int tree_int_cst_sgn (tree);
*** 2161,2166 ****
--- 2161,2167 ----
  extern int tree_expr_nonnegative_p (tree);
  extern int rtl_expr_nonnegative_p (rtx);
  extern tree get_inner_array_type (tree);
+ extern tree canonical_type (tree);
  
  /* From expmed.c.  Since rtl.h is included after tree.h, we can't
     put the prototype here.  Rtl.h does declare the prototype if
*** ./langhooks-def.h.~1~	2003-11-14 16:04:15.000000000 -0500
--- ./langhooks-def.h	2004-01-09 17:29:50.000000000 -0500
*************** extern tree lhd_expr_size (tree);
*** 69,74 ****
--- 69,75 ----
  extern bool lhd_decl_uninit (tree);
  extern tree lhd_get_callee_fndecl (tree);
  extern size_t lhd_tree_size (enum tree_code);
+ extern tree lhd_canonical_type (tree);
  
  /* Declarations of default tree inlining hooks.  */
  extern tree lhd_tree_inlining_walk_subtrees (tree *, int *, walk_tree_fn,
*************** extern int lhd_tree_dump_type_quals (tre
*** 221,226 ****
--- 222,228 ----
  #define LANG_HOOKS_INCOMPLETE_TYPE_ERROR lhd_incomplete_type_error
  #define LANG_HOOKS_TYPE_PROMOTES_TO lhd_type_promotes_to
  #define LANG_HOOKS_REGISTER_BUILTIN_TYPE lhd_register_builtin_type
+ #define LANG_HOOKS_CANONICAL_TYPE lhd_canonical_type
  
  #define LANG_HOOKS_FOR_TYPES_INITIALIZER { \
    LANG_HOOKS_MAKE_TYPE, \
*************** extern int lhd_tree_dump_type_quals (tre
*** 231,237 ****
    LANG_HOOKS_SIGNED_OR_UNSIGNED_TYPE, \
    LANG_HOOKS_TYPE_PROMOTES_TO, \
    LANG_HOOKS_REGISTER_BUILTIN_TYPE, \
!   LANG_HOOKS_INCOMPLETE_TYPE_ERROR \
  }
  
  /* Declaration hooks.  */
--- 233,240 ----
    LANG_HOOKS_SIGNED_OR_UNSIGNED_TYPE, \
    LANG_HOOKS_TYPE_PROMOTES_TO, \
    LANG_HOOKS_REGISTER_BUILTIN_TYPE, \
!   LANG_HOOKS_INCOMPLETE_TYPE_ERROR, \
!   LANG_HOOKS_CANONICAL_TYPE \
  }
  
  /* Declaration hooks.  */

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