diff --git a/.gitignore b/.gitignore index 1187596de..c85ccb4ad 100644 --- a/.gitignore +++ b/.gitignore @@ -277,6 +277,7 @@ odin *.bin demo.bin libLLVM*.so* +*.a # shared collection shared/ diff --git a/check_all.bat b/check_all.bat index 83c7deaa9..c5f7ee399 100644 --- a/check_all.bat +++ b/check_all.bat @@ -1,9 +1,9 @@ @echo off if "%1" == "" ( - echo Checking darwin_amd64 - expect vendor:cgtlf panic + echo Checking darwin_amd64 - expect vendor:cgltf panic odin check examples\all -vet -vet-tabs -strict-style -vet-style -warnings-as-errors -disallow-do -target:darwin_amd64 - echo Checking darwin_arm64 - expect vendor:cgtlf panic + echo Checking darwin_arm64 - expect vendor:cgltf panic odin check examples\all -vet -vet-tabs -strict-style -vet-style -warnings-as-errors -disallow-do -target:darwin_arm64 echo Checking linux_i386 odin check examples\all -vet -vet-tabs -strict-style -vet-style -warnings-as-errors -disallow-do -target:linux_i386 diff --git a/check_all.sh b/check_all.sh old mode 100644 new mode 100755 index fb32b8cc2..568ac55ba --- a/check_all.sh +++ b/check_all.sh @@ -45,9 +45,9 @@ wasm) ;; *) - echo Checking darwin_amd64 - expect vendor:cgtlf panic + echo Checking darwin_amd64 - expect vendor:cgltf panic odin check examples/all -vet -vet-tabs -strict-style -vet-style -warnings-as-errors -disallow-do -target:darwin_amd64 - echo Checking darwin_arm64 - expect vendor:cgtlf panic + echo Checking darwin_arm64 - expect vendor:cgltf panic odin check examples/all -vet -vet-tabs -strict-style -vet-style -warnings-as-errors -disallow-do -target:darwin_arm64 echo Checking linux_i386 odin check examples/all -vet -vet-tabs -strict-style -vet-style -warnings-as-errors -disallow-do -target:linux_i386 diff --git a/core/mem/allocators.odin b/core/mem/allocators.odin index 21e69c463..a5a7d9951 100644 --- a/core/mem/allocators.odin +++ b/core/mem/allocators.odin @@ -2239,6 +2239,7 @@ buddy_allocator_init :: proc(b: ^Buddy_Allocator, data: []byte, alignment: uint, b.head.is_free = true b.tail = buddy_block_next(b.head) b.alignment = alignment + assert(uint(len(data)) >= 2 * buddy_block_size_required(b, 1), "The size of the backing buffer must be large enough to hold at least two 1-byte allocations given the alignment requirements, otherwise it cannot split.", loc) // sanitizer.address_poison(data) } @@ -2247,12 +2248,14 @@ Get required block size to fit in the allocation as well as the alignment paddin */ @(require_results) buddy_block_size_required :: proc(b: ^Buddy_Allocator, size: uint) -> uint { - size := size - actual_size := b.alignment - size += size_of(Buddy_Block) - size = align_forward_uint(size, b.alignment) - for size > actual_size { - actual_size <<= 1 + assert(size > 0) + // NOTE: `size_of(Buddy_Block)` will be accounted for in `b.alignment`. + // This calculation is also previously guarded against being given a `size` + // 0 by `buddy_allocator_alloc_bytes_non_zeroed` checking for that. + actual_size := b.alignment + size + if intrinsics.count_ones(actual_size) != 1 { + // We're not a power of two. Let's fix that. + actual_size = 1 << (size_of(uint) * 8 - intrinsics.count_leading_zeros(actual_size)) } return actual_size } @@ -2315,7 +2318,7 @@ buddy_allocator_alloc_bytes_non_zeroed :: proc(b: ^Buddy_Allocator, size: uint) if size != 0 { actual_size := buddy_block_size_required(b, size) found := buddy_block_find_best(b.head, b.tail, actual_size) - if found != nil { + if found == nil { // Try to coalesce all the free buddy blocks and then search again buddy_block_coalescence(b.head, b.tail) found = buddy_block_find_best(b.head, b.tail, actual_size) diff --git a/tests/issues/run.bat b/tests/issues/run.bat index 76d8f58b6..4a1072e12 100644 --- a/tests/issues/run.bat +++ b/tests/issues/run.bat @@ -17,6 +17,7 @@ set COMMON=-define:ODIN_TEST_FANCY=false -file -vet -strict-style ..\..\..\odin test ..\test_issue_2637.odin %COMMON% || exit /b ..\..\..\odin test ..\test_issue_2666.odin %COMMON% || exit /b ..\..\..\odin test ..\test_issue_2694.odin %COMMON% || exit /b +..\..\..\odin test ..\test_issue_3435.odin %COMMON% || exit /b ..\..\..\odin test ..\test_issue_4210.odin %COMMON% || exit /b ..\..\..\odin test ..\test_issue_4364.odin %COMMON% || exit /b ..\..\..\odin test ..\test_issue_4584.odin %COMMON% || exit /b diff --git a/tests/issues/run.sh b/tests/issues/run.sh index 305329e7d..03d84425a 100755 --- a/tests/issues/run.sh +++ b/tests/issues/run.sh @@ -18,6 +18,7 @@ $ODIN test ../test_issue_2615.odin $COMMON $ODIN test ../test_issue_2637.odin $COMMON $ODIN test ../test_issue_2666.odin $COMMON $ODIN test ../test_issue_2694.odin $COMMON +$ODIN test ../test_issue_3435.odin $COMMON $ODIN test ../test_issue_4210.odin $COMMON $ODIN test ../test_issue_4364.odin $COMMON $ODIN test ../test_issue_4584.odin $COMMON diff --git a/tests/issues/test_issue_3435.odin b/tests/issues/test_issue_3435.odin new file mode 100644 index 000000000..3830869ed --- /dev/null +++ b/tests/issues/test_issue_3435.odin @@ -0,0 +1,38 @@ +package main + +import "base:runtime" +import "core:mem" +import "core:testing" +import "core:time" + +@test +test_issue_3435 :: proc(t: ^testing.T) { + testing.set_fail_timeout(t, time.Second) + allocator: mem.Buddy_Allocator + data := runtime.make_aligned([]byte, 64, 32) + defer delete(data) + + // mem.buddy_allocator_init(&allocator, data, 32) + + // Bypass the assertion that would normally keep this from happening by + // manually putting the allocator together. + allocator.head = cast(^mem.Buddy_Block)raw_data(data) + allocator.head.size = len(data) + allocator.head.is_free = true + allocator.tail = mem.buddy_block_next(allocator.head) + allocator.alignment = 32 + + context.allocator = mem.buddy_allocator(&allocator) + + // Three allocations in the space above is all that's needed to reproduce + // the bug seen in #3435; this is the most minimal reproduction possible. + a := make([]u8, 1) + testing.expect(t, len(a) == 1) + b := make([]u8, 1) + testing.expect(t, len(b) == 0) + c := make([]u8, 1) + testing.expect(t, len(c) == 0) + + // With the bugfix in place, the allocator should be sensible enough to not + // fall into an infinite loop anymore, even if the assertion is disabled. +}