CHERI, take 2021.10

.github/workflows/main.yml

@@ -119,18 +119,28 @@ jobs:
             system-processor: arm
             triple: arm-linux-gnueabihf
             rtld: ld-linux-armhf.so.3
+            ld-flavour: lld
           - name: arm64
             system-processor: aarch64
             triple: aarch64-linux-gnu
             rtld: ld-linux-aarch64.so.1
+            ld-flavour: lld
           - name: ppc64el
             system-processor: powerpc64le
             triple: powerpc64le-linux-gnu
             rtld: ld64.so.2
+            ld-flavour: lld
+          - name: riscv64
+            system-processor: riscv64
+            triple: riscv64-linux-gnu
+            rtld: ld-linux-riscv64-lp64d.so.1
+            extra-packages: binutils-riscv64-linux-gnu
+            ld-flavour: bfd
+            ld: /usr/bin/riscv64-linux-gnu-ld.bfd
       # Don't abort runners if a single one fails
       fail-fast: false
     runs-on: ubuntu-latest
-    name: Cross-build for ${{ matrix.arch.triple }}
+    name: ${{matrix.build-type}} cross-build for ${{ matrix.arch.triple }}
     steps:
     - uses: actions/checkout@v2
     - name: Install cross-compile toolchain and QEMU
@@ -141,6 +151,7 @@ jobs:
         sudo add-apt-repository "deb http://apt.llvm.org/focal/     llvm-toolchain-focal-13  main"
         sudo apt update
         sudo apt install libstdc++-9-dev-${{ matrix.arch.name }}-cross qemu-user ninja-build clang-13 lld-13
+        sudo apt install ${{matrix.arch.extra-packages}}
         # The default PowerPC qemu configuration uses the wrong page size.
         # Wrap it in a script that fixes this.
         sudo update-binfmts --disable qemu-ppc64le
@@ -161,6 +172,8 @@ jobs:
         -DSNMALLOC_QEMU_WORKAROUND=ON
         -DSNMALLOC_STATIC_LIBRARY=OFF
         -DCMAKE_TOOLCHAIN_FILE=ci/Toolchain.cmake
+        -DSNMALLOC_LINKER=${{matrix.arch.ld}}
+        -DSNMALLOC_LINKER_FLAVOUR=${{matrix.arch.ld-flavour}}
     - name: Build
       working-directory: ${{github.workspace}}/build
       run: NINJA_STATUS="%p [%f:%s/%t] %o/s, %es" ninja

CMakeLists.txt

@@ -245,10 +245,21 @@ if(NOT SNMALLOC_HEADER_ONLY_LIBRARY)
     set(${result} ${dirlist} PARENT_SCOPE)
   endfunction()
 
-  set(CMAKE_REQUIRED_LINK_OPTIONS -fuse-ld=lld)
-  check_cxx_source_compiles("int main() { return 1; }" LLD_WORKS)
-  if (LLD_WORKS)
-    message(STATUS "Using LLD to link snmalloc shims")
+  if(NOT (DEFINED SNMALLOC_LINKER_FLAVOUR) OR ("${SNMALLOC_LINKER_FLAVOUR}" MATCHES "^$"))
+    # Linker not specified externally; probe to see if we can make lld work
+    set(CMAKE_REQUIRED_LINK_OPTIONS -fuse-ld=lld)
+    check_cxx_source_compiles("int main() { return 1; }" LLD_WORKS)
+    if (LLD_WORKS)
+      message(STATUS "Using LLD to link snmalloc shims")
+    endif()
+  elseif(SNMALLOC_LINKER_FLAVOUR STREQUAL "lld")
+    # Linker specified externally to be lld; assume it works and that the flags
+    # have also been set for us
+    set(LLD_WORKS TRUE)
+  else()
+    # Linker specified externally as something other than lld; presume it
+    # doesn't work and don't add its flags, below
+    set(LLD_WORKS FALSE)
   endif()
 
   function(add_shim name type)

ci/Toolchain.cmake

@@ -7,7 +7,11 @@ set(CMAKE_C_COMPILER clang-13)
 set(CMAKE_C_COMPILER_TARGET ${triple})
 set(CMAKE_CXX_COMPILER clang++-13)
 set(CMAKE_CXX_COMPILER_TARGET ${triple})
-set(CROSS_LINKER_FLAGS "-fuse-ld=lld -Wl,--dynamic-linker=/usr/${triple}/lib/$ENV{RTLD_NAME},-rpath,/usr/${triple}/lib")
+
+set(CROSS_LINKER_FLAGS "-fuse-ld=${SNMALLOC_LINKER_FLAVOUR} -Wl,--dynamic-linker=/usr/${triple}/lib/$ENV{RTLD_NAME},-rpath,/usr/${triple}/lib")
+if((DEFINED SNMALLOC_LINKER) AND NOT ("${SNMALLOC_LINKER}" MATCHES "^$"))
+  string(APPEND CROSS_LINKER_FLAGS " --ld-path=${SNMALLOC_LINKER}")
+endif()
 set(CMAKE_EXE_LINKER_FLAGS ${CROSS_LINKER_FLAGS})
 set(CMAKE_SHARED_LINKER_FLAGS ${CROSS_LINKER_FLAGS})
 set(CMAKE_MODULE_LINKER_FLAGS ${CROSS_LINKER_FLAGS})

src/aal/aal.h

@@ -32,6 +32,10 @@
 #  define PLATFORM_IS_SPARC
 #endif
 
+#if defined(__riscv)
+#  define PLATFORM_IS_RISCV
+#endif
+
 namespace snmalloc
 {
   /**
@@ -195,27 +199,15 @@ namespace snmalloc
     static SNMALLOC_FAST_PATH CapPtr<T, BOut>
     capptr_bound(CapPtr<U, BIn> a, size_t size) noexcept
     {
-      // Impose constraints on bounds annotations.
-      static_assert(BIn::spatial >= capptr::dimension::Spatial::Chunk);
-      static_assert(capptr_is_spatial_refinement<BIn, BOut>());
+      static_assert(
+        BIn::spatial > capptr::dimension::Spatial::Alloc,
+        "Refusing to re-bound Spatial::Alloc CapPtr");
+      static_assert(
+        capptr::is_spatial_refinement<BIn, BOut>(),
+        "capptr_bound must preserve non-spatial CapPtr dimensions");
 
       UNUSED(size);
-      return CapPtr<T, BOut>(a.template as_static<T>().unsafe_capptr);
-    }
-
-    /**
-     * For architectures which do not enforce StrictProvenance, there's nothing
-     * to be done, so just return the pointer unmodified with new annotation.
-     */
-    template<
-      typename T,
-      SNMALLOC_CONCEPT(capptr::ConceptBound) BOut,
-      SNMALLOC_CONCEPT(capptr::ConceptBound) BIn>
-    static SNMALLOC_FAST_PATH CapPtr<T, BOut>
-    capptr_rebound(CapPtr<void, BOut> a, CapPtr<T, BIn> r) noexcept
-    {
-      UNUSED(a);
-      return CapPtr<T, BOut>(r.unsafe_capptr);
+      return CapPtr<T, BOut>(a.template as_static<T>().unsafe_ptr());
     }
   };
 } // namespace snmalloc
@@ -230,11 +222,21 @@ namespace snmalloc
 #  include "aal_powerpc.h"
 #elif defined(PLATFORM_IS_SPARC)
 #  include "aal_sparc.h"
+#elif defined(PLATFORM_IS_RISCV)
+#  include "aal_riscv.h"
+#endif
+
+#if defined(__CHERI_PURE_CAPABILITY__)
+#  include "aal_cheri.h"
 #endif
 
 namespace snmalloc
 {
+#if defined(__CHERI_PURE_CAPABILITY__)
+  using Aal = AAL_Generic<AAL_CHERI<AAL_Arch>>;
+#else
   using Aal = AAL_Generic<AAL_NoStrictProvenance<AAL_Arch>>;
+#endif
 
   template<AalFeatures F, SNMALLOC_CONCEPT(ConceptAAL) AAL = Aal>
   constexpr static bool aal_supports = (AAL::aal_features & F) == F;

src/aal/aal_cheri.h

@@ -0,0 +1,52 @@
+#pragma once
+
+#include "../ds/defines.h"
+
+#include <stddef.h>
+
+namespace snmalloc
+{
+  /**
+   * A mixin AAL that applies CHERI to a `Base` architecture.  Gives
+   * architectural teeth to the capptr_bound primitive.
+   */
+  template<typename Base>
+  class AAL_CHERI : public Base
+  {
+  public:
+    /**
+     * CHERI pointers are not integers and come with strict provenance
+     * requirements.
+     */
+    static constexpr uint64_t aal_features =
+      (Base::aal_features & ~IntegerPointers) | StrictProvenance;
+
+    /**
+     * On CHERI-aware compilers, ptraddr_t is an integral type that is wide
+     * enough to hold any address that may be contained within a memory
+     * capability.  It does not carry provenance: it is not a capability, but
+     * merely an address.
+     */
+    typedef ptraddr_t address_t;
+
+    template<
+      typename T,
+      SNMALLOC_CONCEPT(capptr::ConceptBound) BOut,
+      SNMALLOC_CONCEPT(capptr::ConceptBound) BIn,
+      typename U = T>
+    static SNMALLOC_FAST_PATH CapPtr<T, BOut>
+    capptr_bound(CapPtr<U, BIn> a, size_t size) noexcept
+    {
+      static_assert(
+        BIn::spatial > capptr::dimension::Spatial::Alloc,
+        "Refusing to re-bound Spatial::Alloc CapPtr");
+      static_assert(
+        capptr::is_spatial_refinement<BIn, BOut>(),
+        "capptr_bound must preserve non-spatial CapPtr dimensions");
+      SNMALLOC_ASSERT(__builtin_cheri_tag_get(a.unsafe_ptr()));
+
+      void* pb = __builtin_cheri_bounds_set_exact(a.unsafe_ptr(), size);
+      return CapPtr<T, BOut>(static_cast<T*>(pb));
+    }
+  };
+} // namespace snmalloc

src/aal/aal_concept.h

@@ -53,12 +53,6 @@ namespace snmalloc
     { AAL::template capptr_bound<void, capptr::bounds::Chunk>(auth, sz) }
       noexcept
       -> ConceptSame<capptr::Chunk<void>>;
-
-    /**
-     * Construct a copy of auth with its target set to that of ret.
-     */
-    { AAL::capptr_rebound(auth, ret) } noexcept
-      -> ConceptSame<capptr::Chunk<void>>;
   };
 
   template<typename AAL>

src/aal/aal_consts.h

@@ -33,5 +33,6 @@ namespace snmalloc
     X86,
     X86_SGX,
     Sparc,
+    RISCV
   };
 } // namespace snmalloc

src/aal/aal_riscv.h

@@ -0,0 +1,54 @@
+#pragma once
+
+#if __riscv_xlen == 64
+#  define SNMALLOC_VA_BITS_64
+#elif __riscv_xlen == 32
+#  define SNMALLOC_VA_BITS_32
+#endif
+
+namespace snmalloc
+{
+  /**
+   * RISC-V architecture layer, phrased as generically as possible.  Specific
+   * implementations may need to adjust some of these.
+   */
+  class AAL_RISCV
+  {
+  public:
+    static constexpr uint64_t aal_features = IntegerPointers;
+
+    static constexpr size_t smallest_page_size = 0x1000;
+
+    static constexpr AalName aal_name = RISCV;
+
+    static void inline pause()
+    {
+      /*
+       * The "Zihintpause" extension claims to be the right thing to do here,
+       * and it is expected to be used in analogous places, e.g., Linux's
+       * cpu_relax(), but...
+       *
+       *  its specification is somewhat unusual, in that it talks about the rate
+       *  at which a HART's instructions retire rather than the rate at which
+       *  they are dispatched (Intel's PAUSE instruction explicitly promises
+       *  that it "de-pipelines" the spin-wait loop, for example) or anything
+       *  about memory semantics (Intel's PAUSE docs talk about a possible
+       *  memory order violation and pipeline flush upon loop exit).
+       *
+       *  we don't yet have examples of what implementations have done.
+       *
+       *  it's not yet understood by C frontends or assembler, meaning we'd have
+       *  to spell it out by hand, as
+       *      __asm__ volatile(".byte 0xF; .byte 0x0; .byte 0x0; .byte 0x1");
+       *
+       * All told, we just leave this function empty for the moment.  The good
+       * news is that, if and when we do add a PAUSE, the instruction is encoded
+       * by stealing some dead space of the FENCE instruction and so should be
+       * available everywhere even if it doesn't do anything on a particular
+       * microarchitecture.
+       */
+    }
+  };
+
+  using AAL_Arch = AAL_RISCV;
+}

src/backend/address_space.h

@@ -53,12 +53,10 @@ namespace snmalloc
       SNMALLOC_ASSERT(size >= sizeof(void*));
 
       /*
-       * For sufficiently large allocations with platforms that support
-       * aligned allocations and architectures that don't require
-       * StrictProvenance, try asking the platform first.
+       * For sufficiently large allocations with platforms that support aligned
+       * allocations, try asking the platform directly.
        */
-      if constexpr (
-        pal_supports<AlignedAllocation, PAL> && !aal_supports<StrictProvenance>)
+      if constexpr (pal_supports<AlignedAllocation, PAL>)
       {
         if (size >= PAL::minimum_alloc_size)
         {

src/backend/address_space_core.h

@@ -20,6 +20,16 @@ namespace snmalloc
    *
    * It cannot unreserve memory, so this does not require the
    * usual complexity of a buddy allocator.
+   *
+   * TODO: This manages pieces of memory smaller than (1U << MIN_CHUNK_BITS) to
+   * source Metaslab and LocalCache objects.  On CHERI, where ASLR and guard
+   * pages are not needed, it may be worth switching to a design where we
+   * bootstrap allocators with at least two embedded Metaslab-s that can be used
+   * to construct slabs for LocalCache and, of course, additional Metaslab
+   * objects.  That would let us stop splitting memory below that threshold
+   * here, and may reduce address space fragmentation or address space committed
+   * to Metaslab objects in perpetuity; it could also make {set,get}_next less
+   * scary.
    */
   class AddressSpaceManagerCore
   {
@@ -77,13 +87,16 @@ namespace snmalloc
     {
       if (align_bits >= MIN_CHUNK_BITS)
       {
-        // The pagemap stores MetaEntrys, abuse the metaslab field to be the
+        // The pagemap stores `MetaEntry`s; abuse the metaslab field to be the
         // next block in the stack of blocks.
         //
         // The pagemap entries here have nullptr (i.e., fake_large_remote) as
         // their remote, and so other accesses to the pagemap (by
         // external_pointer, for example) will not attempt to follow this
         // "Metaslab" pointer.
+        //
+        // dealloc() can reject attempts to free such MetaEntry-s due to the
+        // zero sizeclass.
         MetaEntry t(reinterpret_cast<Metaslab*>(next.unsafe_ptr()), nullptr, 0);
         Pagemap::set_metaentry(local_state, address_cast(base), 1, t);
         return;
@@ -112,7 +125,7 @@ namespace snmalloc
         const MetaEntry& t = Pagemap::template get_metaentry<false>(
           local_state, address_cast(base));
         return capptr::Chunk<FreeChunk>(
-          reinterpret_cast<FreeChunk*>(t.get_metaslab()));
+          reinterpret_cast<FreeChunk*>(t.get_metaslab_no_remote()));
       }
 
       return base->next;

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};

src/backend/pagemap.h

@@ -294,7 +294,7 @@ namespace snmalloc
     void set(address_t p, T t)
     {
 #ifdef SNMALLOC_TRACING
-      std::cout << "Pagemap.Set " << (void*)p << std::endl;
+      std::cout << "Pagemap.Set " << (void*)(uintptr_t)p << std::endl;
 #endif
       if constexpr (has_bounds)
       {

src/ds/address.h

@@ -8,11 +8,8 @@
 namespace snmalloc
 {
   /**
-   * The type used for an address.  Currently, all addresses are assumed to be
-   * provenance-carrying values and so it is possible to cast back from the
-   * result of arithmetic on an address_t.  Eventually, this will want to be
-   * separated into two types, one for raw addresses and one for addresses that
-   * can be cast back to pointers.
+   * The type used for an address.  On CHERI, this is not a provenance-carrying
+   * value and so cannot be converted back to a pointer.
    */
   using address_t = Aal::address_t;