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]

Fix folding through externally keyed vtables


Hi,
currently build of Mozilla with -flto-partition=none fails at:
/tmp/ccQ0smdA.ltrans3.ltrans.o:ccQ0smdA.ltrans3.o:function scriptableInvokeDefault(NPObject*, _NPVariant const*, unsigned int, _NPVariant*) [clone .part.84.4761]: error: undefi
ned reference to 'construction vtable for std::ostream-in-std::basic_ostringstream<char, std::char_traits<char>, std::allocator<char> >'                                        
/tmp/ccQ0smdA.ltrans3.ltrans.o:ccQ0smdA.ltrans3.o:function scriptableInvokeDefault(NPObject*, _NPVariant const*, unsigned int, _NPVariant*) [clone .part.84.4761]: error: undefi
ned reference to 'construction vtable for std::ostream-in-std::basic_ostringstream<char, std::char_traits<char>, std::allocator<char> >'                                        
/tmp/ccQ0smdA.ltrans4.ltrans.o:ccQ0smdA.ltrans4.o:function NPP_New: error: undefined reference to 'construction vtable for std::ostream-in-std::basic_ostringstream<char, std::c
har_traits<char>, std::allocator<char> >'                                                                                                                                       
/tmp/ccQ0smdA.ltrans6.ltrans.o:ccQ0smdA.ltrans6.o:function mozilla::NoteIntentionalCrash(char const*) [clone .local.0] [clone .constprop.67]: error: undefined reference to 'con
struction vtable for std::ostream-in-std::basic_ostringstream<char, std::char_traits<char>, std::allocator<char> >'                                    

and my patch actually makes this failure to happen always.  What happen is the following:

C++ represents vtables as initialized variables. When VTABLE is keyed, they are
DECL_EXTENRAL but their constructor is known.  We do look into them when
folding and derive useful data, primarily for devirtualization.  The
initializers of DECL_EXTERNAL vars however do not exists in current compilatoin
unit. They are build as if they were in the compilation unit they are comming
from.  We previously run into probelm with fact that they can reffer static
sumbol from other unit and added can_refer_decl_in_current_unit_p for this
reason.

This case is even more slipperly - the symbols in question are hidden
and thus they pass the check. Still they can not be referred since they
live in libstdc++ DSO rather than in Mozilla.

Unforutnately there is no way to safely detect these cases - we really don't
know where the unit of vtable will live and if it will be compiled with
-fdefault-visibility=hidden or not, so in order to keep units interoperable no
matter of default visibility setting we can not even look for the hiddden
visibility.  I think we are still safe when visibilities was not defined by
user, since if it is hidden in the other unit, everything will be hidden there
and we won't be able to get to the vtable.

We must however exclude all references where visibility was defined by user.
This means that we can no longer detect this safely. For this reason I added
parameter from_decl to can_refer_decl_in_current_unit_p and execute all
the strange logic only when the value comes from constructor that can't be
output in current unit.

Bootstrapped/regtested x86_64-linux, seems sane?

More conservative approach would be in this case fold only to values really
defined locally.  The patch would not be much different, only
can_refer_decl_in_current_unit_p would be more strict.
There are couple thousdant references in Mozilla that however belongs to
this cagegory. 

Honza

	* cgraphbuild.c (record_reference): Update.
	* lto-cgraph.c (lto_output_varpool_node): External vars
	are not in other partition even if they are not output
	in current partition.
	* gimple-fold.c (can_refer_decl_in_current_unit_p): Take FROM_DECL
	argument; fix.
	(canonicalize_constructor_val): Take FROM_DECL argument.
	(fold_ctor_reference, fold_string_cst_ctor_reference,
	fold_array_ctor_reference, fold_nonarray_ctor_reference,
	fold_ctor_reference): Likewise.
	(fold_const_aggregate_ref_1, gimple_get_virt_method_for_binfo): Update.
	* gimple.h (gimple_fold_builtin): Likewise.
Index: cgraphbuild.c
===================================================================
*** cgraphbuild.c	(revision 187412)
--- cgraphbuild.c	(working copy)
*************** record_reference (tree *tp, int *walk_su
*** 54,60 ****
    tree decl;
    struct record_reference_ctx *ctx = (struct record_reference_ctx *)data;
  
!   t = canonicalize_constructor_val (t);
    if (!t)
      t = *tp;
    else if (t != *tp)
--- 54,60 ----
    tree decl;
    struct record_reference_ctx *ctx = (struct record_reference_ctx *)data;
  
!   t = canonicalize_constructor_val (t, NULL);
    if (!t)
      t = *tp;
    else if (t != *tp)
Index: lto-cgraph.c
===================================================================
*** lto-cgraph.c	(revision 187412)
--- lto-cgraph.c	(working copy)
*************** lto_output_varpool_node (struct lto_simp
*** 585,591 ****
        bp_pack_value (&bp, node->analyzed
  		     && referenced_from_other_partition_p (&node->symbol.ref_list,
  							   set, vset), 1);
!       bp_pack_value (&bp, boundary_p, 1);  /* in_other_partition.  */
      }
    streamer_write_bitpack (&bp);
    if (node->alias_of)
--- 586,593 ----
        bp_pack_value (&bp, node->analyzed
  		     && referenced_from_other_partition_p (&node->symbol.ref_list,
  							   set, vset), 1);
!       bp_pack_value (&bp, boundary_p && !DECL_EXTERNAL (node->symbol.decl), 1);
! 	  /* in_other_partition.  */
      }
    streamer_write_bitpack (&bp);
    if (node->alias_of)
Index: gimple-fold.c
===================================================================
*** gimple-fold.c	(revision 187412)
--- gimple-fold.c	(working copy)
*************** along with GCC; see the file COPYING3.
*** 33,40 ****
  #include "gimple-fold.h"
  
  /* Return true when DECL can be referenced from current unit.
!    We can get declarations that are not possible to reference for
!    various reasons:
  
       1) When analyzing C++ virtual tables.
  	C++ virtual tables do have known constructors even
--- 33,41 ----
  #include "gimple-fold.h"
  
  /* Return true when DECL can be referenced from current unit.
!    FROM_DECL (if non-null) specify constructor of variable DECL was taken from.
!    We can get declarations that are not possible to reference for various
!    reasons:
  
       1) When analyzing C++ virtual tables.
  	C++ virtual tables do have known constructors even
*************** along with GCC; see the file COPYING3.
*** 54,66 ****
          directly.  */
  
  static bool
! can_refer_decl_in_current_unit_p (tree decl)
  {
    struct varpool_node *vnode;
    struct cgraph_node *node;
  
!   if (!TREE_STATIC (decl) && !DECL_EXTERNAL (decl))
      return true;
    /* External flag is set, so we deal with C++ reference
       to static object from other file.  */
    if (DECL_EXTERNAL (decl) && TREE_STATIC (decl)
--- 55,89 ----
          directly.  */
  
  static bool
! can_refer_decl_in_current_unit_p (tree decl, tree from_decl)
  {
    struct varpool_node *vnode;
    struct cgraph_node *node;
+   symtab_node snode;
  
!   /* We will later output the initializer, so we can reffer to it.
!      So we are concerned only when DECL comes from initializer of
!      external var.  */
!   if (!from_decl
!       || TREE_CODE (from_decl) != VAR_DECL
!       || !DECL_EXTERNAL (from_decl)
!       || (symtab_get_node (from_decl)->symbol.in_other_partition))
      return true;
+   /* We are concerned ony about static/external vars and functions.  */
+   if ((!TREE_STATIC (decl) && !DECL_EXTERNAL (decl))
+       || (TREE_CODE (decl) != VAR_DECL && TREE_CODE (decl) != FUNCTION_DECL))
+     return true;
+   /* Weakrefs have somewhat confusing DECL_EXTERNAL flag set; they are always safe.  */
+   if (DECL_EXTERNAL (decl)
+       && lookup_attribute ("weakref", DECL_ATTRIBUTES (node->symbol.decl)))
+     return true;
+   /* We are folding reference from external vtable.  The vtable may reffer
+      to a symbol keyed to other compilation unit.  The other compilation
+      unit may be in separate DSO and the symbol may be hidden.  */
+   if (DECL_VISIBILITY_SPECIFIED (decl)
+       && DECL_EXTERNAL (decl)
+       && (!(snode = symtab_get_node (decl)) || !snode->symbol.in_other_partition))
+     return false;
    /* External flag is set, so we deal with C++ reference
       to static object from other file.  */
    if (DECL_EXTERNAL (decl) && TREE_STATIC (decl)
*************** can_refer_decl_in_current_unit_p (tree d
*** 82,95 ****
    /* We are not at ltrans stage; so don't worry about WHOPR.
       Also when still gimplifying all referred comdat functions will be
       produced.
!      ??? as observed in PR20991 for already optimized out comdat virtual functions
!      we may not neccesarily give up because the copy will be output elsewhere when
!      corresponding vtable is output.  */
    if (!flag_ltrans && (!DECL_COMDAT (decl) || !cgraph_function_flags_ready))
      return true;
!   /* If we already output the function body, we are safe.  */
!   if (TREE_ASM_WRITTEN (decl))
!     return true;
    if (TREE_CODE (decl) == FUNCTION_DECL)
      {
        node = cgraph_get_node (decl);
--- 105,123 ----
    /* We are not at ltrans stage; so don't worry about WHOPR.
       Also when still gimplifying all referred comdat functions will be
       produced.
! 
!      As observed in PR20991 for already optimized out comdat virtual functions
!      it may be tempting to not neccesarily give up because the copy will be
!      output elsewhere when corresponding vtable is output.  
!      This is however not possible - ABI specify that COMDATs are output in
!      units where they are used and when the other unit was compiled with LTO
!      it is possible that vtable was kept public while the function itself
!      was privatized. */
    if (!flag_ltrans && (!DECL_COMDAT (decl) || !cgraph_function_flags_ready))
      return true;
! 
!   /* OK we are seeing either COMDAT or static variable.  In this case we must
!      check that the definition is still around so we can refer it.  */
    if (TREE_CODE (decl) == FUNCTION_DECL)
      {
        node = cgraph_get_node (decl);
*************** can_refer_decl_in_current_unit_p (tree d
*** 99,120 ****
           compilation stage when making a new reference no longer makes callee
           to be compiled.  */
        if (!node || !node->analyzed || node->global.inlined_to)
! 	return false;
      }
    else if (TREE_CODE (decl) == VAR_DECL)
      {
        vnode = varpool_get_node (decl);
!       if (!vnode || !vnode->finalized)
! 	return false;
      }
    return true;
  }
  
  /* CVAL is value taken from DECL_INITIAL of variable.  Try to transform it into
!    acceptable form for is_gimple_min_invariant.   */
  
  tree
! canonicalize_constructor_val (tree cval)
  {
    STRIP_NOPS (cval);
    if (TREE_CODE (cval) == POINTER_PLUS_EXPR
--- 127,155 ----
           compilation stage when making a new reference no longer makes callee
           to be compiled.  */
        if (!node || !node->analyzed || node->global.inlined_to)
! 	{
! 	  gcc_checking_assert (!TREE_ASM_WRITTEN (decl));
! 	  return false;
! 	}
      }
    else if (TREE_CODE (decl) == VAR_DECL)
      {
        vnode = varpool_get_node (decl);
!       if (!vnode || !vnode->analyzed)
! 	{
! 	  gcc_checking_assert (!TREE_ASM_WRITTEN (decl));
! 	  return false;
! 	}
      }
    return true;
  }
  
  /* CVAL is value taken from DECL_INITIAL of variable.  Try to transform it into
!    acceptable form for is_gimple_min_invariant.
!    FROM_DECL (if non-NULL) specify variable whose constructor contains CVAL.  */
  
  tree
! canonicalize_constructor_val (tree cval, tree from_decl)
  {
    STRIP_NOPS (cval);
    if (TREE_CODE (cval) == POINTER_PLUS_EXPR
*************** canonicalize_constructor_val (tree cval)
*** 137,143 ****
  
        if ((TREE_CODE (base) == VAR_DECL
  	   || TREE_CODE (base) == FUNCTION_DECL)
! 	  && !can_refer_decl_in_current_unit_p (base))
  	return NULL_TREE;
        if (TREE_CODE (base) == VAR_DECL)
  	{
--- 172,178 ----
  
        if ((TREE_CODE (base) == VAR_DECL
  	   || TREE_CODE (base) == FUNCTION_DECL)
! 	  && !can_refer_decl_in_current_unit_p (base, from_decl))
  	return NULL_TREE;
        if (TREE_CODE (base) == VAR_DECL)
  	{
*************** get_symbol_constant_value (tree sym)
*** 170,176 ****
        tree val = DECL_INITIAL (sym);
        if (val)
  	{
! 	  val = canonicalize_constructor_val (val);
  	  if (val && is_gimple_min_invariant (val))
  	    return val;
  	  else
--- 205,211 ----
        tree val = DECL_INITIAL (sym);
        if (val)
  	{
! 	  val = canonicalize_constructor_val (val, sym);
  	  if (val && is_gimple_min_invariant (val))
  	    return val;
  	  else
*************** gimple_fold_stmt_to_constant (gimple stm
*** 2630,2636 ****
  
  static tree fold_ctor_reference (tree type, tree ctor,
  				 unsigned HOST_WIDE_INT offset,
! 				 unsigned HOST_WIDE_INT size);
  
  /* See if we can find constructor defining value of BASE.
     When we know the consructor with constant offset (such as
--- 2665,2671 ----
  
  static tree fold_ctor_reference (tree type, tree ctor,
  				 unsigned HOST_WIDE_INT offset,
! 				 unsigned HOST_WIDE_INT size, tree);
  
  /* See if we can find constructor defining value of BASE.
     When we know the consructor with constant offset (such as
*************** fold_string_cst_ctor_reference (tree typ
*** 2738,2744 ****
  static tree
  fold_array_ctor_reference (tree type, tree ctor,
  			   unsigned HOST_WIDE_INT offset,
! 			   unsigned HOST_WIDE_INT size)
  {
    unsigned HOST_WIDE_INT cnt;
    tree cfield, cval;
--- 2773,2780 ----
  static tree
  fold_array_ctor_reference (tree type, tree ctor,
  			   unsigned HOST_WIDE_INT offset,
! 			   unsigned HOST_WIDE_INT size,
! 			   tree from_decl)
  {
    unsigned HOST_WIDE_INT cnt;
    tree cfield, cval;
*************** fold_array_ctor_reference (tree type, tr
*** 2827,2833 ****
        /* Do we have match?  */
        if (double_int_cmp (access_index, index, 1) >= 0
  	  && double_int_cmp (access_index, max_index, 1) <= 0)
! 	return fold_ctor_reference (type, cval, inner_offset, size);
      }
    /* When memory is not explicitely mentioned in constructor,
       it is 0 (or out of range).  */
--- 2863,2870 ----
        /* Do we have match?  */
        if (double_int_cmp (access_index, index, 1) >= 0
  	  && double_int_cmp (access_index, max_index, 1) <= 0)
! 	return fold_ctor_reference (type, cval, inner_offset, size,
! 				    from_decl);
      }
    /* When memory is not explicitely mentioned in constructor,
       it is 0 (or out of range).  */
*************** fold_array_ctor_reference (tree type, tr
*** 2840,2846 ****
  static tree
  fold_nonarray_ctor_reference (tree type, tree ctor,
  			      unsigned HOST_WIDE_INT offset,
! 			      unsigned HOST_WIDE_INT size)
  {
    unsigned HOST_WIDE_INT cnt;
    tree cfield, cval;
--- 2877,2884 ----
  static tree
  fold_nonarray_ctor_reference (tree type, tree ctor,
  			      unsigned HOST_WIDE_INT offset,
! 			      unsigned HOST_WIDE_INT size,
! 			      tree from_decl)
  {
    unsigned HOST_WIDE_INT cnt;
    tree cfield, cval;
*************** fold_nonarray_ctor_reference (tree type,
*** 2895,2901 ****
  	  if (double_int_cmp (uhwi_to_double_int (offset), bitoffset, 0) < 0)
  	    return NULL_TREE;
  	  return fold_ctor_reference (type, cval,
! 				      double_int_to_uhwi (inner_offset), size);
  	}
      }
    /* When memory is not explicitely mentioned in constructor, it is 0.  */
--- 2933,2940 ----
  	  if (double_int_cmp (uhwi_to_double_int (offset), bitoffset, 0) < 0)
  	    return NULL_TREE;
  	  return fold_ctor_reference (type, cval,
! 				      double_int_to_uhwi (inner_offset), size,
! 				      from_decl);
  	}
      }
    /* When memory is not explicitely mentioned in constructor, it is 0.  */
*************** fold_nonarray_ctor_reference (tree type,
*** 2907,2920 ****
  
  static tree
  fold_ctor_reference (tree type, tree ctor, unsigned HOST_WIDE_INT offset,
! 		     unsigned HOST_WIDE_INT size)
  {
    tree ret;
  
    /* We found the field with exact match.  */
    if (useless_type_conversion_p (type, TREE_TYPE (ctor))
        && !offset)
!     return canonicalize_constructor_val (ctor);
  
    /* We are at the end of walk, see if we can view convert the
       result.  */
--- 2946,2959 ----
  
  static tree
  fold_ctor_reference (tree type, tree ctor, unsigned HOST_WIDE_INT offset,
! 		     unsigned HOST_WIDE_INT size, tree from_decl)
  {
    tree ret;
  
    /* We found the field with exact match.  */
    if (useless_type_conversion_p (type, TREE_TYPE (ctor))
        && !offset)
!     return canonicalize_constructor_val (ctor, from_decl);
  
    /* We are at the end of walk, see if we can view convert the
       result.  */
*************** fold_ctor_reference (tree type, tree cto
*** 2923,2929 ****
        && operand_equal_p (TYPE_SIZE (type),
  			  TYPE_SIZE (TREE_TYPE (ctor)), 0))
      {
!       ret = canonicalize_constructor_val (ctor);
        ret = fold_unary (VIEW_CONVERT_EXPR, type, ret);
        if (ret)
  	STRIP_NOPS (ret);
--- 2962,2968 ----
        && operand_equal_p (TYPE_SIZE (type),
  			  TYPE_SIZE (TREE_TYPE (ctor)), 0))
      {
!       ret = canonicalize_constructor_val (ctor, from_decl);
        ret = fold_unary (VIEW_CONVERT_EXPR, type, ret);
        if (ret)
  	STRIP_NOPS (ret);
*************** fold_ctor_reference (tree type, tree cto
*** 2936,2944 ****
  
        if (TREE_CODE (TREE_TYPE (ctor)) == ARRAY_TYPE
  	  || TREE_CODE (TREE_TYPE (ctor)) == VECTOR_TYPE)
! 	return fold_array_ctor_reference (type, ctor, offset, size);
        else
! 	return fold_nonarray_ctor_reference (type, ctor, offset, size);
      }
  
    return NULL_TREE;
--- 2975,2985 ----
  
        if (TREE_CODE (TREE_TYPE (ctor)) == ARRAY_TYPE
  	  || TREE_CODE (TREE_TYPE (ctor)) == VECTOR_TYPE)
! 	return fold_array_ctor_reference (type, ctor, offset, size,
! 					  from_decl);
        else
! 	return fold_nonarray_ctor_reference (type, ctor, offset, size,
! 					     from_decl);
      }
  
    return NULL_TREE;
*************** fold_const_aggregate_ref_1 (tree t, tree
*** 3011,3017 ****
  		return NULL_TREE;
  	      return fold_ctor_reference (TREE_TYPE (t), ctor, offset,
  					  TREE_INT_CST_LOW (unit_size)
! 					  * BITS_PER_UNIT);
  	    }
  	}
        /* Fallthru.  */
--- 3052,3059 ----
  		return NULL_TREE;
  	      return fold_ctor_reference (TREE_TYPE (t), ctor, offset,
  					  TREE_INT_CST_LOW (unit_size)
! 					  * BITS_PER_UNIT,
! 					  base);
  	    }
  	}
        /* Fallthru.  */
*************** fold_const_aggregate_ref_1 (tree t, tree
*** 3037,3043 ****
        if (offset < 0)
  	return NULL_TREE;
  
!       return fold_ctor_reference (TREE_TYPE (t), ctor, offset, size);
  
      case REALPART_EXPR:
      case IMAGPART_EXPR:
--- 3079,3086 ----
        if (offset < 0)
  	return NULL_TREE;
  
!       return fold_ctor_reference (TREE_TYPE (t), ctor, offset, size,
! 				  base);
  
      case REALPART_EXPR:
      case IMAGPART_EXPR:
*************** tree
*** 3071,3079 ****
  gimple_get_virt_method_for_binfo (HOST_WIDE_INT token, tree known_binfo)
  {
    unsigned HOST_WIDE_INT offset, size;
!   tree v, fn;
  
!   v = BINFO_VTABLE (known_binfo);
    /* If there is no virtual methods table, leave the OBJ_TYPE_REF alone.  */
    if (!v)
      return NULL_TREE;
--- 3114,3122 ----
  gimple_get_virt_method_for_binfo (HOST_WIDE_INT token, tree known_binfo)
  {
    unsigned HOST_WIDE_INT offset, size;
!   tree v, fn, vtable;
  
!   vtable = v = BINFO_VTABLE (known_binfo);
    /* If there is no virtual methods table, leave the OBJ_TYPE_REF alone.  */
    if (!v)
      return NULL_TREE;
*************** gimple_get_virt_method_for_binfo (HOST_W
*** 3099,3105 ****
    size = tree_low_cst (TYPE_SIZE (TREE_TYPE (TREE_TYPE (v))), 1);
    offset += token * size;
    fn = fold_ctor_reference (TREE_TYPE (TREE_TYPE (v)), DECL_INITIAL (v),
! 			    offset, size);
    if (!fn || integer_zerop (fn))
      return NULL_TREE;
    gcc_assert (TREE_CODE (fn) == ADDR_EXPR
--- 3142,3148 ----
    size = tree_low_cst (TYPE_SIZE (TREE_TYPE (TREE_TYPE (v))), 1);
    offset += token * size;
    fn = fold_ctor_reference (TREE_TYPE (TREE_TYPE (v)), DECL_INITIAL (v),
! 			    offset, size, false);
    if (!fn || integer_zerop (fn))
      return NULL_TREE;
    gcc_assert (TREE_CODE (fn) == ADDR_EXPR
*************** gimple_get_virt_method_for_binfo (HOST_W
*** 3111,3117 ****
       devirtualize.  This can happen in WHOPR when the actual method
       ends up in other partition, because we found devirtualization
       possibility too late.  */
!   if (!can_refer_decl_in_current_unit_p (fn))
      return NULL_TREE;
  
    /* Make sure we create a cgraph node for functions we'll reference.
--- 3154,3160 ----
       devirtualize.  This can happen in WHOPR when the actual method
       ends up in other partition, because we found devirtualization
       possibility too late.  */
!   if (!can_refer_decl_in_current_unit_p (fn, vtable))
      return NULL_TREE;
  
    /* Make sure we create a cgraph node for functions we'll reference.
Index: gimple.h
===================================================================
*** gimple.h	(revision 187412)
--- gimple.h	(working copy)
*************** tree gimple_fold_builtin (gimple);
*** 5305,5311 ****
  bool fold_stmt (gimple_stmt_iterator *);
  bool fold_stmt_inplace (gimple_stmt_iterator *);
  tree get_symbol_constant_value (tree);
! tree canonicalize_constructor_val (tree);
  extern tree maybe_fold_and_comparisons (enum tree_code, tree, tree, 
  					enum tree_code, tree, tree);
  extern tree maybe_fold_or_comparisons (enum tree_code, tree, tree,
--- 5305,5311 ----
  bool fold_stmt (gimple_stmt_iterator *);
  bool fold_stmt_inplace (gimple_stmt_iterator *);
  tree get_symbol_constant_value (tree);
! tree canonicalize_constructor_val (tree, tree);
  extern tree maybe_fold_and_comparisons (enum tree_code, tree, tree, 
  					enum tree_code, tree, tree);
  extern tree maybe_fold_or_comparisons (enum tree_code, tree, tree,


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