HI,
On 14/06/2016 15:06, Jason Merrill wrote:
On 06/11/2016 12:06 PM, Paolo Carlini wrote:
The remaining issue to be discussed is what to do about that old bug,
c++/27952, a crash in decay_conversion during error recovery, when vtt
is found null. I propose to simply check for it in
build_special_member_call, avoid passing an unusable null argument to
decay_conversion and return back an error_mark_node. We could also do
gcc_assert (seen_error()) before returning error_mark_node?
Why is vtt null?
It's a mismatch caused by the structure of xref_basetypes, which first
computes the number of virtual bases, etc, and then possibly errors out
and then drops the duplicate bases (or other invalid bases, see the
additional testcases below involving unions). Thus for:
struct B : A, virtual A { }
when the second A is dropped the work done in the first part of the
function, thus, in particular:
vec_alloc (CLASSTYPE_VBASECLASSES (ref), max_vbases);
with max_vbases == 1 becomes inconsistent. As an a obvious check, this:
struct B : virtual A, A { }
does not cause problems.
The below goes the most obvious way: keeps track of the number of
dropped virtual bases (direct and indirect, that's important for eg,
unions too) and if so-to-speak nothing virtual remains at the end of the
xref_basetypes loop, does a vec_free which undoes the vec_alloc above. I
considered moving the new code to a separate function, but frankly I'm
not sure it would add much clarity in this case. Also note, vs the patch
I posted the last time, there are a few minor changes before the
xref_basetypes, essentially reverting that 2006 commit more precisely
(ie, no early returns at all).
What do you think?
Thanks,
Paolo.
/////////////
Index: cp/cp-tree.h
===================================================================
--- cp/cp-tree.h (revision 237443)
+++ cp/cp-tree.h (working copy)
@@ -5771,7 +5771,7 @@ extern int grok_ctor_properties
(const_tree, con
extern bool grok_op_properties (tree, bool);
extern tree xref_tag (enum tag_types, tree,
tag_scope, bool);
extern tree xref_tag_from_type (tree, tree, tag_scope);
-extern bool xref_basetypes (tree, tree);
+extern void xref_basetypes (tree, tree);
extern tree start_enum (tree, tree, tree, tree, bool,
bool *);
extern void finish_enum_value_list (tree);
extern void finish_enum (tree);
Index: cp/decl.c
===================================================================
--- cp/decl.c (revision 237443)
+++ cp/decl.c (working copy)
@@ -12871,12 +12871,9 @@ xref_tag_from_type (tree old, tree id, tag_scope s
/* Create the binfo hierarchy for REF with (possibly NULL) base list
BASE_LIST. For each element on BASE_LIST the TREE_PURPOSE is an
access_* node, and the TREE_VALUE is the type of the base-class.
- Non-NULL TREE_TYPE indicates virtual inheritance.
-
- Returns true if the binfo hierarchy was successfully created,
- false if an error was detected. */
+ Non-NULL TREE_TYPE indicates virtual inheritance. */
-bool
+void
xref_basetypes (tree ref, tree base_list)
{
tree *basep;
@@ -12889,7 +12886,7 @@ xref_basetypes (tree ref, tree base_list)
tree igo_prev; /* Track Inheritance Graph Order. */
if (ref == error_mark_node)
- return false;
+ return;
/* The base of a derived class is private by default, all others are
public. */
@@ -12933,11 +12930,7 @@ xref_basetypes (tree ref, tree base_list)
/* The binfo slot should be empty, unless this is an (ill-formed)
redefinition. */
- if (TYPE_BINFO (ref) && !TYPE_SIZE (ref))
- {
- error ("redefinition of %q#T", ref);
- return false;
- }
+ gcc_assert (!TYPE_BINFO (ref) || TYPE_SIZE (ref));
gcc_assert (TYPE_MAIN_VARIANT (ref) == ref);
@@ -12957,19 +12950,13 @@ xref_basetypes (tree ref, tree base_list)
CLASSTYPE_NON_AGGREGATE (ref) = 1;
if (TREE_CODE (ref) == UNION_TYPE)
- {
- error ("derived union %qT invalid", ref);
- return false;
- }
+ error ("derived union %qT invalid", ref);
}
if (max_bases > 1)
{
if (TYPE_FOR_JAVA (ref))
- {
- error ("Java class %qT cannot have multiple bases", ref);
- return false;
- }
+ error ("Java class %qT cannot have multiple bases", ref);
else
warning (OPT_Wmultiple_inheritance,
"%qT defined with multiple direct bases", ref);
@@ -12980,10 +12967,7 @@ xref_basetypes (tree ref, tree base_list)
vec_alloc (CLASSTYPE_VBASECLASSES (ref), max_vbases);
if (TYPE_FOR_JAVA (ref))
- {
- error ("Java class %qT cannot have virtual bases", ref);
- return false;
- }
+ error ("Java class %qT cannot have virtual bases", ref);
else if (max_dvbases)
warning (OPT_Wvirtual_inheritance,
"%qT defined with direct virtual base", ref);
@@ -13006,7 +12990,7 @@ xref_basetypes (tree ref, tree base_list)
{
error ("base type %qT fails to be a struct or class type",
basetype);
- return false;
+ goto dropped_base;
}
if (TYPE_FOR_JAVA (basetype) && (current_lang_depth () == 0))
@@ -13040,7 +13024,7 @@ xref_basetypes (tree ref, tree base_list)
error ("recursive type %qT undefined", basetype);
else
error ("duplicate base type %qT invalid", basetype);
- return false;
+ goto dropped_base;
}
if (PACK_EXPANSION_P (TREE_VALUE (base_list)))
@@ -13056,8 +13040,25 @@ xref_basetypes (tree ref, tree base_list)
BINFO_BASE_APPEND (binfo, base_binfo);
BINFO_BASE_ACCESS_APPEND (binfo, access);
+ continue;
+
+ dropped_base:
+ /* Update max_vbases to reflect the reality that we are dropping
+ this base: if it reaches zero we want to undo the vec_alloc
+ above to avoid inconsistencies during error-recovery: eg, in
+ build_special_member_call, CLASSTYPE_VBASECLASSES non null
+ and vtt null (c++/27952). */
+ if (via_virtual)
+ max_vbases--;
+ if (CLASS_TYPE_P (basetype))
+ max_vbases
+ -= vec_safe_length (CLASSTYPE_VBASECLASSES (basetype));
}
+ if (CLASSTYPE_VBASECLASSES (ref)
+ && max_vbases == 0)
+ vec_free (CLASSTYPE_VBASECLASSES (ref));
+
if (vec_safe_length (CLASSTYPE_VBASECLASSES (ref)) < max_vbases)
/* If we didn't get max_vbases vbases, we must have shared at
least one of them, and are therefore diamond shaped. */
@@ -13088,8 +13089,6 @@ xref_basetypes (tree ref, tree base_list)
else
break;
}
-
- return true;
}
Index: cp/parser.c
===================================================================
--- cp/parser.c (revision 237443)
+++ cp/parser.c (working copy)
@@ -22050,9 +22050,8 @@ cp_parser_class_head (cp_parser* parser,
/* If we're really defining a class, process the base classes.
If they're invalid, fail. */
- if (type && cp_lexer_next_token_is (parser->lexer, CPP_OPEN_BRACE)
- && !xref_basetypes (type, bases))
- type = NULL_TREE;
+ if (type && cp_lexer_next_token_is (parser->lexer, CPP_OPEN_BRACE))
+ xref_basetypes (type, bases);
done:
/* Leave the scope given by the nested-name-specifier. We will
Index: testsuite/g++.dg/inherit/crash6.C
===================================================================
--- testsuite/g++.dg/inherit/crash6.C (revision 0)
+++ testsuite/g++.dg/inherit/crash6.C (working copy)
@@ -0,0 +1,10 @@
+// PR c++/70202
+
+class A
+{
+ virtual void foo () { }
+};
+class B : public A, A { }; // { dg-error "duplicate base type" }
+
+B b1, &b2 = b1;
+A a = b2;
Index: testsuite/g++.dg/inherit/union2.C
===================================================================
--- testsuite/g++.dg/inherit/union2.C (revision 0)
+++ testsuite/g++.dg/inherit/union2.C (working copy)
@@ -0,0 +1,3 @@
+struct A { };
+union U : A { }; // { dg-error "derived union 'U' invalid" }
+U u;
Index: testsuite/g++.dg/inherit/virtual1.C
===================================================================
--- testsuite/g++.dg/inherit/virtual1.C (revision 237443)
+++ testsuite/g++.dg/inherit/virtual1.C (working copy)
@@ -1,4 +1,4 @@
-//PR c++/27952
+// PR c++/27952
struct A
{
@@ -5,8 +5,8 @@ struct A
virtual ~A() {}
};
-struct B : A, virtual A {}; // { dg-error "duplicate base|forward
declaration" }
+struct B : A, virtual A {}; // { dg-error "duplicate base" }
-struct C : A, B {}; // { dg-error "duplicate base|invalid use" }
+struct C : A, B {}; // { dg-message "direct base 'A' inaccessible"
}
-C c; // { dg-error "aggregate" }
+C c;
Index: testsuite/g++.dg/inherit/virtual12.C
===================================================================
--- testsuite/g++.dg/inherit/virtual12.C (revision 0)
+++ testsuite/g++.dg/inherit/virtual12.C (working copy)
@@ -0,0 +1,14 @@
+// PR c++/70202
+
+union U { };
+
+struct A
+{
+ virtual ~A() {}
+};
+
+struct B : A, virtual U { }; // { dg-error "base type 'U' fails" }
+
+struct C : A, B {}; // { dg-message "direct base 'A' inaccessible" }
+
+C c;
Index: testsuite/g++.dg/inherit/virtual13.C
===================================================================
--- testsuite/g++.dg/inherit/virtual13.C (revision 0)
+++ testsuite/g++.dg/inherit/virtual13.C (working copy)
@@ -0,0 +1,16 @@
+// PR c++/70202
+
+struct D { };
+
+union U : virtual D { }; // { dg-error "derived union" }
+
+struct A
+{
+ virtual ~A() {}
+};
+
+struct B : A, virtual U { }; // { dg-error "base type 'U' fails" }
+
+struct C : A, B {}; // { dg-message "direct base 'A' inaccessible" }
+
+C c;
