[PATCH] Fix operand_equal_p hash checking (PR c++/70906, PR c++/70933)
Richard Biener
rguenther@suse.de
Wed May 4 20:20:00 GMT 2016
On May 4, 2016 9:29:37 PM GMT+02:00, Jakub Jelinek <jakub@redhat.com> wrote:
>Hi!
>
>These 2 PRs were DUPed, yet they are actually different, but somewhat
>related.
>One of the ICEs is due to the OEP_ADDRESS_OF consistency checks that
>both operand_equal_p and inchash::add_expr have (that want to verify
>that we don't e.g. have ADDR_EXPR of ADDR_EXPR).
>operand_equal_p never returns true for TARGET_EXPR though, unless there
>is pointer equality, but if we need to hash e.g. ADDR_EXPR of
>TARGET_EXPR with ADDR_EXPR inside of TARGET_EXPR_INITIAL, we currently
>ICE. We could process TARGET_EXPR_{INITIAL,CLEANUP} with
>OEP_ADDRESS_OF masked off, but I believe different TARGET_EXPRs should
>use different TARGET_EXPR_SLOT variables and thus it should be enough
>to hash just the TARGET_EXPR_SLOT and ignore the other arguments.
>
>The second issue is that in the FEs, we can end up calling
>operand_equal_p
>and e.g. for not really equal, but similar (e.g. useless NOP_EXPR of
>SAVE_EXPR
>and the SAVE_EXPR itself) it can be on trees that contain various FE
>specific trees, including constants (like PTRMEM_CST), and others.
>
>The patch arranges to just not ICE in that case if called from the
>operand_equal_p checking, which has the advantage that we will still
>disallow it when people call inchash::add_expr otherwise.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
Thanks,
Richard.
>2016-05-04 Jakub Jelinek <jakub@redhat.com>
>
> PR c++/70906
> PR c++/70933
> * tree-core.h (enum operand_equal_flag): Add OEP_HASH_CHECK.
> * tree.c (inchash::add_expr): If !IS_EXPR_CODE_CLASS (tclass),
> assert flags & OEP_HASH_CHECK, instead of asserting it
> never happens. Handle TARGET_EXPR.
> * fold-const.c (operand_equal_p): For hash verification,
> or in OEP_HASH_CHECK into flags.
>
> * g++.dg/opt/pr70906.C: New test.
> * g++.dg/opt/pr70933.C: New test.
>
>--- gcc/tree-core.h.jj 2016-04-27 15:29:05.000000000 +0200
>+++ gcc/tree-core.h 2016-05-04 12:13:59.361459074 +0200
>@@ -767,7 +767,9 @@ enum operand_equal_flag {
> OEP_MATCH_SIDE_EFFECTS = 4,
> OEP_ADDRESS_OF = 8,
> /* Internal within operand_equal_p: */
>- OEP_NO_HASH_CHECK = 16
>+ OEP_NO_HASH_CHECK = 16,
>+ /* Internal within inchash::add_expr: */
>+ OEP_HASH_CHECK = 32
> };
>
> /* Enum and arrays used for tree allocation stats.
>--- gcc/tree.c.jj 2016-05-03 10:00:25.000000000 +0200
>+++ gcc/tree.c 2016-05-04 12:20:00.354569734 +0200
>@@ -7915,9 +7915,12 @@ add_expr (const_tree t, inchash::hash &h
> && integer_zerop (TREE_OPERAND (t, 1)))
> inchash::add_expr (TREE_OPERAND (TREE_OPERAND (t, 0), 0),
> hstate, flags);
>+ /* Don't ICE on FE specific trees, or their arguments etc.
>+ during operand_equal_p hash verification. */
>+ else if (!IS_EXPR_CODE_CLASS (tclass))
>+ gcc_assert (flags & OEP_HASH_CHECK);
> else
> {
>- gcc_assert (IS_EXPR_CODE_CLASS (tclass));
> unsigned int sflags = flags;
>
> hstate.add_object (code);
>@@ -7966,6 +7969,13 @@ add_expr (const_tree t, inchash::hash &h
> hstate.add_int (CALL_EXPR_IFN (t));
> break;
>
>+ case TARGET_EXPR:
>+ /* For TARGET_EXPR, just hash on the TARGET_EXPR_SLOT.
>+ Usually different TARGET_EXPRs just should use
>+ different temporaries in their slots. */
>+ inchash::add_expr (TARGET_EXPR_SLOT (t), hstate, flags);
>+ return;
>+
> default:
> break;
> }
>--- gcc/fold-const.c.jj 2016-05-02 18:16:00.000000000 +0200
>+++ gcc/fold-const.c 2016-05-04 12:14:33.188000923 +0200
>@@ -2758,8 +2758,8 @@ operand_equal_p (const_tree arg0, const_
> if (arg0 != arg1)
> {
> inchash::hash hstate0 (0), hstate1 (0);
>- inchash::add_expr (arg0, hstate0, flags);
>- inchash::add_expr (arg1, hstate1, flags);
>+ inchash::add_expr (arg0, hstate0, flags | OEP_HASH_CHECK);
>+ inchash::add_expr (arg1, hstate1, flags | OEP_HASH_CHECK);
> hashval_t h0 = hstate0.end ();
> hashval_t h1 = hstate1.end ();
> gcc_assert (h0 == h1);
>--- gcc/testsuite/g++.dg/opt/pr70906.C.jj 2016-05-04 11:33:32.799387826
>+0200
>+++ gcc/testsuite/g++.dg/opt/pr70906.C 2016-05-04 11:33:02.000000000
>+0200
>@@ -0,0 +1,69 @@
>+// PR c++/70906
>+// { dg-do compile { target c++11 } }
>+// { dg-options "-Wall" }
>+
>+template <typename> struct B;
>+template <typename U> struct F { typedef U *t; };
>+template <class> struct D {};
>+template <class VP> struct L {
>+ typedef VP np;
>+ typedef typename F<D<VP>>::t cnp;
>+};
>+struct P { typedef L<void *> nt; };
>+template <class N> struct I { typedef typename N::template A<int> t;
>};
>+template <class O1> struct Q { typedef typename I<O1>::t t; };
>+template <class T, class Hook, Hook T::*> struct G;
>+template <typename P, typename M, M P::*PM>
>+struct mh {
>+ template <class> struct A { typedef G<P, M, PM> pvt; };
>+};
>+template <typename T> struct B<T *> { static T pt(T); };
>+struct R : public D<void *> { typedef P ht; };
>+class lmh : public R {};
>+template <class T, class Hook, Hook T::*P> struct G {
>+ typedef Hook Ht;
>+ typedef typename Ht::ht::nt nt;
>+ typedef T vt;
>+ typedef typename nt::np np;
>+ typedef typename nt::cnp cnp;
>+ static np tnp(T &);
>+ static cnp tnp(const T &p1) {
>+ B<cnp>::pt(static_cast<const Ht &>(p1.*P));
>+ return cnp ();
>+ }
>+};
>+template <class T, class S> struct K {
>+ template <S> struct J;
>+ template <class U> static int foo(J<U::tnp> *, int);
>+ static const int c = sizeof(foo<T>(0, 0));
>+};
>+template <class V> struct W1 {
>+ typedef typename V::vt vt;
>+ static const bool value = K<V, typename V::np (*)(vt &)>::c == K<V,
>typename V::cnp (*)(const vt &)>::c;
>+};
>+template <class V> struct O {
>+ static const bool svt = W1<V>::value;
>+};
>+template <bool> struct M {};
>+template <class V> class C {
>+ static const bool svt = O<V>::svt;
>+ M<svt> m;
>+};
>+template <class V> struct H {
>+ C<V> bar();
>+};
>+template <class O1> struct ml {
>+ typedef typename Q<O1>::t po;
>+ typedef H<typename po::pvt> t;
>+};
>+template <class O1> class list : public ml<O1>::t {};
>+struct N {
>+ struct IV { lmh hk; };
>+ typedef list<mh<IV, lmh, &IV::hk>> ISL;
>+ friend void fn1(int &, N const &);
>+};
>+void fn1(int &, N const &) {
>+ N::ISL xl;
>+ for (xl.bar();;)
>+ ;
>+}
>--- gcc/testsuite/g++.dg/opt/pr70933.C.jj 2016-05-04 11:41:17.838059021
>+0200
>+++ gcc/testsuite/g++.dg/opt/pr70933.C 2016-05-04 11:40:39.000000000
>+0200
>@@ -0,0 +1,29 @@
>+// PR c++/70933
>+// { dg-do compile }
>+// { dg-options "-Wsequence-point" }
>+
>+struct A
>+{
>+ A (const char *);
>+};
>+
>+template <class T>
>+struct B
>+{
>+ typedef T U;
>+ U &baz (const A &);
>+};
>+
>+template <class T>
>+void
>+bar ()
>+{
>+ B<T> b;
>+ T &p = b.baz ("p1") = T(4);
>+}
>+
>+void
>+foo ()
>+{
>+ bar<unsigned> ();
>+}
>
> Jakub
More information about the Gcc-patches
mailing list