[Foundation] Modernize hashing in Foundation's Swift-only types #23832

lorentey · 2019-04-06T00:25:25Z

This PR upgrades the Foundation overlay to use the new hashing API introduced in SE-0206. This gets rid of the deprecation warnings for hashValue implementations and brings the overlay in sync with current Swift best practices.

Background

SE-0206 radically changed how hashing works in Swift 4.2+. It is now the stdlib's responsibility to choose a hash function; types implementing Hashable only need to worry about selecting what they want to feed to the hasher. To make this possible, Hashable now requires hashing to be implemented through the hash(into:) method.

The old hashValue property is deprecated as a Hashable requirement, and it's only kept to support source-level compatibility with existing code. Set and Dictionary have switched over to using hash(into:) as their entry point for hashing; they do not directly call hashValue at all.

Beyond simplifying hashing, the intent of SE-0206 is to enable Swift to provide certain guarantees about its quality. In particular: as long as hash(into:) feeds enough data the hasher to unambiguously decide equality, Swift attempts to guarantee that collision attacks won't be possible. For this to work, it is critically important for Hashable implementations to include everything that Equatable.== looks at; and this is especially the case for the basic boundary types that come built-in with Swift, like Data.

The exact hash algorithm used is a private implementation detail of the stdlib; the Hasher API was carefully designed to leak as little information about it as possible, enabling future versions of the stdlib to switch to other algorithms without worrying about ABI compatibility issues.

While random seeding makes the hash algorithm opaque, it is technically possible for Swift code to rely on the particular hash encoding of a stdlib- (or SDK-)provided type. This is highly unlikely to happen in practice, but the longer we wait to fix hashing issues, the more likely it will be that someone starts relying on hashing particulars. For example, I could imagine this to happen through a well-meaning desire to work around a broken hash implementation. It is therefore important to do this sooner rather than later.

Notes

I'll highlight points specific to a particular changes as self-review comments. Here is a list of general remarks:

It is neither required nor desirable for Swift's hashValue to return reproducible values.
We don't need to ever match hash values between Swift types and the Cocoa classes that they bridge with. The act of bridging involves a full rehashing of all Set/Dictionary values.
The only hard requirement is that hashing needs to be consistent with the equivalence classes implemented by == within the same type.
Hashing is meaningless without equality. Equatable only defines equality for values of a single type. Therefore, Hashable makes no requirements or expectations about the hash values produced by values of different types.
- It is perfectly fine for hash values produced by different types to not match. (0 as Int8).hashValue and (0 as Int16).hashValue will (typically) not be equal in Swift.
- It is also perfectly fine for values of two distinct types to produce matching hash values. Measurement<UnitLength>.hash(into:) and Measurement<UnitDuration>.hash(into:) are allowed to use the same hash encoding, since there is no way they can ever produce large-scale collisions within the same hash table.
While this is not a hard requirement for user code, for boundary types provided in the stdlib/SDK, we require that hashing isn't just consistent with equality, but that it's equivalent to it. The Swift test suite has checks to actively enforce this -- this is possible through repeatedly salting the hash function.

"Optimizing" hashing by omitting some of the data compared by == is generally a mistake in Swift, because it completely breaks all guarantees about the strength of hashing, and opens the door to (accidental or deliberate) collision attacks.

It's perfectly acceptable to hash a gigabyte of data if someone inserts some large value (such as a big collection) as a key in a hash table. Multi-megabyte String keys are easy to protect against; hidden hashing weaknesses aren't.

It would be nice if we could somehow improve hashing in Foundation's Objective-C classes, but this is outside the scope of this PR (or indeed, this repository). However, we should at least make sure Swift programmers can rely on the quality of hashes produced by native Swift value types.
Modernize hashing in Foundation's Swift-only types swift-corelibs-foundation#2118 is the corresponding PR for swift-corelibs-foundation. These PRs should land at the same time.
Replacing hashValue implementations on types with hash(into:) has no effect on the symbols exposed as ABI -- the compiler automatically synthesizes hashValue from hash(into:), and vice versa. (This is not the case in protocol extensions, but we've fixed those in the 4.2/5.0 timeframe.) In the Swift 5.0 ABI, all of these types include definitions for both hashValue and `hash(into:) so we don't need to worry about availability issues.

The sole exception is URL, which implements hashValue but isn't Hashable. Unfortunately we can't add the conformance in 5.1, but we should add a (versioned) hash(into:) definition to discourage people from trying to implement it on their own. (Edit: URL is not a special case; it's Hashable like the rest. I got mislead by an unrelated error.)

rdar://problem/43394032

…*all* components The new definition diverges from the existing hashValue: it feeds all path components to the hasher, rather than just a limited subset. Hashing all the bits that are compared in == is necessary to ensure proper operation of the hash table.

This replaces the compiler-generated variant (based on hashValue) with a better implementation.

Note: URL does not implement Hashable.

…bset

lorentey · 2019-04-06T00:33:28Z

stdlib/public/Darwin/Foundation/Date.swift

-            return Int(bitPattern: __CFHashDouble(_time))
-        } else { // 10.11 and previous behavior fallback; this must allocate a date to reference the hash value and then throw away the reference
-            return NSDate(timeIntervalSinceReferenceDate: _time).hash
-        }


Note: There is no need to emulate NSDate's hashing behavior, or the behavior of any previous version of Date.hashValue. Swift is very explicit that the values returned by hashValue aren't to be treated as stable, and Swift trains programmers to expect randomized hashes.

Granted, in this case (like with many other hashValues removed by this PR), 5.0 did not vend properly seeded hashes, so in theory someone could be relying on the exact values returned. I believe this is still a low-risk change; and since this is not inlinable code, we have the option to re-add specific hashValue implementations with an SDK check if it turns some of them have ABI impact.

lorentey · 2019-04-06T00:37:12Z

stdlib/public/Darwin/Foundation/DateInterval.swift

@@ -155,15 +155,11 @@ public struct DateInterval : ReferenceConvertible, Comparable, Hashable, Codable
        return false
    }

-    public var hashValue: Int {
-        var buf: (UInt, UInt) = (UInt(start.timeIntervalSinceReferenceDate), UInt(end.timeIntervalSinceReferenceDate))


Using the end date here was problematic, since == compares start and duration, not start and end.

end does not always include all the information that's present in duration, so doing this results in weaker hashes.

The new code is careful to match the implementation of ==.

lorentey · 2019-04-06T00:44:23Z

stdlib/public/Darwin/Foundation/Data.swift

-            let range = startIndex ..< Swift.min(startIndex + 80, endIndex)
-            storage.withUnsafeBytes(in: range) {
+            // To ensure strong hashing, all bytes must be fed into the hasher.
+            withUnsafeBytes {


This is one of the cases where the old implementation deliberately neglected to feed all relevant information into the hasher. This makes it trivial to induce an arbitrary number of collisions, despite all the effort that went into implementing a high-quality universal hash function.

This is particularly a bad idea for Data, which is the standard type for safely exchanging byte buffers across module boundaries. It is reasonable to expect Data to implement the same high quality hashing as Array does; silently doing otherwise is not a good idea.

We previously discussed this issue in #21754.

I'm still not convinced about this without Hasher having some guarantees about not being O(n), but I think we should try to set up a meeting to discuss about strategy here.

OK! I've spun off this change into #23876. Data is already implementing the right APIs, so this is just a matter of changing the implementation.

lorentey · 2019-04-06T00:48:17Z

stdlib/public/Darwin/Foundation/Measurement.swift

@@ -36,8 +36,19 @@ public struct Measurement<UnitType : Unit> : ReferenceConvertible, Comparable, E
        self.unit = unit
    }

-    public var hashValue: Int {
-        return Int(bitPattern: __CFHashDouble(value))


The original implementation of Measurement.hashValue is not a valid hash, because it is in conflict with ==.

(A version of this change was originally submitted as #20879.)

stdlib/public/Darwin/Foundation/URL.swift

lorentey · 2019-04-06T00:56:03Z

test/stdlib/NSStringAPI.swift

+    .ascii,
+    .utf8,
+  ]
+  checkHashable(instances, equalityOracle: { $0 == $1 })


checkHashable verifies that instances provided don't violate Equality and Hashable requirements, including the soft requirement for strong hashing. (I.e., that non-equal values must differ in what they feed into the hasher.)

The equalityOracle closure provides the expected equivalence relation over the indices in the collection passed in as the first argument.

lorentey · 2019-04-06T00:57:47Z

test/stdlib/TestAffineTransform.swift

-        expectEqual(AffineTransform.self, type(of: anyHashables[2].base))
-        expectNotEqual(anyHashables[0], anyHashables[1])
-        expectEqual(anyHashables[1], anyHashables[2])
+        checkHashableGroups(values)


checkHashableGroups is like checkHashable, but it represents the equivalence relation as nested arrays: each element in values is supposed to contain values of a distinct equivalence class.

I.e., a and bridged(a) must compare equal, but a must not compare equal to any item outside its own array.

lorentey · 2019-04-15T20:04:36Z

I submitted a PR for swift-corelibs-foundation with matching changes: swiftlang/swift-corelibs-foundation#2118

lorentey · 2019-04-17T03:27:10Z

@swift-ci please test

lorentey · 2019-04-23T21:29:55Z

@swift-ci please test macOS platform

swift-ci · 2019-04-23T22:34:45Z

Build failed
Swift Test OS X Platform
Git Sha - 13bc567

lorentey · 2019-04-24T03:20:15Z

@swift-ci please test macOS platform

lorentey · 2019-04-24T03:20:31Z

@swift-ci please test linux platform

lorentey · 2019-04-24T06:09:20Z

@swift-ci please test

lorentey · 2019-04-24T22:20:31Z

02:13:44 ******************** TEST 'Swift(iphonesimulator-i386) :: stdlib/TestPersonNameComponents.swift' FAILED ********************
...
02:13:44 [ RUN      ] TestPersonNameComponents.test_Hashing
02:13:44 stdout>>> check failed at /Users/buildnode/jenkins/workspace/swift-PR-osx/branch-master/swift/test/stdlib/TestPersonNameComponents.swift, line 54
02:13:44 stdout>>> stacktrace:
02:13:44 stdout>>>   #0: /Users/buildnode/jenkins/workspace/swift-PR-osx/branch-master/swift/stdlib/private/StdlibUnittest/StdlibUnittest.swift:2530
02:13:44 stdout>>> expected: 230665695 (of type Swift.Int)
02:13:44 stdout>>> actual: -422011617 (of type Swift.Int)
02:13:44 stdout>>> hash(into:) expected to match, found to differ
02:13:44 stdout>>> lhs (at index 0): givenName: Kevin familyName: Frank 
02:13:44 stdout>>> rhs (at index 1): givenName: Kevin familyName: Frank

Hm; NSPersonNameComponents.hash seems not to work correctly on iphonesimulator-i386.

lorentey · 2019-04-25T07:42:01Z

@swift-ci please test

airspeedswift · 2019-04-26T21:32:52Z

@parkera @millenomi @itaiferber are we good to go ahead with this? (with the Data part subsetted out for further discussion)

parkera · 2019-04-26T21:48:41Z

Yup

itaiferber

Latest changes LGTM

lorentey added 4 commits April 4, 2019 13:56

[test] StdlibUnittest: Cosmetic changes to checkEquatable/checkHashable

668c1f0

[StdlibUnittest] checkHashable: Add opt-in support for incomplete hashes

68e6449

[Foundation] NSRange: Add an explicit definition for hash(into:)

569e380

This replaces the compiler-generated variant (based on hashValue) with a better implementation.

lorentey mentioned this pull request Apr 6, 2019

[Foundation] Measurement: Add a correct hash(into:) implementation #20879

Closed

lorentey added 21 commits April 5, 2019 18:07

[Foundation] Calendar: Modernize hashing

73ad830

[Foundation] CharacterSet: Modernize hashing

81567ed

[Foundation] String.Encoding: Modernize hashing

0867f7e

[Foundation] AffineTransform: modernize hashing

8b7adb3

[Foundation] Date: Modernize hashing

741122a

[Foundation] DateComponents: Modernize hashing

73be6b8

[Foundation] DateInterval: Modernize hashing

c1049e7

[Foundation] Decimal: Modernize hashing

a000422

[Foundation] Locale: Modernize hashing

9c46858

[Foundation] IndexSet: Modernize hashing

2d5520e

[Foundation] IndexPath: Modernize hashing

5e451e6

[Foundation] Notification: Modernize hashing

4bdc458

[Foundation] NSRange: Modernize hashing

afa2539

[Foundation] TimeZone: Modernize hashing

b41eb97

[Foundation] URL: Add hash(into:) implementation

10cb05a

Note: URL does not implement Hashable.

[Foundation] URLComponents: Modernize hashing

590f407

[Foundation] URLRequest: Modernize hashing

23cc719

[Foundation] UUID: Modernize hashing

a23c3f3

[Foundation] PersonNameComponents: Modernize hashing

28e4688

[Foundation] Measurement: Fix hashing

6f40a4a

[Foundation] Data: Hash the entire contents, not just an arbitrary su…

b711ed9

…bset

lorentey force-pushed the foundation-hashing branch from 5cec1e5 to b711ed9 Compare April 6, 2019 01:07

lorentey commented Apr 6, 2019

View reviewed changes

lorentey requested review from phausler and itaiferber April 6, 2019 01:28

lorentey mentioned this pull request Apr 15, 2019

Modernize hashing in Foundation's Swift-only types swiftlang/swift-corelibs-foundation#2118

Merged

lorentey marked this pull request as ready for review April 17, 2019 03:26

This comment has been minimized.

Sign in to view

lorentey mentioned this pull request Apr 18, 2019

fix warnings #24127

Merged

lorentey mentioned this pull request Apr 23, 2019

[5.1][Foundation] Modernize hashing in Foundation's Swift-only types #24228

Merged

This comment has been minimized.

Sign in to view

[test] TestNotification: Fix checkHashable invocation

6647c50

lorentey force-pushed the foundation-hashing branch from 71833f4 to 6647c50 Compare April 24, 2019 06:07

This comment has been minimized.

Sign in to view

[test] TestPersonNameComponents: adjust OS check for hashing

530687f

This comment has been minimized.

Sign in to view

parkera approved these changes Apr 26, 2019

View reviewed changes

itaiferber approved these changes Apr 26, 2019

View reviewed changes

lorentey merged commit d9c166f into swiftlang:master Apr 26, 2019

lorentey deleted the foundation-hashing branch April 26, 2019 21:59

lorentey mentioned this pull request Apr 29, 2019

[5.1 04-24-2019][Foundation] Modernize hashing in Foundation's Swift-only types #24369

Merged

[Foundation] Modernize hashing in Foundation's Swift-only types #23832

[Foundation] Modernize hashing in Foundation's Swift-only types #23832

Uh oh!

Conversation

lorentey commented Apr 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Background

Notes

Uh oh!

lorentey Apr 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lorentey Apr 6, 2019

Choose a reason for hiding this comment

Uh oh!

lorentey Apr 6, 2019

Choose a reason for hiding this comment

Uh oh!

itaiferber Apr 8, 2019

Choose a reason for hiding this comment

Uh oh!

lorentey Apr 9, 2019

Choose a reason for hiding this comment

Uh oh!

lorentey Apr 6, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lorentey Apr 6, 2019

Choose a reason for hiding this comment

Uh oh!

lorentey Apr 6, 2019

Choose a reason for hiding this comment

Uh oh!

lorentey commented Apr 15, 2019

Uh oh!

lorentey commented Apr 17, 2019

Uh oh!

This comment has been minimized.

This comment has been minimized.

lorentey commented Apr 23, 2019

Uh oh!

swift-ci commented Apr 23, 2019

Uh oh!

lorentey commented Apr 24, 2019

Uh oh!

lorentey commented Apr 24, 2019

Uh oh!

This comment has been minimized.

This comment has been minimized.

lorentey commented Apr 24, 2019

Uh oh!

This comment has been minimized.

This comment has been minimized.

lorentey commented Apr 24, 2019

Uh oh!

lorentey commented Apr 25, 2019

Uh oh!

This comment has been minimized.

airspeedswift commented Apr 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

parkera commented Apr 26, 2019

Uh oh!

itaiferber left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

lorentey commented Apr 6, 2019 •

edited

Loading

lorentey Apr 6, 2019 •

edited

Loading

airspeedswift commented Apr 26, 2019 •

edited

Loading