Skip to content

Commit 71e205f

Browse files
mjp41Copilot
andauthored
Fallible notify using (#790)
On Windows, the most common way to out-of-memory is to fail to commit a page. The current design of the Pal assumes that notify_using always succeeds, thus we have to raise an error if it fails. This changes the specification of notify_using to return a bool, indicating whether the notify succeeded or not. This allows Windows to fail the notification, and then the surrounding code can handle the failure appropriately, such as by throwing an exception or returning the nullptr for the allocation. Co-authored-by: Copilot <[email protected]>
1 parent d4c7e01 commit 71e205f

File tree

14 files changed

+87
-32
lines changed

14 files changed

+87
-32
lines changed

docs/PORTING.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,16 +38,18 @@ The state for a particular region of memory is set with
3838
static void notify_not_using(void* p, size_t size) noexcept;
3939
4040
template<ZeroMem zero_mem>
41-
static void notify_using(void* p, size_t size) noexcept;
41+
static bool notify_using(void* p, size_t size) noexcept;
4242
43-
static void notify_using_readonly(void* p, size_t size) noexcept;
43+
static bool notify_using_readonly(void* p, size_t size) noexcept;
4444
```
4545
These functions notify the system that the range of memory from `p` to `p` +
4646
`size` is in the relevant state.
4747

4848
If the template parameter is set to `YesZero` then `notify_using` must ensure
4949
the range is full of zeros.
5050

51+
The function should return `true` if the memory is now in the requested state, and `false` if it failed to make the memory useable.
52+
5153
```c++
5254
template<bool page_aligned = false>
5355
static void zero(void* p, size_t size) noexcept;

src/snmalloc/backend_helpers/authmap.h

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,10 @@ namespace snmalloc
1313
{
1414
static SNMALLOC_FAST_PATH void init() {}
1515

16-
static SNMALLOC_FAST_PATH void register_range(capptr::Arena<void>, size_t)
17-
{}
16+
static SNMALLOC_FAST_PATH bool register_range(capptr::Arena<void>, size_t)
17+
{
18+
return true;
19+
}
1820

1921
template<bool potentially_out_of_range = false>
2022
static SNMALLOC_FAST_PATH capptr::Arena<void> amplify(capptr::Alloc<void> c)
@@ -44,7 +46,7 @@ namespace snmalloc
4446
concreteAuthmap.template init</* randomize_location */ false>();
4547
}
4648

47-
static SNMALLOC_FAST_PATH void
49+
static SNMALLOC_FAST_PATH bool
4850
register_range(capptr::Arena<void> base, size_t size)
4951
{
5052
concreteAuthmap.register_range(address_cast(base), size);
@@ -55,6 +57,7 @@ namespace snmalloc
5557
{
5658
concreteAuthmap.set(a, base);
5759
}
60+
return true;
5861
}
5962

6063
template<bool potentially_out_of_range = false>

src/snmalloc/backend_helpers/commitrange.h

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,17 @@ namespace snmalloc
3434
PAL::page_size);
3535
auto range = parent.alloc_range(size);
3636
if (range != nullptr)
37-
PAL::template notify_using<NoZero>(range.unsafe_ptr(), size);
37+
{
38+
auto result =
39+
PAL::template notify_using<NoZero>(range.unsafe_ptr(), size);
40+
if (!result)
41+
{
42+
// If notify_using fails, we deallocate the range and return
43+
// nullptr.
44+
parent.dealloc_range(range, size);
45+
return CapPtr<void, ChunkBounds>(nullptr);
46+
}
47+
}
3848
return range;
3949
}
4050

src/snmalloc/backend_helpers/pagemap.h

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,15 +96,22 @@ namespace snmalloc
9696
* Mark the MetaEntry at the bottom of the range as a boundary, preventing
9797
* consolidation with a lower range, unless CONSOLIDATE_PAL_ALLOCS.
9898
*/
99-
static void register_range(capptr::Arena<void> p, size_t sz)
99+
static bool register_range(capptr::Arena<void> p, size_t sz)
100100
{
101-
concretePagemap.register_range(address_cast(p), sz);
101+
auto result = concretePagemap.register_range(address_cast(p), sz);
102+
103+
if (!result)
104+
{
105+
return false;
106+
}
107+
102108
if constexpr (!CONSOLIDATE_PAL_ALLOCS)
103109
{
104110
// Mark start of allocation in pagemap.
105111
auto& entry = get_metaentry_mut(address_cast(p));
106112
entry.set_boundary();
107113
}
114+
return true;
108115
}
109116

110117
/**

src/snmalloc/backend_helpers/pagemapregisterrange.h

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,13 @@ namespace snmalloc
3030

3131
if (base != nullptr)
3232
{
33-
Pagemap::register_range(base, size);
33+
auto result = Pagemap::register_range(base, size);
34+
if (!result)
35+
{
36+
// If register_range fails, typically there is no recovery from this
37+
// so just return nullptr.
38+
return CapPtr<void, ChunkBounds>(nullptr);
39+
}
3440
}
3541

3642
return base;

src/snmalloc/ds/pagemap.h

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ namespace snmalloc
5555
/**
5656
* Ensure this range of pagemap is accessible
5757
*/
58-
void register_range(address_t p, size_t length)
58+
bool register_range(address_t p, size_t length)
5959
{
6060
SNMALLOC_ASSERT(is_initialised());
6161

@@ -76,11 +76,18 @@ namespace snmalloc
7676
auto page_start = pointer_align_down<OS_PAGE_SIZE, char>(first);
7777
auto page_end = pointer_align_up<OS_PAGE_SIZE, char>(last);
7878
size_t using_size = pointer_diff(page_start, page_end);
79-
PAL::template notify_using<NoZero>(page_start, using_size);
79+
auto result = PAL::template notify_using<NoZero>(page_start, using_size);
80+
if (!result)
81+
{
82+
// If notify_using fails, fails this call.
83+
return false;
84+
}
8085
if constexpr (pal_supports<CoreDump, PAL>)
8186
{
8287
PAL::notify_do_dump(page_start, using_size);
8388
}
89+
90+
return true;
8491
}
8592

8693
constexpr FlatPagemap() = default;
@@ -234,22 +241,35 @@ namespace snmalloc
234241
// Only commit readonly memory for this range, if the platform
235242
// supports lazy commit. Otherwise, this would be a lot of memory to
236243
// have mapped.
237-
PAL::notify_using_readonly(
244+
auto result = PAL::notify_using_readonly(
238245
start_page, pointer_diff(start_page, end_page));
246+
if (!result)
247+
{
248+
PAL::error("Failed to initialise snmalloc.");
249+
}
239250
}
240251
}
241252
else
242253
{
243254
if constexpr (pal_supports<LazyCommit, PAL>)
244255
{
245-
PAL::notify_using_readonly(new_body_untyped, REQUIRED_SIZE);
256+
auto result =
257+
PAL::notify_using_readonly(new_body_untyped, REQUIRED_SIZE);
258+
if (!result)
259+
{
260+
PAL::error("Failed to initialise snmalloc.");
261+
}
246262
}
247263
new_body = static_cast<T*>(new_body_untyped);
248264
}
249265
// Ensure bottom page is committed
250266
// ASSUME: new memory is zeroed.
251-
PAL::template notify_using<NoZero>(
267+
auto result = PAL::template notify_using<NoZero>(
252268
pointer_align_down<OS_PAGE_SIZE>(new_body), OS_PAGE_SIZE);
269+
if (!result)
270+
{
271+
PAL::error("Failed to initialise snmalloc.");
272+
}
253273

254274
// Set up zero page
255275
new_body[0] = body[0];
@@ -364,7 +384,8 @@ namespace snmalloc
364384

365385
/**
366386
* Return the starting address corresponding to a given entry within the
367-
* Pagemap. Also checks that the reference actually points to a valid entry.
387+
* Pagemap. Also checks that the reference actually points to a valid
388+
* entry.
368389
*/
369390
[[nodiscard]] address_t get_address(const T& t) const
370391
{

src/snmalloc/mem/backend_concept.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ namespace snmalloc
6161
concept IsPagemapWithRegister = requires(capptr::Arena<void> p, size_t sz) {
6262
{
6363
Pagemap::register_range(p, sz)
64-
} -> ConceptSame<void>;
64+
} -> ConceptSame<bool>;
6565
};
6666

6767
/**

src/snmalloc/pal/pal_apple.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ namespace snmalloc
189189
*
190190
*/
191191
template<ZeroMem zero_mem>
192-
static void notify_using(void* p, size_t size) noexcept
192+
static bool notify_using(void* p, size_t size) noexcept
193193
{
194194
KeepErrno e;
195195
SNMALLOC_ASSERT(
@@ -207,7 +207,7 @@ namespace snmalloc
207207

208208
if (SNMALLOC_LIKELY(r != MAP_FAILED))
209209
{
210-
return;
210+
return true;
211211
}
212212
}
213213

@@ -232,6 +232,7 @@ namespace snmalloc
232232
{
233233
::bzero(p, size);
234234
}
235+
return true;
235236
}
236237

237238
// Apple's `mmap` doesn't support user-specified alignment and only

src/snmalloc/pal/pal_concept.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,10 @@ namespace snmalloc
5353

5454
{
5555
PAL::template notify_using<NoZero>(vp, sz)
56-
} noexcept -> ConceptSame<void>;
56+
} noexcept -> ConceptSame<bool>;
5757
{
5858
PAL::template notify_using<YesZero>(vp, sz)
59-
} noexcept -> ConceptSame<void>;
59+
} noexcept -> ConceptSame<bool>;
6060

6161
{
6262
PAL::template zero<false>(vp, sz)

src/snmalloc/pal/pal_freebsd_kernel.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,14 +51,15 @@ namespace snmalloc
5151

5252
/// Notify platform that we will be using these pages
5353
template<ZeroMem zero_mem>
54-
static void notify_using(void* p, size_t size)
54+
static bool notify_using(void* p, size_t size)
5555
{
5656
vm_offset_t addr = get_vm_offset(p);
5757
int flags = M_WAITOK | ((zero_mem == YesZero) ? M_ZERO : 0);
5858
if (kmem_back(kernel_object, addr, size, flags) != KERN_SUCCESS)
5959
{
60-
error("Out of memory");
60+
return false;
6161
}
62+
return true;
6263
}
6364

6465
/// OS specific function for zeroing memory

0 commit comments

Comments
 (0)