Skip to content

IRGen: Work around RemoteMirror bug generating reflection info for empty builtin types. #29037

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 7, 2020
Merged

IRGen: Work around RemoteMirror bug generating reflection info for empty builtin types. #29037

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 71 additions & 4 deletions lib/IRGen/GenReflection.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -671,7 +671,10 @@ class AssociatedTypeMetadataBuilder : public ReflectionMetadataBuilder {
};

class FieldTypeMetadataBuilder : public ReflectionMetadataBuilder {
const uint32_t fieldRecordSize = 12;
public:
static const uint32_t FieldRecordSize = 12;

private:
const NominalTypeDecl *NTD;

void addFieldDecl(const ValueDecl *value, Type type,
Expand Down Expand Up @@ -714,7 +717,7 @@ class FieldTypeMetadataBuilder : public ReflectionMetadataBuilder {
}

B.addInt16(uint16_t(kind));
B.addInt16(fieldRecordSize);
B.addInt16(FieldRecordSize);

auto properties = NTD->getStoredProperties();
B.addInt32(properties.size());
Expand All @@ -737,7 +740,7 @@ class FieldTypeMetadataBuilder : public ReflectionMetadataBuilder {
}

B.addInt16(uint16_t(kind));
B.addInt16(fieldRecordSize);
B.addInt16(FieldRecordSize);
B.addInt32(strategy.getElementsWithPayload().size()
+ strategy.getElementsWithNoPayload().size());

Expand All @@ -764,7 +767,7 @@ class FieldTypeMetadataBuilder : public ReflectionMetadataBuilder {
else
Kind = FieldDescriptorKind::Protocol;
B.addInt16(uint16_t(Kind));
B.addInt16(fieldRecordSize);
B.addInt16(FieldRecordSize);
B.addInt32(0);
}

Expand Down Expand Up @@ -825,6 +828,57 @@ class FieldTypeMetadataBuilder : public ReflectionMetadataBuilder {
}
};

static bool
deploymentTargetHasRemoteMirrorZeroSizedTypeDescriptorBug(IRGenModule &IGM) {
auto target = IGM.Context.LangOpts.Target;

if (target.isMacOSX() && target.isMacOSXVersionLT(10, 16, 0)) {
return true;
}
if (target.isiOS() && target.isOSVersionLT(14)) { // includes tvOS
return true;
}
if (target.isWatchOS() && target.isOSVersionLT(7)) {
return true;
}

return false;
}

/// Metadata builder that emits a fixed-layout empty type as an empty struct, as
/// a workaround for a RemoteMirror crash in older OSes.
class EmptyStructMetadataBuilder : public ReflectionMetadataBuilder {
const NominalTypeDecl *NTD;

void layout() override {
addNominalRef(NTD);
B.addInt32(0);
B.addInt16(uint16_t(FieldDescriptorKind::Struct));
B.addInt16(FieldTypeMetadataBuilder::FieldRecordSize);
B.addInt32(0);
}

public:
EmptyStructMetadataBuilder(IRGenModule &IGM,
const NominalTypeDecl *NTD)
: ReflectionMetadataBuilder(IGM), NTD(NTD) {
assert(IGM.getTypeInfoForUnlowered(
NTD->getDeclaredTypeInContext()->getCanonicalType())
.isKnownEmpty(ResilienceExpansion::Maximal)
&& "should only be used for known empty types");
}

llvm::GlobalVariable *emit() {
auto section = IGM.getFieldTypeMetadataSectionName();
return ReflectionMetadataBuilder::emit(
[&](IRGenModule &IGM, ConstantInit definition) -> llvm::Constant* {
return IGM.getAddrOfReflectionFieldDescriptor(
NTD->getDeclaredType()->getCanonicalType(), definition);
},
section);
}
};

class FixedTypeMetadataBuilder : public ReflectionMetadataBuilder {
ModuleDecl *module;
CanType type;
Expand Down Expand Up @@ -1338,6 +1392,19 @@ void IRGenModule::emitFieldDescriptor(const NominalTypeDecl *D) {
}

if (needsOpaqueDescriptor) {
// Work around an issue in the RemoteMirror library that ships in
// macOS 10.15/iOS 13 and earlier that causes it to crash on a
// BuiltinTypeDescriptor with zero size. If the type has zero size, emit it
// as an empty struct instead, which will have the same impact on the
// encoded type layout.
auto &TI = getTypeInfoForUnlowered(T);
if (deploymentTargetHasRemoteMirrorZeroSizedTypeDescriptorBug(*this)
&& TI.isKnownEmpty(ResilienceExpansion::Maximal)) {
EmptyStructMetadataBuilder builder(*this, D);
builder.emit();
return;
}

FixedTypeMetadataBuilder builder(*this, D);
builder.emit();
}
Expand Down
3 changes: 2 additions & 1 deletion validation-test/Reflection/reflect_empty_struct.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
// RUN: %empty-directory(%t)
// RUN: %target-build-swift -lswiftSwiftReflectionTest -I %S/Inputs/EmptyStruct/ %s -o %t/reflect_empty_struct
// RUN: %target-build-swift -lswiftSwiftReflectionTest -I %S/Inputs/EmptyStruct/ %s -o %t/reflect_empty_struct -target x86_64-apple-macosx10.99.0
// RUN: %target-codesign %t/reflect_empty_struct

// RUN: %target-run %target-swift-reflection-test %t/reflect_empty_struct | %FileCheck %s --check-prefix=CHECK-%target-ptrsize --dump-input fail

// REQUIRES: objc_interop
// REQUIRES: executable_test
// REQUIRES: OS=macosx

import SwiftReflectionTest

Expand Down
75 changes: 75 additions & 0 deletions validation-test/Reflection/reflect_empty_struct_compat.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
// RUN: %empty-directory(%t)

// RUN: %target-build-swift -lswiftSwiftReflectionTest -I %S/Inputs/EmptyStruct/ %s -o %t/reflect_empty_struct -target x86_64-apple-macosx10.15.0
// RUN: %target-codesign %t/reflect_empty_struct
// RUN: %target-run %target-swift-reflection-test %t/reflect_empty_struct | %FileCheck %s --check-prefix=CHECK-%target-ptrsize --dump-input fail

// RUN: %target-build-swift -lswiftSwiftReflectionTest -I %S/Inputs/EmptyStruct/ %s -o %t/reflect_empty_struct -target x86_64-apple-macosx10.14.0
// RUN: %target-codesign %t/reflect_empty_struct
// RUN: %target-run %target-swift-reflection-test %t/reflect_empty_struct | %FileCheck %s --check-prefix=CHECK-%target-ptrsize --dump-input fail

// REQUIRES: objc_interop
// REQUIRES: executable_test
// REQUIRES: OS=macosx

import SwiftReflectionTest

import EmptyStruct

@_alignment(1) struct EmptyStruct { }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you declaring the alignment here?

Copy link
Contributor Author

@jckarter jckarter Jan 7, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The compiler always generates BuiltinTypeDescriptors for types with declared alignment (because the normal layout algorithm would mis-align them), so this is just to exercise that code path. An empty type should not be able to affect the layout of its enclosing aggregate regardless of its supposed alignment, so it should be OK to fall back to an empty struct layout.

class Class {
var a = EmptyStruct()
var b: Any = EmptyStruct()
var c = EmptyStructC()
var d: Any = EmptyStructC()
}

var obj = Class()

reflect(object: obj)

// CHECK-64: Reflecting an object.
// CHECK-64: Instance pointer in child address space: 0x{{[0-9a-fA-F]+}}
// CHECK-64: Type reference:
// CHECK-64: (class reflect_empty_struct.Class)

// CHECK-64: Type info:
// CHECK-64: (class_instance size=80 alignment=8 stride=80 num_extra_inhabitants=0 bitwise_takable=1
// CHECK-64: (field name=a offset=16
// CHECK-64: (struct size=0 alignment=1 stride=1 num_extra_inhabitants=0 bitwise_takable=1))
// CHECK-64: (field name=b offset=16
// CHECK-64: (opaque_existential size=32 alignment=8 stride=32 num_extra_inhabitants=2147483647 bitwise_takable=1
// CHECK-64: (field name=metadata offset=24
// CHECK-64: (builtin size=8 alignment=8 stride=8 num_extra_inhabitants=2147483647 bitwise_takable=1))))
// CHECK-64: (field name=c offset=48
// CHECK-64: (struct size=0 alignment=1 stride=1 num_extra_inhabitants=0 bitwise_takable=1))
// CHECK-64: (field name=d offset=48
// CHECK-64: (opaque_existential size=32 alignment=8 stride=32 num_extra_inhabitants=2147483647 bitwise_takable=1
// CHECK-64: (field name=metadata offset=24
// CHECK-64: (builtin size=8 alignment=8 stride=8 num_extra_inhabitants=2147483647 bitwise_takable=1)))))

// CHECK-32: Reflecting an object.
// CHECK-32: Instance pointer in child address space: 0x{{[0-9a-fA-F]+}}
// CHECK-32: Type reference:
// CHECK-32: (class reflect_empty_struct.Class)

// CHECK-32: Type info:
// CHECK-32: (class_instance size=40 alignment=4 stride=40 num_extra_inhabitants=0 bitwise_takable=1
// CHECK-32: (field name=a offset=8
// CHECK-32: (struct size=0 alignment=1 stride=1 num_extra_inhabitants=0 bitwise_takable=1))
// CHECK-32: (field name=b offset=8
// CHECK-32: (opaque_existential size=16 alignment=4 stride=16 num_extra_inhabitants=4096 bitwise_takable=1
// CHECK-32: (field name=metadata offset=12
// CHECK-32: (builtin size=4 alignment=4 stride=4 num_extra_inhabitants=4096 bitwise_takable=1))))
// CHECK-32: (field name=c offset=24
// CHECK-32: (struct size=0 alignment=1 stride=1 num_extra_inhabitants=0 bitwise_takable=1))
// CHECK-32: (field name=d offset=24
// CHECK-32: (opaque_existential size=16 alignment=4 stride=16 num_extra_inhabitants=4096 bitwise_takable=1
// CHECK-32: (field name=metadata offset=12
// CHECK-32: (builtin size=4 alignment=4 stride=4 num_extra_inhabitants=4096 bitwise_takable=1)))))

doneReflecting()

// CHECK-64: Done.

// CHECK-32: Done.