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]

RFA (ipa-prop): PATCHes to avoid use of deprecated copy ctor and op=


In C++11 and up, the implicitly-declared copy constructor and
assignment operator are deprecated if one of them, or the destructor,
is user-provided.  Implementing that in G++ turned up a few dodgy uses
in the compiler.

In general it's unsafe to copy an ipa_edge_args, because if one of the
pointers is non-null you get two copies of a vec pointer, and when one
of the objects is destroyed it frees the vec and leaves the other
object pointing to freed memory.  This specific example is safe
because it only copies from an object with null pointers, but it would
be better to avoid the copy.  OK for trunk?

It's unsafe to copy a releasing_vec for the same reason.  There are a
few places where the copy constructor is nominally used to initialize
a releasing_vec variable from a temporary returned from a function; in
these cases no actual copy is done, and the function directly
initializes the variable, so it's safe.  I made this clearer by
declaring the copy constructor but not defining it, so uses that get
elided are accepted, but uses that actually want to copy will fail to
link.

In cp_expr we defined the copy constructor to do the same thing that
the implicit definition would do, causing the copy assignment operator
to be deprecated.  We don't need the copy constructor, so let's remove
it.

Tested x86_64-pc-linux-gnu.  Are the ipa-prop bits OK for trunk?
commit 560b104324b10814dcaaf65606943d4ca55647d6
Author: Jason Merrill <jason@redhat.com>
Date:   Tue May 15 20:47:41 2018 -0400

            * ipa-prop.h (ipa_edge_args::release): Factor out of destructor.
    
            * ipa-prop.c (ipa_free_edge_args_substructures): Use it.

diff --git a/gcc/ipa-prop.c b/gcc/ipa-prop.c
index 38441cc49bc..11bbf356ce1 100644
--- a/gcc/ipa-prop.c
+++ b/gcc/ipa-prop.c
@@ -3714,8 +3714,7 @@ ipa_check_create_edge_args (void)
 void
 ipa_free_edge_args_substructures (struct ipa_edge_args *args)
 {
-  vec_free (args->jump_functions);
-  *args = ipa_edge_args ();
+  args->release ();
 }
 
 /* Free all ipa_edge structures.  */
diff --git a/gcc/ipa-prop.h b/gcc/ipa-prop.h
index a61e06135e3..08919acb46c 100644
--- a/gcc/ipa-prop.h
+++ b/gcc/ipa-prop.h
@@ -569,13 +569,18 @@ class GTY((for_user)) ipa_edge_args
   ipa_edge_args () : jump_functions (NULL), polymorphic_call_contexts (NULL)
     {}
 
-  /* Destructor.  */
-  ~ipa_edge_args ()
+  void release ()
     {
       vec_free (jump_functions);
       vec_free (polymorphic_call_contexts);
     }
 
+  /* Destructor.  */
+  ~ipa_edge_args ()
+    {
+      release ();
+    }
+
   /* Vectors of the callsite's jump function and polymorphic context
      information of each parameter.  */
   vec<ipa_jump_func, va_gc> *jump_functions;
commit 648ffd02e23ac2695de04ab266b4f8862df6c2ed
Author: Jason Merrill <jason@redhat.com>
Date:   Tue May 15 20:46:54 2018 -0400

            * cp-tree.h (cp_expr): Remove copy constructor.
    
            * mangle.c (struct releasing_vec): Declare copy constructor.

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 9a2eb3be4d1..cab926028b8 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -59,9 +59,6 @@ public:
   cp_expr (tree value, location_t loc):
     m_value (value), m_loc (loc) {}
 
-  cp_expr (const cp_expr &other) :
-    m_value (other.m_value), m_loc (other.m_loc) {}
-
   /* Implicit conversions to tree.  */
   operator tree () const { return m_value; }
   tree & operator* () { return m_value; }
diff --git a/gcc/cp/mangle.c b/gcc/cp/mangle.c
index 6a7df804caf..59a3111fba2 100644
--- a/gcc/cp/mangle.c
+++ b/gcc/cp/mangle.c
@@ -1555,6 +1555,10 @@ struct releasing_vec
   releasing_vec (vec_t *v): v(v) { }
   releasing_vec (): v(make_tree_vector ()) { }
 
+  /* Copy constructor is deliberately declared but not defined,
+     copies must always be elided.  */
+  releasing_vec (const releasing_vec &);
+
   vec_t &operator* () const { return *v; }
   vec_t *operator-> () const { return v; }
   vec_t *get () const { return v; }

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