fix race conditions in self-hosted compiler; add test

* fix race condition in std.event.Channel deinit
 * add support to zig build for --no-rosegment
 * add passing self-hosted compare-output test for calling a function
 * put a global lock on LLD linking because it's not thread safe

build.zig

@@ -45,6 +45,7 @@ pub fn build(b: *Builder) !void {
         .c_header_files = nextValue(&index, build_info),
         .dia_guids_lib = nextValue(&index, build_info),
         .llvm = undefined,
+        .no_rosegment = b.option(bool, "no-rosegment", "Workaround to enable valgrind builds") orelse false,
     };
     ctx.llvm = try findLLVM(b, ctx.llvm_config_exe);
 
@@ -228,6 +229,8 @@ fn configureStage2(b: *Builder, exe: var, ctx: Context) !void {
     // TODO turn this into -Dextra-lib-path=/lib option
     exe.addLibPath("/lib");
 
+    exe.setNoRoSegment(ctx.no_rosegment);
+
     exe.addIncludeDir("src");
     exe.addIncludeDir(ctx.cmake_binary_dir);
     addCppLib(b, exe, ctx.cmake_binary_dir, "zig_cpp");
@@ -286,4 +289,5 @@ const Context = struct {
     c_header_files: []const u8,
     dia_guids_lib: []const u8,
     llvm: LibraryDep,
+    no_rosegment: bool,
 };

src-self-hosted/compilation.zig

@@ -35,6 +35,7 @@ const CInt = @import("c_int.zig").CInt;
 pub const EventLoopLocal = struct {
     loop: *event.Loop,
     llvm_handle_pool: std.atomic.Stack(llvm.ContextRef),
+    lld_lock: event.Lock,
 
     /// TODO pool these so that it doesn't have to lock
     prng: event.Locked(std.rand.DefaultPrng),
@@ -55,6 +56,7 @@ pub const EventLoopLocal = struct {
 
         return EventLoopLocal{
             .loop = loop,
+            .lld_lock = event.Lock.init(loop),
             .llvm_handle_pool = std.atomic.Stack(llvm.ContextRef).init(),
             .prng = event.Locked(std.rand.DefaultPrng).init(loop, std.rand.DefaultPrng.init(seed)),
             .native_libc = event.Future(LibCInstallation).init(loop),
@@ -63,6 +65,7 @@ pub const EventLoopLocal = struct {
 
     /// Must be called only after EventLoop.run completes.
     fn deinit(self: *EventLoopLocal) void {
+        self.lld_lock.deinit();
         while (self.llvm_handle_pool.pop()) |node| {
             c.LLVMContextDispose(node.data);
             self.loop.allocator.destroy(node);

src-self-hosted/link.zig

@@ -80,15 +80,22 @@ pub async fn link(comp: *Compilation) !void {
 
     const extern_ofmt = toExternObjectFormatType(comp.target.getObjectFormat());
     const args_slice = ctx.args.toSlice();
-    // Not evented I/O. LLD does its own multithreading internally.
-    if (!ZigLLDLink(extern_ofmt, args_slice.ptr, args_slice.len, linkDiagCallback, @ptrCast(*c_void, &ctx))) {
-        if (!ctx.link_msg.isNull()) {
-            // TODO capture these messages and pass them through the system, reporting them through the
-            // event system instead of printing them directly here.
-            // perhaps try to parse and understand them.
-            std.debug.warn("{}\n", ctx.link_msg.toSliceConst());
+
+    {
+        // LLD is not thread-safe, so we grab a global lock.
+        const held = await (async comp.event_loop_local.lld_lock.acquire() catch unreachable);
+        defer held.release();
+
+        // Not evented I/O. LLD does its own multithreading internally.
+        if (!ZigLLDLink(extern_ofmt, args_slice.ptr, args_slice.len, linkDiagCallback, @ptrCast(*c_void, &ctx))) {
+            if (!ctx.link_msg.isNull()) {
+                // TODO capture these messages and pass them through the system, reporting them through the
+                // event system instead of printing them directly here.
+                // perhaps try to parse and understand them.
+                std.debug.warn("{}\n", ctx.link_msg.toSliceConst());
+            }
+            return error.LinkFailed;
         }
-        return error.LinkFailed;
     }
 }
 
@@ -672,7 +679,13 @@ const DarwinPlatform = struct {
         };
 
         var had_extra: bool = undefined;
-        try darwinGetReleaseVersion(ver_str, &result.major, &result.minor, &result.micro, &had_extra,);
+        try darwinGetReleaseVersion(
+            ver_str,
+            &result.major,
+            &result.minor,
+            &result.micro,
+            &had_extra,
+        );
         if (had_extra or result.major != 10 or result.minor >= 100 or result.micro >= 100) {
             return error.InvalidDarwinVersionString;
         }
@@ -713,7 +726,7 @@ fn darwinGetReleaseVersion(str: []const u8, major: *u32, minor: *u32, micro: *u3
         return error.InvalidDarwinVersionString;
 
     var start_pos: usize = 0;
-    for ([]*u32{major, minor, micro}) |v| {
+    for ([]*u32{ major, minor, micro }) |v| {
         const dot_pos = mem.indexOfScalarPos(u8, str, start_pos, '.');
         const end_pos = dot_pos orelse str.len;
         v.* = std.fmt.parseUnsigned(u32, str[start_pos..end_pos], 10) catch return error.InvalidDarwinVersionString;

std/build.zig

@@ -807,6 +807,7 @@ pub const LibExeObjStep = struct {
     disable_libc: bool,
     frameworks: BufSet,
     verbose_link: bool,
+    no_rosegment: bool,
 
     // zig only stuff
     root_src: ?[]const u8,
@@ -874,6 +875,7 @@ pub const LibExeObjStep = struct {
 
     fn initExtraArgs(builder: *Builder, name: []const u8, root_src: ?[]const u8, kind: Kind, static: bool, ver: *const Version) LibExeObjStep {
         var self = LibExeObjStep{
+            .no_rosegment = false,
             .strip = false,
             .builder = builder,
             .verbose_link = false,
@@ -914,6 +916,7 @@ pub const LibExeObjStep = struct {
 
     fn initC(builder: *Builder, name: []const u8, kind: Kind, version: *const Version, static: bool) LibExeObjStep {
         var self = LibExeObjStep{
+            .no_rosegment = false,
             .builder = builder,
             .name = name,
             .kind = kind,
@@ -953,6 +956,10 @@ pub const LibExeObjStep = struct {
         return self;
     }
 
+    pub fn setNoRoSegment(self: *LibExeObjStep, value: bool) void {
+        self.no_rosegment = value;
+    }
+
     fn computeOutFileNames(self: *LibExeObjStep) void {
         switch (self.kind) {
             Kind.Obj => {
@@ -1306,6 +1313,10 @@ pub const LibExeObjStep = struct {
             }
         }
 
+        if (self.no_rosegment) {
+            try zig_args.append("--no-rosegment");
+        }
+
         try builder.spawnChild(zig_args.toSliceConst());
 
         if (self.kind == Kind.Lib and !self.static and self.target.wantSharedLibSymLinks()) {
@@ -1598,6 +1609,7 @@ pub const TestStep = struct {
     include_dirs: ArrayList([]const u8),
     lib_paths: ArrayList([]const u8),
     object_files: ArrayList([]const u8),
+    no_rosegment: bool,
 
     pub fn init(builder: *Builder, root_src: []const u8) TestStep {
         const step_name = builder.fmt("test {}", root_src);
@@ -1615,9 +1627,14 @@ pub const TestStep = struct {
             .include_dirs = ArrayList([]const u8).init(builder.allocator),
             .lib_paths = ArrayList([]const u8).init(builder.allocator),
             .object_files = ArrayList([]const u8).init(builder.allocator),
+            .no_rosegment = false,
         };
     }
 
+    pub fn setNoRoSegment(self: *TestStep, value: bool) void {
+        self.no_rosegment = value;
+    }
+
     pub fn addLibPath(self: *TestStep, path: []const u8) void {
         self.lib_paths.append(path) catch unreachable;
     }
@@ -1761,6 +1778,10 @@ pub const TestStep = struct {
             try zig_args.append(lib_path);
         }
 
+        if (self.no_rosegment) {
+            try zig_args.append("--no-rosegment");
+        }
+
         try builder.spawnChild(zig_args.toSliceConst());
     }
 };

std/event/channel.zig

@@ -71,11 +71,6 @@ pub fn Channel(comptime T: type) type {
         /// puts a data item in the channel. The promise completes when the value has been added to the
         /// buffer, or in the case of a zero size buffer, when the item has been retrieved by a getter.
         pub async fn put(self: *SelfChannel, data: T) void {
-            // TODO should be able to group memory allocation failure before first suspend point
-            // so that the async invocation catches it
-            var dispatch_tick_node_ptr: *Loop.NextTickNode = undefined;
-            _ = async self.dispatch(&dispatch_tick_node_ptr) catch unreachable;
-
             suspend |handle| {
                 var my_tick_node = Loop.NextTickNode{
                     .next = undefined,
@@ -91,18 +86,13 @@ pub fn Channel(comptime T: type) type {
                 self.putters.put(&queue_node);
                 _ = @atomicRmw(usize, &self.put_count, AtomicRmwOp.Add, 1, AtomicOrder.SeqCst);
 
-                self.loop.onNextTick(dispatch_tick_node_ptr);
+                self.dispatch();
             }
         }
 
         /// await this function to get an item from the channel. If the buffer is empty, the promise will
         /// complete when the next item is put in the channel.
         pub async fn get(self: *SelfChannel) T {
-            // TODO should be able to group memory allocation failure before first suspend point
-            // so that the async invocation catches it
-            var dispatch_tick_node_ptr: *Loop.NextTickNode = undefined;
-            _ = async self.dispatch(&dispatch_tick_node_ptr) catch unreachable;
-
             // TODO integrate this function with named return values
             // so we can get rid of this extra result copy
             var result: T = undefined;
@@ -121,21 +111,12 @@ pub fn Channel(comptime T: type) type {
                 self.getters.put(&queue_node);
                 _ = @atomicRmw(usize, &self.get_count, AtomicRmwOp.Add, 1, AtomicOrder.SeqCst);
 
-                self.loop.onNextTick(dispatch_tick_node_ptr);
+                self.dispatch();
             }
             return result;
         }
 
-        async fn dispatch(self: *SelfChannel, tick_node_ptr: **Loop.NextTickNode) void {
-            // resumed by onNextTick
-            suspend |handle| {
-                var tick_node = Loop.NextTickNode{
-                    .data = handle,
-                    .next = undefined,
-                };
-                tick_node_ptr.* = &tick_node;
-            }
-
+        fn dispatch(self: *SelfChannel) void {
             // set the "need dispatch" flag
             _ = @atomicRmw(u8, &self.need_dispatch, AtomicRmwOp.Xchg, 1, AtomicOrder.SeqCst);
 

test/stage2/compare_output.zig

@@ -2,6 +2,7 @@ const std = @import("std");
 const TestContext = @import("../../src-self-hosted/test.zig").TestContext;
 
 pub fn addCases(ctx: *TestContext) !void {
+    // hello world
     try ctx.testCompareOutputLibC(
         \\extern fn puts([*]const u8) void;
         \\export fn main() c_int {
@@ -9,4 +10,16 @@ pub fn addCases(ctx: *TestContext) !void {
         \\    return 0;
         \\}
     , "Hello, world!" ++ std.cstr.line_sep);
+
+    // function calling another function
+    try ctx.testCompareOutputLibC(
+        \\extern fn puts(s: [*]const u8) void;
+        \\export fn main() c_int {
+        \\    return foo(c"OK");
+        \\}
+        \\fn foo(s: [*]const u8) c_int {
+        \\    puts(s);
+        \\    return 0;
+        \\}
+    , "OK" ++ std.cstr.line_sep);
 }