Skip to content

[IRGen+Runtime] Fix tag bit mask handling for objc, unknown objects a… #77431

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.

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
Nov 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 6 additions & 2 deletions lib/IRGen/TypeLayout.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
//
//===----------------------------------------------------------------------===//
#include "TypeLayout.h"
#include "ClassTypeInfo.h"
#include "ConstantBuilder.h"
#include "EnumPayload.h"
#include "FixedTypeInfo.h"
Expand Down Expand Up @@ -1293,14 +1294,17 @@ bool ScalarTypeLayoutEntry::refCountString(IRGenModule &IGM,
case ScalarKind::BlockReference:
B.addRefCount(LayoutStringBuilder::RefCountingKind::Block, size);
break;
case ScalarKind::ObjCReference:
if (typeInfo.hasFixedSpareBits()) {
case ScalarKind::ObjCReference: {
auto *classTI = dyn_cast<ClassTypeInfo>(&typeInfo);
assert(classTI);
if (!classTI->getClass()->hasClangNode()) {
B.addRefCount(LayoutStringBuilder::RefCountingKind::NativeSwiftObjC,
size);
} else {
B.addRefCount(LayoutStringBuilder::RefCountingKind::ObjC, size);
}
break;
}
case ScalarKind::ThickFunc:
B.addSkip(IGM.getPointerSize().getValue());
B.addRefCount(LayoutStringBuilder::RefCountingKind::NativeStrong,
Expand Down
72 changes: 57 additions & 15 deletions stdlib/public/runtime/BytecodeLayouts.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,16 @@ static uint64_t readTagBytes(const uint8_t *addr, uint8_t byteCount) {
}
}

// This check is used to determine whether or not ObjC references can
// be tagged pointers. If they can't, they have the same spare bits
// as swift references, and we have to mask them out before passing the
// reference to ref counting operations.
static constexpr bool platformSupportsTaggedPointers() {
// Platforms that don't reserve bits for ObjC, don't support tagged
// pointers.
return _swift_abi_ObjCReservedBitsMask != 0;
}

#if defined(__APPLE__) && defined(__arm64__)

#define CONTINUE_WITH_COPY(METADATA, READER, ADDR_OFFSET, DEST, SRC) \
Expand Down Expand Up @@ -281,9 +291,12 @@ static void weakDestroy(const Metadata *metadata, LayoutStringReader1 &reader,
static void unknownDestroy(const Metadata *metadata,
LayoutStringReader1 &reader, uintptr_t &addrOffset,
uint8_t *addr) {
void *object = *(void**)(addr + addrOffset);
uintptr_t object = *(uintptr_t *)(addr + addrOffset);
addrOffset += sizeof(void*);
swift_unknownObjectRelease(object);
if (!platformSupportsTaggedPointers()) {
object &= ~_swift_abi_SwiftSpareBitsMask;
}
swift_unknownObjectRelease((void *)object);
}

static void unknownUnownedDestroy(const Metadata *metadata,
Expand Down Expand Up @@ -769,9 +782,10 @@ multiPayloadEnumGeneric(const Metadata *metadata, LayoutStringReader1 &reader,
static void blockDestroy(const Metadata *metadata, LayoutStringReader1 &reader,
uintptr_t &addrOffset, uint8_t *addr) {
#if SWIFT_OBJC_INTEROP
void* object = (void *)(addr + addrOffset);
uintptr_t object = *(uintptr_t *)(addr + addrOffset);
object &= ~_swift_abi_SwiftSpareBitsMask;
addrOffset += sizeof(void*);
_Block_release(object);
_Block_release((void *)object);
#else
swift_unreachable("Blocks are not available on this platform");
#endif
Expand All @@ -783,6 +797,11 @@ static void objcStrongDestroy(const Metadata *metadata,
#if SWIFT_OBJC_INTEROP
uintptr_t object = *(uintptr_t *)(addr + addrOffset);
addrOffset += sizeof(objc_object*);

if (!platformSupportsTaggedPointers()) {
object &= ~_swift_abi_SwiftSpareBitsMask;
}

objc_release((objc_object *)object);
#else
swift_unreachable("ObjC interop is not available on this platform");
Expand Down Expand Up @@ -959,10 +978,13 @@ static void weakCopyInit(const Metadata *metadata, LayoutStringReader1 &reader,
static void unknownRetain(const Metadata *metadata, LayoutStringReader1 &reader,
uintptr_t &addrOffset, uint8_t *dest, uint8_t *src) {
uintptr_t _addrOffset = addrOffset;
void *object = *(void **)(src + _addrOffset);
uintptr_t object = *(uintptr_t *)(src + _addrOffset);
memcpy(dest + _addrOffset, &object, sizeof(void*));
addrOffset = _addrOffset + sizeof(void *);
swift_unknownObjectRetain(object);
if (!platformSupportsTaggedPointers()) {
object &= ~_swift_abi_SwiftSpareBitsMask;
}
swift_unknownObjectRetain((void *)object);
}

static void unknownUnownedCopyInit(const Metadata *metadata,
Expand Down Expand Up @@ -1000,9 +1022,11 @@ static void blockCopy(const Metadata *metadata, LayoutStringReader1 &reader,
uintptr_t &addrOffset, uint8_t *dest, uint8_t *src) {
#if SWIFT_OBJC_INTEROP
uintptr_t _addrOffset = addrOffset;
auto *copy = _Block_copy(*(void**)(src + _addrOffset));
memcpy(dest + _addrOffset, &copy, sizeof(void*));
uintptr_t object = *(uintptr_t *)(src + _addrOffset);
memcpy(dest + _addrOffset, &object, sizeof(void *));
addrOffset = _addrOffset + sizeof(void*);
object &= ~_swift_abi_SwiftSpareBitsMask;
_Block_copy((void *)object);
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we know for sure that the block has been copied to the heap already? _Block_copy returns its parameter iff the block isn't still on the stack. I recall that the Swift compiler doesn't try too hard to avoid copying blocks so we may never see a stack block here, but this worries me a bit, especially if the compiler got smarter about not copying nonescaping blocks at some point.

#else
swift_unreachable("Blocks are not available on this platform");
#endif
Expand All @@ -1016,6 +1040,11 @@ static void objcStrongRetain(const Metadata *metadata,
uintptr_t object = *(uintptr_t *)(src + _addrOffset);
memcpy(dest + _addrOffset, &object, sizeof(objc_object *));
addrOffset = _addrOffset + sizeof(objc_object *);

if (!platformSupportsTaggedPointers()) {
object &= ~_swift_abi_SwiftSpareBitsMask;
}

objc_retain((objc_object *)object);
#else
swift_unreachable("ObjC interop is not available on this platform");
Expand Down Expand Up @@ -1368,12 +1397,16 @@ static void unknownAssignWithCopy(const Metadata *metadata,
uintptr_t &addrOffset, uint8_t *dest,
uint8_t *src) {
uintptr_t _addrOffset = addrOffset;
void *destObject = *(void **)(dest + _addrOffset);
void *srcObject = *(void **)(src + _addrOffset);
uintptr_t destObject = *(uintptr_t *)(dest + _addrOffset);
uintptr_t srcObject = *(uintptr_t *)(src + _addrOffset);
memcpy(dest + _addrOffset, &srcObject, sizeof(void *));
addrOffset = _addrOffset + sizeof(void *);
swift_unknownObjectRelease(destObject);
swift_unknownObjectRetain(srcObject);
if (!platformSupportsTaggedPointers()) {
destObject &= ~_swift_abi_SwiftSpareBitsMask;
srcObject &= ~_swift_abi_SwiftSpareBitsMask;
}
swift_unknownObjectRelease((void *)destObject);
swift_unknownObjectRetain((void *)srcObject);
}

static void bridgeAssignWithCopy(const Metadata *metadata,
Expand Down Expand Up @@ -1431,10 +1464,14 @@ static void blockAssignWithCopy(const Metadata *metadata,
uint8_t *src) {
#if SWIFT_OBJC_INTEROP
uintptr_t _addrOffset = addrOffset;
_Block_release(*(void **)(dest + _addrOffset));
auto *copy = _Block_copy(*(void **)(src + _addrOffset));
memcpy(dest + _addrOffset, &copy, sizeof(void*));
uintptr_t destObject = *(uintptr_t *)(dest + _addrOffset);
uintptr_t srcObject = *(uintptr_t *)(src + _addrOffset);
memcpy(dest + _addrOffset, &srcObject, sizeof(void *));
addrOffset = _addrOffset + sizeof(void*);
destObject &= ~_swift_abi_SwiftSpareBitsMask;
srcObject &= ~_swift_abi_SwiftSpareBitsMask;
_Block_release((void *)destObject);
_Block_copy((void *)srcObject);
#else
swift_unreachable("Blocks are not available on this platform");
#endif
Expand All @@ -1452,6 +1489,11 @@ static void objcStrongAssignWithCopy(const Metadata *metadata,
memcpy(dest + _addrOffset, &srcObject, sizeof(objc_object*));
addrOffset = _addrOffset + sizeof(objc_object*);

if (!platformSupportsTaggedPointers()) {
destObject &= ~_swift_abi_SwiftSpareBitsMask;
srcObject &= ~_swift_abi_SwiftSpareBitsMask;
}

objc_release((objc_object *)destObject);
objc_retain((objc_object *)srcObject);
#else
Expand Down
6 changes: 6 additions & 0 deletions test/Interpreter/Inputs/layout_string_witnesses_types.swift
Original file line number Diff line number Diff line change
Expand Up @@ -667,6 +667,12 @@ public enum ErrorWrapper {
case y(Error)
}

public enum MultiPayloadAnyObject {
case x(AnyObject)
case y(AnyObject)
case z(AnyObject)
}

@inline(never)
public func consume<T>(_ x: T.Type) {
withExtendedLifetime(x) {}
Expand Down
57 changes: 57 additions & 0 deletions test/Interpreter/layout_string_witnesses_objc.swift
Original file line number Diff line number Diff line change
Expand Up @@ -133,3 +133,60 @@ func testMultiPayloadNativeSwiftObjC() {
}

testMultiPayloadNativeSwiftObjC()

public enum MultiPayloadBlock {
#if _pointerBitWidth(_32)
public typealias PaddingPayload = (Int16, Int8, Bool)
#else
public typealias PaddingPayload = (Int32, Int16, Int8, Bool)
#endif

case x(PaddingPayload)
case y(@convention(block) () -> Void)
}

func testMultiPayloadBlock() {
let ptr = UnsafeMutablePointer<MultiPayloadBlock>.allocate(capacity: 1)

// initWithCopy
do {
let instance = SimpleClass(x: 0)
let x = MultiPayloadBlock.y({ print(instance) })
testInit(ptr, to: x)
}

// assignWithTake
do {
let instance = SimpleClass(x: 1)
let y = MultiPayloadBlock.y({ print(instance) })

// CHECK-NEXT: Before deinit
print("Before deinit")

// CHECK-NEXT: SimpleClass deinitialized!
testAssign(ptr, from: y)
}

// assignWithCopy
do {
let instance = SimpleClass(x: 2)
var z = MultiPayloadBlock.y({ print(instance) })

// CHECK-NEXT: Before deinit
print("Before deinit")

// CHECK-NEXT: SimpleClass deinitialized!
testAssignCopy(ptr, from: &z)
}

// CHECK-NEXT: Before deinit
print("Before deinit")

// destroy
// CHECK-NEXT: SimpleClass deinitialized!
testDestroy(ptr)

ptr.deallocate()
}

testMultiPayloadBlock()
43 changes: 43 additions & 0 deletions test/Interpreter/layout_string_witnesses_static.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1363,6 +1363,49 @@ func testMultiPayloadOneExtraTagValue() {

testMultiPayloadOneExtraTagValue()

func testMultiPayloadAnyObject() {
let ptr = UnsafeMutablePointer<MultiPayloadAnyObject>.allocate(capacity: 1)

// initWithCopy
do {
let x = MultiPayloadAnyObject.y(SimpleClass(x: 0))
testInit(ptr, to: x)
}

// assignWithTake
do {
let y = MultiPayloadAnyObject.z(SimpleClass(x: 1))

// CHECK-NEXT: Before deinit
print("Before deinit")

// CHECK-NEXT: SimpleClass deinitialized!
testAssign(ptr, from: y)
}

// assignWithCopy
do {
var z = MultiPayloadAnyObject.z(SimpleClass(x: 2))

// CHECK-NEXT: Before deinit
print("Before deinit")

// CHECK-NEXT: SimpleClass deinitialized!
testAssignCopy(ptr, from: &z)
}

// CHECK-NEXT: Before deinit
print("Before deinit")

// destroy
// CHECK-NEXT: SimpleClass deinitialized!
testDestroy(ptr)

ptr.deallocate()
}

testMultiPayloadAnyObject()

#if os(macOS)
func testObjc() {
let ptr = UnsafeMutablePointer<ObjcWrapper>.allocate(capacity: 1)
Expand Down