Skip to content

Fix NSNumber.objCType crashes, make it returns same values with darwin platform version #1025

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

Closed
wants to merge 10 commits into from
Closed

Fix NSNumber.objCType crashes, make it returns same values with darwin platform version #1025

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
550afa7
Add `TestNSNumber.test_objCType()` reproducing SR-4907
norio-nomura Jun 1, 2017
075f832
Fix crash on accessing `objCType` property of `NSNumber`
norio-nomura Jun 2, 2017
ee48897
Change `NSNumber.objCType` to returning same values with darwin
norio-nomura Jun 2, 2017
07d2595
Change `NSNumber.init(value: Bool)` to returning `__NSCFBoolean` by u…
norio-nomura Jun 2, 2017
7f49431
Change `JSONSerialization` to use `_cfTypeID` for checking whether `N…
norio-nomura Jun 2, 2017
ea73230
Change `NSNumber.compare(_:)` to use `objCType`
norio-nomura Jun 3, 2017
68befb3
Add missing entry to `TestNumber.allTests`
norio-nomura Jun 3, 2017
7e94118
Fix `NSKeyedUnarchiver.decodeBool(forKey:)` and add test to `TestNSPr…
norio-nomura Jun 3, 2017
7c2b737
Reflect differences of `NSNumber.objCType`'s result by value and arch…
norio-nomura Jun 4, 2017
2a5a611
Remove arch(powerpc)
norio-nomura Jun 5, 2017
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
2 changes: 1 addition & 1 deletion Foundation/NSCFBoolean.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ internal class __NSCFBoolean : NSNumber {
override var objCType: UnsafePointer<Int8> {
// This must never be fixed to be "B", although that would
// cause correct old-style archiving when this is unarchived.
return UnsafePointer<Int8>(bitPattern: UInt(_NSSimpleObjCType.Char.rawValue.value))!
return String(_NSSimpleObjCType.Char.rawValue)._bridgeToObjectiveC().utf8String!
Copy link
Contributor

Choose a reason for hiding this comment

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

this is going to cause leaks, can we have a static allocation somewhere stashed with the type to cstrings?

}

internal override func _cfNumberType() -> CFNumberType {
Expand Down
4 changes: 2 additions & 2 deletions Foundation/NSJSONSerialization.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -458,8 +458,8 @@ private struct JSONWriter {
throw NSError(domain: NSCocoaErrorDomain, code: CocoaError.propertyListReadCorrupt.rawValue, userInfo: ["NSDebugDescription" : "Number cannot be infinity or NaN"])
}

switch num._objCType {
case .Bool:
switch num._cfTypeID {
case CFBooleanGetTypeID():
serializeBool(num.boolValue)
default:
writer(_serializationString(for: num))
Expand Down
4 changes: 2 additions & 2 deletions Foundation/NSKeyedUnarchiver.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -683,10 +683,10 @@ open class NSKeyedUnarchiver : NSCoder {
}

open override func decodeBool(forKey key: String) -> Bool {
guard let result : NSNumber = _decodeValue(forKey: key) else {
guard let result : Bool = _decodeValue(forKey: key) else {
return false
}
return result.boolValue
return result
}

open override func decodeInt32(forKey key: String) -> Int32 {
Expand Down
57 changes: 42 additions & 15 deletions Foundation/NSNumber.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ open class NSNumber : NSValue {
}

open override var objCType: UnsafePointer<Int8> {
return UnsafePointer<Int8>(bitPattern: UInt(_objCType.rawValue.value))!
return String(_objCType.rawValue)._bridgeToObjectiveC().utf8String!
}

deinit {
Expand All @@ -244,7 +244,7 @@ open class NSNumber : NSValue {
}

public init(value: UInt8) {
_objCType = .UChar
_objCType = .Short
super.init()
_CFNumberInitUInt8(_cfObject, value)
}
Expand All @@ -256,31 +256,40 @@ open class NSNumber : NSValue {
}

public init(value: UInt16) {
_objCType = .UShort
_objCType = .Int
super.init()
_CFNumberInitUInt16(_cfObject, value)
}

public init(value: Int32) {
_objCType = .Long
_objCType = .Int
super.init()
_CFNumberInitInt32(_cfObject, value)
}

public init(value: UInt32) {
_objCType = .ULong
_objCType = .LongLong
super.init()
_CFNumberInitUInt32(_cfObject, value)
}

public init(value: Int) {
_objCType = .Int
#if arch(x86_64) || arch(arm64) || arch(s390x) || arch(powerpc64) || arch(powerpc64le)
_objCType = .LongLong
#elseif arch(i386) || arch(arm)
_objCType = .Int
#endif
super.init()
_CFNumberInitInt(_cfObject, value)
}

public init(value: UInt) {
_objCType = .UInt
#if arch(x86_64) || arch(arm64) || arch(s390x) || arch(powerpc64) || arch(powerpc64le)
// When value is lower equal to `Int.max`, it uses '.LongLong' even if using `UInt`
_objCType = value <= UInt(Int.max) ? .LongLong : .ULongLong
#elseif arch(i386) || arch(arm)
_objCType = .LongLong
#endif
super.init()
_CFNumberInitUInt(_cfObject, value)
}
Expand All @@ -292,7 +301,8 @@ open class NSNumber : NSValue {
}

public init(value: UInt64) {
_objCType = .ULongLong
// When value is lower equal to `Int64.max`, it uses '.LongLong' even if using `UInt64`
_objCType = value <= UInt64(Int64.max) ? .LongLong : .ULongLong
super.init()
_CFNumberInitUInt64(_cfObject, value)
}
Expand All @@ -309,12 +319,6 @@ open class NSNumber : NSValue {
_CFNumberInitDouble(_cfObject, value)
}

public init(value: Bool) {
_objCType = .Bool
super.init()
_CFNumberInitBool(_cfObject, value)
}

override internal init() {
_objCType = .Undef
super.init()
Expand Down Expand Up @@ -500,7 +504,24 @@ open class NSNumber : NSValue {
}

open func compare(_ otherNumber: NSNumber) -> ComparisonResult {
return ._fromCF(CFNumberCompare(_cfObject, otherNumber._cfObject, nil))
switch (objCType.pointee, otherNumber.objCType.pointee) {
case (0x66, _), (_, 0x66), (0x66, 0x66): fallthrough // 'f' float
case (0x64, _), (_, 0x64), (0x64, 0x64): // 'd' double
let (lhs, rhs) = (doubleValue, otherNumber.doubleValue)
if lhs < rhs { return .orderedAscending }
if lhs > rhs { return .orderedDescending }
return .orderedSame
case (0x51, _), (_, 0x51), (0x51, 0x51): // 'q' unsigned long long
let (lhs, rhs) = (uint64Value, otherNumber.uint64Value)
if lhs < rhs { return .orderedAscending }
if lhs > rhs { return .orderedDescending }
return .orderedSame
case (_, _):
let (lhs, rhs) = (int64Value, otherNumber.int64Value)
if lhs < rhs { return .orderedAscending }
if lhs > rhs { return .orderedDescending }
return .orderedSame
}
}

open func description(withLocale locale: Locale?) -> String {
Expand Down Expand Up @@ -613,6 +634,12 @@ open class NSNumber : NSValue {
open override var classForCoder: AnyClass { return NSNumber.self }
}

extension NSNumber {
public convenience init(value: Bool) {
self.init(factory: value._bridgeToObjectiveC)
}
}

extension CFNumber : _NSBridgeable {
typealias NSType = NSNumber
internal var _nsObject: NSType { return unsafeBitCast(self, to: NSType.self) }
Expand Down
11 changes: 11 additions & 0 deletions Foundation/NSValue.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,3 +154,14 @@ open class NSValue : NSObject, NSCopying, NSSecureCoding, NSCoding {
}
}

extension NSValue : _Factory {}

protocol _Factory {
Copy link
Contributor

Choose a reason for hiding this comment

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

just for maintainability; can we annotate this as internal?

init(factory: () -> Self)
}

extension _Factory {
init(factory: () -> Self) {
self = factory()
Copy link
Contributor

Choose a reason for hiding this comment

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

hmm this is rather strange, if this works perhaps we should investigate using this pattern further

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, just prototyped a test for this... wow! this will solve a ton of issues with swift-corelibs-foundation!

Copy link
Member

Choose a reason for hiding this comment

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

Just FYI https://bugs.swift.org/browse/SR-5255

}
}
42 changes: 42 additions & 0 deletions TestFoundation/TestNSNumber.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ class TestNSNumber : XCTestCase {
static var allTests: [(String, (TestNSNumber) -> () throws -> Void)] {
return [
("test_NumberWithBool", test_NumberWithBool ),
("test_CFBoolean", test_CFBoolean ),
("test_numberWithChar", test_numberWithChar ),
("test_numberWithUnsignedChar", test_numberWithUnsignedChar ),
("test_numberWithShort", test_numberWithShort ),
Expand All @@ -34,6 +35,7 @@ class TestNSNumber : XCTestCase {
("test_compareNumberWithDouble", test_compareNumberWithDouble ),
("test_description", test_description ),
("test_descriptionWithLocale", test_descriptionWithLocale ),
("test_objCType", test_objCType ),
]
}

Expand Down Expand Up @@ -438,4 +440,44 @@ class TestNSNumber : XCTestCase {
XCTAssertEqual(receivedDesc, expectedDesc, "expected \(expectedDesc) but received \(receivedDesc)")
}
}

func test_objCType() {
let objCType: (NSNumber) -> UnicodeScalar = { number in
return UnicodeScalar(UInt8(number.objCType.pointee))
}

XCTAssertEqual("c" /* 0x63 */, objCType(NSNumber(value: true)))

XCTAssertEqual("c" /* 0x63 */, objCType(NSNumber(value: Int8.max)))
XCTAssertEqual("s" /* 0x73 */, objCType(NSNumber(value: UInt8(Int8.max))))
XCTAssertEqual("s" /* 0x73 */, objCType(NSNumber(value: UInt8(Int8.max) + 1)))

XCTAssertEqual("s" /* 0x73 */, objCType(NSNumber(value: Int16.max)))
XCTAssertEqual("i" /* 0x69 */, objCType(NSNumber(value: UInt16(Int16.max))))
XCTAssertEqual("i" /* 0x69 */, objCType(NSNumber(value: UInt16(Int16.max) + 1)))

XCTAssertEqual("i" /* 0x69 */, objCType(NSNumber(value: Int32.max)))
XCTAssertEqual("q" /* 0x71 */, objCType(NSNumber(value: UInt32(Int32.max))))
XCTAssertEqual("q" /* 0x71 */, objCType(NSNumber(value: UInt32(Int32.max) + 1)))

XCTAssertEqual("q" /* 0x71 */, objCType(NSNumber(value: Int64.max)))
// When value is lower equal to `Int64.max`, it returns 'q' even if using `UInt64`
XCTAssertEqual("q" /* 0x71 */, objCType(NSNumber(value: UInt64(Int64.max))))
XCTAssertEqual("Q" /* 0x51 */, objCType(NSNumber(value: UInt64(Int64.max) + 1)))

// Depends on architectures
#if arch(x86_64) || arch(arm64) || arch(s390x) || arch(powerpc64) || arch(powerpc64le)
XCTAssertEqual("q" /* 0x71 */, objCType(NSNumber(value: Int.max)))
// When value is lower equal to `Int.max`, it returns 'q' even if using `UInt`
XCTAssertEqual("q" /* 0x71 */, objCType(NSNumber(value: UInt(Int.max))))
XCTAssertEqual("Q" /* 0x51 */, objCType(NSNumber(value: UInt(Int.max) + 1)))
#elseif arch(i386) || arch(arm)
XCTAssertEqual("i" /* 0x71 */, objCType(NSNumber(value: Int.max)))
XCTAssertEqual("q" /* 0x71 */, objCType(NSNumber(value: UInt(Int.max))))
XCTAssertEqual("q" /* 0x51 */, objCType(NSNumber(value: UInt(Int.max) + 1)))
#endif

XCTAssertEqual("f" /* 0x66 */, objCType(NSNumber(value: Float.greatestFiniteMagnitude)))
XCTAssertEqual("d" /* 0x64 */, objCType(NSNumber(value: Double.greatestFiniteMagnitude)))
}
}
3 changes: 3 additions & 0 deletions TestFoundation/TestNSPredicate.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,9 @@ class TestNSPredicate: XCTestCase {
let predicateA = NSPredicate(value: true)
let predicateB = NSKeyedUnarchiver.unarchiveObject(with: NSKeyedArchiver.archivedData(withRootObject: predicateA)) as! NSPredicate
XCTAssertEqual(predicateA, predicateB, "Archived then unarchived uuid must be equal.")
let predicateC = NSPredicate(value: false)
let predicateD = NSKeyedUnarchiver.unarchiveObject(with: NSKeyedArchiver.archivedData(withRootObject: predicateC)) as! NSPredicate
XCTAssertEqual(predicateC, predicateD, "Archived then unarchived uuid must be equal.")
}

func test_copy() {
Expand Down