From adc5bce5e8487cf9507ab5dd2b1b60dd7f54cc3d Mon Sep 17 00:00:00 2001 From: Vexu Date: Thu, 20 Aug 2020 10:08:27 +0300 Subject: [PATCH 1/4] translate-c: correct translation of global variables * externs with intializers are translated as exports * non extern without explicit initialization are zero initalized --- lib/std/mem.zig | 20 +++++++++++++++++++- src-self-hosted/translate_c.zig | 27 ++++++++++++++++++++++++--- test/translate_c.zig | 4 ++-- 3 files changed, 45 insertions(+), 6 deletions(-) diff --git a/lib/std/mem.zig b/lib/std/mem.zig index 1ba64f47fa..7f1b5ae618 100644 --- a/lib/std/mem.zig +++ b/lib/std/mem.zig @@ -221,9 +221,19 @@ pub fn zeroes(comptime T: type) T { .Vector => |info| { return @splat(info.len, zeroes(info.child)); }, + .Union => |info| { + if (comptime meta.containerLayout(T) == .Extern) { + // The C language specification states that (global) unions + // should be zero initialized to the first named member. + var item: T = undefined; + @field(item, info.fields[0].name) = zeroes(@TypeOf(@field(item, info.fields[0].name))); + return item; + } + + @compileError("Can't set a " ++ @typeName(T) ++ " to zero."); + }, .ErrorUnion, .ErrorSet, - .Union, .Fn, .BoundFn, .Type, @@ -312,6 +322,14 @@ test "mem.zeroes" { for (b.sentinel) |e| { testing.expectEqual(@as(u8, 0), e); } + + const C_union = extern union { + a: u8, + b: u32, + }; + + var c = zeroes(C_union); + testing.expectEqual(@as(u8, 0), c.a); } /// Sets a slice to zeroes. diff --git a/src-self-hosted/translate_c.zig b/src-self-hosted/translate_c.zig index 98cb68f059..6c6e3f0c7e 100644 --- a/src-self-hosted/translate_c.zig +++ b/src-self-hosted/translate_c.zig @@ -701,8 +701,14 @@ fn visitVarDecl(c: *Context, var_decl: *const ZigClangVarDecl) Error!void { const qual_type = ZigClangVarDecl_getTypeSourceInfo_getType(var_decl); const storage_class = ZigClangVarDecl_getStorageClass(var_decl); const is_const = ZigClangQualType_isConstQualified(qual_type); + const has_init = ZigClangVarDecl_hasInit(var_decl); - const extern_tok = if (storage_class == .Extern) + // In C extern variables with initializers behave like Zig exports. + // extern int foo = 2; + // does the same as: + // extern int foo; + // int foo = 2; + const extern_tok = if (storage_class == .Extern and !has_init) try appendToken(c, .Keyword_extern, "extern") else if (storage_class != .Static) try appendToken(c, .Keyword_export, "export") @@ -730,7 +736,7 @@ fn visitVarDecl(c: *Context, var_decl: *const ZigClangVarDecl) Error!void { // If the initialization expression is not present, initialize with undefined. // If it is an integer literal, we can skip the @as since it will be redundant // with the variable type. - if (ZigClangVarDecl_hasInit(var_decl)) { + if (has_init) { eq_tok = try appendToken(c, .Equal, "="); init_node = if (ZigClangVarDecl_getInit(var_decl)) |expr| transExprCoercing(rp, &c.global_scope.base, expr, .used, .r_value) catch |err| switch (err) { @@ -745,7 +751,22 @@ fn visitVarDecl(c: *Context, var_decl: *const ZigClangVarDecl) Error!void { try transCreateNodeUndefinedLiteral(c); } else if (storage_class != .Extern) { eq_tok = try appendToken(c, .Equal, "="); - init_node = try transCreateNodeIdentifierUnchecked(c, "undefined"); + // The C language specification states that variables with static or threadlocal + // storage without an initializer are initialized to a zero value. + + // @import("std").mem.zeroes(T) + const import_fn_call = try c.createBuiltinCall("@import", 1); + const std_node = try transCreateNodeStringLiteral(c, "\"std\""); + import_fn_call.params()[0] = std_node; + import_fn_call.rparen_token = try appendToken(c, .RParen, ")"); + const inner_field_access = try transCreateNodeFieldAccess(c, &import_fn_call.base, "mem"); + const outer_field_access = try transCreateNodeFieldAccess(c, inner_field_access, "zeroes"); + + const zero_init_call = try c.createCall(outer_field_access, 1); + zero_init_call.params()[0] = type_node; + zero_init_call.rtoken = try appendToken(c, .RParen, ")"); + + init_node = &zero_init_call.base; } const linksection_expr = blk: { diff --git a/test/translate_c.zig b/test/translate_c.zig index f7e983276e..bfbae2b65e 100644 --- a/test/translate_c.zig +++ b/test/translate_c.zig @@ -466,7 +466,7 @@ pub fn addCases(cases: *tests.TranslateCContext) void { , &[_][]const u8{ \\pub extern var extern_var: c_int; \\pub const int_var: c_int = 13; - \\pub export var foo: c_int = undefined; + \\pub export var foo: c_int = @import("std").mem.zeroes(c_int); }); cases.add("const ptr initializer", @@ -1327,7 +1327,7 @@ pub fn addCases(cases: *tests.TranslateCContext) void { \\static char arr1[] = "hello"; \\char arr2[] = "hello"; , &[_][]const u8{ - \\pub extern var arr0: [*c]u8 = "hello"; + \\pub export var arr0: [*c]u8 = "hello"; \\pub var arr1: [*c]u8 = "hello"; \\pub export var arr2: [*c]u8 = "hello"; }); From a553947a51bc5858ff40fb47dfe41daaa9ff75bf Mon Sep 17 00:00:00 2001 From: Vexu Date: Thu, 20 Aug 2020 10:45:55 +0300 Subject: [PATCH 2/4] translate-c: correctly put static and extern local variables in global scope --- src-self-hosted/translate_c.zig | 25 ++++++++++++++++--------- test/run_translated_c.zig | 14 ++++++++++++++ test/translate_c.zig | 17 ++++++++++++++++- 3 files changed, 46 insertions(+), 10 deletions(-) diff --git a/src-self-hosted/translate_c.zig b/src-self-hosted/translate_c.zig index 6c6e3f0c7e..6628f5b99e 100644 --- a/src-self-hosted/translate_c.zig +++ b/src-self-hosted/translate_c.zig @@ -498,7 +498,7 @@ fn declVisitor(c: *Context, decl: *const ZigClangDecl) Error!void { _ = try transRecordDecl(c, @ptrCast(*const ZigClangRecordDecl, decl)); }, .Var => { - return visitVarDecl(c, @ptrCast(*const ZigClangVarDecl, decl)); + return visitVarDecl(c, @ptrCast(*const ZigClangVarDecl, decl), null); }, .Empty => { // Do nothing @@ -679,12 +679,13 @@ fn visitFnDecl(c: *Context, fn_decl: *const ZigClangFunctionDecl) Error!void { return addTopLevelDecl(c, fn_name, &proto_node.base); } -fn visitVarDecl(c: *Context, var_decl: *const ZigClangVarDecl) Error!void { - const var_name = try c.str(ZigClangNamedDecl_getName_bytes_begin(@ptrCast(*const ZigClangNamedDecl, var_decl))); +/// if mangled_name is not null, this var decl was declared in a block scope. +fn visitVarDecl(c: *Context, var_decl: *const ZigClangVarDecl, mangled_name: ?[]const u8) Error!void { + const var_name = mangled_name orelse try c.str(ZigClangNamedDecl_getName_bytes_begin(@ptrCast(*const ZigClangNamedDecl, var_decl))); if (c.global_scope.sym_table.contains(var_name)) return; // Avoid processing this decl twice const rp = makeRestorePoint(c); - const visib_tok = try appendToken(c, .Keyword_pub, "pub"); + const visib_tok = if (mangled_name) |_| null else try appendToken(c, .Keyword_pub, "pub"); const thread_local_token = if (ZigClangVarDecl_getTLSKind(var_decl) == .None) null @@ -1582,15 +1583,22 @@ fn transDeclStmtOne( .Var => { const var_decl = @ptrCast(*const ZigClangVarDecl, decl); - const thread_local_token = if (ZigClangVarDecl_getTLSKind(var_decl) == .None) - null - else - try appendToken(c, .Keyword_threadlocal, "threadlocal"); const qual_type = ZigClangVarDecl_getTypeSourceInfo_getType(var_decl); const name = try c.str(ZigClangNamedDecl_getName_bytes_begin( @ptrCast(*const ZigClangNamedDecl, var_decl), )); const mangled_name = try block_scope.makeMangledName(c, name); + + switch (ZigClangVarDecl_getStorageClass(var_decl)) { + .Extern, .Static => { + // This is actually a global variable, put it in the global scope and reference it. + // `_ = mangled_name;` + try visitVarDecl(rp.c, var_decl, mangled_name); + return try maybeSuppressResult(rp, scope, .unused, try transCreateNodeIdentifier(rp.c, mangled_name)); + }, + else => {}, + } + const mut_tok = if (ZigClangQualType_isConstQualified(qual_type)) try appendToken(c, .Keyword_const, "const") else @@ -1618,7 +1626,6 @@ fn transDeclStmtOne( .mut_token = mut_tok, .semicolon_token = semicolon_token, }, .{ - .thread_local_token = thread_local_token, .eq_token = eq_token, .type_node = type_node, .init_node = init_node, diff --git a/test/run_translated_c.zig b/test/run_translated_c.zig index 3fa183ce3b..2b27e3085e 100644 --- a/test/run_translated_c.zig +++ b/test/run_translated_c.zig @@ -3,6 +3,20 @@ const tests = @import("tests.zig"); const nl = std.cstr.line_sep; pub fn addCases(cases: *tests.RunTranslatedCContext) void { + cases.add("static variable in block scope", + \\#include + \\int foo() { + \\ static int bar; + \\ bar += 1; + \\ return bar; + \\} + \\int main() { + \\ foo(); + \\ foo(); + \\ if (foo() != 3) abort(); + \\} + , ""); + cases.add("array initializer", \\#include \\int main(int argc, char **argv) { diff --git a/test/translate_c.zig b/test/translate_c.zig index bfbae2b65e..41f1e78294 100644 --- a/test/translate_c.zig +++ b/test/translate_c.zig @@ -3,6 +3,20 @@ const std = @import("std"); const CrossTarget = std.zig.CrossTarget; pub fn addCases(cases: *tests.TranslateCContext) void { + cases.add("extern variable in block scope", + \\float bar; + \\int foo() { + \\ _Thread_local static int bar = 2; + \\} + , &[_][]const u8{ + \\pub export var bar: f32 = @import("std").mem.zeroes(f32); + \\threadlocal var bar_1: c_int = 2; + \\pub export fn foo() c_int { + \\ _ = bar_1; + \\ return 0; + \\} + }); + cases.add("missing return stmt", \\int foo() {} \\int bar() { @@ -480,8 +494,9 @@ pub fn addCases(cases: *tests.TranslateCContext) void { \\ static const char v2[] = "2.2.2"; \\} , &[_][]const u8{ + \\const v2: [*c]const u8 = "2.2.2"; \\pub export fn foo() void { - \\ const v2: [*c]const u8 = "2.2.2"; + \\ _ = v2; \\} }); From d25674a51ec09640b0140a541a156869b3e03f81 Mon Sep 17 00:00:00 2001 From: Vexu Date: Thu, 20 Aug 2020 11:04:42 +0300 Subject: [PATCH 3/4] disallow extern variables with initializers --- lib/std/c/darwin.zig | 2 +- lib/std/special/c.zig | 2 +- lib/std/special/compiler_rt.zig | 2 +- src-self-hosted/Module.zig | 11 +++++++---- src/parser.cpp | 3 +++ test/compile_errors.zig | 6 ++++++ test/standalone/global_linkage/obj1.zig | 4 ++-- test/standalone/global_linkage/obj2.zig | 4 ++-- 8 files changed, 23 insertions(+), 11 deletions(-) diff --git a/lib/std/c/darwin.zig b/lib/std/c/darwin.zig index 2426638569..b1f66fa575 100644 --- a/lib/std/c/darwin.zig +++ b/lib/std/c/darwin.zig @@ -41,7 +41,7 @@ const mach_hdr = if (@sizeOf(usize) == 8) mach_header_64 else mach_header; /// on this operating system. However when building object files or libraries, /// the system libc won't be linked until the final executable. So we /// export a weak symbol here, to be overridden by the real one. -pub extern "c" var _mh_execute_header: mach_hdr = undefined; +pub extern "c" var _mh_execute_header: mach_hdr; comptime { if (std.Target.current.isDarwin()) { @export(_mh_execute_header, .{ .name = "_mh_execute_header", .linkage = .Weak }); diff --git a/lib/std/special/c.zig b/lib/std/special/c.zig index 170bb98620..8019db9d96 100644 --- a/lib/std/special/c.zig +++ b/lib/std/special/c.zig @@ -35,7 +35,7 @@ comptime { } } -extern var _fltused: c_int = 1; +var _fltused: c_int = 1; extern fn main(argc: c_int, argv: [*:null]?[*:0]u8) c_int; fn wasm_start() callconv(.C) void { diff --git a/lib/std/special/compiler_rt.zig b/lib/std/special/compiler_rt.zig index 2716de8113..36a731ed5a 100644 --- a/lib/std/special/compiler_rt.zig +++ b/lib/std/special/compiler_rt.zig @@ -335,7 +335,7 @@ fn __stack_chk_fail() callconv(.C) noreturn { @panic("stack smashing detected"); } -extern var __stack_chk_guard: usize = blk: { +var __stack_chk_guard: usize = blk: { var buf = [1]u8{0} ** @sizeOf(usize); buf[@sizeOf(usize) - 1] = 255; buf[@sizeOf(usize) - 2] = '\n'; diff --git a/src-self-hosted/Module.zig b/src-self-hosted/Module.zig index ec69f94e3d..d08e7f5746 100644 --- a/src-self-hosted/Module.zig +++ b/src-self-hosted/Module.zig @@ -324,10 +324,10 @@ pub const Fn = struct { }; pub const Var = struct { + /// if is_extern == true this is undefined init: Value, owner_decl: *Decl, - has_init: bool, is_extern: bool, is_mutable: bool, is_threadlocal: bool, @@ -1456,7 +1456,11 @@ fn astGenAndAnalyzeDecl(self: *Module, decl: *Decl) !bool { const is_extern = blk: { const maybe_extern_token = var_decl.getTrailer("extern_export_token") orelse break :blk false; - break :blk tree.token_ids[maybe_extern_token] == .Keyword_extern; + if (tree.token_ids[maybe_extern_token] != .Keyword_extern) break :blk false; + if (var_decl.getTrailer("init_node")) |some| { + return self.failNode(&block_scope.base, some, "extern variables have no initializers", .{}); + } + break :blk true; }; if (var_decl.getTrailer("lib_name")) |lib_name| { assert(is_extern); @@ -1569,7 +1573,6 @@ fn astGenAndAnalyzeDecl(self: *Module, decl: *Decl) !bool { new_variable.* = .{ .owner_decl = decl, .init = value orelse undefined, - .has_init = value != null, .is_extern = is_extern, .is_mutable = is_mutable, .is_threadlocal = is_threadlocal, @@ -2440,7 +2443,7 @@ fn analyzeVarRef(self: *Module, scope: *Scope, src: usize, tv: TypedValue) Inner const variable = tv.val.cast(Value.Payload.Variable).?.variable; const ty = try self.singlePtrType(scope, src, variable.is_mutable, tv.ty); - if (!variable.is_mutable and !variable.is_extern and variable.has_init) { + if (!variable.is_mutable and !variable.is_extern) { const val_payload = try scope.arena().create(Value.Payload.RefVal); val_payload.* = .{ .val = variable.init }; return self.constInst(scope, src, .{ diff --git a/src/parser.cpp b/src/parser.cpp index 26233ec6a4..1253baf9ea 100644 --- a/src/parser.cpp +++ b/src/parser.cpp @@ -680,6 +680,9 @@ static AstNode *ast_parse_top_level_decl(ParseContext *pc, VisibMod visib_mod, B AstNode *var_decl = ast_parse_var_decl(pc); if (var_decl != nullptr) { assert(var_decl->type == NodeTypeVariableDeclaration); + if (first->id == TokenIdKeywordExtern && var_decl->data.variable_declaration.expr != nullptr) { + ast_error(pc, first, "extern variables have no initializers"); + } var_decl->line = first->start_line; var_decl->column = first->start_column; var_decl->data.variable_declaration.threadlocal_tok = thread_local_kw; diff --git a/test/compile_errors.zig b/test/compile_errors.zig index fd74385d9b..311c5b9319 100644 --- a/test/compile_errors.zig +++ b/test/compile_errors.zig @@ -2,6 +2,12 @@ const tests = @import("tests.zig"); const std = @import("std"); pub fn addCases(cases: *tests.CompileErrorContext) void { + cases.addTest("reject extern variables with initializers", + \\extern var foo: int = 2; + , &[_][]const u8{ + "tmp.zig:1:1: error: extern variables have no initializers", + }); + cases.addTest("duplicate/unused labels", \\comptime { \\ blk: { blk: while (false) {} } diff --git a/test/standalone/global_linkage/obj1.zig b/test/standalone/global_linkage/obj1.zig index 95814cda3d..83b66f336b 100644 --- a/test/standalone/global_linkage/obj1.zig +++ b/test/standalone/global_linkage/obj1.zig @@ -1,5 +1,5 @@ -extern var internal_integer: usize = 1; -extern var obj1_integer: usize = 421; +var internal_integer: usize = 1; +var obj1_integer: usize = 421; comptime { @export(internal_integer, .{ .name = "internal_integer", .linkage = .Internal }); diff --git a/test/standalone/global_linkage/obj2.zig b/test/standalone/global_linkage/obj2.zig index f2d44b2dc0..2908a627fd 100644 --- a/test/standalone/global_linkage/obj2.zig +++ b/test/standalone/global_linkage/obj2.zig @@ -1,5 +1,5 @@ -extern var internal_integer: usize = 2; -extern var obj2_integer: usize = 422; +var internal_integer: usize = 2; +var obj2_integer: usize = 422; comptime { @export(internal_integer, .{ .name = "internal_integer", .linkage = .Internal }); From 717e2a365d9b5103f855668e4765cc154203a2bf Mon Sep 17 00:00:00 2001 From: Vexu Date: Thu, 20 Aug 2020 14:19:11 +0300 Subject: [PATCH 4/4] correct llvm linkage conversion when weakly exporting external declaration we need to pass LLVMExternalWeakLinkage --- lib/std/c/darwin.zig | 5 +++-- src/codegen.cpp | 9 +++++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/std/c/darwin.zig b/lib/std/c/darwin.zig index b1f66fa575..9ff66ac01e 100644 --- a/lib/std/c/darwin.zig +++ b/lib/std/c/darwin.zig @@ -41,10 +41,11 @@ const mach_hdr = if (@sizeOf(usize) == 8) mach_header_64 else mach_header; /// on this operating system. However when building object files or libraries, /// the system libc won't be linked until the final executable. So we /// export a weak symbol here, to be overridden by the real one. -pub extern "c" var _mh_execute_header: mach_hdr; +var dummy_execute_header: mach_hdr = undefined; +pub extern var _mh_execute_header: mach_hdr; comptime { if (std.Target.current.isDarwin()) { - @export(_mh_execute_header, .{ .name = "_mh_execute_header", .linkage = .Weak }); + @export(dummy_execute_header, .{ .name = "_mh_execute_header", .linkage = .Weak }); } } diff --git a/src/codegen.cpp b/src/codegen.cpp index 36ed520069..e2341fe1aa 100644 --- a/src/codegen.cpp +++ b/src/codegen.cpp @@ -292,13 +292,14 @@ static void add_uwtable_attr(CodeGen *g, LLVMValueRef fn_val) { } } -static LLVMLinkage to_llvm_linkage(GlobalLinkageId id) { +static LLVMLinkage to_llvm_linkage(GlobalLinkageId id, bool is_extern) { switch (id) { case GlobalLinkageIdInternal: return LLVMInternalLinkage; case GlobalLinkageIdStrong: return LLVMExternalLinkage; case GlobalLinkageIdWeak: + if (is_extern) return LLVMExternalWeakLinkage; return LLVMWeakODRLinkage; case GlobalLinkageIdLinkOnce: return LLVMLinkOnceODRLinkage; @@ -521,7 +522,7 @@ static LLVMValueRef make_fn_llvm_value(CodeGen *g, ZigFn *fn) { } - LLVMSetLinkage(llvm_fn, to_llvm_linkage(linkage)); + LLVMSetLinkage(llvm_fn, to_llvm_linkage(linkage, fn->body_node == nullptr)); if (linkage == GlobalLinkageIdInternal) { LLVMSetUnnamedAddr(llvm_fn, true); @@ -7962,7 +7963,7 @@ static void do_code_gen(CodeGen *g) { global_value = LLVMAddGlobal(g->module, get_llvm_type(g, var->var_type), symbol_name); // TODO debug info for the extern variable - LLVMSetLinkage(global_value, to_llvm_linkage(linkage)); + LLVMSetLinkage(global_value, to_llvm_linkage(linkage, true)); maybe_import_dll(g, global_value, GlobalLinkageIdStrong); LLVMSetAlignment(global_value, var->align_bytes); LLVMSetGlobalConstant(global_value, var->gen_is_const); @@ -7975,7 +7976,7 @@ static void do_code_gen(CodeGen *g) { global_value = var->const_value->llvm_global; if (exported) { - LLVMSetLinkage(global_value, to_llvm_linkage(linkage)); + LLVMSetLinkage(global_value, to_llvm_linkage(linkage, false)); maybe_export_dll(g, global_value, GlobalLinkageIdStrong); } if (var->section_name) {