Skip to content

Commit

Permalink
translate-c: packed struct implies align(1) on every field
Browse files Browse the repository at this point in the history
This is relevant to the fix in #12735 , but also fixes stage1.

This comes from a careful reading of the GCC docs for type & variable attributes.

The final line of the documentation for __attribute__ ((aligned)) [on types] says:

> When used on a struct, or struct member, *the aligned attribute can only increase the alignment*; in order to decrease it, the packed attribute must be specified as well.

This implies that GCC uses the `packed` attribute for alignment purposes
in addition to eliminating padding.

The documentation for __attribute__((packed)) [on types], does not
directly state this relationship with alignment.
However, it *does* say that the 'packed' attribute is applied
recursively to each member.

> This attribute, attached to a struct, union, or C++ class type definition, specifies that each of its members (other than zero-width bit-fields) is placed to minimize the memory required. **This is equivalent to specifying the packed attribute on each of the members**.

The key is resolving this indirection, and looking at the documentation
for __attribute__((packed)) [on fields]:

> The packed attribute specifies that a **structure member should have the smallest possible alignment** — one bit for a bit-field and one byte otherwise, unless a larger value is specified with the aligned attribute. The attribute does not apply to non-member objects.

(NOTE: Somewhat confusingly, field attributes are documented under "variables")

This means that a C structure with __attribute__((packed)) automatically
implies align(1) on each of its member.

Unfortunately, the current implementation of `translate-c` does not reflect this.

Running `translate-c` on the following code:

```c
struct foo {
    char a;
    int b;
} __attribute__((packed));

struct bar {
    char a;
    int b;
    short c;
    __attribute__((aligned(8))) long d;
} __attribute__((packed));
```

does not apply any alignment attributes to any fields except d.

Sometimes __attribute__((packed)) is used only for alignment purposes.
It is a semi-portable way to do unaligned stores. In particular, the
aarch64-macos.12 headers do this in stdlib.h

This was noted by @ifreund in the discussion of PR #12735

The structures in the MacOS headers were being used chiefly for their alignment
properties, not for padding purposes (they only had one field).

After applying this change, the translated structures have align(1)
explicitly applied to all of their fields (unless explicitly overriden).
This makes Zig behavior for `tranlsate-c` consistent with clang/GCC.

Here is the newly produced (correct) output for the above example:

```zig
pub const struct_foo = packed struct {
    a: u8 align(1),
    b: c_int align(1),
};
pub const struct_bar = packed struct {
    a: u8 align(1),
    b: c_int align(1),
    c: c_short align(1),
    d: c_long align(8),
};
```

This is closely related with PR #12735

Since the last stable release (0.9.1), there was a **change in the language spec**
related to the alignment of packed structures.

The docs for Zig 0.9.1 read:
> Packed structs have 1-byte alignment.

So the old behavior of translate-c (not specifying any alignment) was possibly correct back then.

However the current docs read:

> Packed structs have the same alignment as their backing integer

So now `translate-c` definitely needs to specify align(1) for every field (unless the spec is overriden).
  • Loading branch information
Techcable committed Sep 6, 2022
1 parent 3deb33f commit fb09af8
Show file tree
Hide file tree
Showing 4 changed files with 86 additions and 10 deletions.
6 changes: 6 additions & 0 deletions src/clang.zig
Expand Up @@ -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;

Expand Down Expand Up @@ -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;

Expand Down
70 changes: 60 additions & 10 deletions src/translate_c.zig
Expand Up @@ -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,
Expand Down Expand Up @@ -1153,7 +1153,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);
Expand Down Expand Up @@ -1851,12 +1851,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,
Expand Down Expand Up @@ -1910,7 +1960,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,
Expand Down Expand Up @@ -5054,7 +5104,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;

Expand Down
18 changes: 18 additions & 0 deletions src/zig_clang.cpp
Expand Up @@ -1975,6 +1975,24 @@ unsigned ZigClangFunctionDecl_getAlignedAttribute(const struct ZigClangFunctionD
return 0;
}

bool ZigClangVarDecl_getPackedAttribute(const struct ZigClangVarDecl *self) {
auto casted_self = reinterpret_cast<const clang::VarDecl *>(self);
if (const clang::PackedAttr *PA = casted_self->getAttr<clang::PackedAttr>()) {
return true;
} else {
return false;
}
}

bool ZigClangFieldDecl_getPackedAttribute(const struct ZigClangFieldDecl *self) {
auto casted_self = reinterpret_cast<const clang::FieldDecl *>(self);
if (const clang::PackedAttr *PA = casted_self->getAttr<clang::PackedAttr>()) {
return true;
} else {
return false;
}
}

ZigClangQualType ZigClangParmVarDecl_getOriginalType(const struct ZigClangParmVarDecl *self) {
return bitcast(reinterpret_cast<const clang::ParmVarDecl *>(self)->getOriginalType());
}
Expand Down
2 changes: 2 additions & 0 deletions src/zig_clang.h
Expand Up @@ -1088,6 +1088,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);

Expand Down

0 comments on commit fb09af8

Please sign in to comment.