Skip to content

Commit 664efc6

Browse files
author
Gabor Horvath
committed
[cxx-interop] Fix a crash with [[no_unique_address]]
Swift does not support storing fields in the padding of the previous fields just yet, so let's not import fields like that from C++. Represent them as opaque blobs instead. Fixes #80764
1 parent ff42f2e commit 664efc6

File tree

4 files changed

+79
-2
lines changed

4 files changed

+79
-2
lines changed

lib/ClangImporter/ImportDecl.cpp

+13
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@
6161
#include "clang/AST/DeclCXX.h"
6262
#include "clang/AST/DeclObjCCommon.h"
6363
#include "clang/AST/PrettyPrinter.h"
64+
#include "clang/AST/RecordLayout.h"
6465
#include "clang/AST/Type.h"
6566
#include "clang/Basic/Specifiers.h"
6667
#include "clang/Basic/TargetInfo.h"
@@ -4341,6 +4342,18 @@ namespace {
43414342
}
43424343

43434344
Decl *VisitFieldDecl(const clang::FieldDecl *decl) {
4345+
if (decl->hasAttr<clang::NoUniqueAddressAttr>()) {
4346+
if (const auto *rd = decl->getType()->getAsRecordDecl()) {
4347+
// Clang can store the next field in the padding of this one. Swift
4348+
// does not support this yet so let's not import the field and
4349+
// represent it with an opaque blob in codegen.
4350+
const auto &fieldLayout =
4351+
decl->getASTContext().getASTRecordLayout(rd);
4352+
if (!decl->isZeroSize(decl->getASTContext()) &&
4353+
fieldLayout.getDataSize() != fieldLayout.getSize())
4354+
return nullptr;
4355+
}
4356+
}
43444357
// Fields are imported as variables.
43454358
std::optional<ImportedName> correctSwiftName;
43464359
ImportedName importedName;

lib/IRGen/GenStruct.cpp

+12-1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
#include "GenStruct.h"
1818

19+
#include "IRGen.h"
1920
#include "swift/AST/ClangModuleLoader.h"
2021
#include "swift/AST/ConformanceLookup.h"
2122
#include "swift/AST/Decl.h"
@@ -42,6 +43,7 @@
4243
#include "llvm/ADT/STLExtras.h"
4344
#include "llvm/IR/DerivedTypes.h"
4445
#include "llvm/IR/Function.h"
46+
#include "llvm/Support/Error.h"
4547
#include <iterator>
4648

4749
#include "GenDecl.h"
@@ -1459,6 +1461,14 @@ class ClangRecordLowering {
14591461
unsigned fieldOffset = layout.getFieldOffset(clangField->getFieldIndex());
14601462
assert(!clangField->isBitField());
14611463
Size offset( SubobjectAdjustment.getValue() + fieldOffset / 8);
1464+
std::optional<Size> dataSize;
1465+
if (clangField->hasAttr<clang::NoUniqueAddressAttr>()) {
1466+
if (const auto *rd = clangField->getType()->getAsRecordDecl()) {
1467+
// Clang can store the next field in the padding of this one.
1468+
const auto &fieldLayout = ClangContext.getASTRecordLayout(rd);
1469+
dataSize = Size(fieldLayout.getDataSize().getQuantity());
1470+
}
1471+
}
14621472

14631473
// If we have a Swift import of this type, use our lowered information.
14641474
if (swiftField) {
@@ -1471,7 +1481,8 @@ class ClangRecordLowering {
14711481

14721482
// Otherwise, add it as an opaque blob.
14731483
auto fieldSize = isZeroSized ? clang::CharUnits::Zero() : ClangContext.getTypeSizeInChars(clangField->getType());
1474-
return addOpaqueField(offset, Size(fieldSize.getQuantity()));
1484+
return addOpaqueField(offset,
1485+
dataSize.value_or(Size(fieldSize.getQuantity())));
14751486
}
14761487

14771488
/// Add opaque storage for bitfields spanning the given range of bits.

test/Interop/Cxx/class/Inputs/member-variables.h

+23
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
#ifndef TEST_INTEROP_CXX_CLASS_INPUTS_MEMBER_VARIABLES_H
22
#define TEST_INTEROP_CXX_CLASS_INPUTS_MEMBER_VARIABLES_H
33

4+
#include <cstddef>
5+
#include <optional>
6+
47
class MyClass {
58
public:
69
const int const_member = 23;
@@ -24,6 +27,26 @@ struct HasZeroSizedField {
2427
void set_c(short c) { this->c = c; }
2528
};
2629

30+
struct ReuseFieldPadding {
31+
[[no_unique_address]] std::optional<int> a = {2};
32+
char c;
33+
char get_c() const { return c; }
34+
void set_c(char c) { this->c = c; }
35+
int offset() const { return offsetof(ReuseFieldPadding, c); }
36+
std::optional<int> getOptional() { return a; }
37+
};
38+
39+
using OptInt = std::optional<int>;
40+
41+
struct ReuseFieldPaddingWithTypedef {
42+
[[no_unique_address]] OptInt a;
43+
char c;
44+
char get_c() const { return c; }
45+
void set_c(char c) { this->c = c; }
46+
int offset() const { return offsetof(ReuseFieldPadding, c); }
47+
};
48+
49+
2750
inline int takesZeroSizedInCpp(HasZeroSizedField x) {
2851
return x.a;
2952
}

test/Interop/Cxx/class/zero-sized-field.swift

+31-1
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ FieldsTestSuite.test("Zero sized field") {
1515
s.set_c(7)
1616
takeTypeWithZeroSizedMember(s)
1717
let s2 = s
18-
let myInt : Empty.type = 6
18+
let _ : Empty.type = 6
1919
expectEqual(s.a, 5)
2020
expectEqual(s.a, s.get_a())
2121
expectEqual(s2.c, 7)
@@ -24,4 +24,34 @@ FieldsTestSuite.test("Zero sized field") {
2424
expectEqual(s.b.getNum(), 42)
2525
}
2626

27+
FieldsTestSuite.test("Field padding reused") {
28+
var s = ReuseFieldPadding()
29+
let opt = s.getOptional()
30+
expectEqual(Int(opt.pointee), 2)
31+
s.c = 5
32+
expectEqual(Int(s.offset()), MemoryLayout<ReuseFieldPadding>.offset(of: \.c)!)
33+
expectEqual(s.c, 5)
34+
expectEqual(s.get_c(), 5)
35+
s.set_c(6)
36+
expectEqual(s.c, 6)
37+
expectEqual(s.get_c(), 6)
38+
let s2 = s
39+
expectEqual(s2.c, 6)
40+
expectEqual(s2.get_c(), 6)
41+
}
42+
43+
FieldsTestSuite.test("Typedef'd field padding reused") {
44+
var s = ReuseFieldPaddingWithTypedef()
45+
s.c = 5
46+
expectEqual(Int(s.offset()), MemoryLayout<ReuseFieldPadding>.offset(of: \.c)!)
47+
expectEqual(s.c, 5)
48+
expectEqual(s.get_c(), 5)
49+
s.set_c(6)
50+
expectEqual(s.c, 6)
51+
expectEqual(s.get_c(), 6)
52+
let s2 = s
53+
expectEqual(s2.c, 6)
54+
expectEqual(s2.get_c(), 6)
55+
}
56+
2757
runAllTests()

0 commit comments

Comments
 (0)