From a127693f950e1abb1b2b31f5bb74d1ba2b0fa532 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Tue, 26 Jul 2022 20:04:24 -0700 Subject: [PATCH 1/9] start code: enable segfault handler for stage2 --- lib/std/start.zig | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lib/std/start.zig b/lib/std/start.zig index 788c979d48..8ea261be2d 100644 --- a/lib/std/start.zig +++ b/lib/std/start.zig @@ -455,10 +455,6 @@ fn callMainWithArgs(argc: usize, argv: [*][*:0]u8, envp: [][*:0]u8) u8 { std.os.argv = argv[0..argc]; std.os.environ = envp; - if (builtin.zig_backend == .stage2_llvm) { - return @call(.{ .modifier = .always_inline }, callMain, .{}); - } - std.debug.maybeEnableSegfaultHandler(); return initEventLoopAndCallMain(); From 0bc4726e00e794be9848b4b42dba43a0c7f4558e Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Tue, 26 Jul 2022 20:05:06 -0700 Subject: [PATCH 2/9] LLVM: add probe-stack function attribute --- src/codegen/llvm.zig | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/codegen/llvm.zig b/src/codegen/llvm.zig index b4bf8449ce..e7915e08b3 100644 --- a/src/codegen/llvm.zig +++ b/src/codegen/llvm.zig @@ -673,6 +673,7 @@ pub const Object = struct { ) !void { const decl_index = func.owner_decl; const decl = module.declPtr(decl_index); + const target = module.getTarget(); var dg: DeclGen = .{ .context = o.context, @@ -706,6 +707,17 @@ pub const Object = struct { DeclGen.removeFnAttr(llvm_func, "noinline"); } + // TODO: port these over from stage1 + // addLLVMFnAttr(llvm_fn, "sspstrong"); + // addLLVMFnAttrStr(llvm_fn, "stack-protector-buffer-size", "4"); + + // TODO: disable this if safety is off for the function scope + if (module.comp.bin_file.options.stack_check) { + dg.addFnAttrString(llvm_func, "probe-stack", "__zig_probe_stack"); + } else if (target.os.tag == .uefi) { + dg.addFnAttrString(llvm_func, "no-stack-arg-probe", ""); + } + // Remove all the basic blocks of a function in order to start over, generating // LLVM IR from an empty function body. while (llvm_func.getFirstBasicBlock()) |bb| { @@ -719,7 +731,6 @@ pub const Object = struct { // This gets the LLVM values from the function and stores them in `dg.args`. const fn_info = decl.ty.fnInfo(); - const target = dg.module.getTarget(); const sret = firstParamSRet(fn_info, target); const ret_ptr = if (sret) llvm_func.getParam(0) else null; const gpa = dg.gpa; @@ -730,7 +741,7 @@ pub const Object = struct { }; const err_return_tracing = fn_info.return_type.isError() and - dg.module.comp.bin_file.options.error_return_tracing; + module.comp.bin_file.options.error_return_tracing; const err_ret_trace = if (err_return_tracing) llvm_func.getParam(@boolToInt(ret_ptr != null)) @@ -920,7 +931,7 @@ pub const Object = struct { const line_number = decl.src_line + 1; const is_internal_linkage = decl.val.tag() != .extern_fn and - !dg.module.decl_exports.contains(decl_index); + !module.decl_exports.contains(decl_index); const noret_bit: c_uint = if (fn_info.return_type.isNoReturn()) llvm.DIFlags.NoReturn else @@ -936,7 +947,7 @@ pub const Object = struct { true, // is definition line_number + func.lbrace_line, // scope line llvm.DIFlags.StaticMember | noret_bit, - dg.module.comp.bin_file.options.optimize_mode != .Debug, + module.comp.bin_file.options.optimize_mode != .Debug, null, // decl_subprogram ); try dg.object.di_map.put(gpa, decl, subprogram.toNode()); From ea3db3274d890b7d00c907c037ffe203f41adbd3 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Tue, 26 Jul 2022 20:05:54 -0700 Subject: [PATCH 3/9] link: avoid passing bad ptrs to pwritev At least on Linux, the pwritev syscall checks the pointer and returns EFAULT before it checks if the length is nonzero. Perhaps this should be fixed in the standard library, however, these are still improvements since they make the kernel do less work within the syscall. --- src/link/C.zig | 75 ++++++++++++++++++++++++++++------------------ src/link/Dwarf.zig | 4 +-- 2 files changed, 48 insertions(+), 31 deletions(-) diff --git a/src/link/C.zig b/src/link/C.zig index 4159a577d2..044f08c94d 100644 --- a/src/link/C.zig +++ b/src/link/C.zig @@ -263,11 +263,13 @@ pub fn flushModule(self: *C, comp: *Compilation, prog_node: *std.Progress.Node) // Covers zig.h and err_typedef_item. try f.all_buffers.ensureUnusedCapacity(gpa, 2); - f.all_buffers.appendAssumeCapacity(.{ - .iov_base = zig_h, - .iov_len = zig_h.len, - }); - f.file_size += zig_h.len; + if (zig_h.len != 0) { + f.all_buffers.appendAssumeCapacity(.{ + .iov_base = zig_h, + .iov_len = zig_h.len, + }); + f.file_size += zig_h.len; + } const err_typedef_writer = f.err_typedef_buf.writer(gpa); const err_typedef_index = f.all_buffers.items.len; @@ -301,11 +303,18 @@ pub fn flushModule(self: *C, comp: *Compilation, prog_node: *std.Progress.Node) try flushDecl(self, &f, decl_index); } - f.all_buffers.items[err_typedef_index] = .{ - .iov_base = f.err_typedef_buf.items.ptr, - .iov_len = f.err_typedef_buf.items.len, - }; - f.file_size += f.err_typedef_buf.items.len; + if (f.err_typedef_buf.items.len == 0) { + f.all_buffers.items[err_typedef_index] = .{ + .iov_base = "", + .iov_len = 0, + }; + } else { + f.all_buffers.items[err_typedef_index] = .{ + .iov_base = f.err_typedef_buf.items.ptr, + .iov_len = f.err_typedef_buf.items.len, + }; + f.file_size += f.err_typedef_buf.items.len; + } // Now the function bodies. try f.all_buffers.ensureUnusedCapacity(gpa, f.fn_count); @@ -391,21 +400,25 @@ fn flushDecl(self: *C, f: *Flush, decl_index: Module.Decl.Index) FlushDeclError! if (decl_block.fwd_decl.items.len != 0) { const buf = decl_block.fwd_decl.items; - try f.all_buffers.append(gpa, .{ - .iov_base = buf.ptr, - .iov_len = buf.len, - }); - f.file_size += buf.len; + if (buf.len != 0) { + try f.all_buffers.append(gpa, .{ + .iov_base = buf.ptr, + .iov_len = buf.len, + }); + f.file_size += buf.len; + } } if (decl.getFunction() != null) { f.fn_count += 1; } else if (decl_block.code.items.len != 0) { const buf = decl_block.code.items; - try f.all_buffers.append(gpa, .{ - .iov_base = buf.ptr, - .iov_len = buf.len, - }); - f.file_size += buf.len; + if (buf.len != 0) { + try f.all_buffers.append(gpa, .{ + .iov_base = buf.ptr, + .iov_len = buf.len, + }); + f.file_size += buf.len; + } } } @@ -421,19 +434,23 @@ pub fn flushEmitH(module: *Module) !void { defer all_buffers.deinit(); var file_size: u64 = zig_h.len; - all_buffers.appendAssumeCapacity(.{ - .iov_base = zig_h, - .iov_len = zig_h.len, - }); + if (zig_h.len != 0) { + all_buffers.appendAssumeCapacity(.{ + .iov_base = zig_h, + .iov_len = zig_h.len, + }); + } for (emit_h.decl_table.keys()) |decl_index| { const decl_emit_h = emit_h.declPtr(decl_index); const buf = decl_emit_h.fwd_decl.items; - all_buffers.appendAssumeCapacity(.{ - .iov_base = buf.ptr, - .iov_len = buf.len, - }); - file_size += buf.len; + if (buf.len != 0) { + all_buffers.appendAssumeCapacity(.{ + .iov_base = buf.ptr, + .iov_len = buf.len, + }); + file_size += buf.len; + } } const directory = emit_h.loc.directory orelse module.comp.local_cache_directory; diff --git a/src/link/Dwarf.zig b/src/link/Dwarf.zig index efbd86bc7f..03ba53801b 100644 --- a/src/link/Dwarf.zig +++ b/src/link/Dwarf.zig @@ -1752,7 +1752,7 @@ fn pwriteDbgLineNops( .iov_base = buf.ptr, .iov_len = buf.len, }; - vec_index += 1; + if (buf.len > 0) vec_index += 1; { var padding_left = next_padding_size; @@ -1861,7 +1861,7 @@ fn pwriteDbgInfoNops( .iov_base = buf.ptr, .iov_len = buf.len, }; - vec_index += 1; + if (buf.len > 0) vec_index += 1; { var padding_left = next_padding_size; From 4e53249d769346683d45d4a4cb4a437388f9e186 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Tue, 26 Jul 2022 20:09:48 -0700 Subject: [PATCH 4/9] test-cases harness: improve stage2 compatibility * proper skip_stage1 mechanism that doesn't get side-stepped with manually added test cases. * avoid runtime-known function pointers. * check for type equality more simply without checking the type name. --- src/test.zig | 53 ++++++++++++++++++++-------------------------------- 1 file changed, 20 insertions(+), 33 deletions(-) diff --git a/src/test.zig b/src/test.zig index 39061a98ff..08d8ae188a 100644 --- a/src/test.zig +++ b/src/test.zig @@ -20,7 +20,7 @@ const enable_wasmtime: bool = build_options.enable_wasmtime; const enable_darling: bool = build_options.enable_darling; const enable_rosetta: bool = build_options.enable_rosetta; const glibc_runtimes_dir: ?[]const u8 = build_options.glibc_runtimes_dir; -const skip_stage1 = build_options.skip_stage1; +const skip_stage1 = builtin.zig_backend != .stage1 or build_options.skip_stage1; const hr = "=" ** 80; @@ -233,11 +233,11 @@ const TestManifest = struct { fn ConfigValueIterator(comptime T: type) type { return struct { inner: std.mem.SplitIterator(u8), - parse_fn: ParseFn(T), fn next(self: *@This()) !?T { const next_raw = self.inner.next() orelse return null; - return try self.parse_fn(next_raw); + const parseFn = getDefaultParser(T); + return try parseFn(next_raw); } }; } @@ -313,25 +313,15 @@ const TestManifest = struct { return manifest; } - fn getConfigForKeyCustomParser( - self: TestManifest, - key: []const u8, - comptime T: type, - parse_fn: ParseFn(T), - ) ConfigValueIterator(T) { - const bytes = self.config_map.get(key) orelse TestManifestConfigDefaults.get(self.@"type", key); - return ConfigValueIterator(T){ - .inner = std.mem.split(u8, bytes, ","), - .parse_fn = parse_fn, - }; - } - fn getConfigForKey( self: TestManifest, key: []const u8, comptime T: type, ) ConfigValueIterator(T) { - return self.getConfigForKeyCustomParser(key, T, getDefaultParser(T)); + const bytes = self.config_map.get(key) orelse TestManifestConfigDefaults.get(self.@"type", key); + return ConfigValueIterator(T){ + .inner = std.mem.split(u8, bytes, ","), + }; } fn getConfigForKeyAlloc( @@ -377,6 +367,15 @@ const TestManifest = struct { } fn getDefaultParser(comptime T: type) ParseFn(T) { + if (T == CrossTarget) return struct { + fn parse(str: []const u8) anyerror!T { + var opts = CrossTarget.ParseOptions{ + .arch_os_abi = str, + }; + return try CrossTarget.parse(opts); + } + }.parse; + switch (@typeInfo(T)) { .Int => return struct { fn parse(str: []const u8) anyerror!T { @@ -397,14 +396,7 @@ const TestManifest = struct { }; } }.parse, - .Struct => if (comptime std.mem.eql(u8, @typeName(T), "CrossTarget")) return struct { - fn parse(str: []const u8) anyerror!T { - var opts = CrossTarget.ParseOptions{ - .arch_os_abi = str, - }; - return try CrossTarget.parse(opts); - } - }.parse else @compileError("no default parser for " ++ @typeName(T)), + .Struct => @compileError("no default parser for " ++ @typeName(T)), else => @compileError("no default parser for " ++ @typeName(T)), } } @@ -884,8 +876,6 @@ pub const TestContext = struct { src: [:0]const u8, expected_errors: []const []const u8, ) void { - if (skip_stage1) return; - const case = ctx.addObj(name, .{}); case.backend = .stage1; case.addError(src, expected_errors); @@ -897,8 +887,6 @@ pub const TestContext = struct { src: [:0]const u8, expected_errors: []const []const u8, ) void { - if (skip_stage1) return; - const case = ctx.addTest(name, .{}); case.backend = .stage1; case.addError(src, expected_errors); @@ -910,8 +898,6 @@ pub const TestContext = struct { src: [:0]const u8, expected_errors: []const []const u8, ) void { - if (skip_stage1) return; - const case = ctx.addExe(name, .{}); case.backend = .stage1; case.addError(src, expected_errors); @@ -1143,8 +1129,6 @@ pub const TestContext = struct { // Cross-product to get all possible test combinations for (backends) |backend| { - if (backend == .stage1 and skip_stage1) continue; - for (targets) |target| { const name = try std.fmt.allocPrint(ctx.arena, "{s} ({s}, {s})", .{ name_prefix, @@ -1276,6 +1260,9 @@ pub const TestContext = struct { if (!build_options.have_llvm and case.backend == .llvm) continue; + if (skip_stage1 and case.backend == .stage1) + continue; + if (build_options.test_filter) |test_filter| { if (std.mem.indexOf(u8, case.name, test_filter) == null) continue; } From acf1aa10c2dcd485496c0db1d9c5378b2c23dd96 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Tue, 26 Jul 2022 20:11:52 -0700 Subject: [PATCH 5/9] test-cases harness: refresh just before update() This makes it so that in a -Dsingle-threaded build of test-cases, if a crash happens, the test case name will be printed just before the stderr of the crash. --- src/test.zig | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test.zig b/src/test.zig index 08d8ae188a..2f3f1618e0 100644 --- a/src/test.zig +++ b/src/test.zig @@ -1594,6 +1594,7 @@ pub const TestContext = struct { var module_node = update_node.start("parse/analysis/codegen", 0); module_node.activate(); + module_node.context.refresh(); try comp.makeBinFileWritable(); try comp.update(); module_node.end(); From bdaa915a02eefe2fb2e400069fcb79b41dcabb94 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Tue, 26 Jul 2022 20:12:40 -0700 Subject: [PATCH 6/9] test-cases: remove failing test This causes a stack overflow in a debug build of stage3 unfortunately. I will open an issue to track this test coverage, which we absolutely should get working - users of the compiler should get a compile error, not a segfault if they hit the default branch quota from abusing recursive inline functions. Note that the problem does not occur in a release build of stage3 which has significantly reduced stack usage. On Linux, I tried bumping up the stack size from 32 MiB to 64 MiB and it did not solve the problem. I'm not sure why not. It seems like it should be fine. Note that we also have a problem of running test-cases in multi-threaded mode which is currently the default. Currently Zig threads are spawned with 16 MiB stack space. --- .../endless_loop_in_function_evaluation.zig | 15 --------------- 1 file changed, 15 deletions(-) delete mode 100644 test/cases/compile_errors/endless_loop_in_function_evaluation.zig diff --git a/test/cases/compile_errors/endless_loop_in_function_evaluation.zig b/test/cases/compile_errors/endless_loop_in_function_evaluation.zig deleted file mode 100644 index 7616bfa5e7..0000000000 --- a/test/cases/compile_errors/endless_loop_in_function_evaluation.zig +++ /dev/null @@ -1,15 +0,0 @@ -const seventh_fib_number = fibonacci(7); -fn fibonacci(x: i32) i32 { - return fibonacci(x - 1) + fibonacci(x - 2); -} - -export fn entry() usize { return @sizeOf(@TypeOf(&seventh_fib_number)); } - -// error -// backend=stage2 -// target=native -// -// :3:21: error: evaluation exceeded 1000 backwards branches -// :3:21: note: use @setEvalBranchQuota() to raise the branch limit from 1000 -// :3:21: note: called from here (999 times) -// :1:37: note: called from here From 3de9ffa84de00f5c26f6b5404cb21ff91597efe4 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Tue, 26 Jul 2022 20:15:57 -0700 Subject: [PATCH 7/9] CI: run test-cases with stage3 See #12144 for why I did not `-Denable-llvm` yet. --- ci/zinc/linux_test.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ci/zinc/linux_test.sh b/ci/zinc/linux_test.sh index 0213b20751..41919c48f9 100755 --- a/ci/zinc/linux_test.sh +++ b/ci/zinc/linux_test.sh @@ -62,11 +62,12 @@ stage3/bin/zig build test-fmt -fqemu -fwasmtime -Denable-llvm stage3/bin/zig build test-translate-c -fqemu -fwasmtime -Denable-llvm stage3/bin/zig build test-standalone -fqemu -fwasmtime -Denable-llvm stage3/bin/zig build test-cli -fqemu -fwasmtime -Denable-llvm +# https://github.com/ziglang/zig/issues/12144 +stage3/bin/zig build test-cases -fqemu -fwasmtime $STAGE1_ZIG build test-stack-traces -fqemu -fwasmtime $STAGE1_ZIG build test-run-translated-c -fqemu -fwasmtime $STAGE1_ZIG build docs -fqemu -fwasmtime -$STAGE1_ZIG build test-cases -fqemu -fwasmtime $STAGE1_ZIG build test-link -fqemu -fwasmtime # Produce the experimental std lib documentation. From ffac6a1b9f00d89c238724854440943f67cee9bb Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Tue, 26 Jul 2022 20:23:33 -0700 Subject: [PATCH 8/9] CI: run test-link with stage3 --- ci/zinc/linux_test.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/zinc/linux_test.sh b/ci/zinc/linux_test.sh index 41919c48f9..e24e8c2d53 100755 --- a/ci/zinc/linux_test.sh +++ b/ci/zinc/linux_test.sh @@ -64,11 +64,11 @@ stage3/bin/zig build test-standalone -fqemu -fwasmtime -Denable-llvm stage3/bin/zig build test-cli -fqemu -fwasmtime -Denable-llvm # https://github.com/ziglang/zig/issues/12144 stage3/bin/zig build test-cases -fqemu -fwasmtime +stage3/bin/zig build test-link -fqemu -fwasmtime -Denable-llvm $STAGE1_ZIG build test-stack-traces -fqemu -fwasmtime $STAGE1_ZIG build test-run-translated-c -fqemu -fwasmtime $STAGE1_ZIG build docs -fqemu -fwasmtime -$STAGE1_ZIG build test-link -fqemu -fwasmtime # Produce the experimental std lib documentation. mkdir -p "$RELEASE_STAGING/docs/std" From 20c230cda955936a04af952f9773c0cdda10d9f4 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Wed, 27 Jul 2022 02:13:01 -0700 Subject: [PATCH 9/9] test-cases harness: annotate an optional type Not sure why this is needed by the CI; it's not needed locally. This is a mystery that will have to wait for another day. --- src/test.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test.zig b/src/test.zig index 2f3f1618e0..5f4107a402 100644 --- a/src/test.zig +++ b/src/test.zig @@ -1843,7 +1843,7 @@ pub const TestContext = struct { .qemu => |qemu_bin_name| if (enable_qemu) { const need_cross_glibc = target.isGnuLibC() and case.link_libc; - const glibc_dir_arg = if (need_cross_glibc) + const glibc_dir_arg: ?[]const u8 = if (need_cross_glibc) glibc_runtimes_dir orelse continue :update // glibc dir not available; pass test else null;