From 8aab222ffbde5bfc141b23b2553b2887cf1a3ae3 Mon Sep 17 00:00:00 2001 From: mlugg Date: Thu, 15 May 2025 13:15:23 +0100 Subject: [PATCH] Compilation: add missing link file options to cache manifest Also add a standalone test which covers the `-fentry` case. It does this by performing two reproducible compilations which are identical other than having different entry points, and checking whether the emitted binaries are identical (they should *not* be). Resolves: #23869 --- src/Compilation.zig | 15 +++++++ test/standalone/build.zig.zon | 3 ++ test/standalone/entry_point/build.zig | 46 ++++++++++++++++++++ test/standalone/entry_point/check_differ.zig | 17 ++++++++ test/standalone/entry_point/main.zig | 7 +++ 5 files changed, 88 insertions(+) create mode 100644 test/standalone/entry_point/build.zig create mode 100644 test/standalone/entry_point/check_differ.zig create mode 100644 test/standalone/entry_point/main.zig diff --git a/src/Compilation.zig b/src/Compilation.zig index 065b717931..a855b983c9 100644 --- a/src/Compilation.zig +++ b/src/Compilation.zig @@ -3233,6 +3233,13 @@ fn addNonIncrementalStuffToCacheManifest( man.hash.addOptional(opts.allow_shlib_undefined); man.hash.add(opts.bind_global_refs_locally); + const EntryTag = @typeInfo(link.File.OpenOptions.Entry).@"union".tag_type.?; + man.hash.add(@as(EntryTag, opts.entry)); + switch (opts.entry) { + .default, .disabled, .enabled => {}, + .named => |name| man.hash.addBytes(name), + } + // ELF specific stuff man.hash.add(opts.z_nodelete); man.hash.add(opts.z_notext); @@ -3254,6 +3261,9 @@ fn addNonIncrementalStuffToCacheManifest( man.hash.addOptional(opts.max_memory); man.hash.addOptional(opts.global_base); man.hash.addListOfBytes(opts.export_symbol_names); + man.hash.add(opts.import_symbols); + man.hash.add(opts.import_table); + man.hash.add(opts.export_table); // Mach-O specific stuff try link.File.MachO.hashAddFrameworks(man, opts.frameworks); @@ -3264,6 +3274,9 @@ fn addNonIncrementalStuffToCacheManifest( man.hash.add(opts.dead_strip_dylibs); man.hash.add(opts.force_load_objc); man.hash.add(opts.discard_local_symbols); + man.hash.addOptional(opts.compatibility_version); + man.hash.addOptionalBytes(opts.install_name); + man.hash.addOptional(opts.darwin_sdk_layout); // COFF specific stuff man.hash.addOptional(opts.subsystem); @@ -3272,6 +3285,8 @@ fn addNonIncrementalStuffToCacheManifest( man.hash.add(opts.dynamicbase); man.hash.addOptional(opts.major_subsystem_version); man.hash.addOptional(opts.minor_subsystem_version); + man.hash.addOptionalBytes(opts.pdb_source_path); + man.hash.addOptionalBytes(opts.module_definition_file); } fn emitFromCObject( diff --git a/test/standalone/build.zig.zon b/test/standalone/build.zig.zon index e1829b4878..652f6bbb4a 100644 --- a/test/standalone/build.zig.zon +++ b/test/standalone/build.zig.zon @@ -201,6 +201,9 @@ .config_header = .{ .path = "config_header", }, + .entry_point = .{ + .path = "entry_point", + }, }, .paths = .{ "build.zig", diff --git a/test/standalone/entry_point/build.zig b/test/standalone/entry_point/build.zig new file mode 100644 index 0000000000..42ba6211ae --- /dev/null +++ b/test/standalone/entry_point/build.zig @@ -0,0 +1,46 @@ +pub fn build(b: *std.Build) !void { + const mod = b.createModule(.{ + // Setting the entry point doesn't work properly on all targets right now. Since we're + // really just trying to make sure that the compiler *frontend* respects `-fentry` and + // includes it in the cache manifest, just test for a target where it works. + .target = b.resolveTargetQuery(try .parse(.{ + .arch_os_abi = "x86_64-linux", + })), + .optimize = .ReleaseFast, // non-Debug build for reproducible output + .root_source_file = b.path("main.zig"), + }); + + const exe_foo = b.addExecutable(.{ + .name = "the_exe", // same name for reproducible output + .root_module = mod, + }); + exe_foo.entry = .{ .symbol_name = "foo" }; + const exe_bar = b.addExecutable(.{ + .name = "the_exe", // same name for reproducible output + .root_module = mod, + }); + exe_bar.entry = .{ .symbol_name = "bar" }; + + // Despite the output binary being reproducible, the `entry` differed, so the emitted binaries + // should be different. But the two compilations are otherwise identical, so if `entry` isn't + // being respected properly, we will see identical binaries. + + const check_differ_exe = b.addExecutable(.{ + .name = "check_differ", + .root_module = b.createModule(.{ + .target = b.graph.host, + .optimize = .Debug, + .root_source_file = b.path("check_differ.zig"), + }), + }); + + const diff_cmd = b.addRunArtifact(check_differ_exe); + diff_cmd.addFileArg(exe_foo.getEmittedBin()); + diff_cmd.addFileArg(exe_bar.getEmittedBin()); + diff_cmd.expectExitCode(0); + + const test_step = b.step("test", "Test it"); + b.default_step = test_step; + test_step.dependOn(&diff_cmd.step); +} +const std = @import("std"); diff --git a/test/standalone/entry_point/check_differ.zig b/test/standalone/entry_point/check_differ.zig new file mode 100644 index 0000000000..72945046a7 --- /dev/null +++ b/test/standalone/entry_point/check_differ.zig @@ -0,0 +1,17 @@ +pub fn main() !void { + var arena_state: std.heap.ArenaAllocator = .init(std.heap.page_allocator); + defer arena_state.deinit(); + const arena = arena_state.allocator(); + + const args = try std.process.argsAlloc(arena); + if (args.len != 3) return error.BadUsage; // usage: 'check_differ ' + + const contents_1 = try std.fs.cwd().readFileAlloc(arena, args[1], 1024 * 1024 * 64); // 64 MiB ought to be plenty + const contents_2 = try std.fs.cwd().readFileAlloc(arena, args[2], 1024 * 1024 * 64); // 64 MiB ought to be plenty + + if (std.mem.eql(u8, contents_1, contents_2)) { + return error.FilesMatch; + } + // success, files differ +} +const std = @import("std"); diff --git a/test/standalone/entry_point/main.zig b/test/standalone/entry_point/main.zig new file mode 100644 index 0000000000..c9fff5ce7b --- /dev/null +++ b/test/standalone/entry_point/main.zig @@ -0,0 +1,7 @@ +pub const _start = {}; +export fn foo() u32 { + return 123; +} +export fn bar() u32 { + return 456; +}