From 353c19468df02db925a7b147cabb398d7e63de68 Mon Sep 17 00:00:00 2001 From: xavier Date: Mon, 3 May 2021 08:50:47 +0200 Subject: [PATCH 1/3] test: add a standalone test for mixing c, zig, threadlocal and build modes. based on https://github.com/ziglang/zig/issues/8531 --- test/standalone.zig | 6 +++++ test/standalone/mix_c_files/build.zig | 34 +++++++++++++++++++++++++++ test/standalone/mix_c_files/main.zig | 30 +++++++++++++++++++++++ test/standalone/mix_c_files/test.c | 8 +++++++ 4 files changed, 78 insertions(+) create mode 100644 test/standalone/mix_c_files/build.zig create mode 100644 test/standalone/mix_c_files/main.zig create mode 100644 test/standalone/mix_c_files/test.c diff --git a/test/standalone.zig b/test/standalone.zig index f0a783a8a9..17194471a7 100644 --- a/test/standalone.zig +++ b/test/standalone.zig @@ -13,6 +13,12 @@ pub fn addCases(cases: *tests.StandaloneContext) void { cases.addBuildFile("test/standalone/main_pkg_path/build.zig", .{}); cases.addBuildFile("test/standalone/shared_library/build.zig", .{}); cases.addBuildFile("test/standalone/mix_o_files/build.zig", .{}); + if (std.Target.current.os.tag == .macos) { + // TODO zld cannot link llvm-ir object files for LTO yet. + cases.addBuildFile("test/standalone/mix_c_files/build.zig", .{ .build_modes = false, .cross_targets = true }); + } else { + cases.addBuildFile("test/standalone/mix_c_files/build.zig", .{ .build_modes = true, .cross_targets = true }); + } cases.addBuildFile("test/standalone/global_linkage/build.zig", .{}); cases.addBuildFile("test/standalone/static_c_lib/build.zig", .{}); cases.addBuildFile("test/standalone/link_interdependent_static_c_libs/build.zig", .{}); diff --git a/test/standalone/mix_c_files/build.zig b/test/standalone/mix_c_files/build.zig new file mode 100644 index 0000000000..614dc8a7ab --- /dev/null +++ b/test/standalone/mix_c_files/build.zig @@ -0,0 +1,34 @@ +const std = @import("std"); +const Builder = std.build.Builder; +const CrossTarget = std.zig.CrossTarget; + +fn isUnpecifiedTarget(t: CrossTarget) bool { + return t.cpu_arch == null and t.abi == null and t.os_tag == null; +} +fn isRunnableTarget(t: CrossTarget) bool { + if (t.isNative()) return true; + + return (t.getOsTag() == std.Target.current.os.tag and + t.getCpuArch() == std.Target.current.cpu.arch); +} + +pub fn build(b: *Builder) void { + const mode = b.standardReleaseOptions(); + const target = b.standardTargetOptions(.{}); + + const test_step = b.step("test", "Test the program"); + + const exe = b.addExecutable("test", "main.zig"); + exe.addCSourceFile("test.c", &[_][]const u8{"-std=c11"}); + exe.setBuildMode(mode); + exe.linkLibC(); + exe.setTarget(target); + b.default_step.dependOn(&exe.step); + + if (isRunnableTarget(target)) { + const run_cmd = exe.run(); + test_step.dependOn(&run_cmd.step); + } else { + test_step.dependOn(&exe.step); + } +} diff --git a/test/standalone/mix_c_files/main.zig b/test/standalone/mix_c_files/main.zig new file mode 100644 index 0000000000..913d284fe9 --- /dev/null +++ b/test/standalone/mix_c_files/main.zig @@ -0,0 +1,30 @@ +const std = @import("std"); + +extern fn add_C(x: i32) i32; +extern fn add_C_zig(x: i32) i32; +extern threadlocal var C_k: c_int; + +export var zig_k: c_int = 1; +export fn add_zig(x: i32) i32 { + return x + zig_k + C_k; +} +export fn add_may_panic(x: i32) i32 { + if (x < 0) @panic("negative int"); + return x + zig_k; +} + +pub fn main() anyerror!void { + var x: i32 = 0; + x = add_zig(x); + x = add_C(x); + x = add_C_zig(x); + + C_k = 200; + zig_k = 2; + x = add_zig(x); + x = add_C(x); + x = add_C_zig(x); + + const u = @intCast(u32, x); + try std.testing.expect(u / 100 == u % 100); +} diff --git a/test/standalone/mix_c_files/test.c b/test/standalone/mix_c_files/test.c new file mode 100644 index 0000000000..e8ea24ca96 --- /dev/null +++ b/test/standalone/mix_c_files/test.c @@ -0,0 +1,8 @@ + +extern int zig_k; +extern int add_may_panic(int); + +_Thread_local int C_k = 100; +int unused(int x) { return x*x; } +int add_C(int x) { return x+zig_k+C_k; } +int add_C_zig(int x) { return add_may_panic(x) + C_k; } From 564629f70475a1e3b397179067e22d3f61772104 Mon Sep 17 00:00:00 2001 From: xavier Date: Sun, 2 May 2021 16:46:46 +0200 Subject: [PATCH 2/3] build: workaround link error with LTO and mingw The symbol "_tls_index" gets lost when using LTO. Disabling LTO on the object file that defines it allows the link to work. fixes https://github.com/ziglang/zig/issues/8531 --- src/mingw.zig | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/mingw.zig b/src/mingw.zig index c10dcbe3f0..b9d82ba784 100644 --- a/src/mingw.zig +++ b/src/mingw.zig @@ -92,6 +92,10 @@ pub fn buildCRTFile(comp: *Compilation, crt_file: CRTFile) !void { "-D_WIN32_WINNT=0x0f00", "-D__MSVCRT_VERSION__=0x700", }); + if (std.mem.eql(u8, dep, "tlssup.c")) { + // Can't let LTO drop symbols defined in this file (eg: _tls_index) + try args.append("-fno-lto"); + } c_source_files[i] = .{ .src_path = try comp.zig_lib_directory.join(arena, &[_][]const u8{ "libc", "mingw", "crt", dep, From 08c768ad82280463410eacc5b7f19389cb1ba49b Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Mon, 15 Nov 2021 16:32:15 -0700 Subject: [PATCH 3/3] pre-merge cleanups * Annotate workarounds with their corresponding GitHub issue links. * Enable test coverage for LTO on Windows with the added c_compiler test. --- src/mingw.zig | 6 +++-- test/standalone.zig | 32 ++++++++++++++++++++------- test/standalone/c_compiler/build.zig | 6 ++--- test/standalone/mix_c_files/build.zig | 12 +++++----- 4 files changed, 35 insertions(+), 21 deletions(-) diff --git a/src/mingw.zig b/src/mingw.zig index b9d82ba784..8738bcbbcd 100644 --- a/src/mingw.zig +++ b/src/mingw.zig @@ -92,8 +92,10 @@ pub fn buildCRTFile(comp: *Compilation, crt_file: CRTFile) !void { "-D_WIN32_WINNT=0x0f00", "-D__MSVCRT_VERSION__=0x700", }); - if (std.mem.eql(u8, dep, "tlssup.c")) { - // Can't let LTO drop symbols defined in this file (eg: _tls_index) + if (std.mem.eql(u8, dep, "tlssup.c") and comp.bin_file.options.lto) { + // LLD will incorrectly drop the `_tls_index` symbol. Here we work + // around it by not using LTO for this one file. + // https://github.com/ziglang/zig/issues/8531 try args.append("-fno-lto"); } c_source_files[i] = .{ diff --git a/test/standalone.zig b/test/standalone.zig index 17194471a7..35d30fafca 100644 --- a/test/standalone.zig +++ b/test/standalone.zig @@ -13,18 +13,27 @@ pub fn addCases(cases: *tests.StandaloneContext) void { cases.addBuildFile("test/standalone/main_pkg_path/build.zig", .{}); cases.addBuildFile("test/standalone/shared_library/build.zig", .{}); cases.addBuildFile("test/standalone/mix_o_files/build.zig", .{}); - if (std.Target.current.os.tag == .macos) { - // TODO zld cannot link llvm-ir object files for LTO yet. - cases.addBuildFile("test/standalone/mix_c_files/build.zig", .{ .build_modes = false, .cross_targets = true }); + if (builtin.os.tag == .macos) { + // Zig's macOS linker does not yet support LTO for LLVM IR files: + // https://github.com/ziglang/zig/issues/8680 + cases.addBuildFile("test/standalone/mix_c_files/build.zig", .{ + .build_modes = false, + .cross_targets = true, + }); } else { - cases.addBuildFile("test/standalone/mix_c_files/build.zig", .{ .build_modes = true, .cross_targets = true }); + cases.addBuildFile("test/standalone/mix_c_files/build.zig", .{ + .build_modes = true, + .cross_targets = true, + }); } cases.addBuildFile("test/standalone/global_linkage/build.zig", .{}); cases.addBuildFile("test/standalone/static_c_lib/build.zig", .{}); cases.addBuildFile("test/standalone/link_interdependent_static_c_libs/build.zig", .{}); cases.addBuildFile("test/standalone/link_static_lib_as_system_lib/build.zig", .{}); cases.addBuildFile("test/standalone/link_common_symbols/build.zig", .{}); - cases.addBuildFile("test/standalone/link_frameworks/build.zig", .{ .requires_macos_sdk = true }); + cases.addBuildFile("test/standalone/link_frameworks/build.zig", .{ + .requires_macos_sdk = true, + }); cases.addBuildFile("test/standalone/issue_339/build.zig", .{}); cases.addBuildFile("test/standalone/issue_8550/build.zig", .{}); cases.addBuildFile("test/standalone/issue_794/build.zig", .{}); @@ -39,10 +48,14 @@ pub fn addCases(cases: *tests.StandaloneContext) void { if (builtin.os.tag != .wasi) { cases.addBuildFile("test/standalone/load_dynamic_library/build.zig", .{}); } - if (builtin.cpu.arch == .x86_64) { // TODO add C ABI support for other architectures + // C ABI compatibility issue: https://github.com/ziglang/zig/issues/1481 + if (builtin.cpu.arch == .x86_64) { cases.addBuildFile("test/stage1/c_abi/build.zig", .{}); } - cases.addBuildFile("test/standalone/c_compiler/build.zig", .{ .build_modes = true, .cross_targets = true }); + cases.addBuildFile("test/standalone/c_compiler/build.zig", .{ + .build_modes = true, + .cross_targets = true, + }); if (builtin.os.tag == .windows) { cases.addC("test/standalone/issue_9402/main.zig"); @@ -52,7 +65,10 @@ pub fn addCases(cases: *tests.StandaloneContext) void { cases.addBuildFile("test/standalone/pie/build.zig", .{}); } // Try to build and run an Objective-C executable. - cases.addBuildFile("test/standalone/objc/build.zig", .{ .build_modes = true, .requires_macos_sdk = true }); + cases.addBuildFile("test/standalone/objc/build.zig", .{ + .build_modes = true, + .requires_macos_sdk = true, + }); // Ensure the development tools are buildable. cases.add("tools/gen_spirv_spec.zig"); diff --git a/test/standalone/c_compiler/build.zig b/test/standalone/c_compiler/build.zig index c1efb160ef..aa7217976e 100644 --- a/test/standalone/c_compiler/build.zig +++ b/test/standalone/c_compiler/build.zig @@ -3,6 +3,7 @@ const builtin = @import("builtin"); const Builder = std.build.Builder; const CrossTarget = std.zig.CrossTarget; +// TODO integrate this with the std.build executor API fn isRunnableTarget(t: CrossTarget) bool { if (t.isNative()) return true; @@ -30,12 +31,9 @@ pub fn build(b: *Builder) void { exe_cpp.setTarget(target); exe_cpp.linkSystemLibrary("c++"); - // disable broken LTO links: switch (target.getOsTag()) { - .windows => { - exe_cpp.want_lto = false; - }, .macos => { + // https://github.com/ziglang/zig/issues/8680 exe_cpp.want_lto = false; exe_c.want_lto = false; }, diff --git a/test/standalone/mix_c_files/build.zig b/test/standalone/mix_c_files/build.zig index 614dc8a7ab..68486ea18d 100644 --- a/test/standalone/mix_c_files/build.zig +++ b/test/standalone/mix_c_files/build.zig @@ -1,23 +1,20 @@ const std = @import("std"); +const builtin = @import("builtin"); const Builder = std.build.Builder; const CrossTarget = std.zig.CrossTarget; -fn isUnpecifiedTarget(t: CrossTarget) bool { - return t.cpu_arch == null and t.abi == null and t.os_tag == null; -} +// TODO integrate this with the std.build executor API fn isRunnableTarget(t: CrossTarget) bool { if (t.isNative()) return true; - return (t.getOsTag() == std.Target.current.os.tag and - t.getCpuArch() == std.Target.current.cpu.arch); + return (t.getOsTag() == builtin.os.tag and + t.getCpuArch() == builtin.cpu.arch); } pub fn build(b: *Builder) void { const mode = b.standardReleaseOptions(); const target = b.standardTargetOptions(.{}); - const test_step = b.step("test", "Test the program"); - const exe = b.addExecutable("test", "main.zig"); exe.addCSourceFile("test.c", &[_][]const u8{"-std=c11"}); exe.setBuildMode(mode); @@ -25,6 +22,7 @@ pub fn build(b: *Builder) void { exe.setTarget(target); b.default_step.dependOn(&exe.step); + const test_step = b.step("test", "Test the program"); if (isRunnableTarget(target)) { const run_cmd = exe.run(); test_step.dependOn(&run_cmd.step);