Force a conservative allocation for pointers to methods in Condition objects.

In order for Condition to work on Microsoft platforms, it has to store pointers to methods that are larger than we usually expect. MSVC pointers to methods from class hierarchies that employ multiple inheritance or virtual inheritance are strictly larger than pointers to methods in class hierarchies that only employ single inheritance.

This change introduces an opaque declaration of a class, which is not fulfilled. This declaration is used to calculate the size of the Condition method pointer allocation. Because the declaration is of unspecified inheritance, the compiler is forced to use a conservatively large allocation, which will thereby accommodate all method pointer sizes.

Because the `method_` and `function_` callbacks are only populated in mutually exclusive conditions, they can be allowed to take up the same space in the Condition object. This change combines the `method_` and `function_` fields and renames the new field to `callback_`. The constructor logic is updated to reflect the new field.

PiperOrigin-RevId: 486701312
Change-Id: If06832cc26f27d91e295183e44dc29440af5f9db

1 files changed, 73 insertions, 34 deletions

diff --git a/absl/synchronization/mutex.h b/absl/synchronization/mutex.h
index 8694bb75..54ee703a 100644
--- a/absl/synchronization/mutex.h
+++ b/absl/synchronization/mutex.h
@@ -60,6 +60,8 @@
 
 #include <atomic>
 #include <cstdint>
+#include <cstring>
+#include <iterator>
 #include <string>
 
 #include "absl/base/const_init.h"
@@ -612,12 +614,12 @@ class ABSL_SCOPED_LOCKABLE WriterMutexLock {
 // Condition
 // -----------------------------------------------------------------------------
 //
-// As noted above, `Mutex` contains a number of member functions which take a
-// `Condition` as an argument; clients can wait for conditions to become `true`
-// before attempting to acquire the mutex. These sections are known as
-// "condition critical" sections. To use a `Condition`, you simply need to
-// construct it, and use within an appropriate `Mutex` member function;
-// everything else in the `Condition` class is an implementation detail.
+// `Mutex` contains a number of member functions which take a `Condition` as an
+// argument; clients can wait for conditions to become `true` before attempting
+// to acquire the mutex. These sections are known as "condition critical"
+// sections. To use a `Condition`, you simply need to construct it, and use
+// within an appropriate `Mutex` member function; everything else in the
+// `Condition` class is an implementation detail.
 //
 // A `Condition` is specified as a function pointer which returns a boolean.
 // `Condition` functions should be pure functions -- their results should depend
@@ -742,22 +744,55 @@ class Condition {
   static bool GuaranteedEqual(const Condition *a, const Condition *b);
 
  private:
-  typedef bool (*InternalFunctionType)(void * arg);
-  typedef bool (Condition::*InternalMethodType)();
-  typedef bool (*InternalMethodCallerType)(void * arg,
-                                           InternalMethodType internal_method);
-
-  bool (*eval_)(const Condition*);  // Actual evaluator
-  InternalFunctionType function_;   // function taking pointer returning bool
-  InternalMethodType method_;       // method returning bool
-  void *arg_;                       // arg of function_ or object of method_
-
-  Condition();        // null constructor used only to create kTrue
+  // Sizing an allocation for a method pointer can be subtle. In the Itanium
+  // specifications, a method pointer has a predictable, uniform size. On the
+  // other hand, MSVC ABI, method pointer sizes vary based on the
+  // inheritance of the class. Specifically, method pointers from classes with
+  // multiple inheritance are bigger than those of classes with single
+  // inheritance. Other variations also exist.
+
+  // A good way to allocate enough space for *any* pointer in these ABIs is to
+  // employ a class declaration with no definition. Because the inheritance
+  // structure is not available for this declaration, the compiler must
+  // assume, conservatively, that its method pointers have the largest possible
+  // size.
+  class OpaqueClass;
+  using ConservativeMethodPointer = bool (OpaqueClass::*)();
+  static_assert(sizeof(bool(OpaqueClass::*)()) >= sizeof(bool (*)(void *)),
+                "Unsupported platform.");
+
+  // Allocation for a function pointer or method pointer.
+  // The {0} initializer ensures that all unused bytes of this buffer are
+  // always zeroed out.  This is necessary, because GuaranteedEqual() compares
+  // all of the bytes, unaware of which bytes are relevant to a given `eval_`.
+  char callback_[sizeof(ConservativeMethodPointer)] = {0};
+
+  // Function with which to evaluate callbacks and/or arguments.
+  bool (*eval_)(const Condition*);
+
+  // Either an argument for a function call or an object for a method call.
+  void *arg_;
 
   // Various functions eval_ can point to:
   static bool CallVoidPtrFunction(const Condition*);
   template <typename T> static bool CastAndCallFunction(const Condition* c);
   template <typename T> static bool CastAndCallMethod(const Condition* c);
+
+  // Helper methods for storing, validating, and reading callback arguments.
+  template <typename T>
+  inline void StoreCallback(T callback) {
+    static_assert(
+        sizeof(callback) <= sizeof(callback_),
+        "An overlarge pointer was passed as a callback to Condition.");
+    std::memcpy(callback_, &callback, sizeof(callback));
+  }
+
+  template <typename T>
+  inline void ReadCallback(T *callback) const {
+    std::memcpy(callback, callback_, sizeof(*callback));
+  }
+
+  Condition();        // null constructor used only to create kTrue
 };
 
 // -----------------------------------------------------------------------------
@@ -949,44 +984,48 @@ inline CondVar::CondVar() : cv_(0) {}
 // static
 template <typename T>
 bool Condition::CastAndCallMethod(const Condition *c) {
-  typedef bool (T::*MemberType)();
-  MemberType rm = reinterpret_cast<MemberType>(c->method_);
-  T *x = static_cast<T *>(c->arg_);
-  return (x->*rm)();
+  T *object = static_cast<T *>(c->arg_);
+  bool (T::*method_pointer)();
+  c->ReadCallback(&method_pointer);
+  return (object->*method_pointer)();
 }
 
 // static
 template <typename T>
 bool Condition::CastAndCallFunction(const Condition *c) {
-  typedef bool (*FuncType)(T *);
-  FuncType fn = reinterpret_cast<FuncType>(c->function_);
-  T *x = static_cast<T *>(c->arg_);
-  return (*fn)(x);
+  bool (*function)(T *);
+  c->ReadCallback(&function);
+  T *argument = static_cast<T *>(c->arg_);
+  return (*function)(argument);
 }
 
 template <typename T>
 inline Condition::Condition(bool (*func)(T *), T *arg)
     : eval_(&CastAndCallFunction<T>),
-      function_(reinterpret_cast<InternalFunctionType>(func)),
-      method_(nullptr),
-      arg_(const_cast<void *>(static_cast<const void *>(arg))) {}
+      arg_(const_cast<void *>(static_cast<const void *>(arg))) {
+  static_assert(sizeof(&func) <= sizeof(callback_),
+                "An overlarge function pointer was passed to Condition.");
+  StoreCallback(func);
+}
 
 template <typename T>
 inline Condition::Condition(T *object,
                             bool (absl::internal::identity<T>::type::*method)())
     : eval_(&CastAndCallMethod<T>),
-      function_(nullptr),
-      method_(reinterpret_cast<InternalMethodType>(method)),
-      arg_(object) {}
+      arg_(object) {
+  static_assert(sizeof(&method) <= sizeof(callback_),
+                "An overlarge method pointer was passed to Condition.");
+  StoreCallback(method);
+}
 
 template <typename T>
 inline Condition::Condition(const T *object,
                             bool (absl::internal::identity<T>::type::*method)()
                                 const)
     : eval_(&CastAndCallMethod<T>),
-      function_(nullptr),
-      method_(reinterpret_cast<InternalMethodType>(method)),
-      arg_(reinterpret_cast<void *>(const_cast<T *>(object))) {}
+      arg_(reinterpret_cast<void *>(const_cast<T *>(object))) {
+  StoreCallback(method);
+}
 
 // Register hooks for profiling support.
 //