From 599ae0e632c54f30075a91ca6c00c52dac1e42ed Mon Sep 17 00:00:00 2001
From: Nathaniel Wesley Filardo <nfilardo@microsoft.com>
Date: Sun, 17 Oct 2021 16:38:19 +0100
Subject: [PATCH] Metaslab: add MetaCommon field

This preserves the chunk pointer through the use of a chunk as a slab.  It does
grow the structure by one pointer, but on non-CHERI it is still padded to 64
bytes, even with CHECK_CLIENT guards in place:

 0: MetaCommon chunk pointer
 8: next pointer
16: builder head[0]
24: builder head[1]
32: builder tail[0]
40: builder tail[1]
48: builder length[0] (uint16_t)
50: builder length[1] (uint16_t)
52: padding (4 bytes)
56: needed (uint16_t)
58: sleeping (bool)

(Sadly, on CHERI, even without CHECK_CLIENT guards and with no padding, there
are now four pointers in the structure -- chunk, next, head, tail -- plus five
extra bytes.  We will likely wish to explore encoding the head and tail offsets
relative to the chunk pointer.)

This lets us remove the "subversive amplification" in dealloc() in favor of just
preserving the chunk pointer.  Speaking of, be sure to assign that in all the
right places, and ASSERT that we've got it right.
---
 src/backend/backend.h |  2 ++
 src/mem/corealloc.h   |  9 ++++++---
 src/mem/localalloc.h  | 13 ++++---------
 src/mem/metaslab.h    | 14 ++++++++++++++
 4 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/src/backend/backend.h b/src/backend/backend.h
index 36328f4..1cc9c0b 100644
--- a/src/backend/backend.h
+++ b/src/backend/backend.h
@@ -87,6 +87,8 @@ namespace snmalloc
         return {p, nullptr};
       }
 
+      meta->meta_common.chunk = p;
+
       MetaEntry t(meta, remote, sizeclass);
       Pagemap::set_metaentry(local_state, address_cast(p), size, t);
       return {p, meta};
diff --git a/src/mem/corealloc.h b/src/mem/corealloc.h
index 98c80a8..b0fc28e 100644
--- a/src/mem/corealloc.h
+++ b/src/mem/corealloc.h
@@ -321,13 +321,16 @@ namespace snmalloc
       // have the whole chunk.
       auto start_of_slab = pointer_align_down<void>(
         p, snmalloc::sizeclass_to_slab_size(sizeclass));
-      // TODO Add bounds correctly here
-      chunk_record->meta_common.chunk =
-        capptr::Chunk<void>(start_of_slab.unsafe_ptr());
+
+      SNMALLOC_ASSERT(
+        address_cast(start_of_slab) ==
+        address_cast(chunk_record->meta_common.chunk));
 
 #ifdef SNMALLOC_TRACING
       std::cout << "Slab " << start_of_slab.unsafe_ptr()
                 << " is unused, Object sizeclass " << sizeclass << std::endl;
+#else
+      UNUSED(start_of_slab);
 #endif
       return chunk_record;
     }
diff --git a/src/mem/localalloc.h b/src/mem/localalloc.h
index 1e62123..db4bbfa 100644
--- a/src/mem/localalloc.h
+++ b/src/mem/localalloc.h
@@ -536,15 +536,10 @@ namespace snmalloc
 
         ChunkRecord* slab_record =
           reinterpret_cast<ChunkRecord*>(entry.get_metaslab());
-        /*
-         * StrictProvenance TODO: this is a subversive amplification.  p_tame is
-         * tame but Alloc-bounded, but we're coercing it to Chunk-bounded.  We
-         * should, instead, not be storing ->chunk here, but should be keeping
-         * a CapPtr<void, Chunk> to this region internally even while it's
-         * allocated.
-         */
-        slab_record->meta_common.chunk =
-          capptr::Chunk<void>(p_tame.unsafe_ptr());
+
+        SNMALLOC_ASSERT(
+          address_cast(slab_record->meta_common.chunk) == address_cast(p_tame));
+
         check_init(
           [](
             CoreAlloc* core_alloc,
diff --git a/src/mem/metaslab.h b/src/mem/metaslab.h
index 997439b..1057c14 100644
--- a/src/mem/metaslab.h
+++ b/src/mem/metaslab.h
@@ -9,6 +9,13 @@
 
 namespace snmalloc
 {
+  /**
+   * A guaranteed type-stable sub-structure of all metadata referenced by the
+   * Pagemap.  Use-specific structures (Metaslab, ChunkRecord) are expected to
+   * have this at offset zero so that, even in the face of concurrent mutation
+   * and reuse of the memory backing that metadata, the types of these fields
+   * remain fixed.
+   */
   struct MetaCommon
   {
     capptr::Chunk<void> chunk;
@@ -18,6 +25,8 @@ namespace snmalloc
   class alignas(CACHELINE_SIZE) Metaslab
   {
   public:
+    MetaCommon meta_common;
+
     // Used to link metaslabs together in various other data-structures.
     Metaslab* next{nullptr};
 
@@ -186,6 +195,11 @@ namespace snmalloc
     }
   };
 
+  static_assert(std::is_standard_layout_v<Metaslab>);
+  static_assert(
+    offsetof(Metaslab, meta_common) == 0,
+    "ChunkRecord and Metaslab must share a common prefix");
+
   /**
    * Entry stored in the pagemap.
    */