Merge pull request #12145 from ziglang/fixes

More ABI size and alignment fixes

Author: andrewrk

lib/std/target.zig

@@ -9,6 +9,7 @@ pub const Target = struct {
     cpu: Cpu,
     os: Os,
     abi: Abi,
+    ofmt: ObjectFormat,
 
     pub const Os = struct {
         tag: Tag,
@@ -594,6 +595,20 @@ pub const Target = struct {
                 .nvptx => ".ptx",
             };
         }
+
+        pub fn default(os_tag: Os.Tag, cpu_arch: Cpu.Arch) ObjectFormat {
+            return switch (os_tag) {
+                .windows, .uefi => .coff,
+                .ios, .macos, .watchos, .tvos => .macho,
+                .plan9 => .plan9,
+                else => return switch (cpu_arch) {
+                    .wasm32, .wasm64 => .wasm,
+                    .spirv32, .spirv64 => .spirv,
+                    .nvptx, .nvptx64 => .nvptx,
+                    else => .elf,
+                },
+            };
+        }
     };
 
     pub const SubSystem = enum {
@@ -1381,24 +1396,6 @@ pub const Target = struct {
         return libPrefix_os_abi(self.os.tag, self.abi);
     }
 
-    pub fn getObjectFormatSimple(os_tag: Os.Tag, cpu_arch: Cpu.Arch) ObjectFormat {
-        return switch (os_tag) {
-            .windows, .uefi => .coff,
-            .ios, .macos, .watchos, .tvos => .macho,
-            .plan9 => .plan9,
-            else => return switch (cpu_arch) {
-                .wasm32, .wasm64 => .wasm,
-                .spirv32, .spirv64 => .spirv,
-                .nvptx, .nvptx64 => .nvptx,
-                else => .elf,
-            },
-        };
-    }
-
-    pub fn getObjectFormat(self: Target) ObjectFormat {
-        return getObjectFormatSimple(self.os.tag, self.cpu.arch);
-    }
-
     pub fn isMinGW(self: Target) bool {
         return self.os.tag == .windows and self.isGnu();
     }
@@ -1806,24 +1803,28 @@ pub const Target = struct {
                 else => 4,
             },
 
-            // For x86_64, LLVMABIAlignmentOfType(i128) reports 8. However I think 16
-            // is a better number for two reasons:
-            // 1. Better machine code when loading into SIMD register.
+            // For these, LLVMABIAlignmentOfType(i128) reports 8. Note that 16
+            // is a relevant number in three cases:
+            // 1. Different machine code instruction when loading into SIMD register.
             // 2. The C ABI wants 16 for extern structs.
             // 3. 16-byte cmpxchg needs 16-byte alignment.
-            // Same logic for riscv64, powerpc64, mips64, sparc64.
+            // Same logic for powerpc64, mips64, sparc64.
             .x86_64,
-            .riscv64,
             .powerpc64,
             .powerpc64le,
             .mips64,
             .mips64el,
             .sparc64,
+            => return switch (target.ofmt) {
+                .c => 16,
+                else => 8,
+            },
 
             // Even LLVMABIAlignmentOfType(i128) agrees on these targets.
             .aarch64,
             .aarch64_be,
             .aarch64_32,
+            .riscv64,
             .bpfel,
             .bpfeb,
             .nvptx,

lib/std/zig.zig

@@ -103,16 +103,14 @@ pub const BinNameOptions = struct {
     target: std.Target,
     output_mode: std.builtin.OutputMode,
     link_mode: ?std.builtin.LinkMode = null,
-    object_format: ?std.Target.ObjectFormat = null,
     version: ?std.builtin.Version = null,
 };
 
 /// Returns the standard file system basename of a binary generated by the Zig compiler.
 pub fn binNameAlloc(allocator: std.mem.Allocator, options: BinNameOptions) error{OutOfMemory}![]u8 {
     const root_name = options.root_name;
     const target = options.target;
-    const ofmt = options.object_format orelse target.getObjectFormat();
-    switch (ofmt) {
+    switch (target.ofmt) {
         .coff => switch (options.output_mode) {
             .Exe => return std.fmt.allocPrint(allocator, "{s}{s}", .{ root_name, target.exeFileExt() }),
             .Lib => {
@@ -186,8 +184,12 @@ pub fn binNameAlloc(allocator: std.mem.Allocator, options: BinNameOptions) error
         .raw => return std.fmt.allocPrint(allocator, "{s}.bin", .{root_name}),
         .plan9 => switch (options.output_mode) {
             .Exe => return allocator.dupe(u8, root_name),
-            .Obj => return std.fmt.allocPrint(allocator, "{s}{s}", .{ root_name, ofmt.fileExt(target.cpu.arch) }),
-            .Lib => return std.fmt.allocPrint(allocator, "{s}{s}.a", .{ target.libPrefix(), root_name }),
+            .Obj => return std.fmt.allocPrint(allocator, "{s}{s}", .{
+                root_name, target.ofmt.fileExt(target.cpu.arch),
+            }),
+            .Lib => return std.fmt.allocPrint(allocator, "{s}{s}.a", .{
+                target.libPrefix(), root_name,
+            }),
         },
         .nvptx => return std.fmt.allocPrint(allocator, "{s}", .{root_name}),
     }

lib/std/zig/CrossTarget.zig

@@ -42,6 +42,9 @@ abi: ?Target.Abi = null,
 /// based on the `os_tag`.
 dynamic_linker: DynamicLinker = DynamicLinker{},
 
+/// `null` means default for the cpu/arch/os combo.
+ofmt: ?Target.ObjectFormat = null,
+
 pub const CpuModel = union(enum) {
     /// Always native
     native,
@@ -168,6 +171,7 @@ pub fn toTarget(self: CrossTarget) Target {
         .cpu = self.getCpu(),
         .os = self.getOs(),
         .abi = self.getAbi(),
+        .ofmt = self.getObjectFormat(),
     };
 }
 
@@ -197,6 +201,8 @@ pub const ParseOptions = struct {
     /// detected path, or a standard path.
     dynamic_linker: ?[]const u8 = null,
 
+    object_format: ?[]const u8 = null,
+
     /// If this is provided, the function will populate some information about parsing failures,
     /// so that user-friendly error messages can be delivered.
     diagnostics: ?*Diagnostics = null,
@@ -321,6 +327,11 @@ pub fn parse(args: ParseOptions) !CrossTarget {
         }
     }
 
+    if (args.object_format) |ofmt_name| {
+        result.ofmt = std.meta.stringToEnum(Target.ObjectFormat, ofmt_name) orelse
+            return error.UnknownObjectFormat;
+    }
+
     return result;
 }
 
@@ -620,7 +631,7 @@ pub fn setGnuLibCVersion(self: *CrossTarget, major: u32, minor: u32, patch: u32)
 }
 
 pub fn getObjectFormat(self: CrossTarget) Target.ObjectFormat {
-    return Target.getObjectFormatSimple(self.getOsTag(), self.getCpuArch());
+    return self.ofmt orelse Target.ObjectFormat.default(self.getOsTag(), self.getCpuArch());
 }
 
 pub fn updateCpuFeatures(self: CrossTarget, set: *Target.Cpu.Feature.Set) void {

lib/std/zig/system/NativeTargetInfo.zig

@@ -276,6 +276,7 @@ fn detectAbiAndDynamicLinker(
     };
     var ld_info_list_buffer: [all_abis.len]LdInfo = undefined;
     var ld_info_list_len: usize = 0;
+    const ofmt = cross_target.ofmt orelse Target.ObjectFormat.default(os.tag, cpu.arch);
 
     for (all_abis) |abi| {
         // This may be a nonsensical parameter. We detect this with error.UnknownDynamicLinkerPath and
@@ -284,6 +285,7 @@ fn detectAbiAndDynamicLinker(
             .cpu = cpu,
             .os = os,
             .abi = abi,
+            .ofmt = ofmt,
         };
         const ld = target.standardDynamicLinkerPath();
         if (ld.get() == null) continue;
@@ -346,6 +348,7 @@ fn detectAbiAndDynamicLinker(
                 .cpu = cpu,
                 .os = os_adjusted,
                 .abi = cross_target.abi orelse found_ld_info.abi,
+                .ofmt = cross_target.ofmt orelse Target.ObjectFormat.default(os_adjusted.tag, cpu.arch),
             },
             .dynamic_linker = if (cross_target.dynamic_linker.get() == null)
                 DynamicLinker.init(found_ld_path)
@@ -539,6 +542,7 @@ pub fn abiAndDynamicLinkerFromFile(
             .cpu = cpu,
             .os = os,
             .abi = cross_target.abi orelse Target.Abi.default(cpu.arch, os),
+            .ofmt = cross_target.ofmt orelse Target.ObjectFormat.default(os.tag, cpu.arch),
         },
         .dynamic_linker = cross_target.dynamic_linker,
     };
@@ -829,6 +833,7 @@ fn defaultAbiAndDynamicLinker(cpu: Target.Cpu, os: Target.Os, cross_target: Cros
         .cpu = cpu,
         .os = os,
         .abi = cross_target.abi orelse Target.Abi.default(cpu.arch, os),
+        .ofmt = cross_target.ofmt orelse Target.ObjectFormat.default(os.tag, cpu.arch),
     };
     return NativeTargetInfo{
         .target = target,

src/Compilation.zig

@@ -810,7 +810,6 @@ pub const InitOptions = struct {
     /// this flag would be set to disable this machinery to avoid false positives.
     disable_lld_caching: bool = false,
     cache_mode: CacheMode = .incremental,
-    object_format: ?std.Target.ObjectFormat = null,
     optimize_mode: std.builtin.Mode = .Debug,
     keep_source_files_loaded: bool = false,
     clang_argv: []const []const u8 = &[0][]const u8{},
@@ -1027,8 +1026,6 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation {
         const comp = try arena.create(Compilation);
         const root_name = try arena.dupeZ(u8, options.root_name);
 
-        const ofmt = options.object_format orelse options.target.getObjectFormat();
-
         const use_stage1 = options.use_stage1 orelse blk: {
             // Even though we may have no Zig code to compile (depending on `options.main_pkg`),
             // we may need to use stage1 for building compiler-rt and other dependencies.
@@ -1042,7 +1039,7 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation {
             }
 
             // If LLVM does not support the target, then we can't use it.
-            if (!target_util.hasLlvmSupport(options.target, ofmt))
+            if (!target_util.hasLlvmSupport(options.target, options.target.ofmt))
                 break :blk false;
 
             break :blk build_options.is_stage1;
@@ -1072,7 +1069,7 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation {
                 break :blk true;
 
             // If LLVM does not support the target, then we can't use it.
-            if (!target_util.hasLlvmSupport(options.target, ofmt))
+            if (!target_util.hasLlvmSupport(options.target, options.target.ofmt))
                 break :blk false;
 
             // Prefer LLVM for release builds.
@@ -1115,7 +1112,7 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation {
             if (!build_options.have_llvm)
                 break :blk false;
 
-            if (ofmt == .c)
+            if (options.target.ofmt == .c)
                 break :blk false;
 
             if (options.want_lto) |lto| {
@@ -1374,7 +1371,7 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation {
         cache.hash.add(options.target.os.getVersionRange());
         cache.hash.add(options.is_native_os);
         cache.hash.add(options.target.abi);
-        cache.hash.add(ofmt);
+        cache.hash.add(options.target.ofmt);
         cache.hash.add(pic);
         cache.hash.add(pie);
         cache.hash.add(lto);
@@ -1682,7 +1679,6 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation {
             .sysroot = sysroot,
             .output_mode = options.output_mode,
             .link_mode = link_mode,
-            .object_format = ofmt,
             .optimize_mode = options.optimize_mode,
             .use_lld = use_lld,
             .use_llvm = use_llvm,
@@ -1841,7 +1837,9 @@ pub fn create(gpa: Allocator, options: InitOptions) !*Compilation {
 
     const have_bin_emit = comp.bin_file.options.emit != null or comp.whole_bin_sub_path != null;
 
-    if (have_bin_emit and !comp.bin_file.options.skip_linker_dependencies) {
+    if (have_bin_emit and !comp.bin_file.options.skip_linker_dependencies and
+        options.target.ofmt != .c)
+    {
         if (comp.getTarget().isDarwin()) {
             switch (comp.getTarget().abi) {
                 .none,
@@ -3739,7 +3737,8 @@ fn updateCObject(comp: *Compilation, c_object: *CObject, c_obj_prog_node: *std.P
     else
         c_source_basename[0 .. c_source_basename.len - std.fs.path.extension(c_source_basename).len];
 
-    const o_ext = comp.bin_file.options.object_format.fileExt(comp.bin_file.options.target.cpu.arch);
+    const target = comp.getTarget();
+    const o_ext = target.ofmt.fileExt(target.cpu.arch);
     const digest = if (!comp.disable_c_depfile and try man.hit()) man.final() else blk: {
         var argv = std.ArrayList([]const u8).init(comp.gpa);
         defer argv.deinit();
@@ -4092,7 +4091,7 @@ pub fn addCCArgs(
 
             if (!comp.bin_file.options.strip) {
                 try argv.append("-g");
-                switch (comp.bin_file.options.object_format) {
+                switch (target.ofmt) {
                     .coff => try argv.append("-gcodeview"),
                     else => {},
                 }
@@ -4660,7 +4659,7 @@ fn wantBuildLibCFromSource(comp: Compilation) bool {
     };
     return comp.bin_file.options.link_libc and is_exe_or_dyn_lib and
         comp.bin_file.options.libc_installation == null and
-        comp.bin_file.options.object_format != .c;
+        comp.bin_file.options.target.ofmt != .c;
 }
 
 fn wantBuildGLibCFromSource(comp: Compilation) bool {
@@ -4688,7 +4687,7 @@ fn wantBuildLibUnwindFromSource(comp: *Compilation) bool {
         .Exe => true,
     };
     return is_exe_or_dyn_lib and comp.bin_file.options.link_libunwind and
-        comp.bin_file.options.object_format != .c;
+        comp.bin_file.options.target.ofmt != .c;
 }
 
 fn setAllocFailure(comp: *Compilation) void {
@@ -4747,7 +4746,7 @@ pub fn generateBuiltinZigSource(comp: *Compilation, allocator: Allocator) Alloca
     const zig_backend: std.builtin.CompilerBackend = blk: {
         if (use_stage1) break :blk .stage1;
         if (build_options.have_llvm and comp.bin_file.options.use_llvm) break :blk .stage2_llvm;
-        if (comp.bin_file.options.object_format == .c) break :blk .stage2_c;
+        if (target.ofmt == .c) break :blk .stage2_c;
         break :blk switch (target.cpu.arch) {
             .wasm32, .wasm64 => std.builtin.CompilerBackend.stage2_wasm,
             .arm, .armeb, .thumb, .thumbeb => .stage2_arm,
@@ -4895,6 +4894,7 @@ pub fn generateBuiltinZigSource(comp: *Compilation, allocator: Allocator) Alloca
         \\    .cpu = cpu,
         \\    .os = os,
         \\    .abi = abi,
+        \\    .ofmt = object_format,
         \\}};
         \\pub const object_format = std.Target.ObjectFormat.{};
         \\pub const mode = std.builtin.Mode.{};
@@ -4909,7 +4909,7 @@ pub fn generateBuiltinZigSource(comp: *Compilation, allocator: Allocator) Alloca
         \\pub const code_model = std.builtin.CodeModel.{};
         \\
     , .{
-        std.zig.fmtId(@tagName(comp.bin_file.options.object_format)),
+        std.zig.fmtId(@tagName(target.ofmt)),
         std.zig.fmtId(@tagName(comp.bin_file.options.optimize_mode)),
         link_libc,
         comp.bin_file.options.link_libcpp,

src/Module.zig

@@ -935,13 +935,41 @@ pub const Struct = struct {
         /// If true then `default_val` is the comptime field value.
         is_comptime: bool,
 
-        /// Returns the field alignment, assuming the struct is not packed.
-        pub fn normalAlignment(field: Field, target: Target) u32 {
-            if (field.abi_align == 0) {
-                return field.ty.abiAlignment(target);
-            } else {
+        /// Returns the field alignment. If the struct is packed, returns 0.
+        pub fn alignment(
+            field: Field,
+            target: Target,
+            layout: std.builtin.Type.ContainerLayout,
+        ) u32 {
+            if (field.abi_align != 0) {
+                assert(layout != .Packed);
                 return field.abi_align;
             }
+
+            switch (layout) {
+                .Packed => return 0,
+                .Auto => {
+                    if (target.ofmt == .c) {
+                        return alignmentExtern(field, target);
+                    } else {
+                        return field.ty.abiAlignment(target);
+                    }
+                },
+                .Extern => return alignmentExtern(field, target),
+            }
+        }
+
+        pub fn alignmentExtern(field: Field, target: Target) u32 {
+            // This logic is duplicated in Type.abiAlignmentAdvanced.
+            const ty_abi_align = field.ty.abiAlignment(target);
+
+            if (field.ty.isAbiInt() and field.ty.intInfo(target).bits >= 128) {
+                // The C ABI requires 128 bit integer fields of structs
+                // to be 16-bytes aligned.
+                return @maximum(ty_abi_align, 16);
+            }
+
+            return ty_abi_align;
         }
     };