C++ PATCH for c++/67557 (copy elision clobbering tail padding)

C++ PATCH for c++/67557 (copy elision clobbering tail padding)

[Jason Merrill]

[-- Attachment #1: Type: text/plain, Size: 876 bytes --]

In this testcase, the FontTag constructor was storing the result of 
fontToStartTag into a stack temporary and then bitwise copying the 
temporary into the StartTag base subobject, which is broken for a class 
with a non-trivial copy constructor.  This bogus copy turns out to have 
been introduced by store_field, because the base field is smaller than 
the type of the call.

In fact, the copy elision that the front end is expecting here is 
unsafe, because we can't be sure that the called function won't clobber 
tail padding in the base subobject; we need to force a real copy 
constructor call from the function return value to the base subobject.

I also want to add an assert to store_field to avoid inappropriate 
bitwise copying of TREE_ADDRESSABLE types, but that is waiting for my 
patch for the CONSTRUCTOR case.

Tested x86_64-pc-linux-gnu, applying to trunk.


[-- Attachment #2: 67557.patch --]
[-- Type: text/x-patch, Size: 3231 bytes --]

commit 80057a9d21415f5ccd29328183a2e3d6b3a0c5e1
Author: Jason Merrill <jason@redhat.com>
Date:   Wed Oct 7 16:39:13 2015 -0400

    	PR c++/67557
    
    	* call.c (is_base_field_ref): New.
    	(unsafe_copy_elision_p): New.
    	(build_over_call): Use it.

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 93e28dc..f8db2df 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -7094,6 +7094,39 @@ call_copy_ctor (tree a, tsubst_flags_t complain)
   return r;
 }
 
+/* Return true iff T refers to a base field.  */
+
+static bool
+is_base_field_ref (tree t)
+{
+  STRIP_NOPS (t);
+  if (TREE_CODE (t) == ADDR_EXPR)
+    t = TREE_OPERAND (t, 0);
+  if (TREE_CODE (t) == COMPONENT_REF)
+    t = TREE_OPERAND (t, 1);
+  if (TREE_CODE (t) == FIELD_DECL)
+    return DECL_FIELD_IS_BASE (t);
+  return false;
+}
+
+/* We can't elide a copy from a function returning by value to a base
+   subobject, as the callee might clobber tail padding.  Return true iff this
+   could be that case.  */
+
+static bool
+unsafe_copy_elision_p (tree target, tree exp)
+{
+  tree type = TYPE_MAIN_VARIANT (TREE_TYPE (exp));
+  if (type == CLASSTYPE_AS_BASE (type))
+    return false;
+  if (!is_base_field_ref (target)
+      && resolves_to_fixed_type_p (target, NULL))
+    return false;
+  tree init = TARGET_EXPR_INITIAL (exp);
+  return (TREE_CODE (init) == AGGR_INIT_EXPR
+	  && !AGGR_INIT_VIA_CTOR_P (init));
+}
+
 /* Subroutine of the various build_*_call functions.  Overload resolution
    has chosen a winning candidate CAND; build up a CALL_EXPR accordingly.
    ARGS is a TREE_LIST of the unconverted arguments to the call.  FLAGS is a
@@ -7513,7 +7546,9 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain)
 	  else if (trivial)
 	    return force_target_expr (DECL_CONTEXT (fn), arg, complain);
 	}
-      else if (TREE_CODE (arg) == TARGET_EXPR || trivial)
+      else if (trivial
+	       || (TREE_CODE (arg) == TARGET_EXPR
+		   && !unsafe_copy_elision_p (fa, arg)))
 	{
 	  tree to = stabilize_reference (cp_build_indirect_ref (fa, RO_NULL,
 								complain));
diff --git a/gcc/testsuite/g++.dg/init/elide3.C b/gcc/testsuite/g++.dg/init/elide3.C
new file mode 100644
index 0000000..7eb0389
--- /dev/null
+++ b/gcc/testsuite/g++.dg/init/elide3.C
@@ -0,0 +1,50 @@
+// PR c++/67557
+// { dg-do run }
+
+namespace std
+{
+  struct string
+  {
+    typedef unsigned long size_type;
+    const char* _M_p;
+    char        _M_local_buf[1];
+
+    string(const char* s) : _M_p(_M_local_buf)
+    {
+      __builtin_printf("%p constructed\n", this);
+    }
+
+    string(const string& s) : _M_p(_M_local_buf)
+    {
+      __builtin_printf("%p copied from %p\n", this, &s);
+    }
+
+    ~string()
+    {
+      __builtin_printf("%p destroyed\n", this);
+      if (_M_p != _M_local_buf)
+	__builtin_abort();
+    }
+  };
+}
+
+struct StartTag
+{
+  explicit StartTag(std::string const & tag) : tag_(tag), keepempty_(false) {}
+  std::string tag_;
+  bool keepempty_;
+};
+
+StartTag fontToStartTag() { return StartTag(""); }
+
+struct FontTag : public StartTag
+{
+  FontTag() : StartTag(fontToStartTag()) {}
+};
+
+int main()
+{
+  FontTag x;
+  __builtin_printf("%p x.tag_ in main()\n", &x.tag_);
+  return 0;
+}