From 4d08b7637279c49cb56106a1247e7dfc09265fb2 Mon Sep 17 00:00:00 2001 From: Laytan Laats Date: Fri, 9 May 2025 22:26:49 +0200 Subject: [PATCH 1/4] require the __asan_unpoison_memory_region runtime symbol so empty projects with asan enabled build --- base/runtime/internal.odin | 1 + 1 file changed, 1 insertion(+) diff --git a/base/runtime/internal.odin b/base/runtime/internal.odin index bff5b8380..38b7f662c 100644 --- a/base/runtime/internal.odin +++ b/base/runtime/internal.odin @@ -1109,6 +1109,7 @@ __read_bits :: proc "contextless" (dst, src: [^]byte, offset: uintptr, size: uin when .Address in ODIN_SANITIZER_FLAGS { foreign { + @(require) __asan_unpoison_memory_region :: proc "system" (address: rawptr, size: uint) --- } } From f9b9e9e7dcbb605bc64bc5af1331855375f58494 Mon Sep 17 00:00:00 2001 From: Laytan Laats Date: Fri, 9 May 2025 22:27:35 +0200 Subject: [PATCH 2/4] some ABI fixups and improvements Started with trying to enable asan in the CI for MacOS, noticed it wasn't enabled on the `tests/internal` folder, it came up with a couple of issues with the abi/OdinLLVMBuildTransmute that this also solves. - Looking at clang output for arm64, we should be promoting `{ i64, i32 }` to `{ i64, i64 }` - after doing the previous point, I noticed this is not handled well in OdinLLVMBuildTransmute which was emitting loads and stores into the space of a value that was alignment, asan does not want this, looking at clang output again, a memcpy is the appropriate way of handling this. - Having done this we don't need the hacky "return is packed" set anymore in the amd64 sysv ABI anymore either --- src/llvm_abi.cpp | 42 +++++++++----------------- src/llvm_backend_general.cpp | 58 ++++++++++++++++++++++++++++++++++-- 2 files changed, 69 insertions(+), 31 deletions(-) diff --git a/src/llvm_abi.cpp b/src/llvm_abi.cpp index c8e1ca764..baad3f873 100644 --- a/src/llvm_abi.cpp +++ b/src/llvm_abi.cpp @@ -977,7 +977,7 @@ namespace lbAbiAmd64SysV { return types[0]; } - return LLVMStructTypeInContext(c, types.data, cast(unsigned)types.count, sz == 0); + return LLVMStructTypeInContext(c, types.data, cast(unsigned)types.count, false); } gb_internal void classify_with(LLVMTypeRef t, Array *cls, i64 ix, i64 off) { @@ -1231,38 +1231,24 @@ namespace lbAbiArm64 { } } else { i64 size = lb_sizeof(return_type); - if (size <= 16) { - LLVMTypeRef cast_type = nullptr; - - if (size == 0) { - cast_type = LLVMStructTypeInContext(c, nullptr, 0, false); - } else if (size <= 8) { - cast_type = LLVMIntTypeInContext(c, cast(unsigned)(size*8)); - } else { - unsigned count = cast(unsigned)((size+7)/8); - - LLVMTypeRef llvm_i64 = LLVMIntTypeInContext(c, 64); - LLVMTypeRef *types = gb_alloc_array(temporary_allocator(), LLVMTypeRef, count); - - i64 size_copy = size; - for (unsigned i = 0; i < count; i++) { - if (size_copy >= 8) { - types[i] = llvm_i64; - } else { - types[i] = LLVMIntTypeInContext(c, 8*cast(unsigned)size_copy); - } - size_copy -= 8; - } - GB_ASSERT(size_copy <= 0); - cast_type = LLVMStructTypeInContext(c, types, count, true); - } - return lb_arg_type_direct(return_type, cast_type, nullptr, nullptr); - } else { + if (size > 16) { LB_ABI_MODIFY_RETURN_IF_TUPLE_MACRO(); LLVMAttributeRef attr = lb_create_enum_attribute_with_type(c, "sret", return_type); return lb_arg_type_indirect(return_type, attr); } + + GB_ASSERT(size <= 16); + LLVMTypeRef cast_type = nullptr; + if (size == 0) { + cast_type = LLVMStructTypeInContext(c, nullptr, 0, false); + } else if (size <= 8) { + cast_type = LLVMIntTypeInContext(c, cast(unsigned)(size*8)); + } else { + LLVMTypeRef llvm_i64 = LLVMIntTypeInContext(c, 64); + cast_type = LLVMArrayType2(llvm_i64, 2); + } + return lb_arg_type_direct(return_type, cast_type, nullptr, nullptr); } } diff --git a/src/llvm_backend_general.cpp b/src/llvm_backend_general.cpp index c52551b36..504c8234e 100644 --- a/src/llvm_backend_general.cpp +++ b/src/llvm_backend_general.cpp @@ -2525,10 +2525,13 @@ general_end:; } } - src_size = align_formula(src_size, src_align); - dst_size = align_formula(dst_size, dst_align); + // NOTE(laytan): even though this logic seems sound, the Address Sanitizer does not + // want you to load/store the space of a value that is there for alignment. +#if 0 + i64 aligned_src_size = align_formula(src_size, src_align); + i64 aligned_dst_size = align_formula(dst_size, dst_align); - if (LLVMIsALoadInst(val) && (src_size >= dst_size && src_align >= dst_align)) { + if (LLVMIsALoadInst(val) && (aligned_src_size >= aligned_dst_size && src_align >= dst_align)) { LLVMValueRef val_ptr = LLVMGetOperand(val, 0); val_ptr = LLVMBuildPointerCast(p->builder, val_ptr, LLVMPointerType(dst_type, 0), ""); LLVMValueRef loaded_val = OdinLLVMBuildLoad(p, dst_type, val_ptr); @@ -2536,8 +2539,57 @@ general_end:; // LLVMSetAlignment(loaded_val, gb_min(src_align, dst_align)); return loaded_val; + } +#endif + + if (src_size > dst_size) { + GB_ASSERT(p->decl_block != p->curr_block); + // NOTE(laytan): src is bigger than dst, need to memcpy the part of src we want. + + LLVMValueRef val_ptr; + if (LLVMIsALoadInst(val)) { + val_ptr = LLVMGetOperand(val, 0); + } else if (LLVMIsAAllocaInst(val)) { + val_ptr = LLVMBuildPointerCast(p->builder, val, LLVMPointerType(src_type, 0), ""); + } else { + // NOTE(laytan): we need a pointer to memcpy from. + LLVMValueRef val_copy = llvm_alloca(p, src_type, src_align); + val_ptr = LLVMBuildPointerCast(p->builder, val_copy, LLVMPointerType(src_type, 0), ""); + LLVMBuildStore(p->builder, val, val_ptr); + } + + i64 max_align = gb_max(lb_alignof(src_type), lb_alignof(dst_type)); + max_align = gb_max(max_align, 16); + + LLVMValueRef ptr = llvm_alloca(p, dst_type, max_align); + LLVMValueRef nptr = LLVMBuildPointerCast(p->builder, ptr, LLVMPointerType(dst_type, 0), ""); + + LLVMTypeRef types[3] = { + lb_type(p->module, t_rawptr), + lb_type(p->module, t_rawptr), + lb_type(p->module, t_int) + }; + + LLVMValueRef args[4] = { + nptr, + val_ptr, + LLVMConstInt(LLVMIntTypeInContext(p->module->ctx, 8*cast(unsigned)build_context.int_size), dst_size, 0), + LLVMConstInt(LLVMInt1TypeInContext(p->module->ctx), 0, 0), + }; + + lb_call_intrinsic( + p, + "llvm.memcpy.inline", + args, + gb_count_of(args), + types, + gb_count_of(types) + ); + + return OdinLLVMBuildLoad(p, dst_type, ptr); } else { GB_ASSERT(p->decl_block != p->curr_block); + GB_ASSERT(dst_size >= src_size); i64 max_align = gb_max(lb_alignof(src_type), lb_alignof(dst_type)); max_align = gb_max(max_align, 16); From 221dea76a42a4e41ff3f9a1b889f1439f54287b9 Mon Sep 17 00:00:00 2001 From: Laytan Laats Date: Fri, 9 May 2025 17:12:35 +0200 Subject: [PATCH 3/4] Run MacOS CI with -sanitize:address --- .github/workflows/ci.yml | 68 +++++++++++----------------------------- 1 file changed, 19 insertions(+), 49 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2f03eb359..590c52feb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -75,9 +75,9 @@ jobs: fail-fast: false matrix: # MacOS 13 runs on Intel, 14 runs on ARM - os: [macos-13, macos-14] + os: [macos-13, macos-14, ubuntu-latest] runs-on: ${{ matrix.os }} - name: ${{ matrix.os == 'macos-14' && 'MacOS ARM' || (matrix.os == 'macos-13' && 'MacOS Intel') }} Build, Check, and Test + name: ${{ matrix.os == 'macos-14' && 'MacOS ARM' || (matrix.os == 'macos-13' && 'MacOS Intel') || (matrix.os == 'ubuntu-latest' && 'Ubuntu') }} Build, Check, and Test timeout-minutes: 15 steps: @@ -95,52 +95,8 @@ jobs: brew update brew install llvm@20 wasmtime lua@5.4 lld - - name: Build Odin - run: ./build_odin.sh release - - name: Odin version - run: ./odin version - - name: Odin report - run: ./odin report - - name: Compile needed Vendor - run: | - make -C vendor/stb/src - make -C vendor/cgltf/src - make -C vendor/miniaudio/src - - name: Odin check - run: ./odin check examples/demo -vet - - name: Odin run - run: ./odin run examples/demo - - name: Odin run -debug - run: ./odin run examples/demo -debug - - name: Odin check examples/all - run: ./odin check examples/all -strict-style -vet -disallow-do - - name: Odin check vendor/sdl3 - run: ./odin check vendor/sdl3 -strict-style -vet -disallow-do -no-entry-point - - name: Normal Core library tests - run: ./odin test tests/core/normal.odin -file -all-packages -vet -strict-style -disallow-do -define:ODIN_TEST_FANCY=false -define:ODIN_TEST_FAIL_ON_BAD_MEMORY=true - - name: Optimized Core library tests - run: ./odin test tests/core/speed.odin -o:speed -file -all-packages -vet -strict-style -disallow-do -define:ODIN_TEST_FANCY=false -define:ODIN_TEST_FAIL_ON_BAD_MEMORY=true - - name: Vendor library tests - run: ./odin test tests/vendor -all-packages -vet -strict-style -disallow-do -define:ODIN_TEST_FANCY=false -define:ODIN_TEST_FAIL_ON_BAD_MEMORY=true - - name: Internals tests - run: ./odin test tests/internal -all-packages -vet -strict-style -disallow-do -define:ODIN_TEST_FANCY=false -define:ODIN_TEST_FAIL_ON_BAD_MEMORY=true - - name: GitHub Issue tests - run: | - cd tests/issues - ./run.sh - - - name: Run demo on WASI WASM32 - run: | - ./odin build examples/demo -target:wasi_wasm32 -vet -strict-style -disallow-do -out:demo - wasmtime ./demo.wasm - if: matrix.os == 'macos-14' - - build_ubuntu: - name: Ubuntu Build, Check, and Test - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - - name: Download LLVM + - name: Download LLVM (Ubuntu) + if: matrix.os == 'ubuntu-latest' run: | wget https://apt.llvm.org/llvm.sh chmod +x llvm.sh @@ -175,30 +131,44 @@ jobs: - name: Vendor library tests run: ./odin test tests/vendor -all-packages -vet -strict-style -disallow-do -define:ODIN_TEST_FANCY=false -define:ODIN_TEST_FAIL_ON_BAD_MEMORY=true -sanitize:address - name: Internals tests - run: ./odin test tests/internal -all-packages -vet -strict-style -disallow-do -define:ODIN_TEST_FANCY=false -define:ODIN_TEST_FAIL_ON_BAD_MEMORY=true + run: ./odin test tests/internal -all-packages -vet -strict-style -disallow-do -define:ODIN_TEST_FANCY=false -define:ODIN_TEST_FAIL_ON_BAD_MEMORY=true -sanitize:address - name: GitHub Issue tests run: | cd tests/issues ./run.sh + - name: Run demo on WASI WASM32 + run: | + ./odin build examples/demo -target:wasi_wasm32 -vet -strict-style -disallow-do -out:demo + wasmtime ./demo.wasm + if: matrix.os == 'macos-14' + - name: Check benchmarks run: ./odin check tests/benchmark -vet -strict-style -no-entry-point - name: Odin check examples/all for Linux i386 + if: matrix.os == 'ubuntu-latest' run: ./odin check examples/all -vet -strict-style -disallow-do -target:linux_i386 - name: Odin check examples/all for Linux arm64 + if: matrix.os == 'ubuntu-latest' run: ./odin check examples/all -vet -strict-style -disallow-do -target:linux_arm64 - name: Odin check examples/all for FreeBSD amd64 + if: matrix.os == 'ubuntu-latest' run: ./odin check examples/all -vet -strict-style -disallow-do -target:freebsd_amd64 - name: Odin check examples/all for OpenBSD amd64 + if: matrix.os == 'ubuntu-latest' run: ./odin check examples/all -vet -strict-style -disallow-do -target:openbsd_amd64 - name: Odin check vendor/sdl3 for Linux i386 + if: matrix.os == 'ubuntu-latest' run: ./odin check vendor/sdl3 -vet -strict-style -disallow-do -no-entry-point -target:linux_i386 - name: Odin check vendor/sdl3 for Linux arm64 + if: matrix.os == 'ubuntu-latest' run: ./odin check vendor/sdl3 -vet -strict-style -disallow-do -no-entry-point -target:linux_arm64 - name: Odin check vendor/sdl3 for FreeBSD amd64 + if: matrix.os == 'ubuntu-latest' run: ./odin check vendor/sdl3 -vet -strict-style -disallow-do -no-entry-point -target:freebsd_amd64 - name: Odin check vendor/sdl3 for OpenBSD amd64 + if: matrix.os == 'ubuntu-latest' run: ./odin check vendor/sdl3 -vet -strict-style -disallow-do -no-entry-point -target:openbsd_amd64 build_windows: From 8374854dd557accf3ad8a0136ae1ff5867bc9a29 Mon Sep 17 00:00:00 2001 From: Laytan Laats Date: Fri, 9 May 2025 17:21:08 +0200 Subject: [PATCH 4/4] use brew clang instead of system clang --- .github/workflows/ci.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 590c52feb..8ae39667b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -88,12 +88,14 @@ jobs: run: | brew update brew install llvm@20 lua@5.4 lld + echo "$(brew --prefix llvm@20)/bin" >> $GITHUB_PATH - name: Download LLVM (MacOS ARM) if: matrix.os == 'macos-14' run: | brew update brew install llvm@20 wasmtime lua@5.4 lld + echo "$(brew --prefix llvm@20)/bin" >> $GITHUB_PATH - name: Download LLVM (Ubuntu) if: matrix.os == 'ubuntu-latest'