* RFA (tree.h): C++ PATCH for c++/67407 (ICE with protected access)
@ 2016-01-28 15:04 Jason Merrill
2016-01-28 15:09 ` Jakub Jelinek
0 siblings, 1 reply; 3+ messages in thread
From: Jason Merrill @ 2016-01-28 15:04 UTC (permalink / raw)
To: gcc-patches List
[-- Attachment #1: Type: text/plain, Size: 513 bytes --]
In this testcase, the problem was that we were checking DERIVED_FROM_P,
which uses dfs_walk_once and thus BINFO_MARKED, in the middle of a
dfs_walk_once_accessible, which also uses BINFO_MARKED, and the marks
from one walk were confusing the other walk. Fixed by moving these
binfo walking functions to use a hash_set instead of BINFO_MARKED.
After this change, there are no uses of BINFO_MARKED left in the source
tree, so I'm inclined to rename it to BINFO_LANG_FLAG_0. OK?
Tested x86_64-pc-linux-gnu.
[-- Attachment #2: 67407.patch --]
[-- Type: text/x-patch, Size: 9582 bytes --]
commit 624557b32eadf69d58c712960b9f4c455ca344f5
Author: Jason Merrill <jason@redhat.com>
Date: Wed Jan 27 15:52:11 2016 -0500
PR c++/67407
gcc/
* tree.h (BINFO_FLAG_0): Rename from BINFO_MARKED.
gcc/cp/
* search.c (dfs_walk_once, dfs_walk_once_r)
(dfs_walk_once_accessible_r, dfs_walk_once_accessible): Use
hash_set instead of BINFO_MARKED.
(dfs_unmark_r): Remove.
diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index fc47f91..45d8a24 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -2608,9 +2608,7 @@ update_vtable_entry_for_fn (tree t, tree binfo, tree fn, tree* virtuals,
/* There was no existing virtual thunk (which takes
precedence). So find the binfo of the base function's
return type within the overriding function's return type.
- We cannot call lookup base here, because we're inside a
- dfs_walk, and will therefore clobber the BINFO_MARKED
- flags. Fortunately we know the covariancy is valid (it
+ Fortunately we know the covariancy is valid (it
has already been checked), so we can just iterate along
the binfos, which have been chained in inheritance graph
order. Of course it is lame that we have to repeat the
diff --git a/gcc/cp/search.c b/gcc/cp/search.c
index 455073e..7924611 100644
--- a/gcc/cp/search.c
+++ b/gcc/cp/search.c
@@ -34,9 +34,6 @@ static tree dfs_lookup_base (tree, void *);
static tree dfs_dcast_hint_pre (tree, void *);
static tree dfs_dcast_hint_post (tree, void *);
static tree dfs_debug_mark (tree, void *);
-static tree dfs_walk_once_r (tree, tree (*pre_fn) (tree, void *),
- tree (*post_fn) (tree, void *), void *data);
-static void dfs_unmark_r (tree);
static int check_hidden_convs (tree, int, int, tree, tree, tree);
static tree split_conversions (tree, tree, tree, tree);
static int lookup_conversions_r (tree, int, int,
@@ -44,10 +41,6 @@ static int lookup_conversions_r (tree, int, int,
static int look_for_overrides_r (tree, tree);
static tree lookup_field_r (tree, void *);
static tree dfs_accessible_post (tree, void *);
-static tree dfs_walk_once_accessible_r (tree, bool, bool,
- tree (*pre_fn) (tree, void *),
- tree (*post_fn) (tree, void *),
- void *data);
static tree dfs_walk_once_accessible (tree, bool,
tree (*pre_fn) (tree, void *),
tree (*post_fn) (tree, void *),
@@ -1826,7 +1819,8 @@ dfs_walk_all (tree binfo, tree (*pre_fn) (tree, void *),
static tree
dfs_walk_once_r (tree binfo, tree (*pre_fn) (tree, void *),
- tree (*post_fn) (tree, void *), void *data)
+ tree (*post_fn) (tree, void *), hash_set<tree> *pset,
+ void *data)
{
tree rval;
unsigned ix;
@@ -1849,13 +1843,10 @@ dfs_walk_once_r (tree binfo, tree (*pre_fn) (tree, void *),
for (ix = 0; BINFO_BASE_ITERATE (binfo, ix, base_binfo); ix++)
{
if (BINFO_VIRTUAL_P (base_binfo))
- {
- if (BINFO_MARKED (base_binfo))
- continue;
- BINFO_MARKED (base_binfo) = 1;
- }
+ if (pset->add (base_binfo))
+ continue;
- rval = dfs_walk_once_r (base_binfo, pre_fn, post_fn, data);
+ rval = dfs_walk_once_r (base_binfo, pre_fn, post_fn, pset, data);
if (rval)
return rval;
}
@@ -1872,30 +1863,6 @@ dfs_walk_once_r (tree binfo, tree (*pre_fn) (tree, void *),
return NULL_TREE;
}
-/* Worker for dfs_walk_once. Recursively unmark the virtual base binfos of
- BINFO. */
-
-static void
-dfs_unmark_r (tree binfo)
-{
- unsigned ix;
- tree base_binfo;
-
- /* Process the basetypes. */
- for (ix = 0; BINFO_BASE_ITERATE (binfo, ix, base_binfo); ix++)
- {
- if (BINFO_VIRTUAL_P (base_binfo))
- {
- if (!BINFO_MARKED (base_binfo))
- continue;
- BINFO_MARKED (base_binfo) = 0;
- }
- /* Only walk, if it can contain more virtual bases. */
- if (CLASSTYPE_VBASECLASSES (BINFO_TYPE (base_binfo)))
- dfs_unmark_r (base_binfo);
- }
-}
-
/* Like dfs_walk_all, except that binfos are not multiply walked. For
non-diamond shaped hierarchies this is the same as dfs_walk_all.
For diamond shaped hierarchies we must mark the virtual bases, to
@@ -1918,22 +1885,8 @@ dfs_walk_once (tree binfo, tree (*pre_fn) (tree, void *),
rval = dfs_walk_all (binfo, pre_fn, post_fn, data);
else
{
- rval = dfs_walk_once_r (binfo, pre_fn, post_fn, data);
- if (!BINFO_INHERITANCE_CHAIN (binfo))
- {
- /* We are at the top of the hierarchy, and can use the
- CLASSTYPE_VBASECLASSES list for unmarking the virtual
- bases. */
- vec<tree, va_gc> *vbases;
- unsigned ix;
- tree base_binfo;
-
- for (vbases = CLASSTYPE_VBASECLASSES (BINFO_TYPE (binfo)), ix = 0;
- vec_safe_iterate (vbases, ix, &base_binfo); ix++)
- BINFO_MARKED (base_binfo) = 0;
- }
- else
- dfs_unmark_r (binfo);
+ hash_set<tree> pset;
+ rval = dfs_walk_once_r (binfo, pre_fn, post_fn, &pset, data);
}
active--;
@@ -1947,7 +1900,7 @@ dfs_walk_once (tree binfo, tree (*pre_fn) (tree, void *),
indicates whether bases should be marked during traversal. */
static tree
-dfs_walk_once_accessible_r (tree binfo, bool friends_p, bool once,
+dfs_walk_once_accessible_r (tree binfo, bool friends_p, hash_set<tree> *pset,
tree (*pre_fn) (tree, void *),
tree (*post_fn) (tree, void *), void *data)
{
@@ -1971,9 +1924,9 @@ dfs_walk_once_accessible_r (tree binfo, bool friends_p, bool once,
/* Find the next child binfo to walk. */
for (ix = 0; BINFO_BASE_ITERATE (binfo, ix, base_binfo); ix++)
{
- bool mark = once && BINFO_VIRTUAL_P (base_binfo);
+ bool mark = pset && BINFO_VIRTUAL_P (base_binfo);
- if (mark && BINFO_MARKED (base_binfo))
+ if (mark && pset->contains (base_binfo))
continue;
/* If the base is inherited via private or protected
@@ -1992,9 +1945,9 @@ dfs_walk_once_accessible_r (tree binfo, bool friends_p, bool once,
}
if (mark)
- BINFO_MARKED (base_binfo) = 1;
+ pset->add (base_binfo);
- rval = dfs_walk_once_accessible_r (base_binfo, friends_p, once,
+ rval = dfs_walk_once_accessible_r (base_binfo, friends_p, pset,
pre_fn, post_fn, data);
if (rval)
return rval;
@@ -2021,28 +1974,14 @@ dfs_walk_once_accessible (tree binfo, bool friends_p,
tree (*pre_fn) (tree, void *),
tree (*post_fn) (tree, void *), void *data)
{
- bool diamond_shaped = CLASSTYPE_DIAMOND_SHAPED_P (BINFO_TYPE (binfo));
- tree rval = dfs_walk_once_accessible_r (binfo, friends_p, diamond_shaped,
+ hash_set<tree> *pset = NULL;
+ if (CLASSTYPE_DIAMOND_SHAPED_P (BINFO_TYPE (binfo)))
+ pset = new hash_set<tree>;
+ tree rval = dfs_walk_once_accessible_r (binfo, friends_p, pset,
pre_fn, post_fn, data);
- if (diamond_shaped)
- {
- if (!BINFO_INHERITANCE_CHAIN (binfo))
- {
- /* We are at the top of the hierarchy, and can use the
- CLASSTYPE_VBASECLASSES list for unmarking the virtual
- bases. */
- vec<tree, va_gc> *vbases;
- unsigned ix;
- tree base_binfo;
-
- for (vbases = CLASSTYPE_VBASECLASSES (BINFO_TYPE (binfo)), ix = 0;
- vec_safe_iterate (vbases, ix, &base_binfo); ix++)
- BINFO_MARKED (base_binfo) = 0;
- }
- else
- dfs_unmark_r (binfo);
- }
+ if (pset)
+ delete pset;
return rval;
}
diff --git a/gcc/doc/generic.texi b/gcc/doc/generic.texi
index 5378793..ca63984 100644
--- a/gcc/doc/generic.texi
+++ b/gcc/doc/generic.texi
@@ -2889,9 +2889,8 @@ or @code{access_protected_node}. If bases are always public,
@code{BINFO_BASE_ACCESSES} may be @code{NULL}.
@code{BINFO_VIRTUAL_P} is used to specify whether the binfo is inherited
-virtually or not. The other flags, @code{BINFO_MARKED_P} and
-@code{BINFO_FLAG_1} to @code{BINFO_FLAG_6} can be used for language
-specific use.
+virtually or not. The other flags, @code{BINFO_FLAG_0} to
+@code{BINFO_FLAG_6}, can be used for language specific use.
The following macros can be used on a tree node representing a class-type.
diff --git a/gcc/testsuite/g++.dg/inherit/protected1.C b/gcc/testsuite/g++.dg/inherit/protected1.C
new file mode 100644
index 0000000..c71be53
--- /dev/null
+++ b/gcc/testsuite/g++.dg/inherit/protected1.C
@@ -0,0 +1,51 @@
+// PR c++/67407
+
+template <class> class A;
+template <class> struct B;
+template <class X> struct B<A<X> >
+{
+ static int
+ check ()
+ {
+ A<X> a;
+ a.m_class->m_object;
+ }
+};
+template <class T> class A
+{
+public:
+ template <class X> bool operator== (const X &) const;
+ T *m_class;
+};
+template <class T>
+template <class X>
+bool
+A<T>::operator== (const X &) const
+{
+ B<X>::check;
+}
+class C
+{
+protected:
+ template <class> friend struct B;
+ void *m_object;
+};
+class F : virtual C
+{
+};
+class G : virtual public C
+{
+};
+class H : F, public G
+{
+};
+class D
+{
+ void onBusMessage (const A<int> &);
+ A<H> m_pipeline;
+};
+void
+D::onBusMessage (const A<int> &p1)
+{
+ p1 == m_pipeline;
+}
diff --git a/gcc/tree.h b/gcc/tree.h
index f789785..191f21b 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -2096,7 +2096,7 @@ extern machine_mode element_mode (const_tree t);
#define BINFO_VIRTUAL_P(NODE) (TREE_BINFO_CHECK (NODE)->base.static_flag)
/* Flags for language dependent use. */
-#define BINFO_MARKED(NODE) TREE_LANG_FLAG_0 (TREE_BINFO_CHECK (NODE))
+#define BINFO_FLAG_0(NODE) TREE_LANG_FLAG_0 (TREE_BINFO_CHECK (NODE))
#define BINFO_FLAG_1(NODE) TREE_LANG_FLAG_1 (TREE_BINFO_CHECK (NODE))
#define BINFO_FLAG_2(NODE) TREE_LANG_FLAG_2 (TREE_BINFO_CHECK (NODE))
#define BINFO_FLAG_3(NODE) TREE_LANG_FLAG_3 (TREE_BINFO_CHECK (NODE))
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: RFA (tree.h): C++ PATCH for c++/67407 (ICE with protected access)
2016-01-28 15:04 RFA (tree.h): C++ PATCH for c++/67407 (ICE with protected access) Jason Merrill
@ 2016-01-28 15:09 ` Jakub Jelinek
2016-01-28 15:44 ` Jason Merrill
0 siblings, 1 reply; 3+ messages in thread
From: Jakub Jelinek @ 2016-01-28 15:09 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches List
On Thu, Jan 28, 2016 at 10:03:59AM -0500, Jason Merrill wrote:
> In this testcase, the problem was that we were checking DERIVED_FROM_P,
> which uses dfs_walk_once and thus BINFO_MARKED, in the middle of a
> dfs_walk_once_accessible, which also uses BINFO_MARKED, and the marks from
> one walk were confusing the other walk. Fixed by moving these binfo walking
> functions to use a hash_set instead of BINFO_MARKED.
>
> After this change, there are no uses of BINFO_MARKED left in the source
> tree, so I'm inclined to rename it to BINFO_LANG_FLAG_0. OK?
LGTM.
Do you see any measurable compatile time differences with this patch
on some large C++ preprocessed source (say some boost one)?
Jakub
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: RFA (tree.h): C++ PATCH for c++/67407 (ICE with protected access)
2016-01-28 15:09 ` Jakub Jelinek
@ 2016-01-28 15:44 ` Jason Merrill
0 siblings, 0 replies; 3+ messages in thread
From: Jason Merrill @ 2016-01-28 15:44 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches List
On 01/28/2016 10:09 AM, Jakub Jelinek wrote:
> On Thu, Jan 28, 2016 at 10:03:59AM -0500, Jason Merrill wrote:
>> In this testcase, the problem was that we were checking DERIVED_FROM_P,
>> which uses dfs_walk_once and thus BINFO_MARKED, in the middle of a
>> dfs_walk_once_accessible, which also uses BINFO_MARKED, and the marks from
>> one walk were confusing the other walk. Fixed by moving these binfo walking
>> functions to use a hash_set instead of BINFO_MARKED.
>>
>> After this change, there are no uses of BINFO_MARKED left in the source
>> tree, so I'm inclined to rename it to BINFO_LANG_FLAG_0. OK?
>
> LGTM.
> Do you see any measurable compatile time differences with this patch
> on some large C++ preprocessed source (say some boost one)?
Nope.
Jason
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-01-28 15:44 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-28 15:04 RFA (tree.h): C++ PATCH for c++/67407 (ICE with protected access) Jason Merrill
2016-01-28 15:09 ` Jakub Jelinek
2016-01-28 15:44 ` Jason Merrill
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).