Commit b83c037f9f

Veikka Tuominen <git@vexu.eu>
2022-09-02 19:26:33
Sema: only ABI sized packed structs are extern compatible
1 parent 6aee07c
src/arch/wasm/abi.zig
@@ -23,6 +23,10 @@ pub fn classifyType(ty: Type, target: Target) [2]Class {
     if (!ty.hasRuntimeBitsIgnoreComptime()) return none;
     switch (ty.zigTypeTag()) {
         .Struct => {
+            if (ty.containerLayout() == .Packed) {
+                if (ty.bitSize(target) <= 64) return direct;
+                return .{ .direct, .direct };
+            }
             // When the struct type is non-scalar
             if (ty.structFieldCount() > 1) return memory;
             // When the struct's alignment is non-natural
@@ -57,6 +61,10 @@ pub fn classifyType(ty: Type, target: Target) [2]Class {
             return direct;
         },
         .Union => {
+            if (ty.containerLayout() == .Packed) {
+                if (ty.bitSize(target) <= 64) return direct;
+                return .{ .direct, .direct };
+            }
             const layout = ty.unionGetLayout(target);
             std.debug.assert(layout.tag_size == 0);
             if (ty.unionFields().count() > 1) return memory;
src/arch/x86_64/abi.zig
@@ -174,6 +174,12 @@ pub fn classifySystemV(ty: Type, target: Target) [8]Class {
             // "If the size of the aggregate exceeds a single eightbyte, each is classified
             // separately.".
             const ty_size = ty.abiSize(target);
+            if (ty.containerLayout() == .Packed) {
+                assert(ty_size <= 128);
+                result[0] = .integer;
+                if (ty_size > 64) result[1] = .integer;
+                return result;
+            }
             if (ty_size > 64)
                 return memory_class;
 
@@ -284,6 +290,12 @@ pub fn classifySystemV(ty: Type, target: Target) [8]Class {
             // "If the size of the aggregate exceeds a single eightbyte, each is classified
             // separately.".
             const ty_size = ty.abiSize(target);
+            if (ty.containerLayout() == .Packed) {
+                assert(ty_size <= 128);
+                result[0] = .integer;
+                if (ty_size > 64) result[1] = .integer;
+                return result;
+            }
             if (ty_size > 64)
                 return memory_class;
 
src/codegen/llvm.zig
@@ -9674,22 +9674,7 @@ fn lowerFnRetTy(dg: *DeclGen, fn_info: Type.Payload.Function.Data) !*const llvm.
             }
         },
         .C => {
-            const is_scalar = switch (fn_info.return_type.zigTypeTag()) {
-                .Void,
-                .Bool,
-                .NoReturn,
-                .Int,
-                .Float,
-                .Pointer,
-                .Optional,
-                .ErrorSet,
-                .Enum,
-                .AnyFrame,
-                .Vector,
-                => true,
-
-                else => false,
-            };
+            const is_scalar = isScalar(fn_info.return_type);
             switch (target.cpu.arch) {
                 .mips, .mipsel => return dg.lowerType(fn_info.return_type),
                 .x86_64 => switch (target.os.tag) {
@@ -9837,24 +9822,7 @@ const ParamTypeIterator = struct {
                 @panic("TODO implement async function lowering in the LLVM backend");
             },
             .C => {
-                const is_scalar = switch (ty.zigTypeTag()) {
-                    .Void,
-                    .Bool,
-                    .NoReturn,
-                    .Int,
-                    .Float,
-                    .Pointer,
-                    .Optional,
-                    .ErrorSet,
-                    .Enum,
-                    .AnyFrame,
-                    .Vector,
-                    => true,
-                    .Struct => ty.containerLayout() == .Packed,
-                    .Union => ty.containerLayout() == .Packed,
-
-                    else => false,
-                };
+                const is_scalar = isScalar(ty);
                 switch (it.target.cpu.arch) {
                     .riscv32, .riscv64 => {
                         it.zig_index += 1;
@@ -10108,6 +10076,27 @@ fn isByRef(ty: Type) bool {
     }
 }
 
+fn isScalar(ty: Type) bool {
+    return switch (ty.zigTypeTag()) {
+        .Void,
+        .Bool,
+        .NoReturn,
+        .Int,
+        .Float,
+        .Pointer,
+        .Optional,
+        .ErrorSet,
+        .Enum,
+        .AnyFrame,
+        .Vector,
+        => true,
+
+        .Struct => ty.containerLayout() == .Packed,
+        .Union => ty.containerLayout() == .Packed,
+        else => false,
+    };
+}
+
 /// This function returns true if we expect LLVM to lower x86_fp80 correctly
 /// and false if we expect LLVM to crash if it counters an x86_fp80 type.
 fn backendSupportsF80(target: std.Target) bool {
src/Sema.zig
@@ -5056,7 +5056,7 @@ pub fn analyzeExport(
     try mod.ensureDeclAnalyzed(exported_decl_index);
     const exported_decl = mod.declPtr(exported_decl_index);
 
-    if (!sema.validateExternType(exported_decl.ty, .other)) {
+    if (!try sema.validateExternType(block, src, exported_decl.ty, .other)) {
         const msg = msg: {
             const msg = try sema.errMsg(block, src, "unable to export type '{}'", .{exported_decl.ty.fmt(sema.mod)});
             errdefer msg.destroy(sema.gpa);
@@ -7896,7 +7896,7 @@ fn funcCommon(
             };
             return sema.failWithOwnedErrorMsg(msg);
         }
-        if (!Type.fnCallingConventionAllowsZigTypes(cc_workaround) and !sema.validateExternType(return_type, .ret_ty)) {
+        if (!Type.fnCallingConventionAllowsZigTypes(cc_workaround) and !try sema.validateExternType(block, ret_ty_src, return_type, .ret_ty)) {
             const msg = msg: {
                 const msg = try sema.errMsg(block, ret_ty_src, "return type '{}' not allowed in function with calling convention '{s}'", .{
                     return_type.fmt(sema.mod), @tagName(cc_workaround),
@@ -8115,7 +8115,7 @@ fn analyzeParameter(
         };
         return sema.failWithOwnedErrorMsg(msg);
     }
-    if (!Type.fnCallingConventionAllowsZigTypes(cc) and !sema.validateExternType(param.ty, .param_ty)) {
+    if (!Type.fnCallingConventionAllowsZigTypes(cc) and !try sema.validateExternType(block, param_src, param.ty, .param_ty)) {
         const msg = msg: {
             const msg = try sema.errMsg(block, param_src, "parameter of type '{}' not allowed in function with calling convention '{s}'", .{
                 param.ty.fmt(sema.mod), @tagName(cc),
@@ -15583,7 +15583,7 @@ fn zirPtrType(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Air
     } else if (inst_data.size == .Many and elem_ty.zigTypeTag() == .Opaque) {
         return sema.fail(block, elem_ty_src, "unknown-length pointer to opaque not allowed", .{});
     } else if (inst_data.size == .C) {
-        if (!sema.validateExternType(elem_ty, .other)) {
+        if (!try sema.validateExternType(block, elem_ty_src, elem_ty, .other)) {
             const msg = msg: {
                 const msg = try sema.errMsg(block, elem_ty_src, "C pointers cannot point to non-C-ABI-compatible type '{}'", .{elem_ty.fmt(sema.mod)});
                 errdefer msg.destroy(sema.gpa);
@@ -16681,7 +16681,7 @@ fn zirReify(sema: *Sema, block: *Block, extended: Zir.Inst.Extended.InstData, in
             } else if (ptr_size == .Many and elem_ty.zigTypeTag() == .Opaque) {
                 return sema.fail(block, src, "unknown-length pointer to opaque not allowed", .{});
             } else if (ptr_size == .C) {
-                if (!sema.validateExternType(elem_ty, .other)) {
+                if (!try sema.validateExternType(block, src, elem_ty, .other)) {
                     const msg = msg: {
                         const msg = try sema.errMsg(block, src, "C pointers cannot point to non-C-ABI-compatible type '{}'", .{elem_ty.fmt(sema.mod)});
                         errdefer msg.destroy(sema.gpa);
@@ -20549,7 +20549,14 @@ const ExternPosition = enum {
 
 /// Returns true if `ty` is allowed in extern types.
 /// Does *NOT* require `ty` to be resolved in any way.
-fn validateExternType(sema: *Sema, ty: Type, position: ExternPosition) bool {
+/// Calls `resolveTypeLayout` for packed containers.
+fn validateExternType(
+    sema: *Sema,
+    block: *Block,
+    src: LazySrcLoc,
+    ty: Type,
+    position: ExternPosition,
+) !bool {
     switch (ty.zigTypeTag()) {
         .Type,
         .ComptimeFloat,
@@ -20577,17 +20584,25 @@ fn validateExternType(sema: *Sema, ty: Type, position: ExternPosition) bool {
         .Fn => return !Type.fnCallingConventionAllowsZigTypes(ty.fnCallingConvention()),
         .Enum => {
             var buf: Type.Payload.Bits = undefined;
-            return sema.validateExternType(ty.intTagType(&buf), position);
+            return sema.validateExternType(block, src, ty.intTagType(&buf), position);
         },
         .Struct, .Union => switch (ty.containerLayout()) {
-            .Extern, .Packed => return true,
-            else => return false,
+            .Extern => return true,
+            .Packed => {
+                const target = sema.mod.getTarget();
+                const bit_size = try ty.bitSizeAdvanced(target, sema.kit(block, src));
+                switch (bit_size) {
+                    8, 16, 32, 64, 128 => return true,
+                    else => return false,
+                }
+            },
+            .Auto => return false,
         },
         .Array => {
             if (position == .ret_ty or position == .param_ty) return false;
-            return sema.validateExternType(ty.elemType2(), .other);
+            return sema.validateExternType(block, src, ty.elemType2(), .other);
         },
-        .Vector => return sema.validateExternType(ty.elemType2(), .other),
+        .Vector => return sema.validateExternType(block, src, ty.elemType2(), .other),
         .Optional => return ty.isPtrLikeOptional(),
     }
 }
@@ -20639,8 +20654,8 @@ fn explainWhyTypeIsNotExtern(
             try mod.errNoteNonLazy(src_loc, msg, "enum tag type '{}' is not extern compatible", .{tag_ty.fmt(sema.mod)});
             try sema.explainWhyTypeIsNotExtern(msg, src_loc, tag_ty, position);
         },
-        .Struct => try mod.errNoteNonLazy(src_loc, msg, "only structs with packed or extern layout are extern compatible", .{}),
-        .Union => try mod.errNoteNonLazy(src_loc, msg, "only unions with packed or extern layout are extern compatible", .{}),
+        .Struct => try mod.errNoteNonLazy(src_loc, msg, "only extern structs and ABI sized packed structs are extern compatible", .{}),
+        .Union => try mod.errNoteNonLazy(src_loc, msg, "only extern unions and ABI sized packed unions are extern compatible", .{}),
         .Array => {
             if (position == .ret_ty) {
                 return mod.errNoteNonLazy(src_loc, msg, "arrays are not allowed as a return type", .{});
@@ -24119,7 +24134,7 @@ fn coerceVarArgParam(
     };
 
     const coerced_ty = sema.typeOf(coerced);
-    if (!sema.validateExternType(coerced_ty, .other)) {
+    if (!try sema.validateExternType(block, inst_src, coerced_ty, .other)) {
         const msg = msg: {
             const msg = try sema.errMsg(block, inst_src, "cannot pass '{}' to variadic function", .{coerced_ty.fmt(sema.mod)});
             errdefer msg.destroy(sema.gpa);
@@ -28321,7 +28336,7 @@ fn semaStructFields(mod: *Module, struct_obj: *Module.Struct) CompileError!void
             };
             return sema.failWithOwnedErrorMsg(msg);
         }
-        if (struct_obj.layout == .Extern and !sema.validateExternType(field.ty, .other)) {
+        if (struct_obj.layout == .Extern and !try sema.validateExternType(&block_scope, src, field.ty, .other)) {
             const msg = msg: {
                 const tree = try sema.getAstTree(&block_scope);
                 const fields_src = enumFieldSrcLoc(decl, tree.*, 0, i);
@@ -28658,7 +28673,7 @@ fn semaUnionFields(mod: *Module, union_obj: *Module.Union) CompileError!void {
             };
             return sema.failWithOwnedErrorMsg(msg);
         }
-        if (union_obj.layout == .Extern and !sema.validateExternType(field_ty, .union_field)) {
+        if (union_obj.layout == .Extern and !try sema.validateExternType(&block_scope, src, field_ty, .union_field)) {
             const msg = msg: {
                 const tree = try sema.getAstTree(&block_scope);
                 const field_src = enumFieldSrcLoc(decl, tree.*, 0, field_i);
test/c_abi/cfuncs.c
@@ -86,24 +86,8 @@ struct MedStructMixed {
 void zig_med_struct_mixed(struct MedStructMixed);
 struct MedStructMixed zig_ret_med_struct_mixed();
 
-struct SmallPackedStruct {
-    uint8_t a: 2;
-    uint8_t b: 2;
-    uint8_t c: 2;
-    uint8_t d: 2;
-    uint8_t e: 1;
-};
-
-struct BigPackedStruct {
-    uint64_t a: 64;
-    uint64_t b: 64;
-    uint64_t c: 64;
-    uint64_t d: 64;
-    uint8_t e: 8;
-};
-
-//void zig_small_packed_struct(struct SmallPackedStruct); // #1481
-void zig_big_packed_struct(struct BigPackedStruct);
+void zig_small_packed_struct(uint8_t);
+void zig_big_packed_struct(__int128);
 
 struct SplitStructInts {
     uint64_t a;
@@ -176,13 +160,19 @@ void run_c_tests(void) {
     }
 
     {
-        struct BigPackedStruct s = {1, 2, 3, 4, 5};
+        __int128 s = 0;
+        s |= 1 << 0;
+        s |= (__int128)2 << 64;
         zig_big_packed_struct(s);
     }
 
     {
-        struct SmallPackedStruct s = {0, 1, 2, 3, 1};
-        //zig_small_packed_struct(s);
+        uint8_t s = 0;
+        s |= 0 << 0;
+        s |= 1 << 2;
+        s |= 2 << 4;
+        s |= 3 << 6;
+        zig_small_packed_struct(s);
     }
 
     {
@@ -378,42 +368,32 @@ void c_split_struct_mixed(struct SplitStructMixed x) {
     assert_or_panic(y.c == 1337.0f);
 }
 
-struct SmallPackedStruct c_ret_small_packed_struct() {
-    struct SmallPackedStruct s = {
-        .a = 0,
-        .b = 1,
-        .c = 2,
-        .d = 3,
-        .e = 1,
-    };
+uint8_t c_ret_small_packed_struct() {
+    uint8_t s = 0;
+    s |= 0 << 0;
+    s |= 1 << 2;
+    s |= 2 << 4;
+    s |= 3 << 6;
     return s;
 }
 
-void c_small_packed_struct(struct SmallPackedStruct x) {
-    assert_or_panic(x.a == 0);
-    assert_or_panic(x.a == 1);
-    assert_or_panic(x.a == 2);
-    assert_or_panic(x.a == 3);
-    assert_or_panic(x.e == 1);
+void c_small_packed_struct(uint8_t x) {
+    assert_or_panic(((x >> 0) & 0x3) == 0);
+    assert_or_panic(((x >> 2) & 0x3) == 1);
+    assert_or_panic(((x >> 4) & 0x3) == 2);
+    assert_or_panic(((x >> 6) & 0x3) == 3);
 }
 
-struct BigPackedStruct c_ret_big_packed_struct() {
-    struct BigPackedStruct s = {
-        .a = 1,
-        .b = 2,
-        .c = 3,
-        .d = 4,
-        .e = 5,
-    };
+__int128 c_ret_big_packed_struct() {
+    __int128 s = 0;
+    s |= 1 << 0;
+    s |= (__int128)2 << 64;
     return s;
 }
 
-void c_big_packed_struct(struct BigPackedStruct x) {
-    assert_or_panic(x.a == 1);
-    assert_or_panic(x.b == 2);
-    assert_or_panic(x.c == 3);
-    assert_or_panic(x.d == 4);
-    assert_or_panic(x.e == 5);
+void c_big_packed_struct(__int128 x) {
+    assert_or_panic(((x >> 0) & 0xFFFFFFFFFFFFFFFF) == 1);
+    assert_or_panic(((x >> 64) & 0xFFFFFFFFFFFFFFFF) == 2);
 }
 
 struct SplitStructMixed c_ret_split_struct_mixed() {
test/c_abi/main.zig
@@ -263,37 +263,30 @@ const SmallPackedStruct = packed struct {
     b: u2,
     c: u2,
     d: u2,
-    e: bool,
 };
-const c_small_packed_struct: fn (SmallPackedStruct) callconv(.C) void = @compileError("TODO: #1481");
+extern fn c_small_packed_struct(SmallPackedStruct) void;
 extern fn c_ret_small_packed_struct() SmallPackedStruct;
 
-// waiting on #1481
-//export fn zig_small_packed_struct(x: SmallPackedStruct) void {
-//    expect(x.a == 0) catch @panic("test failure");
-//    expect(x.b == 1) catch @panic("test failure");
-//    expect(x.c == 2) catch @panic("test failure");
-//    expect(x.d == 3) catch @panic("test failure");
-//    expect(x.e) catch @panic("test failure");
-//}
+export fn zig_small_packed_struct(x: SmallPackedStruct) void {
+    expect(x.a == 0) catch @panic("test failure");
+    expect(x.b == 1) catch @panic("test failure");
+    expect(x.c == 2) catch @panic("test failure");
+    expect(x.d == 3) catch @panic("test failure");
+}
 
 test "C ABI small packed struct" {
-    var s = SmallPackedStruct{ .a = 0, .b = 1, .c = 2, .d = 3, .e = true };
-    _ = s; //c_small_packed_struct(s); // waiting on #1481
+    var s = SmallPackedStruct{ .a = 0, .b = 1, .c = 2, .d = 3 };
+    c_small_packed_struct(s);
     var s2 = c_ret_small_packed_struct();
     try expect(s2.a == 0);
     try expect(s2.b == 1);
     try expect(s2.c == 2);
     try expect(s2.d == 3);
-    try expect(s2.e);
 }
 
 const BigPackedStruct = packed struct {
     a: u64,
     b: u64,
-    c: u64,
-    d: u64,
-    e: u8,
 };
 extern fn c_big_packed_struct(BigPackedStruct) void;
 extern fn c_ret_big_packed_struct() BigPackedStruct;
@@ -301,20 +294,14 @@ extern fn c_ret_big_packed_struct() BigPackedStruct;
 export fn zig_big_packed_struct(x: BigPackedStruct) void {
     expect(x.a == 1) catch @panic("test failure");
     expect(x.b == 2) catch @panic("test failure");
-    expect(x.c == 3) catch @panic("test failure");
-    expect(x.d == 4) catch @panic("test failure");
-    expect(x.e == 5) catch @panic("test failure");
 }
 
 test "C ABI big packed struct" {
-    var s = BigPackedStruct{ .a = 1, .b = 2, .c = 3, .d = 4, .e = 5 };
+    var s = BigPackedStruct{ .a = 1, .b = 2 };
     c_big_packed_struct(s);
     var s2 = c_ret_big_packed_struct();
     try expect(s2.a == 1);
     try expect(s2.b == 2);
-    try expect(s2.c == 3);
-    try expect(s2.d == 4);
-    try expect(s2.e == 5);
 }
 
 const SplitStructInt = extern struct {
test/cases/compile_errors/C_pointer_pointing_to_non_C_ABI_compatible_type_or_has_align_attr.zig
@@ -10,5 +10,5 @@ export fn a() void {
 // target=native
 //
 // :3:19: error: C pointers cannot point to non-C-ABI-compatible type 'tmp.Foo'
-// :3:19: note: only structs with packed or extern layout are extern compatible
+// :3:19: note: only extern structs and ABI sized packed structs are extern compatible
 // :1:13: note: struct declared here
test/cases/compile_errors/function_with_non-extern_non-packed_struct_parameter.zig
@@ -10,5 +10,5 @@ export fn entry(foo: Foo) void { _ = foo; }
 // target=native
 //
 // :6:17: error: parameter of type 'tmp.Foo' not allowed in function with calling convention 'C'
-// :6:17: note: only structs with packed or extern layout are extern compatible
+// :6:17: note: only extern structs and ABI sized packed structs are extern compatible
 // :1:13: note: struct declared here
test/cases/compile_errors/function_with_non-extern_non-packed_union_parameter.zig
@@ -10,5 +10,5 @@ export fn entry(foo: Foo) void { _ = foo; }
 // target=native
 //
 // :6:17: error: parameter of type 'tmp.Foo' not allowed in function with calling convention 'C'
-// :6:17: note: only unions with packed or extern layout are extern compatible
+// :6:17: note: only extern unions and ABI sized packed unions are extern compatible
 // :1:13: note: union declared here