From 4ed8e46f1b2e13da2dd158159b27d2ba0e65ae15 Mon Sep 17 00:00:00 2001
From: Abseil Team <absl-team@google.com>
Date: Mon, 7 Nov 2022 10:31:02 -0800
Subject: 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
---
 absl/synchronization/mutex.cc | 38 ++++++++++++++++++++++++--------------
 1 file changed, 24 insertions(+), 14 deletions(-)

(limited to 'absl/synchronization/mutex.cc')

diff --git a/absl/synchronization/mutex.cc b/absl/synchronization/mutex.cc
index b0f412bf..ff18df5d 100644
--- a/absl/synchronization/mutex.cc
+++ b/absl/synchronization/mutex.cc
@@ -37,6 +37,8 @@
 #include <atomic>
 #include <cinttypes>
 #include <cstddef>
+#include <cstring>
+#include <iterator>
 #include <thread>  // NOLINT(build/c++11)
 
 #include "absl/base/attributes.h"
@@ -2780,25 +2782,32 @@ static bool Dereference(void *arg) {
   return *(static_cast<bool *>(arg));
 }
 
-Condition::Condition() {}   // null constructor, used for kTrue only
+Condition::Condition() = default;  // null constructor, used for kTrue only
 const Condition Condition::kTrue;
 
 Condition::Condition(bool (*func)(void *), void *arg)
     : eval_(&CallVoidPtrFunction),
-      function_(func),
-      method_(nullptr),
-      arg_(arg) {}
+      arg_(arg) {
+  static_assert(sizeof(&func) <= sizeof(callback_),
+                "An overlarge function pointer passed to Condition.");
+  StoreCallback(func);
+}
 
 bool Condition::CallVoidPtrFunction(const Condition *c) {
-  return (*c->function_)(c->arg_);
+  using FunctionPointer = bool (*)(void *);
+  FunctionPointer function_pointer;
+  std::memcpy(&function_pointer, c->callback_, sizeof(function_pointer));
+  return (*function_pointer)(c->arg_);
 }
 
 Condition::Condition(const bool *cond)
     : eval_(CallVoidPtrFunction),
-      function_(Dereference),
-      method_(nullptr),
       // const_cast is safe since Dereference does not modify arg
-      arg_(const_cast<bool *>(cond)) {}
+      arg_(const_cast<bool *>(cond)) {
+  using FunctionPointer = bool (*)(void *);
+  const FunctionPointer dereference = Dereference;
+  StoreCallback(dereference);
+}
 
 bool Condition::Eval() const {
   // eval_ == null for kTrue
@@ -2806,14 +2815,15 @@ bool Condition::Eval() const {
 }
 
 bool Condition::GuaranteedEqual(const Condition *a, const Condition *b) {
-  if (a == nullptr) {
+  // kTrue logic.
+  if (a == nullptr || a->eval_ == nullptr) {
     return b == nullptr || b->eval_ == nullptr;
+  }else  if (b == nullptr || b->eval_ == nullptr) {
+    return false;
   }
-  if (b == nullptr || b->eval_ == nullptr) {
-    return a->eval_ == nullptr;
-  }
-  return a->eval_ == b->eval_ && a->function_ == b->function_ &&
-         a->arg_ == b->arg_ && a->method_ == b->method_;
+  // Check equality of the representative fields.
+  return a->eval_ == b->eval_ && a->arg_ == b->arg_ &&
+         !memcmp(a->callback_, b->callback_, sizeof(ConservativeMethodPointer));
 }
 
 ABSL_NAMESPACE_END
-- 
cgit v1.2.3