From 524dc756b30c19db070362c8094459c7c8e4fd8a Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Tue, 26 Dec 2023 23:45:44 -0700 Subject: 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. --- src/Compilation.zig | 148 ++++++++++++++++++++++++++++++++-------------------- src/link.zig | 2 - src/link/Wasm.zig | 24 ++++----- 3 files changed, 102 insertions(+), 72 deletions(-) (limited to 'src') diff --git a/src/Compilation.zig b/src/Compilation.zig index 9029d8ecbc..86caba7651 100644 --- a/src/Compilation.zig +++ b/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); diff --git a/src/link.zig b/src/link.zig index a051d27b12..fdcd5fcf41 100644 --- a/src/link.zig +++ b/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, diff --git a/src/link/Wasm.zig b/src/link/Wasm.zig index 91653ce641..7f3ca877c6 100644 --- a/src/link/Wasm.zig +++ b/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 -- cgit v1.2.3