Additional commentary and more verbose names

Author: nwf-msr

src/backend/backend.h

@@ -217,7 +217,8 @@ namespace snmalloc
      *
      * The template argument is the type of the metadata being allocated.  This
      * allows the backend to allocate different types of metadata in different
-     * places or with different policies.
+     * places or with different policies.  The default implementation, here,
+     * does not avail itself of this degree of freedom.
      */
     template<typename T>
     static capptr::Chunk<void>

src/backend/largebuddyrange.h

@@ -18,17 +18,45 @@ namespace snmalloc
   class BuddyChunkRep
   {
   public:
+    /*
+     * The values we store in our rbtree are the addresses of (combined spans
+     * of) chunks of the address space; as such, bits in (MIN_CHUNK_SIZE - 1)
+     * are unused and so the RED_BIT is packed therein.  However, in practice,
+     * these are not "just any" uintptr_t-s, but specifically the uintptr_t-s
+     * inside the Pagemap's MetaEntry structures.  As such, there are some
+     * additional bit-swizzling concerns; see set() and get() below.
+     */
     using Holder = uintptr_t;
     using Contents = uintptr_t;
 
-    static constexpr address_t RED_BIT = 2;
+    static constexpr address_t RED_BIT = 1 << 1;
+
+    static_assert(RED_BIT < MIN_CHUNK_SIZE);
+    static_assert(RED_BIT != MetaEntry::META_BOUNDARY_BIT);
+    static_assert(RED_BIT != MetaEntry::REMOTE_BACKEND_MARKER);
 
     static constexpr Contents null = 0;
 
     static void set(Holder* ptr, Contents r)
     {
       SNMALLOC_ASSERT((r & (MIN_CHUNK_SIZE - 1)) == 0);
-      // Preserve lower bits.
+      /*
+       * Preserve lower bits, claim as backend, and update contents of holder.
+       *
+       * This is excessive at present but no harder than being more precise
+       * while also being future-proof.  All that is strictly required would be
+       * to preserve META_BOUNDARY_BIT and RED_BIT in ->meta and to assert
+       * REMOTE_BACKEND_MARKER in ->remote_and_sizeclass (if it isn't already
+       * asserted).  However, we don't know which Holder* we have been given,
+       * nor do we know whether this Holder* is completely new (and so we are
+       * the first reasonable opportunity to assert REMOTE_BACKEND_MARKER) or
+       * recycled from the frontend, and so we preserve and assert more than
+       * strictly necessary.
+       *
+       * The use of `address_cast` below is a CHERI-ism; otherwise both `r` and
+       * `*ptr & ...` are plausibly provenance-carrying values and the compiler
+       * balks at the ambiguity.
+       */
       *ptr = r | address_cast(*ptr & (MIN_CHUNK_SIZE - 1)) |
         MetaEntry::REMOTE_BACKEND_MARKER;
     }

src/backend/metatypes.h

@@ -84,17 +84,20 @@ namespace snmalloc
   class sizeclass_t;
 
   /**
-   * Entry stored in the pagemap.
+   * Entry stored in the pagemap.  See docs/AddressSpace.md for the full
+   * MetaEntry lifecycle.
    */
   class MetaEntry
   {
     template<typename Pagemap>
     friend class BuddyChunkRep;
 
     /**
-     * The pointer to the metaslab, the bottom bit is used to indicate if this
-     * is the first chunk in a PAL allocation, that cannot be combined with
-     * the preceeding chunk.
+     * In common cases, the pointer to the metaslab.  See docs/AddressSpace.md
+     * for additional details.
+     *
+     * The bottom bit is used to indicate if this is the first chunk in a PAL
+     * allocation, that cannot be combined with the preceeding chunk.
      */
     uintptr_t meta{0};
 
@@ -107,18 +110,12 @@ namespace snmalloc
      * representable.  It is also true on Windows as you cannot Commit across
      * multiple continuous VirtualAllocs.
      */
-    static constexpr address_t BOUNDARY_BIT = 1;
+    static constexpr address_t META_BOUNDARY_BIT = 1 << 0;
 
     /**
-     * A bit-packed pointer to the owning allocator (if any), and the sizeclass
-     * of this chunk.  The sizeclass here is itself a union between two cases:
-     *
-     *  * log_2(size), at least MIN_CHUNK_BITS, for large allocations.
-     *
-     *  * a value in [0, NUM_SMALL_SIZECLASSES] for small allocations.  These
-     *    may be directly passed to the sizeclass (not slab_sizeclass) functions
-     *    of sizeclasstable.h
-     *
+     * In common cases, a bit-packed pointer to the owning allocator (if any),
+     * and the sizeclass of this chunk.  See mem/metaslab.h:MetaEntryRemote for
+     * details of this case and docs/AddressSpace.md for further details.
      */
     uintptr_t remote_and_sizeclass{0};
 
@@ -160,7 +157,7 @@ namespace snmalloc
     [[nodiscard]] SNMALLOC_FAST_PATH Metaslab* get_metaslab() const
     {
       SNMALLOC_ASSERT(get_remote() != nullptr);
-      return unsafe_from_uintptr<Metaslab>(meta & ~BOUNDARY_BIT);
+      return unsafe_from_uintptr<Metaslab>(meta & ~META_BOUNDARY_BIT);
     }
 
     /**
@@ -183,24 +180,25 @@ namespace snmalloc
     MetaEntry& operator=(const MetaEntry& other)
     {
       // Don't overwrite the boundary bit with the other's
-      meta = (other.meta & ~BOUNDARY_BIT) | address_cast(meta & BOUNDARY_BIT);
+      meta = (other.meta & ~META_BOUNDARY_BIT) |
+        address_cast(meta & META_BOUNDARY_BIT);
       remote_and_sizeclass = other.remote_and_sizeclass;
       return *this;
     }
 
     void set_boundary()
     {
-      meta |= BOUNDARY_BIT;
+      meta |= META_BOUNDARY_BIT;
     }
 
     [[nodiscard]] bool is_boundary() const
     {
-      return meta & BOUNDARY_BIT;
+      return meta & META_BOUNDARY_BIT;
     }
 
     bool clear_boundary_bit()
     {
-      return meta &= ~BOUNDARY_BIT;
+      return meta &= ~META_BOUNDARY_BIT;
     }
   };