Skip to content

[SR-5036]: Segfault using KeyPath with NSObject #10302

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

Jump to bottom
Merged
merged 2 commits into from
Jun 16, 2017
Merged

[SR-5036]: Segfault using KeyPath with NSObject #10302

merged 2 commits into from
Jun 16, 2017

Conversation

jckarter
Copy link
Contributor

We need to use the ivar offset variables in this case, since the Swift field offset vector doesn't pick up the adjusted offsets from the ObjC runtime. Fixes SR-5036 | rdar://problem/32488871.

@jckarter
Copy link
Contributor Author

@swift-ci Please test

@swift-ci
Copy link
Contributor

Build failed
Jenkins build - Swift Test Linux Platform
Git Commit - 9e378186c53d8002ad596d89ae294684d26edbb4
Test requested by - @jckarter

@swift-ci
Copy link
Contributor

Build failed
Jenkins build - Swift Test OS X Platform
Git Commit - 9e378186c53d8002ad596d89ae294684d26edbb4
Test requested by - @jckarter

@jckarter
Copy link
Contributor Author

@aciidb0mb3r This swiftpm failure ring a bell?

/home/buildnode/jenkins/workspace/swift-PR-Linux/branch-master/swiftpm/Tests/FunctionalTests/MiscellaneousTests.swift:360: error: MiscellaneousTestCase.testSecondBuildIsNullInModulemapGen : XCTAssertTrue failed - 
17:16:56 Test Case 'MiscellaneousTestCase.testSecondBuildIsNullInModulemapGen' failed (4.209 seconds)
17:16:56 Test Suite 'MiscellaneousTestCase' failed at 2017-06-15 19:15:58.032
17:16:56 	 Executed 1 test, with 1 failure (0 unexpected) in 4.209 (4.209) seconds
17:16:56 Test Suite 'Selected tests' failed at 2017-06-15 19:15:58.032
17:16:56 	 Executed 1 test, with 1 failure (0 unexpected) in 4.209 (4.209) seconds--- bootstrap: error: tests failed with exit status 1

@aciidgh
Copy link
Contributor

aciidgh commented Jun 16, 2017

@jckarter Nope, haven't seen this failure before.

@jckarter jckarter force-pushed the keypath-indirect-offset branch from 9e37818 to 66dbec0 Compare June 16, 2017 17:38
@jckarter
Copy link
Contributor Author

@swift-ci Please test

1 similar comment
@jckarter
Copy link
Contributor Author

@swift-ci Please test

@swift-ci
Copy link
Contributor

Build failed
Jenkins build - Swift Test OS X Platform
Git Commit - 66dbec060e9c38068949d5d57859c8c5d074a19b
Test requested by - @jckarter

jckarter added 2 commits June 16, 2017 14:42
@jckarter
IRGen: Begin NSObject subclass layout with an NSObject 'isa' header o…
3663c36 
…nly.

We would lay out all classes starting with a Swift-style two-word header, even classes that inherit NSObject and therefore don't have Swift refcounting. The ObjC runtime would slide our ivars down for us at realization time, but it's nice to avoid unnecessarily dirtying memory in the not-uncommon case of direct NSObject subclasses.
@jckarter
KeyPaths: Correctly instantiate offsets for final stored properties i…
fe88bd5 
…n NSObject subclasses.

We need to use the ivar offset variables in this case, since the Swift field offset vector doesn't pick up the adjusted offsets from the ObjC runtime. Fixes SR-5036 | rdar://problem/32488871.
@jckarter jckarter force-pushed the keypath-indirect-offset branch from 66dbec0 to fe88bd5 Compare June 16, 2017 21:42
@jckarter
Copy link
Contributor Author

@swift-ci Please test

@swift-ci
Copy link
Contributor

Build failed
Jenkins build - Swift Test Linux Platform
Git Commit - 66dbec060e9c38068949d5d57859c8c5d074a19b
Test requested by - @jckarter

@jckarter
Copy link
Contributor Author

@swift-ci Please test

@jckarter jckarter merged commit cf3da5f into swiftlang:master Jun 16, 2017
@jckarter jckarter mentioned this pull request Jun 16, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jckarter @swift-ci @aciidgh