[PATCH][4.2/trunk] Fix PR30252 (4.2) and SPEC2k6 dealII miscompile (trunk)

Richard Guenther rguenther@suse.de
Tue Jun 5 18:50:00 GMT 2007


This (controversical as of Danny) patch fixes two problems in the 
points-to solver that cause PR30252 on the 4.2 branch and dealII to
be miscompiled on the trunk.

The Problem with PR30252 is that we
are faced with a C++ struct hierarchy that has empty bases as first
members which are addressable (and actually addressed by the testcase).
We cannot represent a correct points-to set for this, so we miscompute
offsetting the pointer by a component reference later.  This is fixed
by creating variable infos for the empty bases so the solver can point
to them.

The Problem with the dealII miscompile is that taking the address of
a substructure can in some cases cause the wrong points-to set being
computed from a previous solution if the substructure access was through
a pointer.  This looks to us like a problem in solution_set_add, which
relies on all aliases being computable by adding an offset to each of
the old solution vars.  This is not true, so to fix this we simply make
sure to not need to "compute" what an offset points to, as only vars in
the previous solution are valid members of the offsetted solution.

Bootstrapped and tested on x86_64-unknown-linux-gnu.  Danny, this is
now the chance for you to come up with the "correct" solution to these
two problems.  Otherwise I'll make sure to apply this to the branch
before the 4.2.1 release.

(no runtime testcase for the dealII miscompile, but the audit trail
of the bugzilla contains a testcase that has wrong points-to sets
computed)

Thanks,
Richard.

2007-06-05  Richard Guenther  <rguenther@suse.de>
	Michael Matz  <matz@suse.de>

	PR tree-optimization/30252
	* tree-ssa-structalias.c (solution_set_add): Make sure to
	preserve all relevant vars.
	(handle_ptr_arith): Make sure to only handle positive
	offsets.
	(push_fields_onto_fieldstack): Create fields for empty
	bases.

	* g++.dg/opt/pr30252.C: New testcase.

Index: tree-ssa-structalias.c
===================================================================
--- tree-ssa-structalias.c	(revision 125310)
+++ tree-ssa-structalias.c	(working copy)
@@ -708,6 +708,26 @@ solution_set_add (bitmap set, unsigned H
   bitmap result = BITMAP_ALLOC (&iteration_obstack);
   unsigned int i;
   bitmap_iterator bi;
+  unsigned HOST_WIDE_INT min = -1, max = 0;
+
+  /* Compute set of vars we can reach from set + offset.  */
+
+  EXECUTE_IF_SET_IN_BITMAP (set, 0, i, bi)
+    {
+      if (get_varinfo (i)->is_artificial_var
+	  || get_varinfo (i)->has_union
+	  || get_varinfo (i)->is_unknown_size_var)
+	continue;
+
+      if (get_varinfo (i)->offset + offset < min)
+	min = get_varinfo (i)->offset + offset;
+      if (get_varinfo (i)->offset + get_varinfo (i)->size + offset > max)
+	{
+	  max = get_varinfo (i)->offset + get_varinfo (i)->size + offset;
+	  if (max > get_varinfo (i)->fullsize)
+	    max = get_varinfo (i)->fullsize;
+	}
+    }
 
   EXECUTE_IF_SET_IN_BITMAP (set, 0, i, bi)
     {
@@ -715,13 +735,10 @@ solution_set_add (bitmap set, unsigned H
 	 less than end.  Otherwise, it is globbed to a single
 	 variable.  */
 
-      if ((get_varinfo (i)->offset + offset) < get_varinfo (i)->fullsize)
+      if (get_varinfo (i)->offset + get_varinfo (i)->size - 1 >= min
+	  && get_varinfo (i)->offset < max)
 	{
-	  unsigned HOST_WIDE_INT fieldoffset = get_varinfo (i)->offset + offset;
-	  varinfo_t v = first_vi_for_offset (get_varinfo (i), fieldoffset);
-	  if (!v)
-	    continue;
-	  bitmap_set_bit (result, v->id);
+	  bitmap_set_bit (result, i);
 	}
       else if (get_varinfo (i)->is_artificial_var
 	       || get_varinfo (i)->has_union
@@ -3258,7 +3275,7 @@ handle_ptr_arith (VEC (ce_s, heap) *lhsc
   unsigned int i = 0;
   unsigned int j = 0;
   VEC (ce_s, heap) *temp = NULL;
-  unsigned int rhsoffset = 0;
+  unsigned HOST_WIDE_INT rhsoffset = 0;
 
   if (TREE_CODE (expr) != PLUS_EXPR
       && TREE_CODE (expr) != MINUS_EXPR)
@@ -3269,9 +3286,12 @@ handle_ptr_arith (VEC (ce_s, heap) *lhsc
 
   get_constraint_for (op0, &temp);
   if (POINTER_TYPE_P (TREE_TYPE (op0))
-      && TREE_CODE (op1) == INTEGER_CST
+      && host_integerp (op1, 1)
       && TREE_CODE (expr) == PLUS_EXPR)
     {
+      if ((TREE_INT_CST_LOW (op1) * BITS_PER_UNIT) / BITS_PER_UNIT
+	  != TREE_INT_CST_LOW (op1))
+	return false;
       rhsoffset = TREE_INT_CST_LOW (op1) * BITS_PER_UNIT;
     }
   else
@@ -3661,6 +3681,7 @@ push_fields_onto_fieldstack (tree type, 
 {
   tree field;
   int count = 0;
+  unsigned HOST_WIDE_INT minoffset = -1;
 
   if (TREE_CODE (type) == COMPLEX_TYPE)
     {
@@ -3773,8 +3794,24 @@ push_fields_onto_fieldstack (tree type, 
 	  }
 	else
 	  count += pushed;
+
+	if (bitpos_of_field (field) < minoffset)
+	  minoffset = bitpos_of_field (field);
       }
 
+  /* We need to create a fake subvar for empty bases.  */
+  if (minoffset != 0 && count != 0)
+    {
+      fieldoff_s *pair;
+
+      pair = VEC_safe_push (fieldoff_s, heap, *fieldstack, NULL);
+      pair->type = void_type_node;
+      pair->size = build_int_cst (size_type_node, minoffset);
+      pair->decl = NULL;
+      pair->offset = offset;
+      count++;
+    }
+
   return count;
 }
 
Index: testsuite/g++.dg/opt/pr30252.C
===================================================================
--- testsuite/g++.dg/opt/pr30252.C	(revision 0)
+++ testsuite/g++.dg/opt/pr30252.C	(revision 0)
@@ -0,0 +1,226 @@
+/* { dg-do run } */
+/* { dg-options "-O -fstrict-aliasing" } */
+
+extern "C" void abort (void);
+namespace sigc {
+  template <class T_type>
+  struct type_trait
+  {
+    typedef T_type& pass;
+    typedef const T_type& take;
+    typedef T_type* pointer;
+  };
+  template <class T_base, class T_derived>
+  struct is_base_and_derived
+  {
+    struct big {
+      char memory[64];
+    };
+    static big is_base_class_(...);
+    static char is_base_class_(typename type_trait<T_base>::pointer);
+    static const bool value =
+    sizeof(is_base_class_(reinterpret_cast<typename type_trait<T_derived>::pointer>(0))) ==
+    sizeof(char);
+  };
+  struct nil;
+  struct functor_base {};
+  template <class T_functor, bool I_derives_functor_base=is_base_and_derived<functor_base,T_functor>::value>
+  struct functor_trait
+  {
+  };
+  template <class T_functor>
+  struct functor_trait<T_functor,true>
+  {
+    typedef typename T_functor::result_type result_type;
+    typedef T_functor functor_type;
+  };
+  template <class T_arg1, class T_return>
+  class pointer_functor1 : public functor_base
+  {
+    typedef T_return (*function_type)(T_arg1);
+    function_type func_ptr_;
+  public:
+    typedef T_return result_type;
+    explicit pointer_functor1(function_type _A_func): func_ptr_(_A_func) {}
+    T_return operator()(typename type_trait<T_arg1>::take _A_a1) const
+    { return func_ptr_(_A_a1); }
+  };
+  template <class T_arg1, class T_return>
+  inline pointer_functor1<T_arg1, T_return>
+  ptr_fun1(T_return (*_A_func)(T_arg1))
+  { return pointer_functor1<T_arg1, T_return>(_A_func); }
+  struct adaptor_base : public functor_base {};
+  template <class T_functor,
+    class T_arg1=void,
+    bool I_derives_adaptor_base=is_base_and_derived<adaptor_base,T_functor>::value>
+  struct deduce_result_type
+  { typedef typename functor_trait<T_functor>::result_type type; };
+  template <class T_functor>
+  struct adaptor_functor : public adaptor_base
+  {
+    template <class T_arg1=void>
+    struct deduce_result_type
+    { typedef typename sigc::deduce_result_type<T_functor, T_arg1>::type type; };
+    typedef typename functor_trait<T_functor>::result_type result_type;
+    result_type
+    operator()() const;
+    template <class T_arg1>
+    typename deduce_result_type<T_arg1>::type
+    operator()(T_arg1 _A_arg1) const
+    { return functor_(_A_arg1); }
+    explicit adaptor_functor(const T_functor& _A_functor)
+      : functor_(_A_functor)
+    {}
+    mutable T_functor functor_;
+  };
+  template <class T_functor>
+  typename adaptor_functor<T_functor>::result_type
+  adaptor_functor<T_functor>::operator()() const
+  { return functor_(); }
+  template <class T_functor, bool I_isadaptor = is_base_and_derived<adaptor_base, T_functor>::value> struct adaptor_trait;
+  template <class T_functor>
+  struct adaptor_trait<T_functor, true>
+  {
+    typedef T_functor adaptor_type;
+  };
+  template <class T_functor>
+  struct adaptor_trait<T_functor, false>
+  {
+    typedef typename functor_trait<T_functor>::functor_type functor_type;
+    typedef adaptor_functor<functor_type> adaptor_type;
+  };
+  template <class T_functor>
+  struct adapts : public adaptor_base
+  {
+    typedef typename adaptor_trait<T_functor>::adaptor_type adaptor_type;
+    explicit adapts(const T_functor& _A_functor)
+      : functor_(_A_functor)
+    {}
+    mutable adaptor_type functor_;
+  };
+  template <class T_type>
+  struct reference_wrapper
+  {
+  };
+  template <class T_type>
+  struct unwrap_reference
+  {
+    typedef T_type type;
+  };
+  template <class T_type>
+  class bound_argument
+  {
+  public:
+    bound_argument(const T_type& _A_argument)
+      : visited_(_A_argument)
+    {}
+    inline T_type& invoke()
+    { return visited_; }
+    T_type visited_;
+  };
+  template <class T_wrapped>
+  class bound_argument< reference_wrapper<T_wrapped> >
+  {
+  };
+  template <int I_location, class T_functor, class T_type1=nil>
+  struct bind_functor;
+  template <class T_functor, class T_type1>
+  struct bind_functor<-1, T_functor, T_type1> : public adapts<T_functor>
+  {
+    typedef typename adapts<T_functor>::adaptor_type adaptor_type;
+    typedef typename adaptor_type::result_type result_type;
+    result_type
+    operator()()
+    {
+      return this->functor_.template operator()<typename type_trait<typename unwrap_reference<T_type1>::type>::pass> (bound1_.invoke());
+    }
+    bind_functor(typename type_trait<T_functor>::take _A_func, typename type_trait<T_type1>::take _A_bound1)
+      : adapts<T_functor>(_A_func), bound1_(_A_bound1)
+    {}
+    bound_argument<T_type1> bound1_;
+  };
+  template <class T_type1, class T_functor>
+  inline bind_functor<-1, T_functor,
+		      T_type1>
+  bind(const T_functor& _A_func, T_type1 _A_b1)
+  { return bind_functor<-1, T_functor,
+      T_type1>
+      (_A_func, _A_b1);
+  }
+  namespace internal {
+    struct slot_rep;
+    typedef void* (*hook)(slot_rep *);
+    struct slot_rep
+    {
+      hook call_;
+    };
+  }
+  class slot_base : public functor_base
+  {
+  public:
+    typedef internal::slot_rep rep_type;
+    explicit slot_base(rep_type* rep)
+      : rep_(rep)
+    {
+    }
+    mutable rep_type *rep_;
+  };
+  namespace internal {
+    template <class T_functor>
+    struct typed_slot_rep : public slot_rep
+    {
+      typedef typename adaptor_trait<T_functor>::adaptor_type adaptor_type;
+      adaptor_type functor_;
+      inline typed_slot_rep(const T_functor& functor)
+	: functor_(functor)
+      {
+      }
+    };
+    template<class T_functor>
+    struct slot_call0
+    {
+      static void *call_it(slot_rep* rep)
+      {
+	typedef typed_slot_rep<T_functor> typed_slot;
+	typed_slot *typed_rep = static_cast<typed_slot*>(rep);
+	return (typed_rep->functor_)();
+      }
+      static hook address()
+      {
+	return &call_it;
+      }
+    };
+  }
+
+  class slot0 : public slot_base
+  {
+  public:
+    typedef void * (*call_type)(rep_type*);
+    inline void *operator()() const
+    {
+      return slot_base::rep_->call_ (slot_base::rep_);
+    }
+    template <class T_functor>
+    slot0(const T_functor& _A_func)
+      : slot_base(new internal::typed_slot_rep<T_functor>(_A_func))
+    {
+      slot_base::rep_->call_ = internal::slot_call0<T_functor>::address();
+    }
+  };
+}
+struct A
+{
+  static void *foo (void *p) { return p; }
+  typedef sigc::slot0 C;
+  C bar();
+};
+A::C A::bar ()
+{
+  return sigc::bind (sigc::ptr_fun1 (&A::foo), (void*)0);
+}
+int main (void)
+{
+  A a;
+  if (a.bar ()() != 0)
+    abort ();
+}



More information about the Gcc-patches mailing list