Strip top bits from pointers before reading memory contents #320
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kattrali
force-pushed
the
kattrali/swift-assertion-msg-fix
branch
from
ca3311f
to
91d011d
Compare
In the new Xcode 10 build system, Swift object register values have the top bit used as a flag. This change strips the flag while not losing anything relevant to us in our quest to see error messages for assertion failures. This technique does not capture messages which are less than 16 characters, as short strings are stored as raw char arrays on the stack rather than being allocated. (See WWDC 2018 #401 for more info on new string optimizations) While it is possible to check for char arrays as well as pointers when searching for notable address values, sweeping up local variables has a likely chance of capturing unintended data as well from the surrounding code, some of which may be sensitive. It is also not guaranteed that the value would still be on the stack after the message is logged, so it is possible to get only unrelated string values as the message. In the current Swift stdlib, the following messages passed to fatalError, preconditionFailure, and precondition (and their internal func counterparts) are less than 16 characters: * empty string * `unavailable` * `not implemented` * `abstract method` * `unknown value` * `invalid count` (where a dictionary contains < 0 items(?)) * `invalid index` (where a dictionary ceases to be a dictionary) * `don't touch me` (from SpriteKit) * `close() failed` (from the private Subprocess implementation) The vast majority have more meaningful messages. Reference: * https://asciiwwdc.com/2018/sessions/401 Fixes #318
kattrali
force-pushed
the
kattrali/swift-assertion-msg-fix
branch
from
91d011d
to
7094543
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In the new Xcode 10 build system, Swift object register values have the
top bit used as a flag. This change strips the flag while not losing
anything relevant to us in our quest to see error messages for assertion
failures.
Design
Added a new validation to
bsg_kscrw_i_writeMemoryContentsIfNotablewhich strips the top from the address if it is not valid and rechecks the new value. If it is then valid, it is used as the object address.Tests
Discussion
This technique no longer works at all for assertion messages less than 16 characters, as they aren't stored in the register values.
Linked issues
Fixes #318
Review