Make member<T> not heap-allocated with ZCPOINTER_TRACK_REFS.
authorRobert Sesek <rsesek@bluestatic.org>
Wed, 12 Oct 2016 01:23:08 +0000 (21:23 -0400)
committerRobert Sesek <rsesek@bluestatic.org>
Wed, 12 Oct 2016 01:23:08 +0000 (21:23 -0400)
Instead, member<T> is a subclass of T with |operator&| overloaded. And a new
OwnershipBehavior field is added to the OwnedPtrDeleter, to control whether
delete should be invoked on the pointer. For member<T>, the behavior is to not,
while for owned<T> the behavior is to delete.

The new field does increase codesize for member<T> when using TRACK_REFS, since
the extra field is added to all deleters and requires an extra comparison. It is
not possible to do this using a compile-time-only mechansim because templates are
not covariant. That would prevent |owned<T, DELETE>| and |member<T, BORROW>|
from both returning a |ref<T>| even if DELETE and BORROW parameters were related.

But this also makes non-TRACK_REFS mode member<T> just a type alias for T, which
is more conceptually correct.

test.cc
test_helpers.cc
zcpointer.h

diff --git a/test.cc b/test.cc
index c51932250cf47edf2354e4e6d81118082d52e81f..0e46e9437404901e0c0c588a5989185db9d8dd58 100644 (file)
--- a/test.cc
+++ b/test.cc
@@ -169,7 +169,7 @@ void TestStack() {
     zc::member<C> c;
     rc = &c;
     EXPECT(rc == &c);
-    c->DoThing();
+    c.DoThing();
   }
   EXPECT_UAF(rc->DoThing());
 }
@@ -189,6 +189,12 @@ void TestMember() {
   }
   EXPECT_UAF(ref->DoThing());
   EXPECT_UAF(vec_ref->at(1).DoThing());
+
+  {
+    zc::member<X> x("foo bar");
+    ref = x.c();
+  }
+  EXPECT_UAF(ref->DoThing());
 }
 
 #define TEST_FUNC(fn) { #fn , Test##fn }
index 45cba9b3d76f13f0483fd6b49934aa338fdcd832..ae1897c8d08c9814c0314740afa67721ec862d79 100644 (file)
@@ -25,10 +25,10 @@ X::X(const char* v) : foo_(v) {}
 X::~X() {}
 
 size_t X::GetCount() const {
-  return vec_c_->size();
+  return vec_c_.size();
 }
 
 std::string X::DoString() {
-  vec_c_->push_back(C());
+  vec_c_.push_back(C());
   return foo_;
 }
index 9af37d6f1595dbef7e4eeb9d19f5f4a6a7cc0ba1..74a547d0c71a06f4c7b6c7b8120c9630719ba1bd 100644 (file)
@@ -34,24 +34,39 @@ template <typename T> class ref;
 
 namespace internal {
 
+enum class OwnershipBehavior {
+  DELETE_POINTER,
+  BORROW_POINTER,
+};
+
 template <typename T>
 class OwnedPtrDeleter {
  public:
-  OwnedPtrDeleter() {}
+  OwnedPtrDeleter() : refs_(), behavior_(OwnershipBehavior::DELETE_POINTER) {}
   ~OwnedPtrDeleter() {}
 
-  OwnedPtrDeleter(OwnedPtrDeleter&& other) : refs_(std::move(other.refs_)) {
+  explicit OwnedPtrDeleter(OwnershipBehavior behavior)
+      : refs_(),
+        behavior_(behavior) {
+  }
+
+  OwnedPtrDeleter(OwnedPtrDeleter&& other)
+      : refs_(std::move(other.refs_)),
+        behavior_(other.behavior_) {
   }
 
   void operator=(const OwnedPtrDeleter& o) {
     refs_ = o.refs_;
+    behavior_ = o.behavior_;
   }
 
   void operator()(T* t) const {
     for (auto& ref : refs_) {
       ref->MarkDeleted();
     }
-    delete t;
+    if (behavior_ == OwnershipBehavior::DELETE_POINTER) {
+      delete t;
+    }
   }
 
  protected:
@@ -67,6 +82,7 @@ class OwnedPtrDeleter {
 
  private:
   std::forward_list<ref<T>*> refs_;
+  OwnershipBehavior behavior_;
 };
 
 void RaiseUseAfterFree(const char* error) __attribute__((noreturn));
@@ -171,27 +187,18 @@ class ref {
 };
 
 template <typename T>
-class member {
+class member : public T {
  public:
-  template <typename... Args>
-  explicit member(Args&&... args)
-    : t_(new T(std::forward<Args>(args)...)) {
-  }
+  using T::T;
 
   ref<T> operator&() {
-    return t_.get();
-  }
-
-  T* operator->() {
-    return t_.operator->();
-  }
-
-  const T* operator->() const {
-    return t_.operator->();
+    return ptr_.get();
   }
 
  private:
-  owned<T> t_;
+  owned<T> ptr_ = owned<T>(this,
+                           internal::OwnedPtrDeleter<T>(
+                               internal::OwnershipBehavior::BORROW_POINTER));
 };
 
 #else
@@ -203,18 +210,7 @@ template <typename T>
 using ref = T*;
 
 template <typename T>
-class member : public T {
- public:
-  using T::T;
-
-  T* operator->() {
-    return this;
-  }
-
-  const T* operator->() const {
-    return this;
-  }
-};
+using member = T;
 
 #endif