The first patch factors out testing of current_in_charge_parm from
various places in the compiler into a new build_if_in_charge function.
The second patch implements Bernd's suggestion for modifying
-flifetime-dse so that when we have virtual bases, we clobber the entire
object, but only when we are in charge (and therefore know we are in the
constructor for a complete object, and don't need to worry about tail
padding).
The third patch adjusts the -fsanitize=vptr vptr clearing so that we
don't clear the vptr for a virtual base when we aren't in charge of
virtual bases, even if the current class shares the vptr from a primary
virtual base.
The testcase tests all of these changes.
Tested x86_64-pc-linux-gnu, applying to trunk.
commit 4cffe7ea961ed6b602a954c616f5186f27c85db5
Author: Jason Merrill <ja...@redhat.com>
Date: Thu Mar 17 17:22:43 2016 -0400
* class.c (build_if_in_charge): Split out from build_base_path.
* init.c (expand_virtual_init, expand_default_init): Use it.
* call.c (build_special_member_call): Use it.
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 34c1d9b..d445163 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -8082,11 +8082,7 @@ build_special_member_call (tree instance, tree name, vec<tree, va_gc> **args,
vtt = decay_conversion (vtt, complain);
if (vtt == error_mark_node)
return error_mark_node;
- vtt = build3 (COND_EXPR, TREE_TYPE (vtt),
- build2 (EQ_EXPR, boolean_type_node,
- current_in_charge_parm, integer_zero_node),
- current_vtt_parm,
- vtt);
+ vtt = build_if_in_charge (vtt, current_vtt_parm);
if (BINFO_SUBVTT_INDEX (binfo))
sub_vtt = fold_build_pointer_plus (vtt, BINFO_SUBVTT_INDEX (binfo));
else
diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index f8ecfa1..866a0a4 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -225,6 +225,24 @@ int n_convert_harshness = 0;
int n_compute_conversion_costs = 0;
int n_inner_fields_searched = 0;
+/* Return a COND_EXPR that executes TRUE_STMT if this execution of the
+ 'structor is in charge of 'structing virtual bases, or FALSE_STMT
+ otherwise. */
+
+tree
+build_if_in_charge (tree true_stmt, tree false_stmt)
+{
+ gcc_assert (DECL_HAS_IN_CHARGE_PARM_P (current_function_decl));
+ tree cmp = build2 (NE_EXPR, boolean_type_node,
+ current_in_charge_parm, integer_zero_node);
+ tree type = unlowered_expr_type (true_stmt);
+ if (VOID_TYPE_P (type))
+ type = unlowered_expr_type (false_stmt);
+ tree cond = build3 (COND_EXPR, type,
+ cmp, true_stmt, false_stmt);
+ return cond;
+}
+
/* Convert to or from a base subobject. EXPR is an expression of type
`A' or `A*', an expression of type `B' or `B*' is returned. To
convert A to a base B, CODE is PLUS_EXPR and BINFO is the binfo for
@@ -470,12 +488,9 @@ build_base_path (enum tree_code code,
/* Negative fixed_type_p means this is a constructor or destructor;
virtual base layout is fixed in in-charge [cd]tors, but not in
base [cd]tors. */
- offset = build3 (COND_EXPR, ptrdiff_type_node,
- build2 (EQ_EXPR, boolean_type_node,
- current_in_charge_parm, integer_zero_node),
- v_offset,
- convert_to_integer (ptrdiff_type_node,
- BINFO_OFFSET (binfo)));
+ offset = build_if_in_charge
+ (convert_to_integer (ptrdiff_type_node, BINFO_OFFSET (binfo)),
+ v_offset);
else
offset = v_offset;
}
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index a50d92c..497430a 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -5642,6 +5642,7 @@ extern tree get_function_version_dispatcher (tree);
/* in class.c */
extern tree build_vfield_ref (tree, tree);
+extern tree build_if_in_charge (tree true_stmt, tree false_stmt = void_node);
extern tree build_base_path (enum tree_code, tree,
tree, int, tsubst_flags_t);
extern tree convert_to_base (tree, tree, bool, bool,
diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index 22c039b..aee3b84 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -1243,12 +1243,7 @@ expand_virtual_init (tree binfo, tree decl)
/* The actual initializer is the VTT value only in the subobject
constructor. In maybe_clone_body we'll substitute NULL for
the vtt_parm in the case of the non-subobject constructor. */
- vtbl = build3 (COND_EXPR,
- TREE_TYPE (vtbl),
- build2 (EQ_EXPR, boolean_type_node,
- current_in_charge_parm, integer_zero_node),
- vtbl2,
- vtbl);
+ vtbl = build_if_in_charge (vtbl, vtbl2);
}
/* Compute the location of the vtpr. */
@@ -1741,11 +1736,7 @@ expand_default_init (tree binfo, tree true_exp, tree exp, tree init, int flags,
&parms, binfo, flags,
complain);
base = fold_build_cleanup_point_expr (void_type_node, base);
- rval = build3 (COND_EXPR, void_type_node,
- build2 (EQ_EXPR, boolean_type_node,
- current_in_charge_parm, integer_zero_node),
- base,
- complete);
+ rval = build_if_in_charge (complete, base);
}
else
{
commit a8ffdc4d5e991bebdaf4e1154147e42ee6d66e40
Author: Jason Merrill <ja...@redhat.com>
Date: Thu Mar 17 17:09:40 2016 -0400
Avoid clobbering primary virtual base when not in charge.
* decl.c (build_clobber_this): Factor out of
start_preparsed_function and begin_destructor_body. Handle
virtual bases better.
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 497430a..9c2aeb2 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -1991,7 +1991,7 @@ struct GTY(()) lang_type {
#define CLASSTYPE_VBASECLASSES(NODE) (LANG_TYPE_CLASS_CHECK (NODE)->vbases)
/* The type corresponding to NODE when NODE is used as a base class,
- i.e., NODE without virtual base classes. */
+ i.e., NODE without virtual base classes or tail padding. */
#define CLASSTYPE_AS_BASE(NODE) (LANG_TYPE_CLASS_CHECK (NODE)->as_base)
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 6cc7046..4f29163 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -13716,6 +13716,43 @@ implicit_default_ctor_p (tree fn)
&& sufficient_parms_p (FUNCTION_FIRST_USER_PARMTYPE (fn)));
}
+/* Clobber the contents of *this to let the back end know that the object
+ storage is dead when we enter the constructor or leave the destructor. */
+
+static tree
+build_clobber_this ()
+{
+ /* Clobbering an empty base is pointless, and harmful if its one byte
+ TYPE_SIZE overlays real data. */
+ if (is_empty_class (current_class_type))
+ return void_node;
+
+ /* If we have virtual bases, clobber the whole object, but only if we're in
+ charge. If we don't have virtual bases, clobber the as-base type so we
+ don't mess with tail padding. */
+ bool vbases = CLASSTYPE_VBASECLASSES (current_class_type);
+
+ tree ctype = current_class_type;
+ if (!vbases)
+ ctype = CLASSTYPE_AS_BASE (ctype);
+
+ tree clobber = build_constructor (ctype, NULL);
+ TREE_THIS_VOLATILE (clobber) = true;
+
+ tree thisref = current_class_ref;
+ if (ctype != current_class_type)
+ {
+ thisref = build_nop (build_reference_type (ctype), current_class_ptr);
+ thisref = convert_from_reference (thisref);
+ }
+
+ tree exprstmt = build2 (MODIFY_EXPR, void_type_node, thisref, clobber);
+ if (vbases)
+ exprstmt = build_if_in_charge (exprstmt);
+
+ return exprstmt;
+}
+
/* Create the FUNCTION_DECL for a function definition.
DECLSPECS and DECLARATOR are the parts of the declaration;
they describe the function's name and the type it returns,
@@ -14131,17 +14168,7 @@ start_preparsed_function (tree decl1, tree attrs, int flags)
because part of the initialization might happen before we enter the
constructor, via AGGR_INIT_ZERO_FIRST (c++/68006). */
&& !implicit_default_ctor_p (decl1))
- {
- /* Insert a clobber to let the back end know that the object storage
- is dead when we enter the constructor. */
- tree btype = CLASSTYPE_AS_BASE (current_class_type);
- tree clobber = build_constructor (btype, NULL);
- TREE_THIS_VOLATILE (clobber) = true;
- tree bref = build_nop (build_reference_type (btype), current_class_ptr);
- bref = convert_from_reference (bref);
- tree exprstmt = build2 (MODIFY_EXPR, btype, bref, clobber);
- finish_expr_stmt (exprstmt);
- }
+ finish_expr_stmt (build_clobber_this ());
if (!processing_template_decl
&& DECL_CONSTRUCTOR_P (decl1)
@@ -14371,18 +14398,7 @@ begin_destructor_body (void)
if (flag_lifetime_dse
/* Clobbering an empty base is harmful if it overlays real data. */
&& !is_empty_class (current_class_type))
- {
- /* Insert a cleanup to let the back end know that the object is dead
- when we exit the destructor, either normally or via exception. */
- tree btype = CLASSTYPE_AS_BASE (current_class_type);
- tree clobber = build_constructor (btype, NULL);
- TREE_THIS_VOLATILE (clobber) = true;
- tree bref = build_nop (build_reference_type (btype),
- current_class_ptr);
- bref = convert_from_reference (bref);
- tree exprstmt = build2 (MODIFY_EXPR, btype, bref, clobber);
- finish_decl_cleanup (NULL_TREE, exprstmt);
- }
+ finish_decl_cleanup (NULL_TREE, build_clobber_this ());
/* And insert cleanups for our bases and members so that they
will be properly destroyed if we throw. */
commit 2994636e0a3e0fc05367bd8ba7ae6eaeb2969b93
Author: Jason Merrill <ja...@redhat.com>
Date: Wed Mar 16 17:13:50 2016 -0400
PR c++/70147 - handle primary virtual bases
* class.c (vptr_via_virtual_p): New.
(most_primary_binfo): Factor out of build_rtti_vtbl_entries.
* cp-ubsan.c (cp_ubsan_dfs_initialize_vtbl_ptrs): Don't clear
a vptr from any virtual base in a not-in-charge 'structor.
diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index 866a0a4..1fcf61e 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -8493,6 +8493,40 @@ get_primary_binfo (tree binfo)
return copied_binfo (primary_base, binfo);
}
+/* As above, but iterate until we reach the binfo that actually provides the
+ vptr for BINFO. */
+
+static tree
+most_primary_binfo (tree binfo)
+{
+ tree b = binfo;
+ while (CLASSTYPE_HAS_PRIMARY_BASE_P (BINFO_TYPE (b))
+ && !BINFO_LOST_PRIMARY_P (b))
+ {
+ tree primary_base = get_primary_binfo (b);
+ gcc_assert (BINFO_PRIMARY_P (primary_base)
+ && BINFO_INHERITANCE_CHAIN (primary_base) == b);
+ b = primary_base;
+ }
+ return b;
+}
+
+/* Returns true if BINFO gets its vptr from a virtual base of the most derived
+ type. Note that the virtual inheritance might be above or below BINFO in
+ the hierarchy. */
+
+bool
+vptr_via_virtual_p (tree binfo)
+{
+ if (TYPE_P (binfo))
+ binfo = TYPE_BINFO (binfo);
+ tree primary = most_primary_binfo (binfo);
+ /* Don't limit binfo_via_virtual, we want to return true when BINFO itself is
+ a morally virtual base. */
+ tree virt = binfo_via_virtual (primary, NULL_TREE);
+ return virt != NULL_TREE;
+}
+
/* If INDENTED_P is zero, indent to INDENT. Return nonzero. */
static int
@@ -9780,17 +9814,7 @@ build_rtti_vtbl_entries (tree binfo, vtbl_init_data* vid)
/* To find the complete object, we will first convert to our most
primary base, and then add the offset in the vtbl to that value. */
- b = binfo;
- while (CLASSTYPE_HAS_PRIMARY_BASE_P (BINFO_TYPE (b))
- && !BINFO_LOST_PRIMARY_P (b))
- {
- tree primary_base;
-
- primary_base = get_primary_binfo (b);
- gcc_assert (BINFO_PRIMARY_P (primary_base)
- && BINFO_INHERITANCE_CHAIN (primary_base) == b);
- b = primary_base;
- }
+ b = most_primary_binfo (binfo);
offset = size_diffop_loc (input_location,
BINFO_OFFSET (vid->rtti_binfo), BINFO_OFFSET (b));
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 9c2aeb2..999005e 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -5681,6 +5681,7 @@ extern void invalidate_class_lookup_cache (void);
extern void maybe_note_name_used_in_class (tree, tree);
extern void note_name_declared_in_class (tree, tree);
extern tree get_vtbl_decl_for_binfo (tree);
+extern bool vptr_via_virtual_p (tree);
extern void debug_class (tree);
extern void debug_thunks (tree);
extern void set_linkage_according_to_type (tree, tree);
diff --git a/gcc/cp/cp-ubsan.c b/gcc/cp/cp-ubsan.c
index 75aeeb8..be24a5c 100644
--- a/gcc/cp/cp-ubsan.c
+++ b/gcc/cp/cp-ubsan.c
@@ -283,7 +283,7 @@ cp_ubsan_dfs_initialize_vtbl_ptrs (tree binfo, void *data)
if (!TYPE_CONTAINS_VPTR_P (BINFO_TYPE (binfo)))
return dfs_skip_bases;
- if (!BINFO_PRIMARY_P (binfo) || BINFO_VIRTUAL_P (binfo))
+ if (!BINFO_PRIMARY_P (binfo))
{
tree base_ptr = TREE_VALUE ((tree) data);
@@ -301,11 +301,10 @@ cp_ubsan_dfs_initialize_vtbl_ptrs (tree binfo, void *data)
tree vtbl = build_zero_cst (TREE_TYPE (vtbl_ptr));
tree stmt = cp_build_modify_expr (vtbl_ptr, NOP_EXPR, vtbl,
tf_warning_or_error);
- if (BINFO_VIRTUAL_P (binfo))
- stmt = build3 (COND_EXPR, void_type_node,
- build2 (NE_EXPR, boolean_type_node,
- current_in_charge_parm, integer_zero_node),
- stmt, void_node);
+ if (vptr_via_virtual_p (binfo))
+ /* If this vptr comes from a virtual base of the complete object, only
+ clear it if we're in charge of virtual bases. */
+ stmt = build_if_in_charge (stmt);
finish_expr_stmt (stmt);
}
diff --git a/gcc/testsuite/g++.dg/ubsan/vptr-11.C b/gcc/testsuite/g++.dg/ubsan/vptr-11.C
new file mode 100644
index 0000000..4516b1e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ubsan/vptr-11.C
@@ -0,0 +1,84 @@
+// PR c++/70147
+// { dg-do run }
+// { dg-options "-fsanitize=vptr -fno-sanitize-recover=vptr" }
+
+static int ac, ad, bc, bd, cc, cd, dc, dd;
+struct A
+{
+ A ()
+ {
+ ac++;
+ }
+ virtual void f ()
+ {
+ }
+ __attribute__ ((noinline)) ~ A ();
+};
+
+struct D
+{
+ __attribute__ ((noinline)) D (int);
+ ~D ()
+ {
+ dd++;
+ }
+};
+struct B: virtual A, D
+{
+ B ():D (1)
+ {
+ bc++;
+ }
+ virtual void f ()
+ {
+ }
+ ~B ()
+ {
+ bd++;
+ }
+};
+
+struct C: B, virtual A
+{
+ C ()
+ {
+ cc++;
+ }
+ ~C ()
+ {
+ cd++;
+ }
+};
+
+D::D (int x)
+{
+ if (x)
+ throw 1;
+ dc++;
+}
+
+__attribute__ ((noinline, noclone))
+void foo (A * p)
+{
+ p->f ();
+}
+
+A::~A ()
+{
+ foo (this);
+ ad++;
+}
+
+int
+main ()
+{
+ try
+ {
+ C c;
+ }
+ catch ( ...)
+ {
+ }
+ if (ac != 1 || ad != 1 || bc || bd || cc || cd || dc || dd)
+ __builtin_abort ();
+}