From e8090258b5ca045d7b89fdf625115477514880e0 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Mon, 17 Aug 2026 18:03:23 -0700 Subject: [PATCH] frontend: more resistant to absolute paths When using Compilation.Path, we already know the path prefix, so just use that directly instead of taking a detour through absolute paths. Also, when using whole cache mode, don't call `addModuleTableToCacheHash` because it is redundant with the logic in `PerThread.update` which iterates over `zcu.alive_files` and adds those files discovered via `@import` to the whole cache manifest. That was causing files relative to cwd to be added to cache manifest rather than being relative to build_root. --- lib/std/Build/Cache.zig | 37 +++++++++++-------- src/Compilation.zig | 78 ++++++++++++++++++++++++++++++++++++----- src/Zcu/PerThread.zig | 14 +++----- src/codegen/llvm.zig | 2 +- src/link/Dwarf.zig | 2 +- 5 files changed, 98 insertions(+), 35 deletions(-) diff --git a/lib/std/Build/Cache.zig b/lib/std/Build/Cache.zig index 2391e698c4158197c63db208cceb0618b342a208..9019cf5ec37b4702b49c60562f85785fdbad19a7 100644 --- a/lib/std/Build/Cache.zig +++ b/lib/std/Build/Cache.zig @@ -1032,24 +1032,32 @@ pub const Manifest = struct { /// Like `addFilePost` but when the file contents have already been loaded from disk. pub fn addFilePostContents( - self: *Manifest, + man: *Manifest, file_path: []const u8, bytes: []const u8, stat: File.Stat, ) !void { - assert(self.manifest_file != null); - const gpa = self.cache.gpa; + assert(man.manifest_file != null); + const gpa = man.cache.gpa; + const prefixed_path = try man.cache.findPrefix(file_path); + var keep = false; + defer if (!keep) gpa.free(prefixed_path.sub_path); + keep = try addPrefixedPathPostContents(man, prefixed_path, bytes, stat); + } - const prefixed_path = try self.cache.findPrefix(file_path); - errdefer gpa.free(prefixed_path.sub_path); + /// Low level function. `prefixed_path` references cloned memory. Returns + /// whether or not `prefixed_path.sub_path` should be kept. + pub fn addPrefixedPathPostContents( + man: *Manifest, + prefixed_path: PrefixedPath, + bytes: []const u8, + stat: File.Stat, + ) !bool { + const gpa = man.cache.gpa; + const gop = try man.files.getOrPutAdapted(gpa, prefixed_path, FilesAdapter{}); + errdefer _ = man.files.pop(); - const gop = try self.files.getOrPutAdapted(gpa, prefixed_path, FilesAdapter{}); - errdefer _ = self.files.pop(); - - if (gop.found_existing) { - gpa.free(prefixed_path.sub_path); - return; - } + if (gop.found_existing) return false; const new_file = gop.key_ptr; @@ -1062,7 +1070,7 @@ pub const Manifest = struct { .contents = null, }; - if (try self.isProblematicTimestamp(new_file.stat.mtime)) { + if (try man.isProblematicTimestamp(new_file.stat.mtime)) { // The actual file has an unreliable timestamp, force it to be hashed new_file.stat.mtime = .zero; new_file.stat.inode = 0; @@ -1074,7 +1082,8 @@ pub const Manifest = struct { hasher.final(&new_file.bin_digest); } - self.hash.hasher.update(&new_file.bin_digest); + man.hash.hasher.update(&new_file.bin_digest); + return true; } pub fn addDepFilePost(self: *Manifest, dir: Io.Dir, dep_file_sub_path: []const u8) !void { diff --git a/src/Compilation.zig b/src/Compilation.zig index 92d77b97f882816fae92bf88c8db3d18b1e37ec5..b0df1aaeade37e0e53231aa3e35f490f4ea642d9 100644 --- a/src/Compilation.zig +++ b/src/Compilation.zig @@ -506,10 +506,11 @@ pub const Path = struct { // so that we prefer `.root = .local_cache` over `.root = .zig_lib`. The easiest way to do // this is simply to prioritize the longest root path. const PathAndRoot = struct { ?[]const u8, Root }; - var roots: [3]PathAndRoot = .{ + var roots: [4]PathAndRoot = .{ .{ dirs.zig_lib.path, .zig_lib }, .{ dirs.global_cache.path, .global_cache }, .{ dirs.local_cache.path, .local_cache }, + .{ dirs.build_root.path, .build_root }, }; // This must be a stable sort, because the global and local cache directories may be the same, in // which case we need to make a consistent choice. @@ -660,7 +661,7 @@ pub const Path = struct { /// This should not be used for most of the compiler pipeline, but is useful when emitting /// paths from the compilation (e.g. in debug info), because they will not depend on the cwd. /// The returned path is owned by the caller and allocated into `gpa`. - pub fn toAbsolute(p: Path, dirs: std.zig.Directories, gpa: Allocator) Allocator.Error![]u8 { + pub fn toAbsolute(p: Path, dirs: *const std.zig.Directories, gpa: Allocator) Allocator.Error![]u8 { const root_path: []const u8 = switch (p.root) { .zig_lib => dirs.zig_lib.path orelse "", .global_cache => dirs.global_cache.path orelse "", @@ -701,6 +702,66 @@ pub const Path = struct { .no, .different_roots => false, }; } + + pub fn addToCacheManifestPostHit(p: Path, man: *Cache.Manifest, dirs: *const std.zig.Directories) !void { + comptime assert(0 == @backingInt(std.zig.Server.Message.PathPrefix.cwd)); + comptime assert(1 == @backingInt(std.zig.Server.Message.PathPrefix.zig_lib)); + comptime assert(2 == @backingInt(std.zig.Server.Message.PathPrefix.local_cache)); + comptime assert(3 == @backingInt(std.zig.Server.Message.PathPrefix.global_cache)); + comptime assert(4 == @backingInt(std.zig.Server.Message.PathPrefix.build_root)); + comptime assert(@typeInfo(std.zig.Server.Message.PathPrefix).@"enum".field_names.len == 5); + const gpa = man.cache.gpa; + const prefixed_path: Cache.PrefixedPath = .{ + .prefix = switch (p.root) { + .none => { + const path = try p.toAbsolute(dirs, gpa); + defer gpa.free(path); + return man.addFilePost(path); + }, + .zig_lib => 1, + .local_cache => 2, + .global_cache => 3, + .build_root => 4, + }, + .sub_path = try gpa.dupe(u8, p.sub_path), + }; + var keep = false; + defer if (!keep) gpa.free(prefixed_path.sub_path); + keep = try man.addPrefixedPathPost(prefixed_path); + } + + pub fn addToCacheManifestPostHitContents( + p: Path, + man: *Cache.Manifest, + dirs: *const std.zig.Directories, + bytes: []const u8, + stat: Cache.File.Stat, + ) !void { + comptime assert(0 == @backingInt(std.zig.Server.Message.PathPrefix.cwd)); + comptime assert(1 == @backingInt(std.zig.Server.Message.PathPrefix.zig_lib)); + comptime assert(2 == @backingInt(std.zig.Server.Message.PathPrefix.local_cache)); + comptime assert(3 == @backingInt(std.zig.Server.Message.PathPrefix.global_cache)); + comptime assert(4 == @backingInt(std.zig.Server.Message.PathPrefix.build_root)); + comptime assert(@typeInfo(std.zig.Server.Message.PathPrefix).@"enum".field_names.len == 5); + const gpa = man.cache.gpa; + const prefixed_path: Cache.PrefixedPath = .{ + .prefix = switch (p.root) { + .none => { + const path = try p.toAbsolute(dirs, gpa); + defer gpa.free(path); + return man.addFilePostContents(path, bytes, stat); + }, + .zig_lib => 1, + .local_cache => 2, + .global_cache => 3, + .build_root => 4, + }, + .sub_path = try gpa.dupe(u8, p.sub_path), + }; + var keep = false; + defer if (!keep) gpa.free(prefixed_path.sub_path); + keep = try man.addPrefixedPathPostContents(prefixed_path, bytes, stat); + } }; /// This small wrapper function just checks whether debug extensions are enabled before checking @@ -2776,7 +2837,7 @@ pub fn update(comp: *Compilation, main_progress_node: std.Progress.Node) UpdateE man = comp.cache_parent.obtain(); whole.cache_manifest = &man; - try addNonIncrementalStuffToCacheManifest(comp, arena, &man); + try addNonIncrementalStuffToCacheManifest(comp, &man); // Under `--time-report`, ignore cache hits; do the work anyway for those juicy numbers. const ignore_hit = comp.time_report != null; @@ -3328,15 +3389,14 @@ fn renameTmpIntoCache( /// anything from the link cache manifest. pub const link_hash_implementation_version = 14; -fn addNonIncrementalStuffToCacheManifest( - comp: *Compilation, - arena: Allocator, - man: *Cache.Manifest, -) !void { +fn addNonIncrementalStuffToCacheManifest(comp: *Compilation, man: *Cache.Manifest) !void { comptime assert(link_hash_implementation_version == 14); if (comp.zcu) |zcu| { - try addModuleTableToCacheHash(zcu, arena, &man.hash, .{ .files = man }); + // No need to call `addModuleTableToCacheHash` here because it is + // redundant with the logic in `PerThread.update` which iterates over + // `zcu.alive_files` and adds those files discovered via `@import` to + // the whole cache manifest. // Synchronize with other matching comments: ZigOnlyHashStuff man.hash.addListOfBytes(comp.test_filters); diff --git a/src/Zcu/PerThread.zig b/src/Zcu/PerThread.zig index 8b355ee22a5895eb65ef581aa536689078379315..0e4962985a566aa611a326ea1aeb2b61e9928ae4 100644 --- a/src/Zcu/PerThread.zig +++ b/src/Zcu/PerThread.zig @@ -221,22 +221,19 @@ pub fn update( .astgen_failure, .success => {}, // the file was read successfully } - const path = try file.path.toAbsolute(comp.dirs, gpa); - defer gpa.free(path); - const result = res: { try whole.cache_manifest_mutex.lock(io); defer whole.cache_manifest_mutex.unlock(io); if (file.source) |source| { - break :res man.addFilePostContents(path, source, file.stat); + break :res file.path.addToCacheManifestPostHitContents(man, &comp.dirs, source, file.stat); } else { - break :res man.addFilePost(path); + break :res file.path.addToCacheManifestPostHit(man, &comp.dirs); } }; result catch |err| switch (err) { error.OutOfMemory => |e| return e, else => { - try pt.reportRetryableFileError(file_index, "unable to update cache: {s}", .{@errorName(err)}); + try pt.reportRetryableFileError(file_index, "unable to update cache: {t}", .{err}); continue; }, }; @@ -2965,13 +2962,10 @@ fn newEmbedFile( const array_len = Value.fromInterned(new_file.val).typeOf(zcu).childType(zcu).arrayLen(zcu); const contents = ip_str.toSlice(array_len, ip); - const path_str = try path.toAbsolute(comp.dirs, gpa); - defer gpa.free(path_str); - try whole.cache_manifest_mutex.lock(io); defer whole.cache_manifest_mutex.unlock(io); - try man.addFilePostContents(path_str, contents, new_file.stat); + try path.addToCacheManifestPostHitContents(man, &comp.dirs, contents, new_file.stat); } return new_file; diff --git a/src/codegen/llvm.zig b/src/codegen/llvm.zig index 9e6c29c94c5cfdab42e3822d5a62e5e279d510c8..4022a2b901c77443b977a4461e044de40a8230c9 100644 --- a/src/codegen/llvm.zig +++ b/src/codegen/llvm.zig @@ -481,7 +481,7 @@ pub const Object = struct { // way already, but here we throw all that sweet information // into the garbage can by converting into absolute paths. What // a terrible tragedy. - const compile_unit_dir = try zcu.main_mod.root.toAbsolute(comp.dirs, arena); + const compile_unit_dir = try zcu.main_mod.root.toAbsolute(&comp.dirs, arena); const debug_file = try builder.debugFile( try builder.metadataString(comp.root_name), diff --git a/src/link/Dwarf.zig b/src/link/Dwarf.zig index c8eef4c8651367edbc1abffcd3cf24d76b847603..a73874ea0259b566444233452028d36ec99479e3 100644 --- a/src/link/Dwarf.zig +++ b/src/link/Dwarf.zig @@ -4735,7 +4735,7 @@ fn flushWriterError(dwarf: *Dwarf, pt: Zcu.PerThread) (UpdateError || Writer.Err } for (dwarf.mods.keys(), dwarf.mods.values()) |mod, *mod_info| { - const root_dir_path = try mod.root.toAbsolute(zcu.comp.dirs, dwarf.gpa); + const root_dir_path = try mod.root.toAbsolute(&zcu.comp.dirs, dwarf.gpa); defer dwarf.gpa.free(root_dir_path); mod_info.root_dir_path = try dwarf.debug_line_str.addString(dwarf, root_dir_path); } -- 2.54.0