From d9490a4340366818f91fac428981fe662d552ca6 Mon Sep 17 00:00:00 2001 From: Jacob Young Date: Sat, 1 Oct 2022 03:51:36 -0400 Subject: [PATCH 1/5] Sema: avoid undefined fields in file struct decls Fixes original comment of #12399 --- src/Module.zig | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Module.zig b/src/Module.zig index 6056c385e3..37e693c275 100644 --- a/src/Module.zig +++ b/src/Module.zig @@ -4430,6 +4430,8 @@ pub fn semaFile(mod: *Module, file: *File) SemaError!void { new_decl.has_linksection_or_addrspace = false; new_decl.ty = ty_ty; new_decl.val = struct_val; + new_decl.@"align" = 0; + new_decl.@"linksection" = null; new_decl.has_tv = true; new_decl.owns_tv = true; new_decl.alive = true; // This Decl corresponds to a File and is therefore always alive. From 8b66443d5008be91756c3c5567548ed18766ea11 Mon Sep 17 00:00:00 2001 From: Jacob Young Date: Sat, 1 Oct 2022 04:01:35 -0400 Subject: [PATCH 2/5] llvm: avoid undefined values by ensuring the StackTrace decl is analyzed The test builds an object file to prevent StackTrace from already having been analyzed by other code. Fixes #13030 --- src/codegen/llvm.zig | 1 + test/standalone.zig | 2 ++ test/standalone/issue_13030/build.zig | 18 ++++++++++++++++++ test/standalone/issue_13030/main.zig | 8 ++++++++ 4 files changed, 29 insertions(+) create mode 100644 test/standalone/issue_13030/build.zig create mode 100644 test/standalone/issue_13030/main.zig diff --git a/src/codegen/llvm.zig b/src/codegen/llvm.zig index 1b793265da..ce8c169044 100644 --- a/src/codegen/llvm.zig +++ b/src/codegen/llvm.zig @@ -2335,6 +2335,7 @@ pub const Object = struct { const stack_trace_decl = builtin_namespace.decls .getKeyAdapted(stack_trace_str, Module.DeclAdapter{ .mod = mod }).?; + mod.ensureDeclAnalyzed(stack_trace_decl) catch unreachable; return mod.declPtr(stack_trace_decl).val.toType(undefined); } }; diff --git a/test/standalone.zig b/test/standalone.zig index aa6c0f0a14..d3dedb59e6 100644 --- a/test/standalone.zig +++ b/test/standalone.zig @@ -97,4 +97,6 @@ pub fn addCases(cases: *tests.StandaloneContext) void { // Disabled due to tripping LLVM 13 assertion: // https://github.com/ziglang/zig/issues/12015 //cases.add("tools/update_spirv_features.zig"); + + cases.addBuildFile("test/standalone/issue_13030/build.zig", .{ .build_modes = true }); } diff --git a/test/standalone/issue_13030/build.zig b/test/standalone/issue_13030/build.zig new file mode 100644 index 0000000000..8c05e47cf6 --- /dev/null +++ b/test/standalone/issue_13030/build.zig @@ -0,0 +1,18 @@ +const std = @import("std"); +const builtin = @import("builtin"); +const Builder = std.build.Builder; +const CrossTarget = std.zig.CrossTarget; + +pub fn build(b: *Builder) void { + const mode = b.standardReleaseOptions(); + const target = b.standardTargetOptions(.{}); + + const obj = b.addObject("main", "main.zig"); + obj.setBuildMode(mode); + + obj.setTarget(target); + b.default_step.dependOn(&obj.step); + + const test_step = b.step("test", "Test the program"); + test_step.dependOn(&obj.step); +} diff --git a/test/standalone/issue_13030/main.zig b/test/standalone/issue_13030/main.zig new file mode 100644 index 0000000000..95b3a67af2 --- /dev/null +++ b/test/standalone/issue_13030/main.zig @@ -0,0 +1,8 @@ +const std = @import("std"); +fn a() error{}!void {} +fn b() std.meta.FnPtr(fn () error{}!void) { + return &a; +} +export fn c() void { + _ = b(); +} From b7bd44a654671e76f15ed1a4e1226c0d7cc20d92 Mon Sep 17 00:00:00 2001 From: Jacob Young Date: Sat, 1 Oct 2022 08:05:06 -0400 Subject: [PATCH 3/5] Sema: ensure builtin.StackTrace fields are analyzed When encountering a fn type that returns an error (union), a backend that supports error return tracing will want the StackTrace struct and its fields to be analyzed. --- src/Sema.zig | 15 +++++++++++---- src/codegen/llvm.zig | 1 - test/standalone/issue_13030/main.zig | 9 ++++----- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/Sema.zig b/src/Sema.zig index aed09d6201..4e2d868c60 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -8107,6 +8107,13 @@ fn funcCommon( for (comptime_params) |ct| is_generic = is_generic or ct; is_generic = is_generic or ret_ty_requires_comptime; + if (!is_generic and sema.wantErrorReturnTracing(return_type)) { + // Make sure that StackTrace's fields are resolved so that the backend can + // lower this fn type. + const unresolved_stack_trace_ty = try sema.getBuiltinType(block, ret_ty_src, "StackTrace"); + _ = try sema.resolveTypeFields(block, ret_ty_src, unresolved_stack_trace_ty); + } + break :fn_ty try Type.Tag.function.create(sema.arena, .{ .param_types = param_types, .comptime_params = comptime_params.ptr, @@ -15631,7 +15638,7 @@ fn zirRetLoad(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Zir return sema.analyzeRet(block, operand, src); } - if (sema.wantErrorReturnTracing()) { + if (sema.wantErrorReturnTracing(sema.fn_ret_ty)) { const is_non_err = try sema.analyzePtrIsNonErr(block, src, ret_ptr); return retWithErrTracing(sema, block, src, is_non_err, .ret_load, ret_ptr); } @@ -15698,11 +15705,11 @@ fn retWithErrTracing( return always_noreturn; } -fn wantErrorReturnTracing(sema: *Sema) bool { +fn wantErrorReturnTracing(sema: *Sema, fn_ret_ty: Type) bool { // TODO implement this feature in all the backends and then delete this check. const backend_supports_error_return_tracing = sema.mod.comp.bin_file.options.use_llvm; - return sema.fn_ret_ty.isError() and + return fn_ret_ty.isError() and sema.mod.comp.bin_file.options.error_return_tracing and backend_supports_error_return_tracing; } @@ -15754,7 +15761,7 @@ fn analyzeRet( try sema.resolveTypeLayout(block, src, sema.fn_ret_ty); - if (sema.wantErrorReturnTracing()) { + if (sema.wantErrorReturnTracing(sema.fn_ret_ty)) { // Avoid adding a frame to the error return trace in case the value is comptime-known // to be not an error. const is_non_err = try sema.analyzeIsNonErr(block, src, operand); diff --git a/src/codegen/llvm.zig b/src/codegen/llvm.zig index ce8c169044..1b793265da 100644 --- a/src/codegen/llvm.zig +++ b/src/codegen/llvm.zig @@ -2335,7 +2335,6 @@ pub const Object = struct { const stack_trace_decl = builtin_namespace.decls .getKeyAdapted(stack_trace_str, Module.DeclAdapter{ .mod = mod }).?; - mod.ensureDeclAnalyzed(stack_trace_decl) catch unreachable; return mod.declPtr(stack_trace_decl).val.toType(undefined); } }; diff --git a/test/standalone/issue_13030/main.zig b/test/standalone/issue_13030/main.zig index 95b3a67af2..1a16a3e754 100644 --- a/test/standalone/issue_13030/main.zig +++ b/test/standalone/issue_13030/main.zig @@ -1,8 +1,7 @@ -const std = @import("std"); -fn a() error{}!void {} -fn b() std.meta.FnPtr(fn () error{}!void) { - return &a; +fn b(comptime T: type) ?@import("std").meta.FnPtr(fn () error{}!T) { + return null; } + export fn c() void { - _ = b(); + _ = b(void); } From 272e31227c2636afbf286d5154808cab930cec66 Mon Sep 17 00:00:00 2001 From: Jacob Young Date: Sat, 1 Oct 2022 09:03:28 -0400 Subject: [PATCH 4/5] llvm: add assert to reliably catch undefined value use This assert makes it possible to detect a regression of #13030 in the future without relying on undefined value tracking. --- src/codegen/llvm.zig | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/codegen/llvm.zig b/src/codegen/llvm.zig index 1b793265da..ffa710c67a 100644 --- a/src/codegen/llvm.zig +++ b/src/codegen/llvm.zig @@ -2332,10 +2332,13 @@ pub const Object = struct { // buffer is only used for int_type, `builtin` is a struct. const builtin_ty = mod.declPtr(builtin_decl).val.toType(undefined); const builtin_namespace = builtin_ty.getNamespace().?; - const stack_trace_decl = builtin_namespace.decls + const stack_trace_decl_index = builtin_namespace.decls .getKeyAdapted(stack_trace_str, Module.DeclAdapter{ .mod = mod }).?; + const stack_trace_decl = mod.declPtr(stack_trace_decl_index); - return mod.declPtr(stack_trace_decl).val.toType(undefined); + // Sema should have ensured that StackTrace was analyzed. + assert(stack_trace_decl.has_tv); + return stack_trace_decl.val.toType(undefined); } }; From 9cad44770a15d189cddb749bc1b2fca9e24b858f Mon Sep 17 00:00:00 2001 From: Jacob Young Date: Mon, 3 Oct 2022 10:28:30 -0400 Subject: [PATCH 5/5] test/standalone: remove unneeded FnPtr The behavior of this test is not affected by an extra level of indirection. --- test/standalone/issue_13030/main.zig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/standalone/issue_13030/main.zig b/test/standalone/issue_13030/main.zig index 1a16a3e754..5e4c976db3 100644 --- a/test/standalone/issue_13030/main.zig +++ b/test/standalone/issue_13030/main.zig @@ -1,7 +1,7 @@ -fn b(comptime T: type) ?@import("std").meta.FnPtr(fn () error{}!T) { +fn b(comptime T: type) ?*const fn () error{}!T { return null; } -export fn c() void { +export fn entry() void { _ = b(void); }