Skip to content

Cater for new Swift reference count representation #1525

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
Apr 20, 2018
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
3 changes: 1 addition & 2 deletions CoreFoundation/Base.subproj/CFBase.h
Original file line number Diff line number Diff line change
Expand Up @@ -683,8 +683,7 @@ CFTypeRef CFMakeCollectable(CFTypeRef cf) CF_AUTOMATED_REFCOUNT_UNAVAILABLE;
#if DEPLOYMENT_RUNTIME_SWIFT

#define _CF_SWIFT_RC_PINNED_FLAG 0x1
#define _CF_SWIFT_RC_FLAGS_COUNT 2
#define _CF_CONSTANT_OBJECT_STRONG_RC ((1 << _CF_SWIFT_RC_FLAGS_COUNT) | _CF_SWIFT_RC_PINNED_FLAG)
#define _CF_CONSTANT_OBJECT_STRONG_RC (_CF_SWIFT_RC_PINNED_FLAG)

#endif

Expand Down
4 changes: 2 additions & 2 deletions CoreFoundation/Base.subproj/CFInternal.h
Original file line number Diff line number Diff line change
Expand Up @@ -412,11 +412,11 @@ CF_EXPORT void *__CFConstantStringClassReference[];
#endif

#define CONST_STRING_DECL(S, V) \
const struct __CFConstStr __##S CONST_STRING_SECTION = {{(uintptr_t)&__CFConstantStringClassReference, _CF_CONSTANT_OBJECT_STRONG_RC, 0, 0x000007c8U}, (uint8_t *)(V), sizeof(V) - 1}; \
const struct __CFConstStr __##S CONST_STRING_SECTION = {{(uintptr_t)&__CFConstantStringClassReference, _CF_CONSTANT_OBJECT_STRONG_RC, 0x000007c8U}, (uint8_t *)(V), sizeof(V) - 1}; \

Choose a reason for hiding this comment

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

Do you need to change the definition of _CF_CONSTANT_OBJECT_STRONG_RC? My guess is that the bits generated here will be wrong if you do not.

Copy link
Contributor Author

@johnno1962 johnno1962 Apr 18, 2018

Choose a reason for hiding this comment

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

This will work on little-endian only perhaps?

#if DEPLOYMENT_RUNTIME_SWIFT

#define _CF_SWIFT_RC_PINNED_FLAG 0x1
#define _CF_SWIFT_RC_FLAGS_COUNT 2
#define _CF_CONSTANT_OBJECT_STRONG_RC ((1 << _CF_SWIFT_RC_FLAGS_COUNT) | _CF_SWIFT_RC_PINNED_FLAG)

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the point of splitting it into strong/weak here was so we could set that pinned flag. There may now be a better way.

Copy link
Contributor Author

@johnno1962 johnno1962 Apr 18, 2018

Choose a reason for hiding this comment

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

The Android tests run through ok. Are you saying separate them again so big-endian 64 bits works?

Choose a reason for hiding this comment

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

I don't think you were setting the pinned flag correctly before. The pinned flag is the LSB of that uintptr_t on all platforms.

I don't think you are setting the refcount correctly now. The strong refcount is stored differently on 32-bit versus 64-bit, but on all platforms it is biased: a strong refcount of 1 is represented as bit value 0.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#define _CF_CONSTANT_OBJECT_STRONG_RC (_CF_SWIFT_RC_PINNED_FLAG) then?

const CFStringRef S = (CFStringRef)&__##S;

#define PE_CONST_STRING_DECL(S, V) \
const static struct __CFConstStr __##S CONST_STRING_SECTION = {{(uintptr_t)&__CFConstantStringClassReference, _CF_CONSTANT_OBJECT_STRONG_RC, 0, 0x000007c8U}, (uint8_t *)(V), sizeof(V) - 1}; \
const static struct __CFConstStr __##S CONST_STRING_SECTION = {{(uintptr_t)&__CFConstantStringClassReference, _CF_CONSTANT_OBJECT_STRONG_RC, 0x000007c8U}, (uint8_t *)(V), sizeof(V) - 1}; \
CF_PRIVATE const CFStringRef S = (CFStringRef)&__##S;


Expand Down
2 changes: 1 addition & 1 deletion CoreFoundation/Base.subproj/CFRuntime.c
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,7 @@ CFTypeRef _CFRuntimeCreateInstance(CFAllocatorRef allocator, CFTypeID typeID, CF
CFRuntimeBase *memory = (CFRuntimeBase *)swift_allocObject(isa, size, align - 1);

// Zero the rest of the memory, starting at cfinfo
memset(&memory->_cfinfoa, 0, size - (sizeof(memory->_cfisa) + sizeof(memory->_swift_strong_rc) + sizeof(memory->_swift_weak_rc)));
memset(&memory->_cfinfoa, 0, size - (sizeof(memory->_cfisa) + sizeof(memory->_swift_rc)));

// Set up the cfinfo struct
uint32_t *cfinfop = (uint32_t *)&(memory->_cfinfoa);
Expand Down
5 changes: 2 additions & 3 deletions CoreFoundation/Base.subproj/CFRuntime.h
Original file line number Diff line number Diff line change
Expand Up @@ -193,13 +193,12 @@ CF_EXPORT void _CFRuntimeUnregisterClassWithTypeID(CFTypeID typeID);
typedef struct __CFRuntimeBase {
// This matches the isa and retain count storage in Swift
uintptr_t _cfisa;
uint32_t _swift_strong_rc;
uint32_t _swift_weak_rc;
uintptr_t _swift_rc;
// This is for CF's use, and must match _NSCFType layout
_Atomic(uint64_t) _cfinfoa;
} CFRuntimeBase;

#define INIT_CFRUNTIME_BASE(...) {0, _CF_CONSTANT_OBJECT_STRONG_RC, 0, 0x0000000000000080ULL}
#define INIT_CFRUNTIME_BASE(...) {0, _CF_CONSTANT_OBJECT_STRONG_RC, 0x0000000000000080ULL}

#else

Expand Down
2 changes: 1 addition & 1 deletion CoreFoundation/String.subproj/CFString.c
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ CF_INLINE void __CFStrClearHasLengthAndNullBytes(CFMutableStringRef str) {

CF_INLINE Boolean __CFStrIsConstant(CFStringRef str) {
#if DEPLOYMENT_RUNTIME_SWIFT
return str->base._swift_strong_rc & _CF_SWIFT_RC_PINNED_FLAG;
return str->base._swift_rc & _CF_SWIFT_RC_PINNED_FLAG;
#else
return __CFRuntimeIsConstant(str);
#endif
Expand Down
7 changes: 3 additions & 4 deletions CoreFoundation/String.subproj/CFString.h
Original file line number Diff line number Diff line change
Expand Up @@ -155,8 +155,7 @@ since it is the default choice with Mac OS X developer tools.
struct __CFConstStr {
struct {
uintptr_t _cfisa;
uint32_t _swift_strong_rc;
uint32_t _swift_weak_rc;
uintptr_t _swift_rc;
uint64_t _cfinfoa;
} _base;
uint8_t *_ptr;
Expand All @@ -175,12 +174,12 @@ struct __CFConstStr {

#if __BIG_ENDIAN__
#define CFSTR(cStr) ({ \
static struct __CFConstStr str CONST_STRING_LITERAL_SECTION = {{(uintptr_t)&__CFConstantStringClassReference, _CF_CONSTANT_OBJECT_STRONG_RC, 0, 0x00000000C8070000}, (uint8_t *)(cStr), sizeof(cStr) - 1}; \
static struct __CFConstStr str CONST_STRING_LITERAL_SECTION = {{(uintptr_t)&__CFConstantStringClassReference, _CF_CONSTANT_OBJECT_STRONG_RC, 0x00000000C8070000}, (uint8_t *)(cStr), sizeof(cStr) - 1}; \
(CFStringRef)&str; \
})
#else
#define CFSTR(cStr) ({ \
static struct __CFConstStr str CONST_STRING_LITERAL_SECTION = {{(uintptr_t)&__CFConstantStringClassReference, _CF_CONSTANT_OBJECT_STRONG_RC, 0, 0x07C8}, (uint8_t *)(cStr), sizeof(cStr) - 1}; \
static struct __CFConstStr str CONST_STRING_LITERAL_SECTION = {{(uintptr_t)&__CFConstantStringClassReference, _CF_CONSTANT_OBJECT_STRONG_RC, 0x07C8}, (uint8_t *)(cStr), sizeof(cStr) - 1}; \
(CFStringRef)&str; \
})
#endif
Expand Down
9 changes: 4 additions & 5 deletions Foundation/NSCFString.swift
Original file line number Diff line number Diff line change
Expand Up @@ -61,18 +61,17 @@ internal final class _NSCFConstantString : _NSCFString {
internal var _ptr : UnsafePointer<UInt8> {
// FIXME: Split expression as a work-around for slow type
// checking (tracked by SR-5322).
let offTemp1 = MemoryLayout<OpaquePointer>.size + MemoryLayout<Int32>.size
let offTemp2 = MemoryLayout<Int32>.size + MemoryLayout<_CFInfo>.size
let offset = offTemp1 + offTemp2
let offTemp1 = MemoryLayout<OpaquePointer>.size + MemoryLayout<uintptr_t>.size
let offset = offTemp1 + MemoryLayout<_CFInfo>.size
let ptr = Unmanaged.passUnretained(self).toOpaque()
return ptr.load(fromByteOffset: offset, as: UnsafePointer<UInt8>.self)
}

private var _lenOffset : Int {
// FIXME: Split expression as a work-around for slow type
// checking (tracked by SR-5322).
let offTemp1 = MemoryLayout<OpaquePointer>.size + MemoryLayout<Int32>.size
let offTemp2 = MemoryLayout<Int32>.size + MemoryLayout<_CFInfo>.size
let offTemp1 = MemoryLayout<OpaquePointer>.size + MemoryLayout<uintptr_t>.size
let offTemp2 = MemoryLayout<_CFInfo>.size
return offTemp1 + offTemp2 + MemoryLayout<UnsafePointer<UInt8>>.size
}

Expand Down