Bug 1448494 - NonDereferenceable<T> wraps a T* and prevents dereferencing ops - r=froydnj

NonDereferenceable denotes the intent that a pointer will (most likely) not be
dereferenced, but its numeric value may be used for e.g. logging purposes.

Dereferencing operations are explicitly disabled to avoid unintentional misuses.
Casting is still possible between related types (same as with raw pointers),
but pointers stay safely stored inside NonDereferenceable objects. These casts
do not trigger `clang++ -fsanitize=vptr` errors.

MozReview-Commit-ID: 5885pB7hSFR

--HG--
extra : rebase_source : 3c4011da64d84f1b19991742b76bafbffa90d590

mfbt/NonDereferenceable.h

@@ -0,0 +1,139 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#ifndef mozilla_NonDereferenceable_h
+#define mozilla_NonDereferenceable_h
+
+/* A pointer wrapper indicating that the pointer should not be dereferenced. */
+
+#include "mozilla/Attributes.h"
+#include "mozilla/TypeTraits.h"
+
+#include <cstdint>
+
+// Macro indicating that a function manipulates a pointer that will not be
+// dereferenced, and therefore there is no need to check the object.
+#if defined(__clang__)
+#define NO_POINTEE_CHECKS __attribute__((no_sanitize("vptr")))
+#else
+#define NO_POINTEE_CHECKS /* nothing */
+#endif
+
+namespace mozilla {
+
+// NonDereferenceable<T> wraps a raw pointer value of type T*, but prevents
+// dereferencing.
+//
+// The main use case is for pointers that referencing memory that may not
+// contain a valid object, either because the object has already been freed, or
+// is under active construction or destruction (and hence parts of it may be
+// uninitialized or destructed.)
+// Such a pointer may still be useful, e.g., for its numeric value for
+// logging/debugging purposes, which may be accessed with `value()`.
+// Using NonDereferenceable with such pointers will make this intent clearer,
+// and prevent misuses.
+//
+// Note that NonDereferenceable is only a wrapper and is NOT an owning pointer,
+// i.e., it will not release/free the object.
+//
+// NonDereferenceable allows conversions between compatible pointer types, e.g.,
+// to navigate a class hierarchy and identify parent/sub-objects. Note that the
+// converted pointers stay safely NonDereferenceable.
+//
+// Use of NonDereferenceable is required to avoid errors from sanitization tools
+// like `clang++ -fsanitize=vptr`, and should prevent false positives while
+// pointers are manipulated within NonDereferenceable objects.
+//
+template<typename T>
+class NonDereferenceable
+{
+public:
+  // Default construction with a null value.
+  NonDereferenceable()
+    : mPtr(nullptr)
+  {
+  }
+
+  // Default copy construction and assignment.
+  NO_POINTEE_CHECKS
+  NonDereferenceable(const NonDereferenceable&) = default;
+  NO_POINTEE_CHECKS
+  NonDereferenceable<T>& operator=(const NonDereferenceable&) = default;
+  // No move operations, as we're only carrying a non-owning pointer, so
+  // copying is most efficient.
+
+  // Construct/assign from a T* raw pointer.
+  // A raw pointer should usually point at a valid object, however we want to
+  // leave the ability to the user to create a NonDereferenceable from any
+  // pointer. Also, strictly speaking, in a constructor or destructor, `this`
+  // points at an object still being constructed or already partially
+  // destructed, which some very sensitive sanitizers could complain about.
+  NO_POINTEE_CHECKS
+  explicit NonDereferenceable(T* aPtr)
+    : mPtr(aPtr)
+  {
+  }
+  NO_POINTEE_CHECKS
+  NonDereferenceable& operator=(T* aPtr)
+  {
+    mPtr = aPtr;
+    return *this;
+  }
+
+  // Construct/assign from a compatible pointer type.
+  template<typename U>
+  NO_POINTEE_CHECKS explicit NonDereferenceable(U* aOther)
+    : mPtr(static_cast<T*>(aOther))
+  {
+  }
+  template<typename U>
+  NO_POINTEE_CHECKS NonDereferenceable& operator=(U* aOther)
+  {
+    mPtr = static_cast<T*>(aOther);
+    return *this;
+  }
+
+  // Construct/assign from a NonDereferenceable with a compatible pointer type.
+  template<typename U>
+  NO_POINTEE_CHECKS MOZ_IMPLICIT
+  NonDereferenceable(const NonDereferenceable<U>& aOther)
+    : mPtr(static_cast<T*>(aOther.mPtr))
+  {
+  }
+  template<typename U>
+  NO_POINTEE_CHECKS NonDereferenceable& operator=(
+    const NonDereferenceable<U>& aOther)
+  {
+    mPtr = static_cast<T*>(aOther.mPtr);
+    return *this;
+  }
+
+  // Explicitly disallow dereference operators, so that compiler errors point
+  // at these lines:
+  T& operator*() = delete;  // Cannot dereference NonDereferenceable!
+  T* operator->() = delete; // Cannot dereference NonDereferenceable!
+
+  // Null check.
+  NO_POINTEE_CHECKS
+  explicit operator bool() const { return !!mPtr; }
+
+  // Extract the pointer value, untyped.
+  NO_POINTEE_CHECKS
+  uintptr_t value() const { return reinterpret_cast<uintptr_t>(mPtr); }
+
+private:
+  // Let other NonDereferenceable templates access mPtr, to permit construction/
+  // assignment from compatible pointer types.
+  template<typename> friend class NonDereferenceable;
+
+  T* MOZ_NON_OWNING_REF mPtr;
+};
+
+} // namespace mozilla
+
+#undef NO_POINTEE_CHECKS
+
+#endif /* mozilla_NonDereferenceable_h */

mfbt/moz.build

@@ -58,6 +58,7 @@ EXPORTS.mozilla = [
     'MemoryChecking.h',
     'MemoryReporting.h',
     'Move.h',
+    'NonDereferenceable.h',
     'NotNull.h',
     'NullPtr.h',
     'Opaque.h',

mfbt/tests/TestNonDereferenceable.cpp

@@ -0,0 +1,194 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "mozilla/NonDereferenceable.h"
+
+#include "mozilla/Assertions.h"
+#include "mozilla/Move.h"
+
+using mozilla::Move;
+using mozilla::NonDereferenceable;
+
+#define CHECK MOZ_RELEASE_ASSERT
+
+void
+TestNonDereferenceableSimple()
+{
+  // Default construction.
+  NonDereferenceable<int> nd0;
+  CHECK(!nd0);
+  CHECK(!nd0.value());
+
+  int i = 1;
+  int i2 = 2;
+
+  // Construction with pointer.
+  NonDereferenceable<int> nd1(&i);
+  CHECK(!!nd1);
+  CHECK(nd1.value() == reinterpret_cast<uintptr_t>(&i));
+
+  // Assignment with pointer.
+  nd1 = &i2;
+  CHECK(nd1.value() == reinterpret_cast<uintptr_t>(&i2));
+
+  // Copy-construction.
+  NonDereferenceable<int> nd2(nd1);
+  CHECK(nd2.value() == reinterpret_cast<uintptr_t>(&i2));
+
+  // Copy-assignment.
+  nd2 = nd0;
+  CHECK(!nd2.value());
+
+  // Move-construction.
+  NonDereferenceable<int> nd3{ NonDereferenceable<int>(&i) };
+  CHECK(nd3.value() == reinterpret_cast<uintptr_t>(&i));
+
+  // Move-assignment.
+  nd3 = Move(nd1);
+  CHECK(nd3.value() == reinterpret_cast<uintptr_t>(&i2));
+  // Note: Not testing nd1's value because we don't want to assume what state
+  // it is left in after move. But at least it should be reusable:
+  nd1 = &i;
+  CHECK(nd1.value() == reinterpret_cast<uintptr_t>(&i));
+}
+
+void
+TestNonDereferenceableHierarchy()
+{
+  struct Base1
+  {
+    // Member variable, to make sure Base1 is not empty.
+    int x1;
+  };
+  struct Base2
+  {
+    int x2;
+  };
+  struct Derived
+    : Base1
+    , Base2
+  {
+  };
+
+  Derived d;
+
+  // Construct NonDereferenceable from raw pointer.
+  NonDereferenceable<Derived> ndd = NonDereferenceable<Derived>(&d);
+  CHECK(ndd);
+  CHECK(ndd.value() == reinterpret_cast<uintptr_t>(&d));
+
+  // Cast Derived to Base1.
+  NonDereferenceable<Base1> ndb1 = ndd;
+  CHECK(ndb1);
+  CHECK(ndb1.value() == reinterpret_cast<uintptr_t>(static_cast<Base1*>(&d)));
+
+  // Cast Base1 back to Derived.
+  NonDereferenceable<Derived> nddb1 = ndb1;
+  CHECK(nddb1.value() == reinterpret_cast<uintptr_t>(&d));
+
+  // Cast Derived to Base2.
+  NonDereferenceable<Base2> ndb2 = ndd;
+  CHECK(ndb2);
+  CHECK(ndb2.value() == reinterpret_cast<uintptr_t>(static_cast<Base2*>(&d)));
+  // Sanity check that Base2 should be offset from the start of Derived.
+  CHECK(ndb2.value() != ndd.value());
+
+  // Cast Base2 back to Derived.
+  NonDereferenceable<Derived> nddb2 = ndb2;
+  CHECK(nddb2.value() == reinterpret_cast<uintptr_t>(&d));
+
+  // Note that it's not possible to jump between bases, as they're not obviously
+  // related, i.e.: `NonDereferenceable<Base2> ndb22 = ndb1;` doesn't compile.
+  // However it's possible to explicitly navigate through the derived object:
+  NonDereferenceable<Base2> ndb22 = NonDereferenceable<Derived>(ndb1);
+  CHECK(ndb22.value() == reinterpret_cast<uintptr_t>(static_cast<Base2*>(&d)));
+
+  // Handling nullptr; should stay nullptr even for offset bases.
+  ndd = nullptr;
+  CHECK(!ndd);
+  CHECK(!ndd.value());
+  ndb1 = ndd;
+  CHECK(!ndb1);
+  CHECK(!ndb1.value());
+  ndb2 = ndd;
+  CHECK(!ndb2);
+  CHECK(!ndb2.value());
+  nddb2 = ndb2;
+  CHECK(!nddb2);
+  CHECK(!nddb2.value());
+}
+
+template<typename T, size_t Index>
+struct CRTPBase
+{
+  // Convert `this` from `CRTPBase*` to `T*` while construction is still in
+  // progress; normally UBSan -fsanitize=vptr would catch this, but using
+  // NonDereferenceable should keep UBSan happy.
+  CRTPBase()
+    : mDerived(this)
+  {
+  }
+  NonDereferenceable<T> mDerived;
+};
+
+void
+TestNonDereferenceableCRTP()
+{
+  struct Derived
+    : CRTPBase<Derived, 1>
+    , CRTPBase<Derived, 2>
+  {
+  };
+  using Base1 = Derived::CRTPBase<Derived, 1>;
+  using Base2 = Derived::CRTPBase<Derived, 2>;
+
+  Derived d;
+  // Verify that base constructors have correctly captured the address of the
+  // (at the time still incomplete) derived object.
+  CHECK(d.Base1::mDerived.value() == reinterpret_cast<uintptr_t>(&d));
+  CHECK(d.Base2::mDerived.value() == reinterpret_cast<uintptr_t>(&d));
+
+  // Construct NonDereferenceable from raw pointer.
+  NonDereferenceable<Derived> ndd = NonDereferenceable<Derived>(&d);
+  CHECK(ndd);
+  CHECK(ndd.value() == reinterpret_cast<uintptr_t>(&d));
+
+  // Cast Derived to Base1.
+  NonDereferenceable<Base1> ndb1 = ndd;
+  CHECK(ndb1);
+  CHECK(ndb1.value() == reinterpret_cast<uintptr_t>(static_cast<Base1*>(&d)));
+
+  // Cast Base1 back to Derived.
+  NonDereferenceable<Derived> nddb1 = ndb1;
+  CHECK(nddb1.value() == reinterpret_cast<uintptr_t>(&d));
+
+  // Cast Derived to Base2.
+  NonDereferenceable<Base2> ndb2 = ndd;
+  CHECK(ndb2);
+  CHECK(ndb2.value() == reinterpret_cast<uintptr_t>(static_cast<Base2*>(&d)));
+  // Sanity check that Base2 should be offset from the start of Derived.
+  CHECK(ndb2.value() != ndd.value());
+
+  // Cast Base2 back to Derived.
+  NonDereferenceable<Derived> nddb2 = ndb2;
+  CHECK(nddb2.value() == reinterpret_cast<uintptr_t>(&d));
+
+  // Note that it's not possible to jump between bases, as they're not obviously
+  // related, i.e.: `NonDereferenceable<Base2> ndb22 = ndb1;` doesn't compile.
+  // However it's possible to explicitly navigate through the derived object:
+  NonDereferenceable<Base2> ndb22 = NonDereferenceable<Derived>(ndb1);
+  CHECK(ndb22.value() == reinterpret_cast<uintptr_t>(static_cast<Base2*>(&d)));
+}
+
+int
+main()
+{
+  TestNonDereferenceableSimple();
+  TestNonDereferenceableHierarchy();
+  TestNonDereferenceableCRTP();
+
+  return 0;
+}

mfbt/tests/moz.build

@@ -38,6 +38,7 @@ CppUnitTests([
     'TestMacroForEach',
     'TestMathAlgorithms',
     'TestMaybe',
+    'TestNonDereferenceable',
     'TestNotNull',
     'TestPair',
     'TestRange',