From 7df5be2131b25d638a3aed31054a58a63be66e11 Mon Sep 17 00:00:00 2001 From: Laytan Laats Date: Wed, 12 Feb 2025 19:09:21 +0100 Subject: [PATCH 1/3] fix wrong out of memory in edge cases, just try allocate from block for one source of truth --- base/runtime/default_temp_allocator_arena.odin | 13 +++++++------ core/mem/virtual/arena.odin | 12 +++++++----- tests/core/mem/test_core_mem.odin | 16 +++++++++++++++- tests/core/runtime/test_core_runtime.odin | 14 ++++++++++++++ 4 files changed, 43 insertions(+), 12 deletions(-) diff --git a/base/runtime/default_temp_allocator_arena.odin b/base/runtime/default_temp_allocator_arena.odin index 6e2900411..5f25dac95 100644 --- a/base/runtime/default_temp_allocator_arena.odin +++ b/base/runtime/default_temp_allocator_arena.odin @@ -104,13 +104,15 @@ arena_alloc :: proc(arena: ^Arena, size, alignment: uint, loc := #caller_locatio if size == 0 { return } - - needed := align_forward_uint(size, alignment) - if arena.curr_block == nil || (safe_add(arena.curr_block.used, needed) or_else 0) > arena.curr_block.capacity { + + prev_used := 0 if arena.curr_block == nil else arena.curr_block.used + data, err = alloc_from_memory_block(arena.curr_block, size, alignment) + if err == .Out_Of_Memory { if arena.minimum_block_size == 0 { arena.minimum_block_size = DEFAULT_ARENA_GROWING_MINIMUM_BLOCK_SIZE } + needed := align_forward_uint(size, alignment) block_size := max(needed, arena.minimum_block_size) if arena.backing_allocator.procedure == nil { @@ -121,10 +123,9 @@ arena_alloc :: proc(arena: ^Arena, size, alignment: uint, loc := #caller_locatio new_block.prev = arena.curr_block arena.curr_block = new_block arena.total_capacity += new_block.capacity + prev_used = 0 + data, err = alloc_from_memory_block(arena.curr_block, size, alignment) } - - prev_used := arena.curr_block.used - data, err = alloc_from_memory_block(arena.curr_block, size, alignment) arena.total_used += arena.curr_block.used - prev_used return } diff --git a/core/mem/virtual/arena.odin b/core/mem/virtual/arena.odin index 5191505cf..73084f77b 100644 --- a/core/mem/virtual/arena.odin +++ b/core/mem/virtual/arena.odin @@ -107,8 +107,9 @@ arena_alloc :: proc(arena: ^Arena, size: uint, alignment: uint, loc := #caller_l switch arena.kind { case .Growing: - needed := mem.align_forward_uint(size, alignment) - if arena.curr_block == nil || (safe_add(arena.curr_block.used, needed) or_else 0) > arena.curr_block.reserved { + prev_used := 0 if arena.curr_block == nil else arena.curr_block.used + data, err = alloc_from_memory_block(arena.curr_block, size, alignment, default_commit_size=arena.default_commit_size) + if err == .Out_Of_Memory { if arena.minimum_block_size == 0 { arena.minimum_block_size = DEFAULT_ARENA_GROWING_MINIMUM_BLOCK_SIZE arena.minimum_block_size = mem.align_forward_uint(arena.minimum_block_size, DEFAULT_PAGE_SIZE) @@ -124,6 +125,7 @@ arena_alloc :: proc(arena: ^Arena, size: uint, alignment: uint, loc := #caller_l max(arena.default_commit_size, arena.minimum_block_size) } + needed := mem.align_forward_uint(size, alignment) needed = max(needed, arena.default_commit_size) block_size := max(needed, arena.minimum_block_size) @@ -131,10 +133,10 @@ arena_alloc :: proc(arena: ^Arena, size: uint, alignment: uint, loc := #caller_l new_block.prev = arena.curr_block arena.curr_block = new_block arena.total_reserved += new_block.reserved - } - prev_used := arena.curr_block.used - data, err = alloc_from_memory_block(arena.curr_block, size, alignment, default_commit_size=arena.default_commit_size) + prev_used = 0 + data, err = alloc_from_memory_block(arena.curr_block, size, alignment, default_commit_size=arena.default_commit_size) + } arena.total_used += arena.curr_block.used - prev_used case .Static: if arena.curr_block == nil { diff --git a/tests/core/mem/test_core_mem.odin b/tests/core/mem/test_core_mem.odin index d282ae1fd..3a1517747 100644 --- a/tests/core/mem/test_core_mem.odin +++ b/tests/core/mem/test_core_mem.odin @@ -1,6 +1,7 @@ package test_core_mem import "core:mem/tlsf" +import "core:mem/virtual" import "core:testing" @test @@ -38,4 +39,17 @@ test_tlsf_bitscan :: proc(t: ^testing.T) { testing.expectf(t, res == test.exp, "Expected tlsf.fls_uint(0x%16x) == %v, got %v", test.v, test.exp, res) } } -} \ No newline at end of file +} + +@(test) +test_align_bumping_block_limit :: proc(t: ^testing.T) { + a: virtual.Arena + + data, err := virtual.arena_alloc(&a, 4193371, 1) + testing.expect_value(t, err, nil) + testing.expect(t, len(data) == 4193371) + + data, err = virtual.arena_alloc(&a, 896, 64) + testing.expect_value(t, err, nil) + testing.expect(t, len(data) == 896) +} diff --git a/tests/core/runtime/test_core_runtime.odin b/tests/core/runtime/test_core_runtime.odin index be6c24c72..0d02ae2d8 100644 --- a/tests/core/runtime/test_core_runtime.odin +++ b/tests/core/runtime/test_core_runtime.odin @@ -31,6 +31,20 @@ test_temp_allocator_big_alloc_and_alignment :: proc(t: ^testing.T) { testing.expect(t, err == nil) } +@(test) +test_align_bumping_block_limit :: proc(t: ^testing.T) { + a: runtime.Arena + a.minimum_block_size = 8*mem.Megabyte + + data, err := runtime.arena_alloc(&a, 4193371, 1) + testing.expect_value(t, err, nil) + testing.expect(t, len(data) == 4193371) + + data, err = runtime.arena_alloc(&a, 896, 64) + testing.expect_value(t, err, nil) + testing.expect(t, len(data) == 896) +} + @(test) test_temp_allocator_returns_correct_size :: proc(t: ^testing.T) { arena: runtime.Arena From ae0f69fbe28b3ee6764da999cccea9b14ac80e36 Mon Sep 17 00:00:00 2001 From: Laytan Laats Date: Wed, 12 Feb 2025 19:13:16 +0100 Subject: [PATCH 2/3] cleanup test arenas --- tests/core/mem/test_core_mem.odin | 1 + tests/core/runtime/test_core_runtime.odin | 1 + 2 files changed, 2 insertions(+) diff --git a/tests/core/mem/test_core_mem.odin b/tests/core/mem/test_core_mem.odin index 3a1517747..4f2095fca 100644 --- a/tests/core/mem/test_core_mem.odin +++ b/tests/core/mem/test_core_mem.odin @@ -44,6 +44,7 @@ test_tlsf_bitscan :: proc(t: ^testing.T) { @(test) test_align_bumping_block_limit :: proc(t: ^testing.T) { a: virtual.Arena + defer virtual.arena_destroy(&a) data, err := virtual.arena_alloc(&a, 4193371, 1) testing.expect_value(t, err, nil) diff --git a/tests/core/runtime/test_core_runtime.odin b/tests/core/runtime/test_core_runtime.odin index 0d02ae2d8..472a5527d 100644 --- a/tests/core/runtime/test_core_runtime.odin +++ b/tests/core/runtime/test_core_runtime.odin @@ -35,6 +35,7 @@ test_temp_allocator_big_alloc_and_alignment :: proc(t: ^testing.T) { test_align_bumping_block_limit :: proc(t: ^testing.T) { a: runtime.Arena a.minimum_block_size = 8*mem.Megabyte + defer runtime.arena_destroy(&a) data, err := runtime.arena_alloc(&a, 4193371, 1) testing.expect_value(t, err, nil) From cae3f13d9f61fa4ec82dbdcfe3c9bcb6f9f8d344 Mon Sep 17 00:00:00 2001 From: Laytan Laats Date: Tue, 18 Feb 2025 18:33:19 +0100 Subject: [PATCH 3/3] mem/virtual: specify max protection on mmap call in NetBSD and FreeBSD --- core/mem/virtual/virtual_darwin.odin | 20 ++++++++++++++++++ core/mem/virtual/virtual_freebsd.odin | 26 ++++++++++++++++++++++++ core/mem/virtual/virtual_netbsd.odin | 25 +++++++++++++++++++++++ core/mem/virtual/virtual_openbsd.odin | 20 ++++++++++++++++++ core/mem/virtual/virtual_platform.odin | 4 +++- core/mem/virtual/virtual_posix.odin | 28 +++++--------------------- 6 files changed, 99 insertions(+), 24 deletions(-) create mode 100644 core/mem/virtual/virtual_darwin.odin create mode 100644 core/mem/virtual/virtual_freebsd.odin create mode 100644 core/mem/virtual/virtual_netbsd.odin create mode 100644 core/mem/virtual/virtual_openbsd.odin diff --git a/core/mem/virtual/virtual_darwin.odin b/core/mem/virtual/virtual_darwin.odin new file mode 100644 index 000000000..0635c83d4 --- /dev/null +++ b/core/mem/virtual/virtual_darwin.odin @@ -0,0 +1,20 @@ +package mem_virtual + +import "core:sys/posix" + +_reserve :: proc "contextless" (size: uint) -> (data: []byte, err: Allocator_Error) { + result := posix.mmap(nil, size, {}, {.ANONYMOUS, .PRIVATE}) + if result == posix.MAP_FAILED { + assert_contextless(posix.errno() == .ENOMEM) + return nil, .Out_Of_Memory + } + + return ([^]byte)(uintptr(result))[:size], nil +} + +_decommit :: proc "contextless" (data: rawptr, size: uint) { + MADV_FREE :: 5 + + posix.mprotect(data, size, {}) + posix.posix_madvise(data, size, transmute(posix.MAdvice)i32(MADV_FREE)) +} diff --git a/core/mem/virtual/virtual_freebsd.odin b/core/mem/virtual/virtual_freebsd.odin new file mode 100644 index 000000000..af0f31733 --- /dev/null +++ b/core/mem/virtual/virtual_freebsd.odin @@ -0,0 +1,26 @@ +package mem_virtual + +import "core:sys/posix" + +_reserve :: proc "contextless" (size: uint) -> (data: []byte, err: Allocator_Error) { + + PROT_MAX :: proc "contextless" (flags: posix.Prot_Flags) -> posix.Prot_Flags { + _PROT_MAX_SHIFT :: 16 + return transmute(posix.Prot_Flags)(transmute(i32)flags << _PROT_MAX_SHIFT) + } + + result := posix.mmap(nil, size, PROT_MAX({.READ, .WRITE, .EXEC}), {.ANONYMOUS, .PRIVATE}) + if result == posix.MAP_FAILED { + assert_contextless(posix.errno() == .ENOMEM) + return nil, .Out_Of_Memory + } + + return ([^]byte)(uintptr(result))[:size], nil +} + +_decommit :: proc "contextless" (data: rawptr, size: uint) { + MADV_FREE :: 5 + + posix.mprotect(data, size, {}) + posix.posix_madvise(data, size, transmute(posix.MAdvice)i32(MADV_FREE)) +} diff --git a/core/mem/virtual/virtual_netbsd.odin b/core/mem/virtual/virtual_netbsd.odin new file mode 100644 index 000000000..588625cc7 --- /dev/null +++ b/core/mem/virtual/virtual_netbsd.odin @@ -0,0 +1,25 @@ +package mem_virtual + +import "core:sys/posix" + +_reserve :: proc "contextless" (size: uint) -> (data: []byte, err: Allocator_Error) { + + PROT_MPROTECT :: proc "contextless" (flags: posix.Prot_Flags) -> posix.Prot_Flags { + return transmute(posix.Prot_Flags)(transmute(i32)flags << 3) + } + + result := posix.mmap(nil, size, PROT_MPROTECT({.READ, .WRITE, .EXEC}), {.ANONYMOUS, .PRIVATE}) + if result == posix.MAP_FAILED { + assert_contextless(posix.errno() == .ENOMEM) + return nil, .Out_Of_Memory + } + + return ([^]byte)(uintptr(result))[:size], nil +} + +_decommit :: proc "contextless" (data: rawptr, size: uint) { + MADV_FREE :: 6 + + posix.mprotect(data, size, {}) + posix.posix_madvise(data, size, transmute(posix.MAdvice)i32(MADV_FREE)) +} diff --git a/core/mem/virtual/virtual_openbsd.odin b/core/mem/virtual/virtual_openbsd.odin new file mode 100644 index 000000000..83f7ca9ca --- /dev/null +++ b/core/mem/virtual/virtual_openbsd.odin @@ -0,0 +1,20 @@ +package mem_virtual + +import "core:sys/posix" + +_reserve :: proc "contextless" (size: uint) -> (data: []byte, err: Allocator_Error) { + result := posix.mmap(nil, size, {}, {.ANONYMOUS, .PRIVATE}) + if result == posix.MAP_FAILED { + assert_contextless(posix.errno() == .ENOMEM) + return nil, .Out_Of_Memory + } + + return ([^]byte)(uintptr(result))[:size], nil +} + +_decommit :: proc "contextless" (data: rawptr, size: uint) { + MADV_FREE :: 6 + + posix.mprotect(data, size, {}) + posix.posix_madvise(data, size, transmute(posix.MAdvice)i32(MADV_FREE)) +} diff --git a/core/mem/virtual/virtual_platform.odin b/core/mem/virtual/virtual_platform.odin index 54c42ce4b..31e9cfca8 100644 --- a/core/mem/virtual/virtual_platform.odin +++ b/core/mem/virtual/virtual_platform.odin @@ -15,7 +15,9 @@ platform_memory_alloc :: proc "contextless" (to_commit, to_reserve: uint) -> (bl to_commit = clamp(to_commit, size_of(Platform_Memory_Block), total_to_reserved) data := reserve(total_to_reserved) or_return - commit(raw_data(data), to_commit) + + commit_err := commit(raw_data(data), to_commit) + assert_contextless(commit_err == nil) block = (^Platform_Memory_Block)(raw_data(data)) block.committed = to_commit diff --git a/core/mem/virtual/virtual_posix.odin b/core/mem/virtual/virtual_posix.odin index c3d6a9095..0b304a5e7 100644 --- a/core/mem/virtual/virtual_posix.odin +++ b/core/mem/virtual/virtual_posix.odin @@ -4,36 +4,18 @@ package mem_virtual import "core:sys/posix" -// Define non-posix needed flags: -when ODIN_OS == .Darwin || ODIN_OS == .FreeBSD { - MADV_FREE :: 5 /* pages unneeded, discard contents */ -} else when ODIN_OS == .OpenBSD || ODIN_OS == .NetBSD { - MADV_FREE :: 6 -} - -_reserve :: proc "contextless" (size: uint) -> (data: []byte, err: Allocator_Error) { - flags := posix.Map_Flags{ .ANONYMOUS, .PRIVATE } - result := posix.mmap(nil, size, {}, flags) - if result == posix.MAP_FAILED { - return nil, .Out_Of_Memory - } - - return ([^]byte)(uintptr(result))[:size], nil -} - _commit :: proc "contextless" (data: rawptr, size: uint) -> Allocator_Error { if posix.mprotect(data, size, { .READ, .WRITE }) != .OK { - return .Out_Of_Memory + #partial switch posix.errno() { + case .EACCES, .EPERM: return .Invalid_Pointer + case .ENOTSUP, .EINVAL: return .Invalid_Argument + case: return .Out_Of_Memory + } } return nil } -_decommit :: proc "contextless" (data: rawptr, size: uint) { - posix.mprotect(data, size, {}) - posix.posix_madvise(data, size, transmute(posix.MAdvice)i32(MADV_FREE)) -} - _release :: proc "contextless" (data: rawptr, size: uint) { posix.munmap(data, size) }