From 1a117d7e09b4856b863fd2ef91eed9327341f667 Mon Sep 17 00:00:00 2001 From: Mehdi Bouaziz Date: Wed, 6 Dec 2017 03:16:00 -0800 Subject: [PATCH] Fix unique_ptr model Summary: Our model of unique_ptr and shared_ptr relied on the fact that we could C-style cast a pointer to the internal pointer type used in the smart pointer. This is wrong when the smart pointer is used with a custom deleter that declares its own pointer type whose is not constructible from just a single pointer. Reviewed By: dulmarod Differential Revision: D6496203 fbshipit-source-id: 1305137 --- .../cpp/include/infer_model/shared_ptr.h | 4 +- .../cpp/include/infer_model/unique_ptr.h | 12 +----- .../tests/codetoanalyze/cpp/errors/issues.exp | 1 + .../cpp/errors/smart_ptr/unique_ptr_deref.cpp | 37 +++++++++++++++++++ 4 files changed, 41 insertions(+), 13 deletions(-) diff --git a/infer/models/cpp/include/infer_model/shared_ptr.h b/infer/models/cpp/include/infer_model/shared_ptr.h index 72d2db9c8..5dcf0b021 100644 --- a/infer/models/cpp/include/infer_model/shared_ptr.h +++ b/infer/models/cpp/include/infer_model/shared_ptr.h @@ -77,8 +77,6 @@ class shared_ptr : public std__shared_ptr { model_set(other, nullptr); } - static T* model_get(infer_shared_ptr_t self) { return (T*)(*self); } - static void model_swap(infer_shared_ptr_t infer_self, infer_shared_ptr_t infer_other) { const void* t = *infer_self; @@ -268,7 +266,7 @@ class shared_ptr : public std__shared_ptr { explicit operator bool() const noexcept { // for some reason analyzer can't cast to bool correctly, trick with two // negations creates right specs for this function - return !!(bool)(model_get(__cast_to_infer_ptr(this))); + return !!(bool)(*__cast_to_infer_ptr(this)); } template bool owner_before(shared_ptr const& b) const { diff --git a/infer/models/cpp/include/infer_model/unique_ptr.h b/infer/models/cpp/include/infer_model/unique_ptr.h index c65d7738b..53221a840 100644 --- a/infer/models/cpp/include/infer_model/unique_ptr.h +++ b/infer/models/cpp/include/infer_model/unique_ptr.h @@ -81,10 +81,6 @@ struct unique_ptr { model_set(other, nullptr); } - static pointer model_get(infer_unique_ptr_t self) noexcept { - return (pointer)(*self); - } - static void model_swap(infer_unique_ptr_t infer_self, infer_unique_ptr_t infer_other) noexcept { const void* t = *infer_self; @@ -179,7 +175,7 @@ struct unique_ptr { _Dp_reference get_deleter() {} explicit operator bool() const { - return !!(bool)(model_get(__cast_to_infer_ptr(this))); + return !!(bool)(*__cast_to_infer_ptr(this)); } pointer release() INFER_MODEL_AS_DEREF_FIRST_ARG; @@ -235,10 +231,6 @@ struct unique_ptr<_Tp[], _Dp> { model_set(other, nullptr); } - static pointer model_get(infer_unique_ptr_t self) noexcept { - return (pointer)(*self); - } - static void model_swap(infer_unique_ptr_t infer_self, infer_unique_ptr_t infer_other) noexcept { const void* t = *infer_self; @@ -329,7 +321,7 @@ struct unique_ptr<_Tp[], _Dp> { _Dp_reference get_deleter() {} explicit operator bool() const { - return !!(bool)(model_get(__cast_to_infer_ptr(this))); + return !!(bool)(*__cast_to_infer_ptr(this)); } pointer release() INFER_MODEL_AS_DEREF_FIRST_ARG; diff --git a/infer/tests/codetoanalyze/cpp/errors/issues.exp b/infer/tests/codetoanalyze/cpp/errors/issues.exp index 978b3ed4a..0115a0c2e 100644 --- a/infer/tests/codetoanalyze/cpp/errors/issues.exp +++ b/infer/tests/codetoanalyze/cpp/errors/issues.exp @@ -1,4 +1,5 @@ INFER_MODEL/cpp/include/infer_model/shared_ptr.h, std::make_shared, 1, MEMORY_LEAK, [start of procedure std::make_shared(),Skipping lol: function or method not found] +INFER_MODEL/cpp/include/infer_model/unique_ptr.h, std::operator!=<65d659492edc5cb5>, 1, Abduction_case_not_implemented, [start of procedure std::operator!=<65d659492edc5cb5>()] codetoanalyze/cpp/errors/c_tests/c_bugs.cpp, crash_fgetc, 4, NULL_DEREFERENCE, [start of procedure crash_fgetc()] codetoanalyze/cpp/errors/c_tests/c_bugs.cpp, crash_getc, 4, NULL_DEREFERENCE, [start of procedure crash_getc()] codetoanalyze/cpp/errors/c_tests/c_bugs.cpp, malloc_fail_gets_reported, 2, NULL_DEREFERENCE, [start of procedure malloc_fail_gets_reported()] diff --git a/infer/tests/codetoanalyze/cpp/errors/smart_ptr/unique_ptr_deref.cpp b/infer/tests/codetoanalyze/cpp/errors/smart_ptr/unique_ptr_deref.cpp index 745a78a67..8af29c801 100644 --- a/infer/tests/codetoanalyze/cpp/errors/smart_ptr/unique_ptr_deref.cpp +++ b/infer/tests/codetoanalyze/cpp/errors/smart_ptr/unique_ptr_deref.cpp @@ -119,4 +119,41 @@ int unique_ptr_move_null_deref() { std::unique_ptr p2 = std::move(p1); return *p1; } + +} // namespace unique_ptr + +namespace unique_ptr_with_deleter { + +/* This is just a compilation test */ + +template +class Pointer { + public: + /* No constructor with only one T* argument */ + /* implicit */ Pointer(std::nullptr_t = nullptr) noexcept {} + Pointer(T* ptr, int n) noexcept {} + + friend bool operator==(Pointer a, Pointer b) noexcept { return true; } + friend bool operator!=(Pointer a, Pointer b) noexcept { return true; } + explicit operator bool() const noexcept { return true; } + T* operator->() const noexcept { return get(); } + T& operator*() const noexcept { return *get(); } + T* get() const noexcept { return nullptr; } +}; + +template +struct Deleter { + using pointer = Pointer; + + void operator()(pointer ptr) const {} +}; + +template +using my_unique_ptr = std::unique_ptr>; + +bool instantiate() { + my_unique_ptr p; + my_unique_ptr q; + return p != nullptr && q != nullptr; +} }