Merge pull request #12745 from Techcable/translate-c/packed-struct-implies-align1

translate-c: Translate clang packed struct C into Zig extern struct with align(1)

6 files changed, 174 insertions, 50 deletions

diff --git a/src/clang.zig b/src/clang.zig
index 205a3cccc6..d3eacbfec4 100644
--- a/src/clang.zig
+++ b/src/clang.zig
@@ -470,6 +470,9 @@ pub const FieldDecl = opaque {
     pub const getAlignedAttribute = ZigClangFieldDecl_getAlignedAttribute;
     extern fn ZigClangFieldDecl_getAlignedAttribute(*const FieldDecl, *const ASTContext) c_uint;
 
+    pub const getPackedAttribute = ZigClangFieldDecl_getPackedAttribute;
+    extern fn ZigClangFieldDecl_getPackedAttribute(*const FieldDecl) bool;
+
     pub const isAnonymousStructOrUnion = ZigClangFieldDecl_isAnonymousStructOrUnion;
     extern fn ZigClangFieldDecl_isAnonymousStructOrUnion(*const FieldDecl) bool;
 
@@ -1015,6 +1018,9 @@ pub const VarDecl = opaque {
     pub const getAlignedAttribute = ZigClangVarDecl_getAlignedAttribute;
     extern fn ZigClangVarDecl_getAlignedAttribute(*const VarDecl, *const ASTContext) c_uint;
 
+    pub const getPackedAttribute = ZigClangVarDecl_getPackedAttribute;
+    extern fn ZigClangVarDecl_getPackedAttribute(*const VarDecl) bool;
+
     pub const getCleanupAttribute = ZigClangVarDecl_getCleanupAttribute;
     extern fn ZigClangVarDecl_getCleanupAttribute(*const VarDecl) ?*const FunctionDecl;
 
diff --git a/src/translate_c.zig b/src/translate_c.zig
index f969bf1c8b..e24c78f052 100644
--- a/src/translate_c.zig
+++ b/src/translate_c.zig
@@ -878,7 +878,7 @@ fn visitVarDecl(c: *Context, var_decl: *const clang.VarDecl, mangled_name: ?[]co
         .is_export = is_export,
         .is_threadlocal = is_threadlocal,
         .linksection_string = linksection_string,
-        .alignment = zigAlignment(var_decl.getAlignedAttribute(c.clang_context)),
+        .alignment = ClangAlignment.forVar(c, var_decl).zigAlignment(),
         .name = var_name,
         .type = type_node,
         .init = init_node,
@@ -1096,7 +1096,6 @@ fn transRecordDecl(c: *Context, scope: *Scope, record_decl: *const clang.RecordD
             break :blk Tag.opaque_literal.init();
         };
 
-        const is_packed = record_decl.getPackedAttribute();
         var fields = std.ArrayList(ast.Payload.Record.Field).init(c.gpa);
         defer fields.deinit();
 
@@ -1153,7 +1152,7 @@ fn transRecordDecl(c: *Context, scope: *Scope, record_decl: *const clang.RecordD
             const alignment = if (has_flexible_array and field_decl.getFieldIndex() == 0)
                 @intCast(c_uint, record_alignment)
             else
-                zigAlignment(field_decl.getAlignedAttribute(c.clang_context));
+                ClangAlignment.forField(c, field_decl, record_def).zigAlignment();
 
             if (is_anon) {
                 try c.decl_table.putNoClobber(c.gpa, @ptrToInt(field_decl.getCanonicalDecl()), field_name);
@@ -1166,15 +1165,11 @@ fn transRecordDecl(c: *Context, scope: *Scope, record_decl: *const clang.RecordD
             });
         }
 
-        if (!c.zig_is_stage1 and is_packed) {
-            return failDecl(c, record_loc, name, "cannot translate packed record union", .{});
-        }
-
         const record_payload = try c.arena.create(ast.Payload.Record);
         record_payload.* = .{
             .base = .{ .tag = ([2]Tag{ .@"struct", .@"union" })[@boolToInt(is_union)] },
             .data = .{
-                .layout = if (is_packed) .@"packed" else .@"extern",
+                .layout = .@"extern",
                 .fields = try c.arena.dupe(ast.Payload.Record.Field, fields.items),
                 .functions = try c.arena.dupe(Node, functions.items),
                 .variables = &.{},
@@ -1851,12 +1846,62 @@ fn transCStyleCastExprClass(
     return maybeSuppressResult(c, scope, result_used, cast_node);
 }
 
-/// Clang reports the alignment in bits, we use bytes
-/// Clang uses 0 for "no alignment specified", we use null
-fn zigAlignment(bit_alignment: c_uint) ?c_uint {
-    if (bit_alignment == 0) return null;
-    return bit_alignment / 8;
-}
+/// The alignment of a variable or field
+const ClangAlignment = struct {
+    /// Clang reports the alignment in bits, we use bytes
+    /// Clang uses 0 for "no alignment specified", we use null
+    bit_alignment: c_uint,
+    /// If the field or variable is marked as 'packed'
+    ///
+    /// According to the GCC variable attribute docs, this impacts alignment
+    /// https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html
+    ///
+    /// > The packed attribute specifies that a structure member
+    /// > should have the smallest possible alignment
+    ///
+    /// Note also that specifying the 'packed' attribute on a structure
+    /// implicitly packs all its fields (making their alignment 1).
+    ///
+    /// This will be null if the AST node doesn't support packing (functions)
+    is_packed: ?bool,
+
+    /// Get the alignment for a field, optionally taking into account the parent record
+    pub fn forField(c: *const Context, field: *const clang.FieldDecl, parent: ?*const clang.RecordDecl) ClangAlignment {
+        const parent_packed = if (parent) |record| record.getPackedAttribute() else false;
+        // NOTE: According to GCC docs, parent attribute packed implies child attribute packed
+        return ClangAlignment{
+            .bit_alignment = field.getAlignedAttribute(c.clang_context),
+            .is_packed = field.getPackedAttribute() or parent_packed,
+        };
+    }
+
+    pub fn forVar(c: *const Context, var_decl: *const clang.VarDecl) ClangAlignment {
+        return ClangAlignment{
+            .bit_alignment = var_decl.getAlignedAttribute(c.clang_context),
+            .is_packed = var_decl.getPackedAttribute(),
+        };
+    }
+
+    pub fn forFunc(c: *const Context, fun: *const clang.FunctionDecl) ClangAlignment {
+        return ClangAlignment{
+            .bit_alignment = fun.getAlignedAttribute(c.clang_context),
+            .is_packed = null, // not supported by GCC/clang (or meaningful),
+        };
+    }
+
+    /// Translate the clang alignment info into a zig alignment
+    ///
+    /// Returns null if there is no special alignment info
+    pub fn zigAlignment(self: ClangAlignment) ?c_uint {
+        if (self.bit_alignment != 0) {
+            return self.bit_alignment / 8;
+        } else if (self.is_packed orelse false) {
+            return 1;
+        } else {
+            return null;
+        }
+    }
+};
 
 fn transDeclStmtOne(
     c: *Context,
@@ -1910,7 +1955,7 @@ fn transDeclStmtOne(
                 .is_export = false,
                 .is_threadlocal = var_decl.getTLSKind() != .None,
                 .linksection_string = null,
-                .alignment = zigAlignment(var_decl.getAlignedAttribute(c.clang_context)),
+                .alignment = ClangAlignment.forVar(c, var_decl).zigAlignment(),
                 .name = var_name,
                 .type = type_node,
                 .init = init_node,
@@ -5054,7 +5099,7 @@ fn finishTransFnProto(
         break :blk null;
     };
 
-    const alignment = if (fn_decl) |decl| zigAlignment(decl.getAlignedAttribute(c.clang_context)) else null;
+    const alignment = if (fn_decl) |decl| ClangAlignment.forFunc(c, decl).zigAlignment() else null;
 
     const explicit_callconv = if ((is_inline or is_export or is_extern) and cc == .C) null else cc;
 
diff --git a/src/zig_clang.cpp b/src/zig_clang.cpp
index 223dc29067..bd06ead4b0 100644
--- a/src/zig_clang.cpp
+++ b/src/zig_clang.cpp
@@ -1941,10 +1941,7 @@ const char* ZigClangVarDecl_getSectionAttribute(const struct ZigClangVarDecl *se
 
 bool ZigClangRecordDecl_getPackedAttribute(const ZigClangRecordDecl *zig_record_decl) {
     const clang::RecordDecl *record_decl = reinterpret_cast<const clang::RecordDecl *>(zig_record_decl);
-    if (record_decl->getAttr<clang::PackedAttr>()) {
-      return true;
-    }
-  return false;
+    return record_decl->hasAttr<clang::PackedAttr>();
 }
 
 unsigned ZigClangVarDecl_getAlignedAttribute(const struct ZigClangVarDecl *self, const ZigClangASTContext* ctx) {
@@ -1985,6 +1982,16 @@ unsigned ZigClangFunctionDecl_getAlignedAttribute(const struct ZigClangFunctionD
     return 0;
 }
 
+bool ZigClangVarDecl_getPackedAttribute(const struct ZigClangVarDecl *self) {
+    auto casted_self = reinterpret_cast<const clang::VarDecl *>(self);
+    return casted_self->hasAttr<clang::PackedAttr>();
+}
+
+bool ZigClangFieldDecl_getPackedAttribute(const struct ZigClangFieldDecl *self) {
+    auto casted_self = reinterpret_cast<const clang::FieldDecl *>(self);
+    return casted_self->hasAttr<clang::PackedAttr>();
+}
+
 ZigClangQualType ZigClangParmVarDecl_getOriginalType(const struct ZigClangParmVarDecl *self) {
     return bitcast(reinterpret_cast<const clang::ParmVarDecl *>(self)->getOriginalType());
 }
diff --git a/src/zig_clang.h b/src/zig_clang.h
index 5d556700e6..fab5b83b2d 100644
--- a/src/zig_clang.h
+++ b/src/zig_clang.h
@@ -1101,6 +1101,8 @@ ZIG_EXTERN_C const struct ZigClangFunctionDecl *ZigClangVarDecl_getCleanupAttrib
 ZIG_EXTERN_C unsigned ZigClangVarDecl_getAlignedAttribute(const struct ZigClangVarDecl *self, const ZigClangASTContext* ctx);
 ZIG_EXTERN_C unsigned ZigClangFunctionDecl_getAlignedAttribute(const struct ZigClangFunctionDecl *self, const ZigClangASTContext* ctx);
 ZIG_EXTERN_C unsigned ZigClangFieldDecl_getAlignedAttribute(const struct ZigClangFieldDecl *self, const ZigClangASTContext* ctx);
+ZIG_EXTERN_C bool ZigClangVarDecl_getPackedAttribute(const struct ZigClangVarDecl *self);
+ZIG_EXTERN_C bool ZigClangFieldDecl_getPackedAttribute(const struct ZigClangFieldDecl *self);
 
 ZIG_EXTERN_C const struct ZigClangStringLiteral *ZigClangFileScopeAsmDecl_getAsmString(const struct ZigClangFileScopeAsmDecl *self);
 
diff --git a/test/run_translated_c.zig b/test/run_translated_c.zig
index e62c33eaa8..71654afba4 100644
--- a/test/run_translated_c.zig
+++ b/test/run_translated_c.zig
@@ -250,20 +250,18 @@ pub fn addCases(cases: *tests.RunTranslatedCContext) void {
         \\}
     , "");
 
-    if (@import("builtin").zig_backend == .stage1) {
-        cases.add("struct initializer - packed",
-            \\#define _NO_CRT_STDIO_INLINE 1
-            \\#include <stdint.h>
-            \\#include <stdlib.h>
-            \\struct s {uint8_t x,y;
-            \\          uint32_t z;} __attribute__((packed)) s0 = {1, 2};
-            \\int main() {
-            \\  /* sizeof nor offsetof currently supported */
-            \\  if (((intptr_t)&s0.z - (intptr_t)&s0.x) != 2) abort();
-            \\  return 0;
-            \\}
-        , "");
-    }
+    cases.add("struct initializer - packed",
+        \\#define _NO_CRT_STDIO_INLINE 1
+        \\#include <stdint.h>
+        \\#include <stdlib.h>
+        \\struct s {uint8_t x,y;
+        \\          uint32_t z;} __attribute__((packed)) s0 = {1, 2};
+        \\int main() {
+        \\  /* sizeof nor offsetof currently supported */
+        \\  if (((intptr_t)&s0.z - (intptr_t)&s0.x) != 2) abort();
+        \\  return 0;
+        \\}
+    , "");
 
     cases.add("cast signed array index to unsigned",
         \\#include <stdlib.h>
diff --git a/test/translate_c.zig b/test/translate_c.zig
index 0e38444ad2..9854e783d4 100644
--- a/test/translate_c.zig
+++ b/test/translate_c.zig
@@ -728,22 +728,20 @@ pub fn addCases(cases: *tests.TranslateCContext) void {
         \\}
     });
 
-    if (builtin.zig_backend == .stage1) {
-        cases.add("struct initializer - packed",
-            \\struct {int x,y,z;} __attribute__((packed)) s0 = {1, 2};
-        , &[_][]const u8{
-            \\const struct_unnamed_1 = packed struct {
-            \\    x: c_int,
-            \\    y: c_int,
-            \\    z: c_int,
-            \\};
-            \\pub export var s0: struct_unnamed_1 = struct_unnamed_1{
-            \\    .x = @as(c_int, 1),
-            \\    .y = @as(c_int, 2),
-            \\    .z = 0,
-            \\};
-        });
-    }
+    cases.add("struct initializer - packed",
+        \\struct {int x,y,z;} __attribute__((packed)) s0 = {1, 2};
+    , &[_][]const u8{
+        \\const struct_unnamed_1 = extern struct {
+        \\    x: c_int align(1),
+        \\    y: c_int align(1),
+        \\    z: c_int align(1),
+        \\};
+        \\pub export var s0: struct_unnamed_1 = struct_unnamed_1{
+        \\    .x = @as(c_int, 1),
+        \\    .y = @as(c_int, 2),
+        \\    .z = 0,
+        \\};
+    });
 
     // Test case temporarily disabled:
     // https://github.com/ziglang/zig/issues/12055
@@ -1393,6 +1391,74 @@ pub fn addCases(cases: *tests.TranslateCContext) void {
         \\pub const Foo = union_Foo;
     });
 
+    cases.add("packed union - simple",
+        \\union Foo {
+        \\  char x;
+        \\  double y;
+        \\} __attribute__((packed));
+    , &[_][]const u8{
+        \\pub const union_Foo = extern union {
+        \\    x: u8 align(1),
+        \\    y: f64 align(1),
+        \\};
+        ,
+        \\pub const Foo = union_Foo;
+    });
+
+    cases.add("packed union - nested unpacked",
+        \\union Foo{
+        \\  char x;
+        \\  double y;
+        \\  struct {
+        \\      char a;
+        \\      int b;
+        \\  } z;
+        \\} __attribute__((packed));
+    , &[_][]const u8{
+        // NOTE: The nested struct is *not* packed/aligned,
+        // even though the parent struct is
+        // this is consistent with GCC docs
+        \\const struct_unnamed_1 = extern struct {
+        \\    a: u8,
+        \\    b: c_int,
+        \\};
+        ,
+        \\pub const union_Foo = extern union {
+        \\    x: u8 align(1),
+        \\    y: f64 align(1),
+        \\    z: struct_unnamed_1 align(1),
+        \\};
+        ,
+        \\pub const Foo = union_Foo;
+    });
+
+    cases.add("packed union - nested packed",
+        \\union Foo{
+        \\  char x;
+        \\  double y;
+        \\  struct {
+        \\      char a;
+        \\      int b;
+        \\  } __attribute__((packed)) z;
+        \\} __attribute__((packed));
+    , &[_][]const u8{
+        // in order for the nested struct to be packed, it must
+        // have an independent packed declaration on
+        // the nested type (see GCC docs for details)
+        \\const struct_unnamed_1 = extern struct {
+        \\    a: u8 align(1),
+        \\    b: c_int align(1),
+        \\};
+        ,
+        \\pub const union_Foo = extern union {
+        \\    x: u8 align(1),
+        \\    y: f64 align(1),
+        \\    z: struct_unnamed_1 align(1),
+        \\};
+        ,
+        \\pub const Foo = union_Foo;
+    });
+
     cases.add("string literal",
         \\const char *foo(void) {
         \\    return "bar";