Skip to content

Commit

Permalink
translate-c: Allow translating packed C structs iff ABI-sized
Browse files Browse the repository at this point in the history
This relaxes the restriction added in c7884af
and fixes #12733

Right now, the compiler only considers packed structs to be
C compatible (extern) if the structs are "ABI sized".

See also b83c037 for details on
the extern restrictions on `packed struct`

This is an issue on all platforms, but is of particular
importance on aarch64-macos (seeissue for details)

Before this commit, a type like

``
struct example {
    int val;
} __attribute__((__packed__));
``

would fail to translate and turn into

``
pub const example = @CompileError("cannot translate packed record union");
``

This fixes this issue for ABI-sized structs, making the translate-c
rules consistent with the regular compiler.

However, we still reject packed structs that are not "ABI-sized",
giving the same `@compileError` message

So
``
struct example {
    long a, b, c
} __attribute__((__packed__));
``

will still fail (in the sense that it will emit a `@compileError`)

**Implementation details**:

For the purposes of translate-c, a packed struct is considered
"ABI-sized" when sizeof(target_struct) <= sizeof(void*)

Unfortunately, right now the translate-c `*Context` structure (on the Zig side)
doesn't directly have any information on the target we're translating into.

However, clang has this target information, and keeps it  in clang::ASTContext.
I added a helper function to extract the `clang::TargetInfo` from this,
then query the pointer size.

This is potentially controversial, since it technically makes the
output target-dependent.
The decision of whether to accept/reject a packed struct varies depending
on the target's pointer size.

However, the translator already asks for (integer) field offsets elsewhere,
so I'm pretty sure target info already impacts translation.

We also add getSize() and getDataSize() functions to *clang.RecordDecl.
These return padded and unpadded sizes respectively.
For a packed struct, they should be the same.

The overall effect is 4 more functions added to zig_clang.cpp,
in order to extract target info & query record sizes.
  • Loading branch information
Techcable committed Sep 4, 2022
1 parent b7d5582 commit eb1eea0
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 3 deletions.
14 changes: 14 additions & 0 deletions src/clang.zig
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,9 @@ pub const APSInt = opaque {
pub const ASTContext = opaque {
pub const getPointerType = ZigClangASTContext_getPointerType;
extern fn ZigClangASTContext_getPointerType(*const ASTContext, T: QualType) QualType;

pub const getTargetInfo = ZigClangASTContext_getTargetInfo;
extern fn ZigClangASTContext_getTargetInfo(*const ASTContext) *const TargetInfo;
};

pub const ASTUnit = opaque {
Expand Down Expand Up @@ -194,6 +197,12 @@ pub const ASTRecordLayout = opaque {

pub const getAlignment = ZigClangASTRecordLayout_getAlignment;
extern fn ZigClangASTRecordLayout_getAlignment(*const ASTRecordLayout) i64;

pub const getSize = ZigClangASTRecordLayout_getSize;
extern fn ZigClangASTRecordLayout_getSize(*const ASTRecordLayout) i64;

pub const getDataSize = ZigClangASTRecordLayout_getSize;
extern fn ZigClangASTRecordLayout_getDataSize(*const ASTRecordLayout) i64;
};

pub const AttributedType = opaque {
Expand Down Expand Up @@ -895,6 +904,11 @@ pub const TagDecl = opaque {
extern fn ZigClangTagDecl_isThisDeclarationADefinition(*const TagDecl) bool;
};

pub const TargetInfo = opaque {
pub const getMaxPointerWidth = ZigClangTargetInfo_getMaxPointerWidth;
extern fn ZigClangTargetInfo_getMaxPointerWidth(*const TargetInfo) u64;
};

pub const Type = opaque {
pub const getTypeClass = ZigClangType_getTypeClass;
extern fn ZigClangType_getTypeClass(*const Type) TypeClass;
Expand Down
37 changes: 35 additions & 2 deletions src/translate_c.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1166,8 +1166,8 @@ fn transRecordDecl(c: *Context, scope: *Scope, record_decl: *const clang.RecordD
});
}

if (!c.zig_is_stage1 and is_packed) {
return failDecl(c, record_loc, bare_name, "cannot translate packed record union", .{});
if (!c.zig_is_stage1 and is_packed and !isAbiSizedPackedRecord(c, record_decl)) {
return failDecl(c, record_loc, bare_name, "cannot translate packed record union (unless ABI sized)", .{});
}

const record_payload = try c.arena.create(ast.Payload.Record);
Expand Down Expand Up @@ -1204,6 +1204,39 @@ fn transRecordDecl(c: *Context, scope: *Scope, record_decl: *const clang.RecordD
}
}

/// Check if the declaration is an ABI sized, packed structure
///
/// aCurrently, a packed struct is only extern if it is "ABI-sized"
fn isAbiSizedPackedRecord(c: *const Context, decl: *const clang.RecordDecl) bool {
assert(decl.getPackedAttribute()); // should be packed
// For the purposes of translate-c, we consider a implement something "ABI sized"
// if it is smaller than a pointer
const struct_layout = decl.getASTRecordLayout(c.clang_context);
const ptr_bits = c.clang_context.getTargetInfo().getMaxPointerWidth();
// double check we're in bits
assert(ptr_bits >= 16);
assert(ptr_bits % 8 == 0);
const ptr_size = ptr_bits / 8;
const struct_size = struct_layout.getSize();
{
// verify the result from getSize() matches the result
// returned from getDataSize().
//
// According to clang doxygen, getDataSize() returns record
// size "without tail padding"
//
// Since this is a packed struct, this should match the regular size
const struct_data_size = struct_layout.getDataSize();
if (struct_size != struct_data_size) {
std.debug.panic(
"Expected getSize() {} to match getDataSize() {} (packed struct should have no padding)",
.{struct_size, struct_data_size},
);
}
}
return struct_size <= ptr_size;
}

fn transEnumDecl(c: *Context, scope: *Scope, enum_decl: *const clang.EnumDecl) Error!void {
if (c.decl_table.get(@ptrToInt(enum_decl.getCanonicalDecl()))) |_|
return; // Avoid processing this decl twice
Expand Down
22 changes: 21 additions & 1 deletion src/zig_clang.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
#include <clang/AST/Attr.h>
#include <clang/AST/Expr.h>
#include <clang/AST/RecordLayout.h>
#include <clang/Basic/TargetInfo.h>

#if __GNUC__ >= 8
#pragma GCC diagnostic pop
Expand Down Expand Up @@ -1832,14 +1833,23 @@ const char* ZigClangSourceManager_getCharacterData(const ZigClangSourceManager *
return reinterpret_cast<const clang::SourceManager *>(self)->getCharacterData(bitcast(SL));
}

ZigClangQualType ZigClangASTContext_getPointerType(const ZigClangASTContext* self, ZigClangQualType T) {
ZigClangQualType ZigClangASTContext_getPointerType(const ZigClangASTContext *self, ZigClangQualType T) {
return bitcast(reinterpret_cast<const clang::ASTContext *>(self)->getPointerType(bitcast(T)));
}
const ZigClangTargetInfo *ZigClangASTContext_getTargetInfo(const ZigClangASTContext* self) {
const clang::ASTContext *ctx = reinterpret_cast<const clang::ASTContext *>(self);
return reinterpret_cast<const ZigClangTargetInfo *>(&ctx->getTargetInfo());
}

// TODO: UNUSED??
unsigned ZigClangASTContext_getTypeAlign(const ZigClangASTContext* self, ZigClangQualType T) {
return reinterpret_cast<const clang::ASTContext *>(self)->getTypeAlign(bitcast(T));
}

uint64_t ZigClangTargetInfo_getMaxPointerWidth(const ZigClangTargetInfo *self) {
return reinterpret_cast<const clang::TargetInfo *>(self)->getMaxPointerWidth();
}

ZigClangASTContext *ZigClangASTUnit_getASTContext(ZigClangASTUnit *self) {
clang::ASTContext *result = &reinterpret_cast<clang::ASTUnit *>(self)->getASTContext();
return reinterpret_cast<ZigClangASTContext *>(result);
Expand Down Expand Up @@ -2910,6 +2920,16 @@ int64_t ZigClangASTRecordLayout_getAlignment(const struct ZigClangASTRecordLayou
return casted_self->getAlignment().getQuantity();
}

int64_t ZigClangASTRecordLayout_getSize(const struct ZigClangASTRecordLayout *self) {
auto casted_self = reinterpret_cast<const clang::ASTRecordLayout *>(self);
return casted_self->getSize().getQuantity();
}

int64_t ZigClangASTRecordLayout_getDataSize(const struct ZigClangASTRecordLayout *self) {
auto casted_self = reinterpret_cast<const clang::ASTRecordLayout *>(self);
return casted_self->getDataSize().getQuantity();
}

bool ZigClangIntegerLiteral_EvaluateAsInt(const struct ZigClangIntegerLiteral *self, struct ZigClangExprEvalResult *result, const struct ZigClangASTContext *ctx) {
auto casted_self = reinterpret_cast<const clang::IntegerLiteral *>(self);
auto casted_ctx = reinterpret_cast<const clang::ASTContext *>(ctx);
Expand Down
6 changes: 6 additions & 0 deletions src/zig_clang.h
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,7 @@ struct ZigClangStringLiteral;
struct ZigClangStringRef;
struct ZigClangSwitchStmt;
struct ZigClangTagDecl;
struct ZigClangTargetInfo;
struct ZigClangType;
struct ZigClangTypedefNameDecl;
struct ZigClangTypedefType;
Expand Down Expand Up @@ -1049,6 +1050,7 @@ ZIG_EXTERN_C const char* ZigClangSourceManager_getCharacterData(const struct Zig
struct ZigClangSourceLocation SL);

ZIG_EXTERN_C struct ZigClangQualType ZigClangASTContext_getPointerType(const struct ZigClangASTContext*, struct ZigClangQualType T);
ZIG_EXTERN_C const struct ZigClangTargetInfo* ZigClangASTContext_getTargetInfo(const struct ZigClangASTContext*);

ZIG_EXTERN_C struct ZigClangSourceLocation ZigClangLexer_getLocForEndOfToken(struct ZigClangSourceLocation,
const ZigClangSourceManager *, const ZigClangASTUnit *);
Expand Down Expand Up @@ -1077,6 +1079,8 @@ ZIG_EXTERN_C const struct ZigClangEnumDecl *ZigClangEnumType_getDecl(const struc

ZIG_EXTERN_C bool ZigClangTagDecl_isThisDeclarationADefinition(const struct ZigClangTagDecl *);

ZIG_EXTERN_C uint64_t ZigClangTargetInfo_getMaxPointerWidth(const struct ZigClangTargetInfo *);

ZIG_EXTERN_C const struct ZigClangTagDecl *ZigClangRecordDecl_getCanonicalDecl(const struct ZigClangRecordDecl *record_decl);
ZIG_EXTERN_C const struct ZigClangTagDecl *ZigClangEnumDecl_getCanonicalDecl(const struct ZigClangEnumDecl *);
ZIG_EXTERN_C const struct ZigClangFieldDecl *ZigClangFieldDecl_getCanonicalDecl(const ZigClangFieldDecl *);
Expand Down Expand Up @@ -1106,6 +1110,8 @@ ZIG_EXTERN_C const struct ZigClangASTRecordLayout *ZigClangRecordDecl_getASTReco

ZIG_EXTERN_C uint64_t ZigClangASTRecordLayout_getFieldOffset(const struct ZigClangASTRecordLayout *, unsigned);
ZIG_EXTERN_C int64_t ZigClangASTRecordLayout_getAlignment(const struct ZigClangASTRecordLayout *);
ZIG_EXTERN_C int64_t ZigClangASTRecordLayout_getSize(const struct ZigClangASTRecordLayout *);
ZIG_EXTERN_C int64_t ZigClangASTRecordLayout_getDataSize(const struct ZigClangASTRecordLayout *);

ZIG_EXTERN_C struct ZigClangQualType ZigClangFunctionDecl_getType(const struct ZigClangFunctionDecl *);
ZIG_EXTERN_C struct ZigClangSourceLocation ZigClangFunctionDecl_getLocation(const struct ZigClangFunctionDecl *);
Expand Down

0 comments on commit eb1eea0

Please sign in to comment.