Skip site navigation (1)Skip section navigation (2)
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>