From a7056f2b4f8ac7c5fe78b00cbae686da4867e206 Mon Sep 17 00:00:00 2001 From: Laytan Laats Date: Thu, 4 Apr 2024 16:58:22 +0200 Subject: fix lbArg_Ignore logic Fixes #2698 --- src/llvm_abi.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/llvm_abi.cpp') diff --git a/src/llvm_abi.cpp b/src/llvm_abi.cpp index 724e4e35a..49b5224ec 100644 --- a/src/llvm_abi.cpp +++ b/src/llvm_abi.cpp @@ -192,7 +192,7 @@ gb_internal void lb_add_function_type_attributes(LLVMValueRef fn, lbFunctionType // } LLVMSetFunctionCallConv(fn, cc_kind); if (calling_convention == ProcCC_Odin) { - unsigned context_index = offset+arg_count; + unsigned context_index = arg_index; LLVMAddAttributeAtIndex(fn, context_index, noalias_attr); LLVMAddAttributeAtIndex(fn, context_index, nonnull_attr); LLVMAddAttributeAtIndex(fn, context_index, nocapture_attr); -- cgit v1.2.3 From 31407d9b1b8d0d35ba53a0245d86ed37007a28e2 Mon Sep 17 00:00:00 2001 From: Laytan Laats Date: Thu, 4 Apr 2024 18:39:41 +0200 Subject: fix 128 bit int alignment on arm64 Fixes #2403 --- src/build_settings.cpp | 2 +- src/llvm_abi.cpp | 2 +- tests/internal/Makefile | 7 ++++-- tests/internal/build.bat | 4 ++- tests/internal/test_128.odin | 59 ++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 69 insertions(+), 5 deletions(-) create mode 100644 tests/internal/test_128.odin (limited to 'src/llvm_abi.cpp') diff --git a/src/build_settings.cpp b/src/build_settings.cpp index 1ac9e451f..a96c4491b 100644 --- a/src/build_settings.cpp +++ b/src/build_settings.cpp @@ -519,7 +519,7 @@ gb_global TargetMetrics target_darwin_amd64 = { gb_global TargetMetrics target_darwin_arm64 = { TargetOs_darwin, TargetArch_arm64, - 8, 8, 8, 16, + 8, 8, 16, 16, str_lit("arm64-apple-macosx"), // NOTE: Changes during initialization based on build flags. str_lit("e-m:o-i64:64-i128:128-n32:64-S128"), }; diff --git a/src/llvm_abi.cpp b/src/llvm_abi.cpp index 724e4e35a..507881f99 100644 --- a/src/llvm_abi.cpp +++ b/src/llvm_abi.cpp @@ -275,7 +275,7 @@ gb_internal i64 lb_alignof(LLVMTypeRef type) { case LLVMIntegerTypeKind: { unsigned w = LLVMGetIntTypeWidth(type); - return gb_clamp((w + 7)/8, 1, build_context.ptr_size); + return gb_clamp((w + 7)/8, 1, build_context.max_align); } case LLVMHalfTypeKind: return 2; diff --git a/tests/internal/Makefile b/tests/internal/Makefile index 00e46197b..daefd5959 100644 --- a/tests/internal/Makefile +++ b/tests/internal/Makefile @@ -1,6 +1,6 @@ ODIN=../../odin -all: rtti_test map_test pow_test +all: rtti_test map_test pow_test 128_test rtti_test: $(ODIN) run test_rtti.odin -file -vet -strict-style -o:minimal @@ -9,4 +9,7 @@ map_test: $(ODIN) run test_map.odin -file -vet -strict-style -o:minimal pow_test: - $(ODIN) run test_pow.odin -file -vet -strict-style -o:minimal \ No newline at end of file + $(ODIN) run test_pow.odin -file -vet -strict-style -o:minimal + +128_test: + $(ODIN) run test_128.odin -file -vet -strict-style -o:minimal diff --git a/tests/internal/build.bat b/tests/internal/build.bat index f289d17fa..da4fe890d 100644 --- a/tests/internal/build.bat +++ b/tests/internal/build.bat @@ -3,4 +3,6 @@ set PATH_TO_ODIN==..\..\odin rem %PATH_TO_ODIN% run test_rtti.odin -file -vet -strict-style -o:minimal || exit /b %PATH_TO_ODIN% run test_map.odin -file -vet -strict-style -o:minimal || exit /b rem -define:SEED=42 -%PATH_TO_ODIN% run test_pow.odin -file -vet -strict-style -o:minimal || exit /b \ No newline at end of file +%PATH_TO_ODIN% run test_pow.odin -file -vet -strict-style -o:minimal || exit /b + +%PATH_TO_ODIN% run test_128.odin -file -vet -strict-style -o:minimal || exit /b diff --git a/tests/internal/test_128.odin b/tests/internal/test_128.odin new file mode 100644 index 000000000..11ef068ed --- /dev/null +++ b/tests/internal/test_128.odin @@ -0,0 +1,59 @@ +package test_128 + +import "core:fmt" +import "core:os" +import "core:testing" + +TEST_count := 0 +TEST_fail := 0 + +when ODIN_TEST { + expect :: testing.expect + log :: testing.log +} else { + expect :: proc(t: ^testing.T, condition: bool, message: string, loc := #caller_location) { + TEST_count += 1 + if !condition { + TEST_fail += 1 + fmt.printf("[%v] %v\n", loc, message) + return + } + } + log :: proc(t: ^testing.T, v: any, loc := #caller_location) { + fmt.printf("[%v] ", loc) + fmt.printf("log: %v\n", v) + } +} + +main :: proc() { + t := testing.T{} + + test_128_align(&t) + + fmt.printf("%v/%v tests successful.\n", TEST_count - TEST_fail, TEST_count) + if TEST_fail > 0 { + os.exit(1) + } +} + +@test +test_128_align :: proc(t: ^testing.T) { + Danger_Struct :: struct { + x: u128, + y: u64, + } + + list := [?]Danger_Struct{{0, 0}, {1, 0}, {2, 0}, {3, 0}} + + expect(t, list[0].x == 0, fmt.tprintf("[0].x (%v) != 0", list[0].x)) + expect(t, list[0].y == 0, fmt.tprintf("[0].y (%v) != 0", list[0].y)) + + expect(t, list[1].x == 1, fmt.tprintf("[1].x (%v) != 1", list[1].x)) + expect(t, list[1].y == 0, fmt.tprintf("[1].y (%v) != 0", list[1].y)) + + expect(t, list[2].x == 2, fmt.tprintf("[2].x (%v) != 2", list[2].x)) + expect(t, list[2].y == 0, fmt.tprintf("[2].y (%v) != 0", list[2].y)) + + expect(t, list[3].x == 3, fmt.tprintf("[3].x (%v) != 3", list[3].x)) + expect(t, list[3].y == 0, fmt.tprintf("[3].y (%v) != 0", list[3].y)) +} -- cgit v1.2.3 From 133b45d843657215bbb4121d0d361134b2731255 Mon Sep 17 00:00:00 2001 From: Laytan Laats Date: Thu, 4 Apr 2024 23:30:06 +0200 Subject: fix amd64 sysv abi to pass asan everywhere I verified the PR by running the entire test suite of Odin itself with `-sanitize:address` and also the ols test suite (which caused unique problems before). A test has also been added with some problematic code, Windows seems to have problems with asan in CI or in general so it is not ran there. The LB_ABI_COMPUTE_RETURN_TYPES block has been removed entirely because it was unused, I got pretty confused why it didn't effect anything at first. Fixes #3211 --- src/llvm_abi.cpp | 47 +++++++++++--------------------- tests/internal/Makefile | 5 +++- tests/internal/test_asan.odin | 62 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 81 insertions(+), 33 deletions(-) create mode 100644 tests/internal/test_asan.odin (limited to 'src/llvm_abi.cpp') diff --git a/src/llvm_abi.cpp b/src/llvm_abi.cpp index 4cee0e443..88bb58c55 100644 --- a/src/llvm_abi.cpp +++ b/src/llvm_abi.cpp @@ -558,7 +558,6 @@ namespace lbAbiAmd64SysV { Amd64TypeAttribute_StructRect, }; - gb_internal LB_ABI_COMPUTE_RETURN_TYPE(compute_return_type); gb_internal void classify_with(LLVMTypeRef t, Array *cls, i64 ix, i64 off); gb_internal void fixup(LLVMTypeRef t, Array *cls); gb_internal lbArgType amd64_type(LLVMContextRef c, LLVMTypeRef type, Amd64TypeAttributeKind attribute_kind, ProcCallingConvention calling_convention); @@ -796,10 +795,10 @@ namespace lbAbiAmd64SysV { } } + i64 sz = lb_sizeof(type); if (all_ints) { - i64 sz = lb_sizeof(type); for_array(i, reg_classes) { - GB_ASSERT(sz != 0); + GB_ASSERT(sz > 0); // TODO(bill): is this even correct? BECAUSE LLVM DOES NOT DOCUMENT ANY OF THIS!!! if (sz >= 8) { array_add(&types, LLVMIntTypeInContext(c, 64)); @@ -811,12 +810,16 @@ namespace lbAbiAmd64SysV { } } else { for (isize i = 0; i < reg_classes.count; /**/) { + GB_ASSERT(sz > 0); RegClass reg_class = reg_classes[i]; switch (reg_class) { case RegClass_Int: - // TODO(bill): is this even correct? BECAUSE LLVM DOES NOT DOCUMENT ANY OF THIS!!! - array_add(&types, LLVMIntTypeInContext(c, 64)); - break; + { + i64 rs = gb_min(sz, 8); + array_add(&types, LLVMIntTypeInContext(c, cast(unsigned)(rs*8))); + sz -= rs; + break; + } case RegClass_SSEFv: case RegClass_SSEDv: case RegClass_SSEInt8: @@ -856,15 +859,18 @@ namespace lbAbiAmd64SysV { unsigned vec_len = llvec_len(reg_classes, i+1); LLVMTypeRef vec_type = LLVMVectorType(elem_type, vec_len * elems_per_word); array_add(&types, vec_type); + sz -= lb_sizeof(vec_type); i += vec_len; continue; } break; case RegClass_SSEFs: array_add(&types, LLVMFloatTypeInContext(c)); + sz -= 4; break; case RegClass_SSEDs: array_add(&types, LLVMDoubleTypeInContext(c)); + sz -= 8; break; default: GB_PANIC("Unhandled RegClass"); @@ -876,8 +882,8 @@ namespace lbAbiAmd64SysV { if (types.count == 1) { return types[0]; } - // TODO(bill): this should be packed but it causes code generation issues - return LLVMStructTypeInContext(c, types.data, cast(unsigned)types.count, false); + + return LLVMStructTypeInContext(c, types.data, cast(unsigned)types.count, sz == 0); } gb_internal void classify_with(LLVMTypeRef t, Array *cls, i64 ix, i64 off) { @@ -980,28 +986,6 @@ namespace lbAbiAmd64SysV { break; } } - - gb_internal LB_ABI_COMPUTE_RETURN_TYPE(compute_return_type) { - if (!return_is_defined) { - return lb_arg_type_direct(LLVMVoidTypeInContext(c)); - } else if (lb_is_type_kind(return_type, LLVMStructTypeKind)) { - i64 sz = lb_sizeof(return_type); - switch (sz) { - case 1: return lb_arg_type_direct(return_type, LLVMIntTypeInContext(c, 8), nullptr, nullptr); - case 2: return lb_arg_type_direct(return_type, LLVMIntTypeInContext(c, 16), nullptr, nullptr); - case 4: return lb_arg_type_direct(return_type, LLVMIntTypeInContext(c, 32), nullptr, nullptr); - case 8: return lb_arg_type_direct(return_type, LLVMIntTypeInContext(c, 64), nullptr, nullptr); - } - - 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); - } else if (build_context.metrics.os == TargetOs_windows && lb_is_type_kind(return_type, LLVMIntegerTypeKind) && lb_sizeof(return_type) == 16) { - return lb_arg_type_direct(return_type, LLVMIntTypeInContext(c, 128), nullptr, nullptr); - } - return non_struct(c, return_type); - } }; @@ -1166,8 +1150,7 @@ namespace lbAbiArm64 { size_copy -= 8; } GB_ASSERT(size_copy <= 0); - // TODO(bill): this should be packed but it causes code generation issues - cast_type = LLVMStructTypeInContext(c, types, count, false); + cast_type = LLVMStructTypeInContext(c, types, count, true); } return lb_arg_type_direct(return_type, cast_type, nullptr, nullptr); } else { diff --git a/tests/internal/Makefile b/tests/internal/Makefile index daefd5959..c5c612cdd 100644 --- a/tests/internal/Makefile +++ b/tests/internal/Makefile @@ -1,6 +1,6 @@ ODIN=../../odin -all: rtti_test map_test pow_test 128_test +all: rtti_test map_test pow_test 128_test asan_test rtti_test: $(ODIN) run test_rtti.odin -file -vet -strict-style -o:minimal @@ -13,3 +13,6 @@ pow_test: 128_test: $(ODIN) run test_128.odin -file -vet -strict-style -o:minimal + +asan_test: + $(ODIN) run test_asan.odin -file -sanitize:address -debug diff --git a/tests/internal/test_asan.odin b/tests/internal/test_asan.odin new file mode 100644 index 000000000..2384ada76 --- /dev/null +++ b/tests/internal/test_asan.odin @@ -0,0 +1,62 @@ +// Intended to contain code that would trigger asan easily if the abi was set up badly. +package test_asan + +import "core:fmt" +import "core:testing" +import "core:os" + +TEST_count := 0 +TEST_fail := 0 + +when ODIN_TEST { + expect :: testing.expect + log :: testing.log +} else { + expect :: proc(t: ^testing.T, condition: bool, message: string, loc := #caller_location) { + TEST_count += 1 + if !condition { + TEST_fail += 1 + fmt.printf("[%v] %v\n", loc, message) + return + } + } + log :: proc(t: ^testing.T, v: any, loc := #caller_location) { + fmt.printf("[%v] ", loc) + fmt.printf("log: %v\n", v) + } +} + +main :: proc() { + t := testing.T{} + + test_12_bytes(&t) + test_12_bytes_two(&t) + + fmt.printf("%v/%v tests successful.\n", TEST_count - TEST_fail, TEST_count) + if TEST_fail > 0 { + os.exit(1) + } +} + +@(test) +test_12_bytes :: proc(t: ^testing.T) { + internal :: proc() -> (a, b: f32, ok: bool) { + return max(f32), 0, true + } + + a, b, ok := internal() + expect(t, a == max(f32), fmt.tprintf("a (%v) != max(f32)", a)) + expect(t, b == 0, fmt.tprintf("b (%v) != 0", b)) + expect(t, ok, fmt.tprintf("ok (%v) != true", ok)) +} + +@(test) +test_12_bytes_two :: proc(t: ^testing.T) { + internal :: proc() -> (a: f32, b: int) { + return 100., max(int) + } + + a, b := internal() + expect(t, a == 100., fmt.tprintf("a (%v) != 100.", a)) + expect(t, b == max(int), fmt.tprintf("b (%v) != max(int)", b)) +} -- cgit v1.2.3