commit 8a8bf5ad023451a22fd7abf438e6c7ee105ac6bf (tree)
parent affe5ed867009c889f2df7b624387c2bceefcc0e
Author: Andrew Kelley <andrew@ziglang.org>
Date: Thu, 7 May 2026 15:32:02 -0700
maker: update ObjCopy to new system
Diffstat:
6 files changed, 215 insertions(+), 114 deletions(-)
diff --git a/BRANCH_TODO b/BRANCH_TODO
@@ -20,6 +20,7 @@
- and adjust dependencyInner to not openDir()
## Followup Issues
+* stop leaking into global process arena
* reduce the size of Maker.Step.Extended (make Run smaller) probably by using an arena per make
* link_eh_frame_hdr should be DefaultingBool
* make --foo, --no-foo CLI args uniform (make them -f args instead)
diff --git a/lib/compiler/Maker.zig b/lib/compiler/Maker.zig
@@ -612,7 +612,7 @@ pub fn main(init: process.Init.Minimal) !void {
step.state = .precheck_done;
const deps = step_index.ptr(c).deps.slice(c);
step.pending_deps = @intCast(deps.len);
- step.reset(gpa);
+ step.reset(&maker);
}
continue :rebuild;
},
@@ -1506,10 +1506,9 @@ fn constructGraphAndCheckForDependencyLoop(
/// invalidated.
pub fn invalidateResult(maker: *Maker, step: *Step) bool {
if (step.state == .precheck_done) return false;
- const gpa = maker.gpa;
assert(step.pending_deps == 0);
step.state = .precheck_done;
- step.reset(gpa);
+ step.reset(maker);
for (step.dependants.items) |dependant_index| {
const dependant = maker.stepByIndex(dependant_index);
_ = invalidateResult(maker, dependant);
diff --git a/lib/compiler/Maker/Step.zig b/lib/compiler/Maker/Step.zig
@@ -23,6 +23,7 @@ pub const Run = @import("Step/Run.zig");
pub const InstallArtifact = @import("Step/InstallArtifact.zig");
pub const InstallFile = @import("Step/InstallFile.zig");
pub const UpdateSourceFiles = @import("Step/UpdateSourceFiles.zig");
+pub const ObjCopy = @import("Step/ObjCopy.zig");
/// Avoid false sharing.
_: void align(std.atomic.cache_line) = {},
@@ -76,7 +77,7 @@ pub const Extended = union(enum) {
install_artifact: InstallArtifact,
install_dir: Todo,
install_file: InstallFile,
- obj_copy: Todo,
+ obj_copy: ObjCopy,
options: Todo,
remove_dir: Todo,
run: Run,
@@ -306,8 +307,9 @@ pub fn make(
}
/// Prepares the step for being re-evaluated.
-pub fn reset(step: *Step, gpa: Allocator) void {
+pub fn reset(step: *Step, maker: *Maker) void {
assert(step.state == .precheck_done);
+ const gpa = maker.gpa;
if (step.result_failed_command) |cmd| gpa.free(cmd);
@@ -318,7 +320,7 @@ pub fn reset(step: *Step, gpa: Allocator) void {
step.result_peak_rss = 0;
step.result_failed_command = null;
step.test_results = .{};
- step.clearWatchInputs();
+ step.clearWatchInputs(maker);
step.result_error_bundle.deinit(gpa);
step.result_error_bundle = std.zig.ErrorBundle.empty;
@@ -732,7 +734,7 @@ fn failWithCacheError(
pub fn writeManifest(s: *Step, maker: *Maker, man: *Cache.Manifest) !void {
if (s.test_results.isSuccess()) {
man.writeManifest() catch |err| {
- try s.addError(maker, "unable to write cache manifest: {t}", .{err});
+ try s.addError(maker, "failed writing cache manifest: {t}", .{err});
};
}
}
diff --git a/lib/compiler/Maker/Step/ObjCopy.zig b/lib/compiler/Maker/Step/ObjCopy.zig
@@ -2,6 +2,7 @@ const ObjCopy = @This();
const std = @import("std");
const Io = std.Io;
+const Path = std.Build.Cache.Path;
const allocPrint = std.fmt.allocPrint;
const Configuration = std.Build.Configuration;
@@ -23,120 +24,152 @@ pub fn make(
const conf_step = step_index.ptr(conf);
const conf_oc = conf_step.extended.get(conf.extra).obj_copy;
const cache_root = graph.local_cache_root;
+ const input_lazy_path = conf_oc.input_file.get(conf);
+ const only_section: ?[]const u8 = if (conf_oc.only_section.value) |s| s.slice(conf) else null;
+ const opt_basename: ?[]const u8 = if (conf_oc.basename.value) |s| s.slice(conf) else null;
+ const opt_debug_basename: ?[]const u8 = if (conf_oc.debug_basename.value) |s| s.slice(conf) else null;
- try step.singleUnchangingWatchInput(maker, arena, conf_oc.input_file);
+ try step.singleUnchangingWatchInput(maker, arena, input_lazy_path);
var man = graph.cache.obtain();
defer man.deinit();
- const src_path = try maker.resolveLazyPathIndex(arena, conf_oc.input_file, step_index);
- _ = try man.addFilePath(src_path, null);
- man.hash.addOptionalBytes(conf_oc.only_section);
- man.hash.addOptional(conf_oc.pad_to);
- man.hash.addOptional(conf_oc.format);
- man.hash.add(conf_oc.compress_debug);
- man.hash.add(conf_oc.strip);
- man.hash.add(conf_oc.output_file_debug != null);
+ const input_path = try maker.resolveLazyPath(arena, input_lazy_path, step_index);
+ _ = try man.addFilePath(input_path, null);
+ man.hash.addOptionalBytes(only_section);
+ man.hash.addOptionalBytes(opt_basename);
+ man.hash.addOptionalBytes(opt_debug_basename);
+ man.hash.addOptional(conf_oc.pad_to.value);
+ man.hash.add(conf_oc.flags.format);
+ man.hash.add(conf_oc.flags.compress_debug);
+ man.hash.add(conf_oc.flags.strip);
+ man.hash.add(conf_oc.debug_file.value != null);
- if (try step.cacheHit(&man)) {
+ const basename = opt_basename orelse Io.Dir.path.basename(input_path.sub_path);
+
+ if (try step.cacheHit(maker, &man)) {
// Cache hit, skip subprocess execution.
const digest = man.final();
- conf_oc.output_file.path = try cache_root.join(arena, &.{
- "o", &digest, conf_oc.basename,
- });
- if (conf_oc.output_file_debug) |*file| {
- file.path = try cache_root.join(arena, &.{
- "o", &digest, try allocPrint(arena, "{s}.debug", .{conf_oc.basename}),
+ maker.generatedPath(conf_oc.output_file).* = .{
+ .root_dir = cache_root,
+ .sub_path = try Io.Dir.path.join(arena, &.{ "o", &digest, basename }),
+ };
+ if (conf_oc.debug_file.value) |debug_file| {
+ const debug_basename = opt_debug_basename orelse try allocPrint(arena, "{s}.debug", .{
+ Io.Dir.path.basename(input_path.sub_path),
});
+ maker.generatedPath(debug_file).* = .{
+ .root_dir = cache_root,
+ .sub_path = try Io.Dir.path.join(arena, &.{ "o", &digest, debug_basename }),
+ };
}
return;
}
+ // We don't find out more input files while executing objcopy so we can
+ // already obtain the digest and use it directly as the output path.
const digest = man.final();
- const cache_path = "o" ++ Io.Dir.path.sep_str ++ digest;
- const full_dest_path = try cache_root.join(arena, &.{ cache_path, conf_oc.basename });
- const full_dest_path_debug = try cache_root.join(arena, &.{
- cache_path, try allocPrint(arena, "{s}.debug", .{conf_oc.basename}),
- });
- cache_root.handle.createDirPath(io, cache_path) catch |err|
- return step.fail("unable to make path {s}: {t}", .{ cache_path, err });
+ const dest_path: Path = .{
+ .root_dir = cache_root,
+ .sub_path = try Io.Dir.path.join(arena, &.{ "o", &digest, basename }),
+ };
+ const dest_dirname = dest_path.dirname().?;
+ dest_dirname.root_dir.handle.createDirPath(io, dest_dirname.sub_path) catch |err|
+ return step.fail(maker, "failed to create path {f}: {t}", .{ dest_dirname, err });
var argv: std.ArrayList([]const u8) = .empty;
try argv.ensureUnusedCapacity(arena, 11);
argv.addManyAsArrayAssumeCapacity(2).* = .{ graph.zig_exe, "objcopy" };
- if (conf_oc.only_section) |only_section|
- argv.addManyAsArrayAssumeCapacity(2).* = .{ "-j", only_section };
+ if (only_section) |s| argv.addManyAsArrayAssumeCapacity(2).* = .{ "-j", s };
- switch (conf_oc.strip) {
+ switch (conf_oc.flags.strip) {
.none => {},
.debug => argv.appendAssumeCapacity("--strip-debug"),
.debug_and_symbols => argv.appendAssumeCapacity("--strip-all"),
}
- if (conf_oc.pad_to) |pad_to| {
+ if (conf_oc.pad_to.value) |pad_to| {
argv.addManyAsArrayAssumeCapacity(2).* = .{
"--pad-to", try allocPrint(arena, "{d}", .{pad_to}),
};
}
- if (conf_oc.format) |format| {
- argv.addManyAsArrayAssumeCapacity(2).* = .{
- "-O",
- switch (format) {
- .bin => "binary",
- .hex => "hex",
- .elf => "elf",
- },
- };
+ switch (conf_oc.flags.format) {
+ .default => {},
+ else => |t| argv.addManyAsArrayAssumeCapacity(2).* = .{ "-O", @tagName(t) },
}
- if (conf_oc.compress_debug)
+ if (conf_oc.flags.compress_debug)
argv.appendAssumeCapacity("--compress-debug-sections");
- if (conf_oc.output_file_debug != null)
- argv.appendAssumeCapacity(try allocPrint(arena, "--extract-to={s}", .{full_dest_path_debug}));
+ if (conf_oc.debug_file.value) |debug_file| {
+ const debug_basename = opt_debug_basename orelse try allocPrint(arena, "{s}.debug", .{
+ Io.Dir.path.basename(input_path.sub_path),
+ });
+ const debug_dest_path: Path = .{
+ .root_dir = cache_root,
+ .sub_path = try Io.Dir.path.join(arena, &.{ "o", &digest, debug_basename }),
+ };
+ argv.appendAssumeCapacity(try allocPrint(arena, "--extract-to={f}", .{debug_dest_path}));
+ maker.generatedPath(debug_file).* = debug_dest_path;
+ }
- try argv.ensureUnusedCapacity(arena, 9);
+ try argv.ensureUnusedCapacity(arena, conf_oc.add_section.slice.len * 2);
- if (conf_oc.add_section) |section| {
+ for (conf_oc.add_section.slice) |section| {
argv.appendAssumeCapacity("--add-section");
argv.appendAssumeCapacity(try allocPrint(arena, "{s}={f}", .{
- section.section_name, try maker.resolveLazyPathIndex(arena, section.file_path, step_index),
+ section.section_name.slice(conf),
+ try maker.resolveLazyPathIndex(arena, section.file_path, step_index),
}));
}
- if (conf_oc.set_section_alignment) |set_align| {
- argv.appendAssumeCapacity("--set-section-alignment");
- argv.appendAssumeCapacity(try allocPrint(arena, "{s}={d}", .{ set_align.section_name, set_align.alignment }));
- }
+ for (conf_oc.update_section.slice) |update| {
+ const name = update.section_name.slice(conf);
- if (conf_oc.set_section_flags) |set_flags| {
- const f = set_flags.flags;
- // trailing comma is allowed
- argv.appendAssumeCapacity("--set-section-flags");
- argv.appendAssumeCapacity(try allocPrint(arena, "{s}={s}{s}{s}{s}{s}{s}{s}{s}{s}", .{
- set_flags.section_name,
- if (f.alloc) "alloc," else "",
- if (f.contents) "contents," else "",
- if (f.load) "load," else "",
- if (f.readonly) "readonly," else "",
- if (f.code) "code," else "",
- if (f.exclude) "exclude," else "",
- if (f.large) "large," else "",
- if (f.merge) "merge," else "",
- if (f.strings) "strings," else "",
- }));
+ try argv.ensureUnusedCapacity(arena, 4);
+
+ if (update.flags.alignment.toBytes()) |a| {
+ argv.appendAssumeCapacity("--set-section-alignment");
+ argv.appendAssumeCapacity(try allocPrint(arena, "{s}={d}", .{ name, a }));
+ }
+
+ const f = update.flags.section_flags;
+ const default_flags: Configuration.Step.ObjCopy.SectionFlags = .{};
+
+ if (f != default_flags) {
+ // trailing comma is allowed
+ argv.appendAssumeCapacity("--set-section-flags");
+ argv.appendAssumeCapacity(try allocPrint(arena, "{s}={s}{s}{s}{s}{s}{s}{s}{s}{s}", .{
+ name,
+ if (f.alloc) "alloc," else "",
+ if (f.contents) "contents," else "",
+ if (f.load) "load," else "",
+ if (f.readonly) "readonly," else "",
+ if (f.code) "code," else "",
+ if (f.exclude) "exclude," else "",
+ if (f.large) "large," else "",
+ if (f.merge) "merge," else "",
+ if (f.strings) "strings," else "",
+ }));
+ }
}
- argv.appendAssumeCapacity(src_path);
- argv.appendAssumeCapacity(full_dest_path);
+ argv.appendAssumeCapacity(try allocPrint(arena, "{f}", .{input_path}));
+ argv.appendAssumeCapacity(try allocPrint(arena, "{f}", .{dest_path}));
argv.appendAssumeCapacity("--listen=-");
- _ = try Step.evalZigProcess(step_index, maker, argv.items, progress_node, false);
+ _ = Step.evalZigProcess(step_index, maker, argv.items, progress_node, false) catch |err| switch (err) {
+ error.NeedCompileErrorCheck => unreachable,
+ else => |e| return e,
+ };
+
+ maker.generatedPath(conf_oc.output_file).* = dest_path;
- conf_oc.output_file.path = full_dest_path;
- if (conf_oc.output_file_debug) |*file| file.path = full_dest_path_debug;
- try man.writeManifest();
+ man.writeManifest() catch |err| switch (err) {
+ error.Canceled => |e| return e,
+ else => |e| try step.addError(maker, "failed writing cache manifest: {t}", .{e}),
+ };
}
diff --git a/lib/std/Build/Configuration.zig b/lib/std/Build/Configuration.zig
@@ -1102,10 +1102,87 @@ pub const Step = extern struct {
pub const ObjCopy = struct {
flags: @This().Flags,
+ input_file: LazyPath.Index,
+ output_file: GeneratedFileIndex,
+ basename: Storage.FlagOptional(.flags, .basename, String),
+ debug_file: Storage.FlagOptional(.flags, .debug_file, GeneratedFileIndex),
+ debug_basename: Storage.FlagOptional(.flags, .debug_basename, String),
+ only_section: Storage.FlagOptional(.flags, .only_section, String),
+ pad_to: Storage.FlagOptional(.flags, .pad_to, u64),
+ add_section: Storage.FlagLengthPrefixedList(.flags, .add_section, AddSection),
+ update_section: Storage.FlagLengthPrefixedList(.flags, .update_section, UpdateSection),
+
+ pub const Format = enum(u2) {
+ binary,
+ hex,
+ elf,
+ default,
+
+ pub fn init(f: ?std.Build.Step.ObjCopy.Format) @This() {
+ return switch (f orelse return .default) {
+ .binary => .binary,
+ .hex => .hex,
+ .elf => .elf,
+ };
+ }
+ };
+
+ pub const Strip = enum(u2) {
+ none,
+ debug,
+ debug_and_symbols,
+ };
+
+ pub const AddSection = extern struct {
+ section_name: String,
+ file_path: LazyPath.Index,
+ };
+
+ pub const UpdateSection = extern struct {
+ section_name: String,
+ flags: @This().Flags,
+
+ pub const Flags = packed struct(u32) {
+ section_flags: SectionFlags,
+ alignment: Alignment,
+ _: u17 = 0,
+ };
+ };
+
+ pub const SectionFlags = packed struct(u9) {
+ /// add SHF_ALLOC
+ alloc: bool = false,
+ /// if section is SHT_NOBITS, set SHT_PROGBITS, otherwise do nothing
+ contents: bool = false,
+ /// if section is SHT_NOBITS, set SHT_PROGBITS, otherwise do nothing (same as contents)
+ load: bool = false,
+ /// readonly: clear default SHF_WRITE flag
+ readonly: bool = false,
+ /// add SHF_EXECINSTR
+ code: bool = false,
+ /// add SHF_EXCLUDE
+ exclude: bool = false,
+ /// add SHF_X86_64_LARGE. Fatal error if target is not x86_64
+ large: bool = false,
+ /// add SHF_MERGE
+ merge: bool = false,
+ /// add SHF_STRINGS
+ strings: bool = false,
+ };
pub const Flags = packed struct(u32) {
tag: Tag = .obj_copy,
- _: u27 = 0,
+ basename: bool,
+ debug_file: bool,
+ debug_basename: bool,
+ format: Format,
+ strip: Strip,
+ compress_debug: bool,
+ only_section: bool,
+ pad_to: bool,
+ add_section: bool,
+ update_section: bool,
+ _: u15 = 0,
};
};
@@ -1658,6 +1735,26 @@ pub const Bytes = extern struct {
}
};
+/// Stored as a power-of-two, with one special value to indicate none.
+pub const Alignment = enum(u6) {
+ @"1" = 0,
+ @"2" = 1,
+ @"4" = 2,
+ @"8" = 3,
+ @"16" = 4,
+ @"32" = 5,
+ @"64" = 6,
+ none = std.math.maxInt(u6),
+ _,
+
+ pub fn toBytes(a: @This()) ?u64 {
+ return switch (a) {
+ .none => null,
+ else => @as(u64, 1) << @intFromEnum(a),
+ };
+ }
+};
+
pub const DefaultingBool = enum(u2) {
false,
true,
diff --git a/lib/std/Build/Step/ObjCopy.zig b/lib/std/Build/Step/ObjCopy.zig
@@ -6,11 +6,11 @@ const Configuration = std.Build.Configuration;
step: Step,
input_file: std.Build.LazyPath,
-basename: []const u8,
+basename: ?[]const u8,
output_file: Configuration.GeneratedFileIndex,
output_file_debug: Configuration.OptionalGeneratedFileIndex,
-format: ?RawFormat,
+format: ?Format,
only_section: ?[]const u8,
pad_to: ?u64,
strip: Strip,
@@ -22,38 +22,9 @@ set_section_flags: ?SetSectionFlags,
pub const base_tag: Step.Tag = .obj_copy;
-pub const RawFormat = enum {
- bin,
- hex,
- elf,
-};
-
-pub const Strip = enum {
- none,
- debug,
- debug_and_symbols,
-};
-
-pub const SectionFlags = packed struct {
- /// add SHF_ALLOC
- alloc: bool = false,
- /// if section is SHT_NOBITS, set SHT_PROGBITS, otherwise do nothing
- contents: bool = false,
- /// if section is SHT_NOBITS, set SHT_PROGBITS, otherwise do nothing (same as contents)
- load: bool = false,
- /// readonly: clear default SHF_WRITE flag
- readonly: bool = false,
- /// add SHF_EXECINSTR
- code: bool = false,
- /// add SHF_EXCLUDE
- exclude: bool = false,
- /// add SHF_X86_64_LARGE. Fatal error if target is not x86_64
- large: bool = false,
- /// add SHF_MERGE
- merge: bool = false,
- /// add SHF_STRINGS
- strings: bool = false,
-};
+pub const Format = enum { binary, hex, elf };
+pub const Strip = Configuration.Step.ObjCopy.Strip;
+pub const SectionFlags = Configuration.Step.ObjCopy.SectionFlags;
pub const AddSection = struct {
section_name: []const u8,
@@ -72,7 +43,7 @@ pub const SetSectionFlags = struct {
pub const Options = struct {
basename: ?[]const u8 = null,
- format: ?RawFormat = null,
+ format: ?Format = null,
only_section: ?[]const u8 = null,
pad_to: ?u64 = null,
@@ -95,7 +66,6 @@ pub fn create(
options: Options,
) *ObjCopy {
const graph = owner.graph;
- const arena = graph.arena;
const obj_copy = graph.create(ObjCopy);
obj_copy.* = .{
.step = .init(.{
@@ -104,8 +74,7 @@ pub fn create(
.owner = owner,
}),
.input_file = input_file,
- .basename = options.basename orelse
- std.fmt.allocPrint(arena, "{f}", .{input_file.fmt(graph)}) catch @panic("OOM"),
+ .basename = options.basename,
.output_file = graph.addGeneratedFile(&obj_copy.step),
.output_file_debug = if (options.strip != .none and options.extract_to_separate_file)
.init(graph.addGeneratedFile(&obj_copy.step))
