diff --git a/libutils/RefBase.cpp b/libutils/RefBase.cpp index 40b6bf07e..5199ef60e 100644 --- a/libutils/RefBase.cpp +++ b/libutils/RefBase.cpp @@ -149,6 +149,29 @@ namespace android { // Same for weak counts. #define BAD_WEAK(c) ((c) == 0 || ((c) & (~MAX_COUNT)) != 0) +// see utils/StrongPointer.h - declared there for legacy reasons +void sp_report_stack_pointer(); + +// Check whether address is definitely on the calling stack. We actually check whether it is on +// the same 4K page as the frame pointer. +// +// Assumptions: +// - Pages are never smaller than 4K (MIN_PAGE_SIZE) +// - Malloced memory never shares a page with a stack. +// +// It does not appear safe to broaden this check to include adjacent pages; apparently this code +// is used in environments where there may not be a guard page below (at higher addresses than) +// the bottom of the stack. +static void check_not_on_stack(const void* ptr) { + static constexpr int MIN_PAGE_SIZE = 0x1000; // 4K. Safer than including sys/user.h. + static constexpr uintptr_t MIN_PAGE_MASK = ~static_cast(MIN_PAGE_SIZE - 1); + uintptr_t my_frame_address = + reinterpret_cast(__builtin_frame_address(0 /* this frame */)); + if (((reinterpret_cast(ptr) ^ my_frame_address) & MIN_PAGE_MASK) == 0) { + sp_report_stack_pointer(); + } +} + // --------------------------------------------------------------------------- class RefBase::weakref_impl : public RefBase::weakref_type @@ -432,6 +455,8 @@ void RefBase::incStrong(const void* id) const return; } + check_not_on_stack(this); + int32_t old __unused = refs->mStrong.fetch_sub(INITIAL_STRONG_VALUE, std::memory_order_relaxed); // A decStrong() must still happen after us. ALOG_ASSERT(old > INITIAL_STRONG_VALUE, "0x%x too small", old); diff --git a/libutils/include/utils/StrongPointer.h b/libutils/include/utils/StrongPointer.h index bb1941b79..cbdcb5b02 100644 --- a/libutils/include/utils/StrongPointer.h +++ b/libutils/include/utils/StrongPointer.h @@ -120,7 +120,6 @@ private: template friend class sp; template friend class wp; void set_pointer(T* ptr); - static inline void check_not_on_stack(const void* ptr); T* m_ptr; }; @@ -190,27 +189,6 @@ void sp_report_stack_pointer(); // --------------------------------------------------------------------------- // No user serviceable parts below here. -// Check whether address is definitely on the calling stack. We actually check whether it is on -// the same 4K page as the frame pointer. -// -// Assumptions: -// - Pages are never smaller than 4K (MIN_PAGE_SIZE) -// - Malloced memory never shares a page with a stack. -// -// It does not appear safe to broaden this check to include adjacent pages; apparently this code -// is used in environments where there may not be a guard page below (at higher addresses than) -// the bottom of the stack. -template -void sp::check_not_on_stack(const void* ptr) { - static constexpr int MIN_PAGE_SIZE = 0x1000; // 4K. Safer than including sys/user.h. - static constexpr uintptr_t MIN_PAGE_MASK = ~static_cast(MIN_PAGE_SIZE - 1); - uintptr_t my_frame_address = - reinterpret_cast(__builtin_frame_address(0 /* this frame */)); - if (((reinterpret_cast(ptr) ^ my_frame_address) & MIN_PAGE_MASK) == 0) { - sp_report_stack_pointer(); - } -} - // TODO: Ideally we should find a way to increment the reference count before running the // constructor, so that generating an sp<> to this in the constructor is no longer dangerous. template @@ -219,14 +197,13 @@ sp sp::make(Args&&... args) { T* t = new T(std::forward(args)...); sp result; result.m_ptr = t; - t->incStrong(t); // bypass check_not_on_stack for heap allocation + t->incStrong(t); return result; } template sp sp::fromExisting(T* other) { if (other) { - check_not_on_stack(other); other->incStrongRequireStrong(other); sp result; result.m_ptr = other; @@ -240,7 +217,6 @@ template sp::sp(T* other) : m_ptr(other) { if (other) { - check_not_on_stack(other); other->incStrong(this); } } @@ -249,7 +225,6 @@ template template sp::sp(U* other) : m_ptr(other) { if (other) { - check_not_on_stack(other); (static_cast(other))->incStrong(this); } } @@ -258,7 +233,6 @@ template sp& sp::operator=(T* other) { T* oldPtr(*const_cast(&m_ptr)); if (other) { - check_not_on_stack(other); other->incStrong(this); } if (oldPtr) oldPtr->decStrong(this);