Merge pull request #4836 from laytan/fix-wrong-out-of-memory

fix wrong out of memory in edge cases, just try allocate from block for one source of truth

10 files changed, 144 insertions, 36 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 42f0054c2..4fc2e0e35 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/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)
 }
diff --git a/tests/core/mem/test_core_mem.odin b/tests/core/mem/test_core_mem.odin
index d282ae1fd..4f2095fca 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,18 @@ 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
+	defer virtual.arena_destroy(&a)
+
+	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..472a5527d 100644
--- a/tests/core/runtime/test_core_runtime.odin
+++ b/tests/core/runtime/test_core_runtime.odin
@@ -32,6 +32,21 @@ test_temp_allocator_big_alloc_and_alignment :: proc(t: ^testing.T) {
 }
 
 @(test)
+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)
+	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
 	context.allocator = runtime.arena_allocator(&arena)