Remove multiple inheritance from "unintesting call" mock classes.refs/pull/1557/head

Internal CL 156157936, which was published in commit
fe402c27790ff1cc9a7e17c5d0aea4ebe7fd8a71, introduced undefined behavior
by casting a base class (internal::{Naggy,Nice,Strict}Base<MockClass>,
using the curiously recurring template pattern) pointer to a derived
class ({Naggy,Nice,Strict}Mock<MockClass>), in the base class'
constructor. At that point, the object isn't guaranteed to have taken on
the shape of the derived class, and casting is undefined behavior.

The undefined behavior was caught by Chrome's CFI build bot [1], and
prevents rolling googletest past that commit / CL.

This commit simplifies the {Naggy,Nice,Strict}Mock class hierarchy in
a way that removes the undefined behavior.

[1] https://www.chromium.org/developers/testing/control-flow-integrity

1 files changed, 26 insertions, 44 deletions

diff --git a/googlemock/include/gmock/gmock-generated-nice-strict.h.pump b/googlemock/include/gmock/gmock-generated-nice-strict.h.pump
index 378c40f..2e50e98 100644
--- a/googlemock/include/gmock/gmock-generated-nice-strict.h.pump
+++ b/googlemock/include/gmock/gmock-generated-nice-strict.h.pump
@@ -83,79 +83,61 @@ $var method=[[$if kind==0 [[AllowUninterestingCalls]]
              $elif kind==1 [[WarnUninterestingCalls]]
              $else [[FailUninterestingCalls]]]]
 
-namespace internal {
-
-// $clazz[[]]Base serves as a mix-in to establish the "uninteresting call"
-// behavior for $clazz on construction. It accomplishes this via CRTP to get
-// access to the derived MockClass.
 template <class MockClass>
-class $clazz[[]]Base {
- protected:
-  $clazz[[]]Base();
-
-  ~$clazz[[]]Base();
-};
-
-}  // namespace internal
-
-template <class MockClass>
-class $clazz : public MockClass, public internal::$clazz[[]]Base<MockClass> {
+class $clazz : public MockClass {
  public:
-  $clazz() : MockClass() {}
+  $clazz() : MockClass() {
+    ::testing::Mock::$method(
+        internal::ImplicitCast_<MockClass*>(this));
+  }
 
 #if GTEST_LANG_CXX11
-  // Ideally, we would inherit base class's constructors through a using
-  // declaration, which would preserve their visibility. However, many existing
-  // tests rely on the fact that current implementation reexports protected
-  // constructors as public. These tests would need to be cleaned up first.
-
   // Single argument constructor is special-cased so that it can be
   // made explicit.
   template <typename A>
-  explicit $clazz(A&& arg) : MockClass(std::forward<A>(arg)) {}
+  explicit $clazz(A&& arg) : MockClass(std::forward<A>(arg)) {
+    ::testing::Mock::$method(
+        internal::ImplicitCast_<MockClass*>(this));
+  }
 
   template <typename A1, typename A2, typename... An>
   $clazz(A1&& arg1, A2&& arg2, An&&... args)
       : MockClass(std::forward<A1>(arg1), std::forward<A2>(arg2),
-                  std::forward<An>(args)...) {}
+                  std::forward<An>(args)...) {
+    ::testing::Mock::$method(
+        internal::ImplicitCast_<MockClass*>(this));
+  }
 #else
   // C++98 doesn't have variadic templates, so we have to define one
   // for each arity.
   template <typename A1>
-  explicit $clazz(const A1& a1) : MockClass(a1) {}
+  explicit $clazz(const A1& a1) : MockClass(a1) {
+    ::testing::Mock::$method(
+        internal::ImplicitCast_<MockClass*>(this));
+  }
 
 $range i 2..n
 $for i [[
 $range j 1..i
   template <$for j, [[typename A$j]]>
-  $clazz($for j, [[const A$j& a$j]]) : MockClass($for j, [[a$j]]) {}
+  $clazz($for j, [[const A$j& a$j]]) : MockClass($for j, [[a$j]]) {
+    ::testing::Mock::$method(
+        internal::ImplicitCast_<MockClass*>(this));
+  }
 
 
 ]]
 #endif  // GTEST_LANG_CXX11
 
+  ~$clazz() {
+    ::testing::Mock::UnregisterCallReaction(
+        internal::ImplicitCast_<MockClass*>(this));
+  }
+
  private:
   GTEST_DISALLOW_COPY_AND_ASSIGN_($clazz);
 };
 
-namespace internal {
-
-template <typename MockClass>
-$clazz[[]]Base<MockClass>::$clazz[[]]Base() {
-  ::testing::Mock::$method(
-      internal::ImplicitCast_<MockClass*>(
-          static_cast<$clazz<MockClass> *>(this)));
-}
-
-template <typename MockClass>
-$clazz[[]]Base<MockClass>::~$clazz[[]]Base() {
-  ::testing::Mock::UnregisterCallReaction(
-      internal::ImplicitCast_<MockClass*>(
-          static_cast<$clazz<MockClass>*>(this)));
-}
-
-}  // namespace internal
-
 ]]
 
 // The following specializations catch some (relatively more common)