Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

windows-gnu: Adhere to MinGW convention for build outputs #22415

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
4 changes: 2 additions & 2 deletions lib/compiler/aro/aro/Driver.zig
Original file line number Diff line number Diff line change
Expand Up @@ -765,7 +765,7 @@ fn processSource(
const fmt_template = "{s}{s}";
const fmt_args = .{
std.fs.path.stem(source.path),
d.comp.target.ofmt.fileExt(d.comp.target.cpu.arch),
d.comp.target.objFileExt(),
};
break :blk d.output_name orelse
std.fmt.bufPrint(&name_buf, fmt_template, fmt_args) catch return d.fatal("Filename too long for filesystem: " ++ fmt_template, fmt_args);
Expand All @@ -781,7 +781,7 @@ fn processSource(
const fmt_template = "/tmp/{s}{s}";
const fmt_args = .{
random_name,
d.comp.target.ofmt.fileExt(d.comp.target.cpu.arch),
d.comp.target.objFileExt(),
};
break :blk std.fmt.bufPrint(&name_buf, fmt_template, fmt_args) catch return d.fatal("Filename too long for filesystem: " ++ fmt_template, fmt_args);
};
Expand Down
22 changes: 14 additions & 8 deletions lib/std/Build/Module.zig
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import_table: std.StringArrayHashMapUnmanaged(*Module),

resolved_target: ?std.Build.ResolvedTarget = null,
optimize: ?std.builtin.OptimizeMode = null,
dwarf_format: ?std.dwarf.Format,
debug_format: ?std.builtin.DebugFormat,

c_macros: std.ArrayListUnmanaged([]const u8),
include_dirs: std.ArrayListUnmanaged(IncludeDir),
Expand Down Expand Up @@ -219,7 +219,7 @@ pub const CreateOptions = struct {
single_threaded: ?bool = null,
strip: ?bool = null,
unwind_tables: ?std.builtin.UnwindTables = null,
dwarf_format: ?std.dwarf.Format = null,
debug_format: ?std.builtin.DebugFormat = null,
code_model: std.builtin.CodeModel = .default,
stack_protector: ?bool = null,
stack_check: ?bool = null,
Expand Down Expand Up @@ -259,7 +259,7 @@ pub fn init(
.optimize = options.optimize,
.link_libc = options.link_libc,
.link_libcpp = options.link_libcpp,
.dwarf_format = options.dwarf_format,
.debug_format = options.debug_format,
.c_macros = .{},
.include_dirs = .{},
.lib_paths = .{},
Expand Down Expand Up @@ -525,11 +525,17 @@ pub fn appendZigProcessFlags(
try addFlag(zig_args, m.pic, "-fPIC", "-fno-PIC");
try addFlag(zig_args, m.red_zone, "-mred-zone", "-mno-red-zone");

if (m.dwarf_format) |dwarf_format| {
try zig_args.append(switch (dwarf_format) {
.@"32" => "-gdwarf32",
.@"64" => "-gdwarf64",
});
if (m.debug_format) |debug_format| {
switch (debug_format) {
.strip => {},
.dwarf => |fmt| switch (fmt) {
.@"32" => try zig_args.append("-gdwarf32"),
.@"64" => try zig_args.append("-gdwarf64"),
},
.code_view => {
try zig_args.append("-gcodeview");
},
}
}

if (m.unwind_tables) |unwind_tables| {
Expand Down
20 changes: 17 additions & 3 deletions lib/std/Build/Step/Compile.zig
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,10 @@ pub fn create(owner: *std.Build, options: Options) *Compile {
compile.name_only_filename = owner.fmt("lib{s}.dylib", .{compile.name});
compile.out_lib_filename = compile.out_filename;
} else if (target.os.tag == .windows) {
compile.out_lib_filename = owner.fmt("{s}.lib", .{compile.name});
compile.out_lib_filename = if (target.abi.isGnu())
owner.fmt("lib{s}.dll.a", .{compile.name})
else
owner.fmt("{s}.lib", .{compile.name});
} else {
compile.major_only_filename = owner.fmt("lib{s}.so.{d}", .{ compile.name, version.major });
compile.name_only_filename = owner.fmt("lib{s}.so", .{compile.name});
Expand All @@ -474,7 +477,10 @@ pub fn create(owner: *std.Build, options: Options) *Compile {
if (target.isDarwin()) {
compile.out_lib_filename = compile.out_filename;
} else if (target.os.tag == .windows) {
compile.out_lib_filename = owner.fmt("{s}.lib", .{compile.name});
compile.out_lib_filename = if (target.abi.isGnu())
owner.fmt("lib{s}.dll.a", .{compile.name})
else
owner.fmt("{s}.lib", .{compile.name});
} else {
compile.out_lib_filename = compile.out_filename;
}
Expand Down Expand Up @@ -651,6 +657,11 @@ pub fn producesPdbFile(compile: *Compile) bool {
.windows, .uefi => {},
else => return false,
}
if (compile.root_module.debug_format) |fmt| {
if (fmt != .code_view) return false;
} else {
if (target.abi.isGnu()) return false;
}
if (target.ofmt == .c) return false;
if (compile.root_module.strip == true or
(compile.root_module.strip == null and compile.root_module.optimize == .ReleaseSmall))
Expand Down Expand Up @@ -1801,7 +1812,10 @@ fn make(step: *Step, options: Step.MakeOptions) !void {

// -femit-implib[=path] (default) Produce an import .lib when building a Windows DLL
if (compile.generated_implib) |implib| {
implib.path = b.fmt("{}" ++ sep ++ "{s}.lib", .{ output_dir, compile.name });
if (compile.rootModuleTarget().abi.isGnu())
implib.path = b.fmt("{}" ++ sep ++ "lib{s}.dll.a", .{ output_dir, compile.name })
else
implib.path = b.fmt("{}" ++ sep ++ "{s}.lib", .{ output_dir, compile.name });
}

// -femit-h[=path] Generate a C header file (.h)
Expand Down
14 changes: 10 additions & 4 deletions lib/std/Target.zig
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,8 @@ pub const Os = struct {
return switch (abi) {
.msvc, .itanium => ".lib",
else => switch (tag) {
.windows, .uefi => ".lib",
.uefi => ".lib",
.windows => if (abi.isGnu()) ".a" else ".lib",
else => ".a",
},
};
Expand All @@ -138,7 +139,8 @@ pub const Os = struct {
return switch (abi) {
.msvc, .itanium => "",
else => switch (tag) {
.windows, .uefi => "",
.uefi => "",
.windows => if (abi.isGnu()) "lib" else "",
else => "lib",
},
};
Expand Down Expand Up @@ -1033,10 +1035,10 @@ pub const ObjectFormat = enum {
// LLVM tags deliberately omitted:
// - dxcontainer

pub fn fileExt(of: ObjectFormat, arch: Cpu.Arch) [:0]const u8 {
pub fn fileExt(of: ObjectFormat, abi: Abi, arch: Cpu.Arch) [:0]const u8 {
alexrp marked this conversation as resolved.
Show resolved Hide resolved
return switch (of) {
.c => ".c",
.coff => ".obj",
.coff => if (abi.isGnu()) ".o" else ".obj",
.elf, .goff, .macho, .wasm, .xcoff => ".o",
.hex => ".ihex",
.nvptx => ".ptx",
Expand Down Expand Up @@ -2043,6 +2045,10 @@ pub fn exeFileExt(target: Target) [:0]const u8 {
return target.os.tag.exeFileExt(target.cpu.arch);
}

pub fn objFileExt(target: Target) [:0]const u8 {
return target.ofmt.fileExt(target.abi, target.cpu.arch);
}

pub fn staticLibSuffix(target: Target) [:0]const u8 {
return target.os.tag.staticLibSuffix(target.abi);
}
Expand Down
8 changes: 8 additions & 0 deletions lib/std/builtin.zig
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,14 @@ pub const OptimizeMode = enum {
/// Deprecated; use OptimizeMode.
pub const Mode = OptimizeMode;

/// DebugFormat specifies which type of debug information to generate:
pub const DebugFormat = union(enum) {
/// No debug information.
strip,
dwarf: std.dwarf.Format,
code_view,
};

/// The calling convention of a function defines how arguments and return values are passed, as well
/// as any other requirements which callers and callees must respect, such as register preservation
/// and stack alignment.
Expand Down
2 changes: 1 addition & 1 deletion lib/std/debug/SelfInfo.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1003,7 +1003,7 @@ fn readCoffDebugInfo(allocator: Allocator, coff_obj: *coff.Coff) !Module {
di.dwarf = dwarf;
}

const raw_path = try coff_obj.getPdbPath() orelse return di;
const raw_path = (coff_obj.getPdbPath() catch return di) orelse return di;
const path = blk: {
if (fs.path.isAbsolute(raw_path)) {
break :blk raw_path;
Expand Down
12 changes: 8 additions & 4 deletions lib/std/zig.zig
Original file line number Diff line number Diff line change
Expand Up @@ -157,13 +157,17 @@ pub fn binNameAlloc(allocator: Allocator, options: BinNameOptions) error{OutOfMe
.coff => switch (options.output_mode) {
.Exe => return std.fmt.allocPrint(allocator, "{s}{s}", .{ root_name, t.exeFileExt() }),
.Lib => {
const prefix = if (t.abi.isGnu()) "lib" else "";
const suffix = switch (options.link_mode orelse .static) {
.static => ".lib",
.static => if (t.abi.isGnu()) ".a" else ".lib",
.dynamic => ".dll",
};
return std.fmt.allocPrint(allocator, "{s}{s}", .{ root_name, suffix });
return std.fmt.allocPrint(allocator, "{s}{s}{s}", .{ prefix, root_name, suffix });
},
.Obj => return std.fmt.allocPrint(allocator, "{s}.obj", .{root_name}),
.Obj => if (t.abi.isGnu())
return std.fmt.allocPrint(allocator, "{s}.o", .{root_name})
else
return std.fmt.allocPrint(allocator, "{s}.obj", .{root_name}),
},
.elf, .goff, .xcoff => switch (options.output_mode) {
.Exe => return allocator.dupe(u8, root_name),
Expand Down Expand Up @@ -228,7 +232,7 @@ pub fn binNameAlloc(allocator: Allocator, options: BinNameOptions) error{OutOfMe
.plan9 => switch (options.output_mode) {
.Exe => return allocator.dupe(u8, root_name),
.Obj => return std.fmt.allocPrint(allocator, "{s}{s}", .{
root_name, t.ofmt.fileExt(t.cpu.arch),
root_name, t.objFileExt(),
}),
.Lib => return std.fmt.allocPrint(allocator, "{s}{s}.a", .{
t.libPrefix(), root_name,
Expand Down
12 changes: 7 additions & 5 deletions src/Compilation.zig
Original file line number Diff line number Diff line change
Expand Up @@ -883,13 +883,13 @@ pub const cache_helpers = struct {
addEmitLoc(hh, optional_emit_loc orelse return);
}

pub fn addOptionalDebugFormat(hh: *Cache.HashHelper, x: ?Config.DebugFormat) void {
pub fn addOptionalDebugFormat(hh: *Cache.HashHelper, x: ?std.builtin.DebugFormat) void {
hh.add(x != null);
addDebugFormat(hh, x orelse return);
}

pub fn addDebugFormat(hh: *Cache.HashHelper, x: Config.DebugFormat) void {
const tag: @typeInfo(Config.DebugFormat).@"union".tag_type.? = x;
pub fn addDebugFormat(hh: *Cache.HashHelper, x: std.builtin.DebugFormat) void {
const tag: @typeInfo(std.builtin.DebugFormat).@"union".tag_type.? = x;
hh.add(tag);
switch (x) {
.strip, .code_view => {},
Expand Down Expand Up @@ -4724,7 +4724,7 @@ fn updateCObject(comp: *Compilation, c_object: *CObject, c_obj_prog_node: std.Pr
c_source_basename[0 .. c_source_basename.len - std.fs.path.extension(c_source_basename).len];

const target = comp.getTarget();
const o_ext = target.ofmt.fileExt(target.cpu.arch);
const o_ext = target.objFileExt();
const digest = if (!comp.disable_c_depfile and try man.hit()) man.final() else blk: {
var argv = std.ArrayList([]const u8).init(gpa);
defer argv.deinit();
Expand Down Expand Up @@ -5992,7 +5992,7 @@ pub const FileExt = enum {
.assembly => ".s",
.assembly_with_cpp => ".S",
.shared_library => target.dynamicLibSuffix(),
.object => target.ofmt.fileExt(target.cpu.arch),
.object => target.objFileExt(),
.static_library => target.staticLibSuffix(),
.zig => ".zig",
.def => ".def",
Expand Down Expand Up @@ -6321,6 +6321,7 @@ fn buildOutputFromZig(
.emit_bin = true,
.root_optimize_mode = optimize_mode,
.root_strip = strip,
.debug_format = comp.config.debug_format,
.link_libc = comp.config.link_libc,
});

Expand Down Expand Up @@ -6457,6 +6458,7 @@ pub fn build_crt_file(
.emit_bin = true,
.root_optimize_mode = comp.compilerRtOptMode(),
.root_strip = comp.compilerRtStrip(),
.debug_format = comp.config.debug_format,
.link_libc = false,
.lto = switch (output_mode) {
.Lib => comp.config.lto,
Expand Down
17 changes: 6 additions & 11 deletions src/Compilation/Config.zig
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ import_memory: bool,
export_memory: bool,
shared_memory: bool,
is_test: bool,
debug_format: DebugFormat,
debug_format: std.builtin.DebugFormat,
root_strip: bool,
root_error_tracing: bool,
dll_export_fns: bool,
Expand All @@ -67,12 +67,6 @@ pub const CFrontend = enum { clang, aro };

pub const LtoMode = enum { none, full, thin };

pub const DebugFormat = union(enum) {
strip,
dwarf: std.dwarf.Format,
code_view,
};

pub const Options = struct {
output_mode: std.builtin.OutputMode,
resolved_target: Module.ResolvedTarget,
Expand Down Expand Up @@ -109,7 +103,7 @@ pub const Options = struct {
import_memory: ?bool = null,
export_memory: ?bool = null,
shared_memory: ?bool = null,
debug_format: ?DebugFormat = null,
debug_format: ?std.builtin.DebugFormat = null,
dll_export_fns: ?bool = null,
rdynamic: ?bool = null,
san_cov_trace_pc_guard: bool = false,
Expand Down Expand Up @@ -441,14 +435,15 @@ pub fn resolve(options: Options) ResolveError!Config {
break :b false;
};

const debug_format: DebugFormat = b: {
const debug_format: std.builtin.DebugFormat = b: {
if (root_strip and !options.any_non_stripped) break :b .strip;
if (options.debug_format) |x| break :b x;
break :b switch (target.ofmt) {
.elf, .goff, .macho, .wasm, .xcoff => .{ .dwarf = .@"32" },
.coff => .code_view,
.coff => if (target.abi.isGnu()) .{ .dwarf = .@"32" } else .code_view,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change -- DWARF by default (i.e. embedded into the binary; no PDB) for MinGW -- is probably the only controversial change in this PR. I don't personally have a problem with it since you can still opt into getting a PDB, but we should probably think carefully about this change nonetheless.

cc @squeek502 @andrewrk for thoughts, maybe others?

Copy link
Collaborator

@squeek502 squeek502 Jan 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't feel like I know enough about why MinGW made this choice to give any useful feedback here.

I have a more general question, though (and this is also coming from ignorance, this is not necessarily intended as a criticism of these changes): is the stuff in this PR related to 'targeting the MinGW ABI' or is it more related to 'emulating the MinGW toolchain'? If it's the latter, is that something that Zig-the-compiler is/should be interested in? Would it make any sense to limit any 'emulating the MinGW toolchain' to zig cc, specifically?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Native debuggers on Windows all expect PDBs, I think this change would hurt debuggability there - it would mean users would have to always specify a non-default option to use the default platform debuggers.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An interesting data point for this discussion: msys2/MINGW-packages#5646

Copy link
Collaborator

@squeek502 squeek502 Jan 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An interesting data point for this discussion: msys2/MINGW-packages#5646

This is also mentioned in #20039:

Zig uses PDB or CodeView format for debug symbols, while Rust and GCC use DWARF

  • When using Zig library in Rust or C on Windows, the mix of debug symbol formats makes either language un-debuggable
  • GDB has no support of PDB files (which use CodeView debug format) and LLDB's support is problematic ([1], [2]), which makes open-sourced VS Code debuggers like CodeLLDB unusable for zig debugging purposes

I'm not sure there's a default that's "good" here, and gnu being the default ABI for Windows in Zig makes the default matter.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the stuff in this PR related to 'targeting the MinGW ABI' or is it more related to 'emulating the MinGW toolchain'?

It's more about the latter. The goal of this PR is to improve compatibility.

.c => switch (target.os.tag) {
.windows, .uefi => .code_view,
.uefi => .code_view,
.windows => if (target.abi.isGnu()) .{ .dwarf = .@"32" } else .code_view,
else => .{ .dwarf = .@"32" },
},
.spirv, .nvptx, .hex, .raw, .plan9 => .strip,
Expand Down
18 changes: 16 additions & 2 deletions src/clang_options_data.zig
Original file line number Diff line number Diff line change
Expand Up @@ -4148,7 +4148,14 @@ flagpd1("g3"),
.pd2 = false,
.psl = false,
},
flagpd1("gcodeview"),
.{
.name = "gcodeview",
.syntax = .flag,
.zig_equivalent = .gcodeview,
.pd1 = true,
.pd2 = false,
.psl = false,
},
flagpd1("gcodeview-command-line"),
flagpd1("gcodeview-ghash"),
flagpd1("gcolumn-info"),
Expand Down Expand Up @@ -7350,7 +7357,14 @@ joinpd1("mthreads"),
.pd2 = false,
.psl = false,
},
joinpd1("mwindows"),
.{
.name = "mwindows",
.syntax = .joined,
.zig_equivalent = .mingw_subsystem_windows,
.pd1 = true,
.pd2 = false,
.psl = false,
},
.{
.name = "offload=",
.syntax = .comma_joined,
Expand Down
40 changes: 26 additions & 14 deletions src/link.zig
Original file line number Diff line number Diff line change
Expand Up @@ -2096,20 +2096,32 @@ fn resolveLibInput(
return finishResolveLibInput(resolved_inputs, test_path, file, link_mode, name_query.query);
}

// In the case of MinGW, the main check will be .lib but we also need to
// look for `libfoo.a`.
if (target.isMinGW() and link_mode == .static) mingw: {
const test_path: Path = .{
.root_dir = lib_directory,
.sub_path = try std.fmt.allocPrint(arena, "lib{s}.a", .{lib_name}),
};
try checked_paths.writer(gpa).print("\n {}", .{test_path});
var file = test_path.root_dir.handle.openFile(test_path.sub_path, .{}) catch |err| switch (err) {
error.FileNotFound => break :mingw,
else => |e| fatal("unable to search for static library '{}': {s}", .{ test_path, @errorName(e) }),
};
errdefer file.close();
return finishResolveLibInput(resolved_inputs, test_path, file, link_mode, name_query.query);
// In the case of MinGW, the main check will be `libfoo.dll` and `libfoo.a`, but we also need to
// look for `foo.dll`, `foo.lib` and `libfoo.dll.a`.
if (target.isMinGW()) {
const sub_paths = if (link_mode == .dynamic)
&[_][]const u8{
try std.fmt.allocPrint(arena, "lib{s}.dll.a", .{lib_name}),
try std.fmt.allocPrint(arena, "{s}.dll", .{lib_name}),
try std.fmt.allocPrint(arena, "{s}.lib", .{lib_name}),
}
else
&[_][]const u8{
try std.fmt.allocPrint(arena, "{s}.lib", .{lib_name}),
};
for (sub_paths) |sub_path| {
const test_path: Path = .{
.root_dir = lib_directory,
.sub_path = sub_path,
};
try checked_paths.writer(gpa).print("\n {}", .{test_path});
var file = test_path.root_dir.handle.openFile(test_path.sub_path, .{}) catch |err| switch (err) {
error.FileNotFound => continue,
else => |e| fatal("unable to search for {s} library '{}': {s}", .{ @tagName(link_mode), test_path, @errorName(e) }),
};
errdefer file.close();
return finishResolveLibInput(resolved_inputs, test_path, file, link_mode, name_query.query);
}
}

return .no_match;
Expand Down
Loading
Loading