This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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.

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;
+}

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]