From c93e0d86187cb589d6726acd36f741f3d87a96be Mon Sep 17 00:00:00 2001 From: mlugg Date: Thu, 9 Mar 2023 21:34:06 +0000 Subject: [PATCH] Sema: @extern fixes * There was an edge case where the arena could be destroyed twice on error: once from the arena itself and once from the decl destruction. * The type of the created decl was incorrect (it should have been the pointer child type), but it's not required anyway, so it's now just initialized to anyopaque (which more accurately reflects what's actually at that memory, since e.g. [*]T may correspond to nothing). * A runtime bitcast of the pointer was performed, meaning @extern didn't work at comptime. This is unnecessary: the decl_ref can just be initialized with the correct pointer type. --- src/Sema.zig | 59 +++++++++++++++--------------- test/standalone.zig | 1 + test/standalone/extern/build.zig | 20 ++++++++++ test/standalone/extern/exports.zig | 12 ++++++ test/standalone/extern/main.zig | 21 +++++++++++ 5 files changed, 84 insertions(+), 29 deletions(-) create mode 100644 test/standalone/extern/build.zig create mode 100644 test/standalone/extern/exports.zig create mode 100644 test/standalone/extern/main.zig diff --git a/src/Sema.zig b/src/Sema.zig index 6c24746da8..aef07b7988 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -22287,7 +22287,6 @@ fn zirBuiltinExtern( extended: Zir.Inst.Extended.InstData, ) CompileError!Air.Inst.Ref { const extra = sema.code.extraData(Zir.Inst.BinNode, extended.operand).data; - const src = LazySrcLoc.nodeOffset(extra.node); const ty_src: LazySrcLoc = .{ .node_offset_builtin_call_arg0 = extra.node }; const options_src: LazySrcLoc = .{ .node_offset_builtin_call_arg1 = extra.node }; @@ -22315,39 +22314,41 @@ fn zirBuiltinExtern( const new_decl = sema.mod.declPtr(new_decl_index); new_decl.name = try sema.gpa.dupeZ(u8, options.name); - var new_decl_arena = std.heap.ArenaAllocator.init(sema.gpa); - errdefer new_decl_arena.deinit(); - const new_decl_arena_allocator = new_decl_arena.allocator(); + { + var new_decl_arena = std.heap.ArenaAllocator.init(sema.gpa); + errdefer new_decl_arena.deinit(); + const new_decl_arena_allocator = new_decl_arena.allocator(); - const new_var = try new_decl_arena_allocator.create(Module.Var); - errdefer new_decl_arena_allocator.destroy(new_var); + const new_var = try new_decl_arena_allocator.create(Module.Var); + new_var.* = .{ + .owner_decl = sema.owner_decl_index, + .init = Value.initTag(.unreachable_value), + .is_extern = true, + .is_mutable = false, + .is_threadlocal = options.is_thread_local, + .is_weak_linkage = options.linkage == .Weak, + .lib_name = null, + }; - new_var.* = .{ - .owner_decl = sema.owner_decl_index, - .init = Value.initTag(.unreachable_value), - .is_extern = true, - .is_mutable = false, - .is_threadlocal = options.is_thread_local, - .is_weak_linkage = options.linkage == .Weak, - .lib_name = null, - }; + new_decl.src_line = sema.owner_decl.src_line; + // We only access this decl through the decl_ref with the correct type created + // below, so this type doesn't matter + new_decl.ty = Type.Tag.init(.anyopaque); + new_decl.val = try Value.Tag.variable.create(new_decl_arena_allocator, new_var); + new_decl.@"align" = 0; + new_decl.@"linksection" = null; + new_decl.has_tv = true; + new_decl.analysis = .complete; + new_decl.generation = sema.mod.generation; - new_decl.src_line = sema.owner_decl.src_line; - new_decl.ty = try ty.copy(new_decl_arena_allocator); - new_decl.val = try Value.Tag.variable.create(new_decl_arena_allocator, new_var); - new_decl.@"align" = 0; - new_decl.@"linksection" = null; - new_decl.has_tv = true; - new_decl.analysis = .complete; - new_decl.generation = sema.mod.generation; + try new_decl.finalizeNewArena(&new_decl_arena); + } - const arena_state = try new_decl_arena_allocator.create(std.heap.ArenaAllocator.State); - arena_state.* = new_decl_arena.state; - new_decl.value_arena = arena_state; + try sema.mod.declareDeclDependency(sema.owner_decl_index, new_decl_index); + try sema.ensureDeclAnalyzed(new_decl_index); - const ref = try sema.analyzeDeclRef(new_decl_index); - try sema.requireRuntimeBlock(block, src, null); - return block.addBitCast(ty, ref); + const ref = try Value.Tag.decl_ref.create(sema.arena, new_decl_index); + return sema.addConstant(ty, ref); } fn requireRuntimeBlock(sema: *Sema, block: *Block, src: LazySrcLoc, runtime_src: ?LazySrcLoc) !void { diff --git a/test/standalone.zig b/test/standalone.zig index 965139235c..7aa4d81f97 100644 --- a/test/standalone.zig +++ b/test/standalone.zig @@ -107,6 +107,7 @@ pub fn addCases(cases: *tests.StandaloneContext) void { cases.addBuildFile("test/standalone/emit_asm_and_bin/build.zig", .{}); cases.addBuildFile("test/standalone/issue_12588/build.zig", .{}); cases.addBuildFile("test/standalone/embed_generated_file/build.zig", .{}); + cases.addBuildFile("test/standalone/extern/build.zig", .{}); cases.addBuildFile("test/standalone/dep_diamond/build.zig", .{}); cases.addBuildFile("test/standalone/dep_triangle/build.zig", .{}); diff --git a/test/standalone/extern/build.zig b/test/standalone/extern/build.zig new file mode 100644 index 0000000000..8a44a6ca8f --- /dev/null +++ b/test/standalone/extern/build.zig @@ -0,0 +1,20 @@ +const std = @import("std"); + +pub fn build(b: *std.Build) void { + const optimize = b.standardOptimizeOption(.{}); + + const obj = b.addObject(.{ + .name = "exports", + .root_source_file = .{ .path = "exports.zig" }, + .target = .{}, + .optimize = optimize, + }); + const main = b.addTest(.{ + .root_source_file = .{ .path = "main.zig" }, + .optimize = optimize, + }); + main.addObject(obj); + + const test_step = b.step("test", "Test it"); + test_step.dependOn(&main.step); +} diff --git a/test/standalone/extern/exports.zig b/test/standalone/extern/exports.zig new file mode 100644 index 0000000000..93351c4581 --- /dev/null +++ b/test/standalone/extern/exports.zig @@ -0,0 +1,12 @@ +var hidden: u32 = 0; +export fn updateHidden(val: u32) void { + hidden = val; +} +export fn getHidden() u32 { + return hidden; +} + +const T = extern struct { x: u32 }; + +export var mut_val: f64 = 1.23; +export const const_val: T = .{ .x = 42 }; diff --git a/test/standalone/extern/main.zig b/test/standalone/extern/main.zig new file mode 100644 index 0000000000..4cbed184c3 --- /dev/null +++ b/test/standalone/extern/main.zig @@ -0,0 +1,21 @@ +const assert = @import("std").debug.assert; + +const updateHidden = @extern(*const fn (u32) callconv(.C) void, .{ .name = "updateHidden" }); +const getHidden = @extern(*const fn () callconv(.C) u32, .{ .name = "getHidden" }); + +const T = extern struct { x: u32 }; + +test { + var mut_val_ptr = @extern(*f64, .{ .name = "mut_val" }); + var const_val_ptr = @extern(*const T, .{ .name = "const_val" }); + + assert(getHidden() == 0); + updateHidden(123); + assert(getHidden() == 123); + + assert(mut_val_ptr.* == 1.23); + mut_val_ptr.* = 10.0; + assert(mut_val_ptr.* == 10.0); + + assert(const_val_ptr.x == 42); +}