Commit 524dc756b3

Andrew Kelley <andrew@ziglang.org>
2023-12-27 07:45:44
Compilation: several branch regression fixes
* move wasi_emulated_libs into Compilation - It needs to be accessed from Compilation, which needs to potentially build those artifacts. * Compilation: improve error reporting for two cases - the setMiscFailure mechanism is handy - let's use it! * fix one instance of incorrectly checking for emit_bin via `comp.bin_file != null`. There are more instances of this that need to be fixed in a future commit. * fix renameTmpIntoCache not handling the case where it needs to make the "o" directory in the zig-cache directory. - while I'm at it, simplify the logic for handling the fact that Windows returns error.AccessDenied rather than error.PathAlreadyExists for failure to rename a directory over another one. * fix missing cache hash additions - there are still more to add in a future commit - addNonIncrementalStuffToCacheManifest is called when bin_file is always null, and then it incorrectly checks if bin_file is non-null and only then adds a bunch of stuff to the cache hash. It needs to instead add to the cache hash based on lf_open_opts.
1 parent 57562c8
Changed files (3)
src/link/Wasm.zig
@@ -43,7 +43,6 @@ export_symbol_names: []const []const u8,
 global_base: ?u64,
 initial_memory: ?u64,
 max_memory: ?u64,
-wasi_emulated_libs: []const wasi_libc.CRTFile,
 /// Output name of the file
 name: []const u8,
 /// If this is not null, an object file is created by LLVM and linked with LLD afterwards.
@@ -435,7 +434,6 @@ pub fn createEmpty(
         .global_base = options.global_base,
         .initial_memory = options.initial_memory,
         .max_memory = options.max_memory,
-        .wasi_emulated_libs = options.wasi_emulated_libs,
 
         .entry_name = switch (options.entry) {
             .disabled => null,
@@ -3626,7 +3624,7 @@ fn linkWithZld(wasm: *Wasm, comp: *Compilation, prog_node: *std.Progress.Node) l
         const is_exe_or_dyn_lib = output_mode == .Exe or
             (output_mode == .Lib and link_mode == .Dynamic);
         if (is_exe_or_dyn_lib) {
-            for (wasm.wasi_emulated_libs) |crt_file| {
+            for (comp.wasi_emulated_libs) |crt_file| {
                 try positionals.append(try comp.get_libc_crt_file(
                     arena,
                     wasi_libc.emulatedLibCRFileLibName(crt_file),
@@ -4601,7 +4599,7 @@ fn linkWithLLD(wasm: *Wasm, comp: *Compilation, prog_node: *std.Progress.Node) !
     const import_memory = comp.config.import_memory;
     const target = comp.root_mod.resolved_target.result;
 
-    const gpa = wasm.base.comp.gpa;
+    const gpa = comp.gpa;
     var arena_allocator = std.heap.ArenaAllocator.init(gpa);
     defer arena_allocator.deinit();
     const arena = arena_allocator.allocator();
@@ -4611,7 +4609,7 @@ fn linkWithLLD(wasm: *Wasm, comp: *Compilation, prog_node: *std.Progress.Node) !
 
     // If there is no Zig code to compile, then we should skip flushing the output file because it
     // will not be part of the linker line anyway.
-    const module_obj_path: ?[]const u8 = if (wasm.base.comp.module != null) blk: {
+    const module_obj_path: ?[]const u8 = if (comp.module != null) blk: {
         try wasm.flushModule(comp, prog_node);
 
         if (fs.path.dirname(full_out_path)) |dirname| {
@@ -4626,7 +4624,7 @@ fn linkWithLLD(wasm: *Wasm, comp: *Compilation, prog_node: *std.Progress.Node) !
     sub_prog_node.context.refresh();
     defer sub_prog_node.end();
 
-    const is_obj = wasm.base.comp.config.output_mode == .Obj;
+    const is_obj = comp.config.output_mode == .Obj;
     const compiler_rt_path: ?[]const u8 = blk: {
         if (comp.compiler_rt_lib) |lib| break :blk lib.full_object_path;
         if (comp.compiler_rt_obj) |obj| break :blk obj.full_object_path;
@@ -4823,7 +4821,7 @@ fn linkWithLLD(wasm: *Wasm, comp: *Compilation, prog_node: *std.Progress.Node) !
             try argv.append("--allow-undefined");
         }
 
-        if (wasm.base.comp.config.output_mode == .Lib and wasm.base.comp.config.link_mode == .Dynamic) {
+        if (comp.config.output_mode == .Lib and comp.config.link_mode == .Dynamic) {
             try argv.append("--shared");
         }
         if (comp.config.pie) {
@@ -4842,10 +4840,10 @@ fn linkWithLLD(wasm: *Wasm, comp: *Compilation, prog_node: *std.Progress.Node) !
         }
 
         if (target.os.tag == .wasi) {
-            const is_exe_or_dyn_lib = wasm.base.comp.config.output_mode == .Exe or
-                (wasm.base.comp.config.output_mode == .Lib and wasm.base.comp.config.link_mode == .Dynamic);
+            const is_exe_or_dyn_lib = comp.config.output_mode == .Exe or
+                (comp.config.output_mode == .Lib and comp.config.link_mode == .Dynamic);
             if (is_exe_or_dyn_lib) {
-                for (wasm.wasi_emulated_libs) |crt_file| {
+                for (comp.wasi_emulated_libs) |crt_file| {
                     try argv.append(try comp.get_libc_crt_file(
                         arena,
                         wasi_libc.emulatedLibCRFileLibName(crt_file),
@@ -4891,7 +4889,7 @@ fn linkWithLLD(wasm: *Wasm, comp: *Compilation, prog_node: *std.Progress.Node) !
             try argv.append(p);
         }
 
-        if (wasm.base.comp.config.output_mode != .Obj and
+        if (comp.config.output_mode != .Obj and
             !comp.skip_linker_dependencies and
             !comp.config.link_libc)
         {
@@ -4902,7 +4900,7 @@ fn linkWithLLD(wasm: *Wasm, comp: *Compilation, prog_node: *std.Progress.Node) !
             try argv.append(p);
         }
 
-        if (wasm.base.comp.verbose_link) {
+        if (comp.verbose_link) {
             // Skip over our own name so that the LLD linker name is the first argv item.
             Compilation.dump_argv(argv.items[1..]);
         }
@@ -4977,7 +4975,7 @@ fn linkWithLLD(wasm: *Wasm, comp: *Compilation, prog_node: *std.Progress.Node) !
         // it, and then can react to that in the same way as trying to run an ELF file
         // from a foreign CPU architecture.
         if (fs.has_executable_bit and target.os.tag == .wasi and
-            wasm.base.comp.config.output_mode == .Exe)
+            comp.config.output_mode == .Exe)
         {
             // TODO: what's our strategy for reporting linker errors from this function?
             // report a nice error here with the file path if it fails instead of
src/Compilation.zig
@@ -190,6 +190,7 @@ compiler_rt_lib: ?CRTFile = null,
 compiler_rt_obj: ?CRTFile = null,
 
 glibc_so_files: ?glibc.BuiltSharedObjects = null,
+wasi_emulated_libs: []const wasi_libc.CRTFile,
 
 /// For example `Scrt1.o` and `libc_nonshared.a`. These are populated after building libc from source,
 /// The set of needed CRT (C runtime) files differs depending on the target and compilation settings.
@@ -707,6 +708,8 @@ pub const Win32Resource = struct {
 
 pub const MiscTask = enum {
     write_builtin_zig,
+    rename_results,
+    check_whole_cache,
     glibc_crt_file,
     glibc_shared_objects,
     musl_crt_file,
@@ -1500,6 +1503,7 @@ pub fn create(gpa: Allocator, options: CreateOptions) !*Compilation {
             .function_sections = options.function_sections,
             .data_sections = options.data_sections,
             .native_system_include_paths = options.native_system_include_paths,
+            .wasi_emulated_libs = options.wasi_emulated_libs,
         };
 
         // Prevent some footguns by making the "any" fields of config reflect
@@ -1521,7 +1525,6 @@ pub fn create(gpa: Allocator, options: CreateOptions) !*Compilation {
             .z_max_page_size = options.linker_z_max_page_size,
             .darwin_sdk_layout = libc_dirs.darwin_sdk_layout,
             .frameworks = options.frameworks,
-            .wasi_emulated_libs = options.wasi_emulated_libs,
             .lib_dirs = options.lib_dirs,
             .rpath_list = options.rpath_list,
             .symbol_wrap_set = options.symbol_wrap_set,
@@ -1672,10 +1675,10 @@ pub fn create(gpa: Allocator, options: CreateOptions) !*Compilation {
     };
     errdefer comp.destroy();
 
-    const target = options.root_mod.resolved_target.result;
+    const target = comp.root_mod.resolved_target.result;
 
-    const capable_of_building_compiler_rt = canBuildLibCompilerRt(target, options.config.use_llvm);
-    const capable_of_building_zig_libc = canBuildZigLibC(target, options.config.use_llvm);
+    const capable_of_building_compiler_rt = canBuildLibCompilerRt(target, comp.config.use_llvm);
+    const capable_of_building_zig_libc = canBuildZigLibC(target, comp.config.use_llvm);
 
     // Add a `CObject` for each `c_source_files`.
     try comp.c_object_table.ensureTotalCapacity(gpa, options.c_source_files.len);
@@ -1768,25 +1771,21 @@ pub fn create(gpa: Allocator, options: CreateOptions) !*Compilation {
             });
         }
 
-        if (comp.bin_file) |lf| {
-            if (lf.cast(link.File.Wasm)) |wasm| {
-                if (comp.wantBuildWasiLibcFromSource()) {
-                    if (!target_util.canBuildLibC(target)) return error.LibCUnavailable;
+        if (comp.wantBuildWasiLibcFromSource()) {
+            if (!target_util.canBuildLibC(target)) return error.LibCUnavailable;
 
-                    // worst-case we need all components
-                    try comp.work_queue.ensureUnusedCapacity(wasm.wasi_emulated_libs.len + 2);
+            // worst-case we need all components
+            try comp.work_queue.ensureUnusedCapacity(comp.wasi_emulated_libs.len + 2);
 
-                    for (wasm.wasi_emulated_libs) |crt_file| {
-                        comp.work_queue.writeItemAssumeCapacity(.{
-                            .wasi_libc_crt_file = crt_file,
-                        });
-                    }
-                    comp.work_queue.writeAssumeCapacity(&[_]Job{
-                        .{ .wasi_libc_crt_file = wasi_libc.execModelCrtFile(options.config.wasi_exec_model) },
-                        .{ .wasi_libc_crt_file = .libc_a },
-                    });
-                }
+            for (comp.wasi_emulated_libs) |crt_file| {
+                comp.work_queue.writeItemAssumeCapacity(.{
+                    .wasi_libc_crt_file = crt_file,
+                });
             }
+            comp.work_queue.writeAssumeCapacity(&[_]Job{
+                .{ .wasi_libc_crt_file = wasi_libc.execModelCrtFile(comp.config.wasi_exec_model) },
+                .{ .wasi_libc_crt_file = .libc_a },
+            });
         }
 
         if (comp.wantBuildMinGWFromSource()) {
@@ -1832,11 +1831,11 @@ pub fn create(gpa: Allocator, options: CreateOptions) !*Compilation {
         }
 
         if (comp.bin_file) |lf| {
-            if (comp.getTarget().isMinGW() and comp.config.any_non_single_threaded) {
+            if (target.isMinGW() and comp.config.any_non_single_threaded) {
                 // LLD might drop some symbols as unused during LTO and GCing, therefore,
                 // we force mark them for resolution here.
 
-                const tls_index_sym = switch (comp.getTarget().cpu.arch) {
+                const tls_index_sym = switch (target.cpu.arch) {
                     .x86 => "__tls_index",
                     else => "_tls_index",
                 };
@@ -2024,12 +2023,14 @@ pub fn update(comp: *Compilation, main_progress_node: *std.Progress.Node) !void
             try comp.addNonIncrementalStuffToCacheManifest(&man);
 
             const is_hit = man.hit() catch |err| {
-                // TODO properly bubble these up instead of emitting a warning
                 const i = man.failed_file_index orelse return err;
                 const pp = man.files.items[i].prefixed_path orelse return err;
                 const prefix = man.cache.prefixes()[pp.prefix].path orelse "";
-                std.log.warn("{s}: {s}{s}", .{ @errorName(err), prefix, pp.sub_path });
-                return err;
+                return comp.setMiscFailure(
+                    .check_whole_cache,
+                    "unable to check cache: stat file '{}{s}{s}' failed: {s}",
+                    .{ comp.local_cache_directory, prefix, pp.sub_path, @errorName(err) },
+                );
             };
             if (is_hit) {
                 comp.last_update_was_cache_hit = true;
@@ -2221,7 +2222,17 @@ pub fn update(comp: *Compilation, main_progress_node: *std.Progress.Node) !void
             const tmp_dir_sub_path = "tmp" ++ s ++ Package.Manifest.hex64(tmp_dir_rand_int);
             const o_sub_path = "o" ++ s ++ digest;
 
-            try renameTmpIntoCache(comp.local_cache_directory, tmp_dir_sub_path, o_sub_path);
+            renameTmpIntoCache(comp.local_cache_directory, tmp_dir_sub_path, o_sub_path) catch |err| {
+                return comp.setMiscFailure(
+                    .rename_results,
+                    "failed to rename compilation results ('{}{s}') into local cache ('{}{s}'): {s}",
+                    .{
+                        comp.local_cache_directory, tmp_dir_sub_path,
+                        comp.local_cache_directory, o_sub_path,
+                        @errorName(err),
+                    },
+                );
+            };
             comp.wholeCacheModeSetBinFilePath(whole, &digest);
 
             // Failure here only means an unnecessary cache miss.
@@ -2251,42 +2262,36 @@ fn renameTmpIntoCache(
     tmp_dir_sub_path: []const u8,
     o_sub_path: []const u8,
 ) !void {
+    var seen_eaccess = false;
     while (true) {
-        if (builtin.os.tag == .windows) {
-            // Work around windows `renameW` can't fail with `PathAlreadyExists`
+        std.fs.rename(
+            cache_directory.handle,
+            tmp_dir_sub_path,
+            cache_directory.handle,
+            o_sub_path,
+        ) catch |err| switch (err) {
+            // On Windows, rename fails with `AccessDenied` rather than `PathAlreadyExists`.
             // See https://github.com/ziglang/zig/issues/8362
-            if (cache_directory.handle.access(o_sub_path, .{})) |_| {
-                try cache_directory.handle.deleteTree(o_sub_path);
-                continue;
-            } else |err| switch (err) {
-                error.FileNotFound => {},
-                else => |e| return e,
-            }
-            std.fs.rename(
-                cache_directory.handle,
-                tmp_dir_sub_path,
-                cache_directory.handle,
-                o_sub_path,
-            ) catch |err| {
-                log.err("unable to rename cache dir {s} to {s}: {s}", .{ tmp_dir_sub_path, o_sub_path, @errorName(err) });
-                return err;
-            };
-            break;
-        } else {
-            std.fs.rename(
-                cache_directory.handle,
-                tmp_dir_sub_path,
-                cache_directory.handle,
-                o_sub_path,
-            ) catch |err| switch (err) {
-                error.PathAlreadyExists => {
+            error.AccessDenied => switch (builtin.os.tag) {
+                .windows => {
+                    if (!seen_eaccess) return error.AccessDenied;
+                    seen_eaccess = true;
                     try cache_directory.handle.deleteTree(o_sub_path);
                     continue;
                 },
-                else => |e| return e,
-            };
-            break;
-        }
+                else => return error.AccessDenied,
+            },
+            error.PathAlreadyExists => {
+                try cache_directory.handle.deleteTree(o_sub_path);
+                continue;
+            },
+            error.FileNotFound => {
+                try cache_directory.handle.makePath("o");
+                continue;
+            },
+            else => |e| return e,
+        };
+        break;
     }
 }
 
@@ -2386,6 +2391,10 @@ fn addNonIncrementalStuffToCacheManifest(comp: *Compilation, man: *Cache.Manifes
         try addModuleTableToCacheHash(gpa, arena, &man.hash, mod.main_mod, .{ .files = man });
 
         // Synchronize with other matching comments: ZigOnlyHashStuff
+        man.hash.add(comp.config.use_llvm);
+        man.hash.add(comp.config.use_lib_llvm);
+        man.hash.add(comp.config.dll_export_fns);
+        man.hash.add(comp.config.is_test);
         man.hash.add(comp.config.test_evented_io);
         man.hash.addOptionalBytes(comp.test_filter);
         man.hash.addOptionalBytes(comp.test_name_prefix);
@@ -2393,6 +2402,20 @@ fn addNonIncrementalStuffToCacheManifest(comp: *Compilation, man: *Cache.Manifes
         man.hash.add(comp.formatted_panics);
         man.hash.add(mod.emit_h != null);
         man.hash.add(mod.error_limit);
+    } else {
+        cache_helpers.addResolvedTarget(&man.hash, comp.root_mod.resolved_target);
+        man.hash.add(comp.root_mod.optimize_mode);
+        man.hash.add(comp.root_mod.code_model);
+        man.hash.add(comp.root_mod.single_threaded);
+        man.hash.add(comp.root_mod.error_tracing);
+        man.hash.add(comp.root_mod.pic);
+        man.hash.add(comp.root_mod.omit_frame_pointer);
+        man.hash.add(comp.root_mod.stack_check);
+        man.hash.add(comp.root_mod.red_zone);
+        man.hash.add(comp.root_mod.sanitize_c);
+        man.hash.add(comp.root_mod.sanitize_thread);
+        man.hash.add(comp.root_mod.unwind_tables);
+        man.hash.add(comp.root_mod.structured_cfg);
     }
 
     for (comp.objects) |obj| {
@@ -3829,8 +3852,19 @@ pub fn obtainCObjectCacheManifest(
     // Only things that need to be added on top of the base hash, and only things
     // that apply both to @cImport and compiling C objects. No linking stuff here!
     // Also nothing that applies only to compiling .zig code.
+    cache_helpers.addResolvedTarget(&man.hash, owner_mod.resolved_target);
+    man.hash.add(owner_mod.optimize_mode);
+    man.hash.add(owner_mod.code_model);
+    man.hash.add(owner_mod.single_threaded);
+    man.hash.add(owner_mod.error_tracing);
+    man.hash.add(owner_mod.pic);
+    man.hash.add(owner_mod.omit_frame_pointer);
+    man.hash.add(owner_mod.stack_check);
+    man.hash.add(owner_mod.red_zone);
     man.hash.add(owner_mod.sanitize_c);
     man.hash.add(owner_mod.sanitize_thread);
+    man.hash.add(owner_mod.unwind_tables);
+    man.hash.add(owner_mod.structured_cfg);
     man.hash.addListOfBytes(owner_mod.cc_argv);
     man.hash.add(comp.config.link_libcpp);
 
src/link.zig
@@ -170,8 +170,6 @@ pub const File = struct {
         /// (Windows) .def file to specify when linking
         module_definition_file: ?[]const u8,
 
-        wasi_emulated_libs: []const wasi_libc.CRTFile,
-
         pub const Entry = union(enum) {
             default,
             disabled,