Date: Sun, 6 Jun 2021 18:14:32 GMT From: Dimitry Andric <dim@FreeBSD.org> To: ports-committers@FreeBSD.org, dev-commits-ports-all@FreeBSD.org, dev-commits-ports-main@FreeBSD.org Subject: git: 4572c48fc4d2 - main - net-im/licq: apply googletest patches which fix null pointer accesses Message-ID: <202106061814.156IEWCs051362@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by dim (src committer): URL: https://cgit.FreeBSD.org/ports/commit/?id=4572c48fc4d2a3326e85380b3f658dc8e1721817 commit 4572c48fc4d2a3326e85380b3f658dc8e1721817 Author: Dimitry Andric <dim@FreeBSD.org> AuthorDate: 2021-05-15 22:49:52 +0000 Commit: Dimitry Andric <dim@FreeBSD.org> CommitDate: 2021-06-06 18:13:58 +0000 net-im/licq: apply googletest patches which fix null pointer accesses During an exp-run for llvm 12 (see bug 255570), it turned out that net-im/licq does not build with clang 12.0.0: [ 92%] Running unit test cd /wrkdirs/usr/ports/net-im/licq/work/.build/src && /usr/local/bin/ctest Test project /wrkdirs/usr/ports/net-im/licq/work/.build/src Start 1: licq 1/1 Test #1: licq .............................***Exception: SegFault 0.03 sec This is due to licq using a relatively ancient version of googletest, which has a few issues with more modern compilers. In particular, it does not handle mocking methods with move-only return types, and in the case of this port, this leads to a null pointer access and consequently a segfault. To fix the null pointer accesses, apply a few upstream googletest commits. Approved by: maintainer timeout (2 weeks) PR: 255915 MFH: 2021Q2 --- net-im/licq/Makefile | 2 +- ...ch-3rdparty_gmock_include_gmock_gmock-actions.h | 92 +++++++++++ ...party_gmock_include_gmock_gmock-spec-builders.h | 183 +++++++++++++++++++++ ...k_include_gmock_internal_gmock-internal-utils.h | 38 +++++ ...patch-3rdparty_gmock_src_gmock-spec-builders.cc | 29 ++++ ...party_gtest_include_gtest_internal_gtest-port.h | 34 ++++ 6 files changed, 377 insertions(+), 1 deletion(-) diff --git a/net-im/licq/Makefile b/net-im/licq/Makefile index 748bcb91ed94..40553b15096c 100644 --- a/net-im/licq/Makefile +++ b/net-im/licq/Makefile @@ -2,7 +2,7 @@ PORTNAME= base PORTVERSION= 1.9.0 -PORTREVISION= 3 +PORTREVISION= 4 CATEGORIES= net-im PKGNAMEPREFIX= licq- PKGNAMESUFFIX= ${SOCKS_SUFFIX}${PKGNAMESUFFIX2} diff --git a/net-im/licq/files/patch-3rdparty_gmock_include_gmock_gmock-actions.h b/net-im/licq/files/patch-3rdparty_gmock_include_gmock_gmock-actions.h new file mode 100644 index 000000000000..bf13ef99eafa --- /dev/null +++ b/net-im/licq/files/patch-3rdparty_gmock_include_gmock_gmock-actions.h @@ -0,0 +1,92 @@ +--- 3rdparty/gmock/include/gmock/gmock-actions.h.orig 2014-10-22 20:07:20 UTC ++++ 3rdparty/gmock/include/gmock/gmock-actions.h +@@ -163,18 +163,27 @@ class DefaultValue { + // Sets the default value for type T; requires T to be + // copy-constructable and have a public destructor. + static void Set(T x) { +- delete value_; +- value_ = new T(x); ++ delete producer_; ++ producer_ = new FixedValueProducer(x); + } + ++ // Provides a factory function to be called to generate the default value. ++ // This method can be used even if T is only move-constructible, but it is not ++ // limited to that case. ++ typedef T (*FactoryFunction)(); ++ static void SetFactory(FactoryFunction factory) { ++ delete producer_; ++ producer_ = new FactoryValueProducer(factory); ++ } ++ + // Unsets the default value for type T. + static void Clear() { +- delete value_; +- value_ = NULL; ++ delete producer_; ++ producer_ = NULL; + } + + // Returns true iff the user has set the default value for type T. +- static bool IsSet() { return value_ != NULL; } ++ static bool IsSet() { return producer_ != NULL; } + + // Returns true if T has a default return value set by the user or there + // exists a built-in default value. +@@ -183,15 +192,42 @@ class DefaultValue { + } + + // Returns the default value for type T if the user has set one; +- // otherwise returns the built-in default value if there is one; +- // otherwise aborts the process. ++ // otherwise returns the built-in default value. Requires that Exists() ++ // is true, which ensures that the return value is well-defined. + static T Get() { +- return value_ == NULL ? +- internal::BuiltInDefaultValue<T>::Get() : *value_; ++ return producer_ == NULL ? ++ internal::BuiltInDefaultValue<T>::Get() : producer_->Produce(); + } + + private: +- static const T* value_; ++ class ValueProducer { ++ public: ++ virtual ~ValueProducer() {} ++ virtual T Produce() = 0; ++ }; ++ ++ class FixedValueProducer : public ValueProducer { ++ public: ++ explicit FixedValueProducer(T value) : value_(value) {} ++ virtual T Produce() { return value_; } ++ ++ private: ++ const T value_; ++ GTEST_DISALLOW_COPY_AND_ASSIGN_(FixedValueProducer); ++ }; ++ ++ class FactoryValueProducer : public ValueProducer { ++ public: ++ explicit FactoryValueProducer(FactoryFunction factory) ++ : factory_(factory) {} ++ virtual T Produce() { return factory_(); } ++ ++ private: ++ const FactoryFunction factory_; ++ GTEST_DISALLOW_COPY_AND_ASSIGN_(FactoryValueProducer); ++ }; ++ ++ static ValueProducer* producer_; + }; + + // This partial specialization allows a user to set default values for +@@ -241,7 +277,7 @@ class DefaultValue<void> { + + // Points to the user-set default value for type T. + template <typename T> +-const T* DefaultValue<T>::value_ = NULL; ++typename DefaultValue<T>::ValueProducer* DefaultValue<T>::producer_ = NULL; + + // Points to the user-set default value for type T&. + template <typename T> diff --git a/net-im/licq/files/patch-3rdparty_gmock_include_gmock_gmock-spec-builders.h b/net-im/licq/files/patch-3rdparty_gmock_include_gmock_gmock-spec-builders.h new file mode 100644 index 000000000000..7a89bd91a857 --- /dev/null +++ b/net-im/licq/files/patch-3rdparty_gmock_include_gmock_gmock-spec-builders.h @@ -0,0 +1,183 @@ +--- 3rdparty/gmock/include/gmock/gmock-spec-builders.h.orig 2014-10-22 20:07:20 UTC ++++ 3rdparty/gmock/include/gmock/gmock-spec-builders.h +@@ -211,7 +211,7 @@ class GTEST_API_ UntypedFunctionMockerBase { + // arguments. This function can be safely called from multiple + // threads concurrently. The caller is responsible for deleting the + // result. +- const UntypedActionResultHolderBase* UntypedInvokeWith( ++ UntypedActionResultHolderBase* UntypedInvokeWith( + const void* untyped_args) + GTEST_LOCK_EXCLUDED_(g_gmock_mutex); + +@@ -1289,6 +1289,58 @@ class MockSpec { + GTEST_DISALLOW_ASSIGN_(MockSpec); + }; // class MockSpec + ++// Wrapper type for generically holding an ordinary value or lvalue reference. ++// If T is not a reference type, it must be copyable or movable. ++// ReferenceOrValueWrapper<T> is movable, and will also be copyable unless ++// T is a move-only value type (which means that it will always be copyable ++// if the current platform does not support move semantics). ++// ++// The primary template defines handling for values, but function header ++// comments describe the contract for the whole template (including ++// specializations). ++template <typename T> ++class ReferenceOrValueWrapper { ++ public: ++ // Constructs a wrapper from the given value/reference. ++ explicit ReferenceOrValueWrapper(T value) ++ : value_(GTEST_MOVE_(value)) {} ++ ++ // Unwraps and returns the underlying value/reference, exactly as ++ // originally passed. The behavior of calling this more than once on ++ // the same object is unspecified. ++ T Unwrap() { ++ return GTEST_MOVE_(value_); ++ } ++ ++ // Provides nondestructive access to the underlying value/reference. ++ // Always returns a const reference (more precisely, ++ // const RemoveReference<T>&). The behavior of calling this after ++ // calling Unwrap on the same object is unspecified. ++ const T& Peek() const { ++ return value_; ++ } ++ ++ private: ++ T value_; ++}; ++ ++// Specialization for lvalue reference types. See primary template ++// for documentation. ++template <typename T> ++class ReferenceOrValueWrapper<T&> { ++ public: ++ // Workaround for debatable pass-by-reference lint warning (c-library-team ++ // policy precludes NOLINT in this context) ++ typedef T& reference; ++ explicit ReferenceOrValueWrapper(reference ref) ++ : value_ptr_(&ref) {} ++ T& Unwrap() { return *value_ptr_; } ++ const T& Peek() const { return *value_ptr_; } ++ ++ private: ++ T* value_ptr_; ++}; ++ + // MSVC warns about using 'this' in base member initializer list, so + // we need to temporarily disable the warning. We have to do it for + // the entire class to suppress the warning, even though it's about +@@ -1320,23 +1372,16 @@ class UntypedActionResultHolderBase { + template <typename T> + class ActionResultHolder : public UntypedActionResultHolderBase { + public: +- explicit ActionResultHolder(T a_value) : value_(a_value) {} +- +- // The compiler-generated copy constructor and assignment operator +- // are exactly what we need, so we don't need to define them. +- +- // Returns the held value and deletes this object. +- T GetValueAndDelete() const { +- T retval(value_); +- delete this; +- return retval; ++ // Returns the held value. Must not be called more than once. ++ T Unwrap() { ++ return result_.Unwrap(); + } + + // Prints the held value as an action's result to os. + virtual void PrintAsActionResult(::std::ostream* os) const { + *os << "\n Returns: "; + // T may be a reference type, so we don't use UniversalPrint(). +- UniversalPrinter<T>::Print(value_, os); ++ UniversalPrinter<T>::Print(result_.Peek(), os); + } + + // Performs the given mock function's default action and returns the +@@ -1346,8 +1391,8 @@ class ActionResultHolder : public UntypedActionResultH + const FunctionMockerBase<F>* func_mocker, + const typename Function<F>::ArgumentTuple& args, + const string& call_description) { +- return new ActionResultHolder( +- func_mocker->PerformDefaultAction(args, call_description)); ++ return new ActionResultHolder(Wrapper( ++ func_mocker->PerformDefaultAction(args, call_description))); + } + + // Performs the given action and returns the result in a new-ed +@@ -1356,42 +1401,52 @@ class ActionResultHolder : public UntypedActionResultH + static ActionResultHolder* + PerformAction(const Action<F>& action, + const typename Function<F>::ArgumentTuple& args) { +- return new ActionResultHolder(action.Perform(args)); ++ return new ActionResultHolder(Wrapper(action.Perform(args))); + } + + private: +- T value_; ++ typedef ReferenceOrValueWrapper<T> Wrapper; + +- // T could be a reference type, so = isn't supported. +- GTEST_DISALLOW_ASSIGN_(ActionResultHolder); ++ explicit ActionResultHolder(Wrapper result) ++ : result_(GTEST_MOVE_(result)) {} ++ ++ Wrapper result_; ++ ++ GTEST_DISALLOW_COPY_AND_ASSIGN_(ActionResultHolder); + }; + + // Specialization for T = void. + template <> + class ActionResultHolder<void> : public UntypedActionResultHolderBase { + public: +- void GetValueAndDelete() const { delete this; } ++ void Unwrap() { } + + virtual void PrintAsActionResult(::std::ostream* /* os */) const {} + +- // Performs the given mock function's default action and returns NULL; ++ // Performs the given mock function's default action and returns ownership ++ // of an empty ActionResultHolder*. + template <typename F> + static ActionResultHolder* PerformDefaultAction( + const FunctionMockerBase<F>* func_mocker, + const typename Function<F>::ArgumentTuple& args, + const string& call_description) { + func_mocker->PerformDefaultAction(args, call_description); +- return NULL; ++ return new ActionResultHolder; + } + +- // Performs the given action and returns NULL. ++ // Performs the given action and returns ownership of an empty ++ // ActionResultHolder*. + template <typename F> + static ActionResultHolder* PerformAction( + const Action<F>& action, + const typename Function<F>::ArgumentTuple& args) { + action.Perform(args); +- return NULL; ++ return new ActionResultHolder; + } ++ ++ private: ++ ActionResultHolder() {} ++ GTEST_DISALLOW_COPY_AND_ASSIGN_(ActionResultHolder); + }; + + // The base of the function mocker class for the given function type. +@@ -1526,8 +1581,9 @@ class FunctionMockerBase : public UntypedFunctionMocke + // threads concurrently. + Result InvokeWith(const ArgumentTuple& args) + GTEST_LOCK_EXCLUDED_(g_gmock_mutex) { +- return static_cast<const ResultHolder*>( +- this->UntypedInvokeWith(&args))->GetValueAndDelete(); ++ scoped_ptr<ResultHolder> holder( ++ DownCast_<ResultHolder*>(this->UntypedInvokeWith(&args))); ++ return holder->Unwrap(); + } + + // Adds and returns a default action spec for this mock function. diff --git a/net-im/licq/files/patch-3rdparty_gmock_include_gmock_internal_gmock-internal-utils.h b/net-im/licq/files/patch-3rdparty_gmock_include_gmock_internal_gmock-internal-utils.h new file mode 100644 index 000000000000..5d65f7c9fb05 --- /dev/null +++ b/net-im/licq/files/patch-3rdparty_gmock_include_gmock_internal_gmock-internal-utils.h @@ -0,0 +1,38 @@ +--- 3rdparty/gmock/include/gmock/internal/gmock-internal-utils.h.orig 2014-10-22 20:07:20 UTC ++++ 3rdparty/gmock/include/gmock/internal/gmock-internal-utils.h +@@ -361,17 +361,30 @@ template <typename T> struct DecayArray<T[]> { + typedef const T* type; + }; + +-// Invalid<T>() returns an invalid value of type T. This is useful ++// Disable MSVC warnings for infinite recursion, since in this case the ++// the recursion is unreachable. ++#ifdef _MSC_VER ++# pragma warning(push) ++# pragma warning(disable:4717) ++#endif ++ ++// Invalid<T>() is usable as an expression of type T, but will terminate ++// the program with an assertion failure if actually run. This is useful + // when a value of type T is needed for compilation, but the statement + // will not really be executed (or we don't care if the statement + // crashes). + template <typename T> + inline T Invalid() { +- return const_cast<typename remove_reference<T>::type&>( +- *static_cast<volatile typename remove_reference<T>::type*>(NULL)); ++ Assert(false, "", -1, "Internal error: attempt to return invalid value"); ++ // This statement is unreachable, and would never terminate even if it ++ // could be reached. It is provided only to placate compiler warnings ++ // about missing return statements. ++ return Invalid<T>(); + } +-template <> +-inline void Invalid<void>() {} ++ ++#ifdef _MSC_VER ++# pragma warning(pop) ++#endif + + // Given a raw type (i.e. having no top-level reference or const + // modifier) RawContainer that's either an STL-style container or a diff --git a/net-im/licq/files/patch-3rdparty_gmock_src_gmock-spec-builders.cc b/net-im/licq/files/patch-3rdparty_gmock_src_gmock-spec-builders.cc new file mode 100644 index 000000000000..ce84096cad90 --- /dev/null +++ b/net-im/licq/files/patch-3rdparty_gmock_src_gmock-spec-builders.cc @@ -0,0 +1,29 @@ +--- 3rdparty/gmock/src/gmock-spec-builders.cc.orig 2014-10-22 20:07:20 UTC ++++ 3rdparty/gmock/src/gmock-spec-builders.cc +@@ -325,7 +325,7 @@ const char* UntypedFunctionMockerBase::Name() const + // Calculates the result of invoking this mock function with the given + // arguments, prints it, and returns it. The caller is responsible + // for deleting the result. +-const UntypedActionResultHolderBase* ++UntypedActionResultHolderBase* + UntypedFunctionMockerBase::UntypedInvokeWith(const void* const untyped_args) + GTEST_LOCK_EXCLUDED_(g_gmock_mutex) { + if (untyped_expectations_.size() == 0) { +@@ -363,7 +363,7 @@ UntypedFunctionMockerBase::UntypedInvokeWith(const voi + this->UntypedDescribeUninterestingCall(untyped_args, &ss); + + // Calculates the function result. +- const UntypedActionResultHolderBase* const result = ++ UntypedActionResultHolderBase* const result = + this->UntypedPerformDefaultAction(untyped_args, ss.str()); + + // Prints the function result. +@@ -410,7 +410,7 @@ UntypedFunctionMockerBase::UntypedInvokeWith(const voi + untyped_expectation->DescribeLocationTo(&loc); + } + +- const UntypedActionResultHolderBase* const result = ++ UntypedActionResultHolderBase* const result = + untyped_action == NULL ? + this->UntypedPerformDefaultAction(untyped_args, ss.str()) : + this->UntypedPerformAction(untyped_action, untyped_args); diff --git a/net-im/licq/files/patch-3rdparty_gtest_include_gtest_internal_gtest-port.h b/net-im/licq/files/patch-3rdparty_gtest_include_gtest_internal_gtest-port.h new file mode 100644 index 000000000000..2c1a502110fc --- /dev/null +++ b/net-im/licq/files/patch-3rdparty_gtest_include_gtest_internal_gtest-port.h @@ -0,0 +1,34 @@ +--- 3rdparty/gtest/include/gtest/internal/gtest-port.h.orig 2014-10-22 20:07:20 UTC ++++ 3rdparty/gtest/include/gtest/internal/gtest-port.h +@@ -141,6 +141,10 @@ + // GTEST_DISALLOW_COPY_AND_ASSIGN_ - disables copy ctor and operator=. + // GTEST_MUST_USE_RESULT_ - declares that a function's result must be used. + // ++// C++11 feature wrappers: ++// ++// GTEST_MOVE_ - portability wrapper for std::move. ++// + // Synchronization: + // Mutex, MutexLock, ThreadLocal, GetThreadCount() + // - synchronization primitives. +@@ -211,6 +215,7 @@ + #include <iostream> // NOLINT + #include <sstream> // NOLINT + #include <string> // NOLINT ++#include <utility> + + #define GTEST_DEV_EMAIL_ "googletestframework@@googlegroups.com" + #define GTEST_FLAG_PREFIX_ "gtest_" +@@ -737,6 +742,12 @@ using ::std::tuple_size; + #else + # define GTEST_MUST_USE_RESULT_ + #endif // __GNUC__ && (GTEST_GCC_VER_ >= 30400) && !COMPILER_ICC ++ ++#if GTEST_LANG_CXX11 ++# define GTEST_MOVE_(x) ::std::move(x) // NOLINT ++#else ++# define GTEST_MOVE_(x) x ++#endif + + // Determine whether the compiler supports Microsoft's Structured Exception + // Handling. This is supported by several Windows compilers but generally
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202106061814.156IEWCs051362>