From 41210b092c1acc83ee33d7becc24298e2fbe4bfb Mon Sep 17 00:00:00 2001 From: Robert Sesek Date: Tue, 11 Oct 2016 21:23:08 -0400 Subject: [PATCH] Make member not heap-allocated with ZCPOINTER_TRACK_REFS. Instead, member 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, the behavior is to not, while for owned the behavior is to delete. The new field does increase codesize for member 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| and |member| from both returning a |ref| even if DELETE and BORROW parameters were related. But this also makes non-TRACK_REFS mode member just a type alias for T, which is more conceptually correct. --- test.cc | 8 ++++++- test_helpers.cc | 4 ++-- zcpointer.h | 56 +++++++++++++++++++++++-------------------------- 3 files changed, 35 insertions(+), 33 deletions(-) diff --git a/test.cc b/test.cc index c519322..0e46e94 100644 --- a/test.cc +++ b/test.cc @@ -169,7 +169,7 @@ void TestStack() { zc::member 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("foo bar"); + ref = x.c(); + } + EXPECT_UAF(ref->DoThing()); } #define TEST_FUNC(fn) { #fn , Test##fn } diff --git a/test_helpers.cc b/test_helpers.cc index 45cba9b..ae1897c 100644 --- a/test_helpers.cc +++ b/test_helpers.cc @@ -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_; } diff --git a/zcpointer.h b/zcpointer.h index 9af37d6..74a547d 100644 --- a/zcpointer.h +++ b/zcpointer.h @@ -34,24 +34,39 @@ template class ref; namespace internal { +enum class OwnershipBehavior { + DELETE_POINTER, + BORROW_POINTER, +}; + template 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*> refs_; + OwnershipBehavior behavior_; }; void RaiseUseAfterFree(const char* error) __attribute__((noreturn)); @@ -171,27 +187,18 @@ class ref { }; template -class member { +class member : public T { public: - template - explicit member(Args&&... args) - : t_(new T(std::forward(args)...)) { - } + using T::T; ref operator&() { - return t_.get(); - } - - T* operator->() { - return t_.operator->(); - } - - const T* operator->() const { - return t_.operator->(); + return ptr_.get(); } private: - owned t_; + owned ptr_ = owned(this, + internal::OwnedPtrDeleter( + internal::OwnershipBehavior::BORROW_POINTER)); }; #else @@ -203,18 +210,7 @@ template using ref = T*; template -class member : public T { - public: - using T::T; - - T* operator->() { - return this; - } - - const T* operator->() const { - return this; - } -}; +using member = T; #endif -- 2.22.5