Skip to content

Commit c33f355

Browse files
Fix the sandbox use case and add a test. (#269)
Summary of changes: - Add a new PAL that doesn't allocate memory, which can be used with a memory provider that is pre-initialised with a range of memory. - Add a `NoAllocation` PAL property so that the methods on a PAL that doesn't support dynamically reserving address space will never be called and therefore don't need to be implemented. - Slightly refactor the memory provider class so that it has a narrower interface with LargeAlloc and is easier to proxy. - Allow the address space manager and the memory provider to be initialised with a range of memory. This may eventually also remove the need for (or, at least, simplify) the Open Enclave PAL. This commit also ends up with a few other cleanups: - The `malloc_useable_size` CMake test that checks whether the parameter is const qualified was failing on FreeBSD where this function is declared in `malloc_np.h` but where including `malloc.h` raises an error. This should now be more robust. - The BSD aligned PAL inherited from the BSD PAL, which does not expose aligned allocation. This meant that it exposed both the aligned and non-aligned allocation interfaces and so happily accepted incorrect `constexpr` if blocks that expected one or the other but accidentally required both to exist. The unaligned function is now deleted so the same failures that appear in CI should appear locally for anyone using this PAL.
1 parent 4837c82 commit c33f355

File tree

10 files changed

+427
-11
lines changed

10 files changed

+427
-11
lines changed

CMakeLists.txt

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,16 @@ option(SNMALLOC_OPTIMISE_FOR_CURRENT_MACHINE "Compile for current machine archit
2121
set(CACHE_FRIENDLY_OFFSET OFF CACHE STRING "Base offset to place linked-list nodes.")
2222
set(SNMALLOC_STATIC_LIBRARY_PREFIX "sn_" CACHE STRING "Static library function prefix")
2323

24+
# malloc.h will error if you include it on FreeBSD, so this test must not
25+
# unconditionally include it.
2426
CHECK_C_SOURCE_COMPILES("
27+
#if __has_include(<malloc_np.h>)
28+
#include <malloc_np.h>
29+
#if __has_include(<malloc/malloc.h>)
30+
#include <malloc/malloc.h>
31+
#else
2532
#include <malloc.h>
33+
#endif
2634
size_t malloc_usable_size(const void* ptr) { return 0; }
2735
int main() { return 0; }
2836
" CONST_QUALIFIED_MALLOC_USABLE_SIZE)
@@ -307,6 +315,9 @@ if(NOT DEFINED SNMALLOC_ONLY_HEADER_LIBRARY)
307315
if (${SUPER_SLAB_SIZE} STREQUAL "malloc")
308316
target_compile_definitions(${TESTNAME} PRIVATE SNMALLOC_PASS_THROUGH)
309317
endif()
318+
if(CONST_QUALIFIED_MALLOC_USABLE_SIZE)
319+
target_compile_definitions(${TESTNAME} PRIVATE -DMALLOC_USABLE_SIZE_QUALIFIER=const)
320+
endif()
310321
target_link_libraries(${TESTNAME} snmalloc_lib)
311322
if (${TEST} MATCHES "release-.*")
312323
message(STATUS "Adding test: ${TESTNAME} only for release configs")

src/mem/address_space.h

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -191,14 +191,14 @@ namespace snmalloc
191191
if (res == nullptr)
192192
{
193193
// Allocation failed ask OS for more memory
194-
void* block;
195-
size_t block_size;
194+
void* block = nullptr;
195+
size_t block_size = 0;
196196
if constexpr (pal_supports<AlignedAllocation, PAL>)
197197
{
198198
block_size = PAL::minimum_alloc_size;
199199
block = PAL::template reserve_aligned<false>(block_size);
200200
}
201-
else
201+
else if constexpr (!pal_supports<NoAllocation, PAL>)
202202
{
203203
// Need at least 2 times the space to guarantee alignment.
204204
// Hold lock here as a race could cause additional requests to
@@ -236,5 +236,21 @@ namespace snmalloc
236236

237237
return res;
238238
}
239+
240+
/**
241+
* Default constructor. An address-space manager constructed in this way
242+
* does not own any memory at the start and will request any that it needs
243+
* from the PAL.
244+
*/
245+
AddressSpaceManager() = default;
246+
247+
/**
248+
* Constructor that pre-initialises the address-space manager with a region
249+
* of memory.
250+
*/
251+
AddressSpaceManager(void* base, size_t length)
252+
{
253+
add_range(base, length);
254+
}
239255
};
240256
} // namespace snmalloc

src/mem/allocstats.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,7 @@ namespace snmalloc
196196
UNUSED(sc);
197197

198198
#ifdef USE_SNMALLOC_STATS
199+
SNMALLOC_ASSUME(sc < LARGE_N);
199200
large_pop_count[sc]++;
200201
#endif
201202
}

src/mem/largealloc.h

Lines changed: 47 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,6 @@ namespace snmalloc
7575
*/
7676
std::atomic<size_t> peak_memory_used_bytes{0};
7777

78-
public:
79-
using Pal = PAL;
80-
8178
/**
8279
* Memory current available in large_stacks
8380
*/
@@ -88,6 +85,51 @@ namespace snmalloc
8885
*/
8986
ModArray<NUM_LARGE_CLASSES, MPMCStack<Largeslab, RequiresInit>> large_stack;
9087

88+
public:
89+
using Pal = PAL;
90+
91+
/**
92+
* Pop an allocation from a large-allocation stack. This is safe to call
93+
* concurrently with other acceses. If there is no large allocation on a
94+
* particular stack then this will return `nullptr`.
95+
*/
96+
SNMALLOC_FAST_PATH void* pop_large_stack(size_t large_class)
97+
{
98+
void* p = large_stack[large_class].pop();
99+
if (p != nullptr)
100+
{
101+
const size_t rsize = bits::one_at_bit(SUPERSLAB_BITS) << large_class;
102+
available_large_chunks_in_bytes -= rsize;
103+
}
104+
return p;
105+
}
106+
107+
/**
108+
* Push `slab` onto the large-allocation stack associated with the size
109+
* class specified by `large_class`. Always succeeds.
110+
*/
111+
SNMALLOC_FAST_PATH void
112+
push_large_stack(Largeslab* slab, size_t large_class)
113+
{
114+
const size_t rsize = bits::one_at_bit(SUPERSLAB_BITS) << large_class;
115+
available_large_chunks_in_bytes += rsize;
116+
large_stack[large_class].push(slab);
117+
}
118+
119+
/**
120+
* Default constructor. This constructs a memory provider that doesn't yet
121+
* own any memory, but which can claim memory from the PAL.
122+
*/
123+
MemoryProviderStateMixin() = default;
124+
125+
/**
126+
* Construct a memory provider owning some memory. The PAL provided with
127+
* memory providers constructed in this way does not have to be able to
128+
* allocate memory, if the initial reservation is sufficient.
129+
*/
130+
MemoryProviderStateMixin(void* start, size_t len)
131+
: address_space(start, len)
132+
{}
91133
/**
92134
* Make a new memory provide for this PAL.
93135
*/
@@ -253,7 +295,7 @@ namespace snmalloc
253295
if (large_class == 0)
254296
size = rsize;
255297

256-
void* p = memory_provider.large_stack[large_class].pop();
298+
void* p = memory_provider.pop_large_stack(large_class);
257299

258300
if (p == nullptr)
259301
{
@@ -265,7 +307,6 @@ namespace snmalloc
265307
else
266308
{
267309
stats.superslab_pop();
268-
memory_provider.available_large_chunks_in_bytes -= rsize;
269310

270311
// Cross-reference alloc.h's large_dealloc decommitment condition.
271312
bool decommitted =
@@ -323,8 +364,7 @@ namespace snmalloc
323364
}
324365

325366
stats.superslab_push();
326-
memory_provider.available_large_chunks_in_bytes += rsize;
327-
memory_provider.large_stack[large_class].push(static_cast<Largeslab*>(p));
367+
memory_provider.push_large_stack(static_cast<Largeslab*>(p), large_class);
328368
}
329369
};
330370

src/pal/pal.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
# include "pal_haiku.h"
1717
# include "pal_linux.h"
1818
# include "pal_netbsd.h"
19+
# include "pal_noalloc.h"
1920
# include "pal_openbsd.h"
2021
# include "pal_solaris.h"
2122
# include "pal_windows.h"

src/pal/pal_bsd_aligned.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,5 +50,17 @@ namespace snmalloc
5050

5151
return p;
5252
}
53+
54+
/**
55+
* Explicitly deleted method for returning non-aligned memory. This causes
56+
* incorrect use of `constexpr if` to fail on platforms with aligned
57+
* allocation. Without this, this PAL and its subclasses exported both
58+
* allocation functions and so callers would type-check if they called
59+
* either in `constexpr if` branches and then fail on platforms such as
60+
* Linux or Windows, which expose only unaligned or aligned allocations,
61+
* respectively.
62+
*/
63+
static std::pair<void*, size_t>
64+
reserve_at_least(size_t size) noexcept = delete;
5365
};
5466
} // namespace snmalloc

src/pal/pal_concept.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,10 +92,11 @@ namespace snmalloc
9292
ConceptPAL_memops<PAL> &&
9393
(!(PAL::pal_features & LowMemoryNotification) ||
9494
ConceptPAL_mem_low_notify<PAL>) &&
95+
(!(PAL::pal_features & NoAllocation) && (
9596
(!!(PAL::pal_features & AlignedAllocation) ||
9697
ConceptPAL_reserve_at_least<PAL>) &&
9798
(!(PAL::pal_features & AlignedAllocation) ||
98-
ConceptPAL_reserve_aligned<PAL>);
99+
ConceptPAL_reserve_aligned<PAL>)));
99100

100101
} // namespace snmalloc
101102
#endif

src/pal/pal_consts.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,11 @@ namespace snmalloc
3636
* exposed in the Pal.
3737
*/
3838
LazyCommit = (1 << 2),
39+
/**
40+
* This Pal does not support allocation. All memory used with this Pal
41+
* should be pre-allocated.
42+
*/
43+
NoAllocation = (1 << 3),
3944
};
4045
/**
4146
* Flag indicating whether requested memory should be zeroed.

src/pal/pal_noalloc.h

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
#pragma once
2+
3+
namespace snmalloc
4+
{
5+
/**
6+
* Platform abstraction layer that does not allow allocation.
7+
*
8+
* This is a minimal PAL for pre-reserved memory regions, where the
9+
* address-space manager is initialised with all of the memory that it will
10+
* ever use.
11+
*
12+
* It takes an error handler delegate as a template argument. This is
13+
* expected to forward to the default PAL in most cases.
14+
*/
15+
template<typename ErrorHandler>
16+
struct PALNoAlloc
17+
{
18+
/**
19+
* Bitmap of PalFeatures flags indicating the optional features that this
20+
* PAL supports.
21+
*/
22+
static constexpr uint64_t pal_features = NoAllocation;
23+
24+
static constexpr size_t page_size = Aal::smallest_page_size;
25+
26+
/**
27+
* Print a stack trace.
28+
*/
29+
static void print_stack_trace()
30+
{
31+
ErrorHandler::print_stack_trace();
32+
}
33+
34+
/**
35+
* Report a fatal error an exit.
36+
*/
37+
[[noreturn]] static void error(const char* const str) noexcept
38+
{
39+
ErrorHandler::error(str);
40+
}
41+
42+
/**
43+
* Notify platform that we will not be using these pages.
44+
*
45+
* This is a no-op in this stub.
46+
*/
47+
static void notify_not_using(void*, size_t) noexcept {}
48+
49+
/**
50+
* Notify platform that we will be using these pages.
51+
*
52+
* This is a no-op in this stub, except for zeroing memory if required.
53+
*/
54+
template<ZeroMem zero_mem>
55+
static void notify_using(void* p, size_t size) noexcept
56+
{
57+
if constexpr (zero_mem == YesZero)
58+
{
59+
zero<true>(p, size);
60+
}
61+
else
62+
{
63+
UNUSED(p);
64+
UNUSED(size);
65+
}
66+
}
67+
68+
/**
69+
* OS specific function for zeroing memory.
70+
*
71+
* This just calls memset - we don't assume that we have access to any
72+
* virtual-memory functions.
73+
*/
74+
template<bool page_aligned = false>
75+
static void zero(void* p, size_t size) noexcept
76+
{
77+
memset(p, 0, size);
78+
}
79+
};
80+
} // namespace snmalloc

0 commit comments

Comments
 (0)