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
[SR-10124] String.UTF16View.distance crashes on using misaligned String.Index with native String #52526
Comments
cc @milseman |
Prior to this crash in asserts configurations, we seem to be failing the invariant check that the Substring's indices are scalar-aligned. Given that we maintain the invariant elsewhere that Substrings are scalar-aligned, this is surprising. I'll look further. edit: I am mistaken, scalar-alignment was done for the failable inits on String from a Substring's code unit views. Looking for the best way to get this fix out there. |
Ok, sorry for the brain-fart, I originally thought this through but there is a bug in a fast-path for the String(Substring) initializer. According to SE-0180: This means that comparison involving sub-scalar indices might scalar align first, and so Substring has a offsetRange property for this purpose. However, that was being misused as part of a fast-path by String's initializer from Substring: we should be comparing the whole indices not the encoded offsets. This is more fall-out from SE-0180. I don't think we should have allowed Substrings that fall on sub-scalar boundaries, we should at least rounded to the nearest scalar. This is a definite bug here, but I'll try and see if there's more from this example or elsewhere. |
Interesting, fixing this crash means that one of our expected to crash tests no longer crashes. My targeted fix involves scalar aligning UTF8 indices inside UTF16View's getNativeOffset for an index. I think we need another targeted fix that scalar aligns indices when forming a Substring, because there's a lot of wacky invariants that will be violated if we don't and follow SE-0180's reasoning. More updates to come edit: JIRA doesn't support it if I try to put the code in here, probably because there's an emoji or something. Neither code nor noformat blocks work... swift/test/stdlib/StringTraps.swift Line 171 in 9a0d6e8
|
Here's another crasher, because contiguous substrings keep encoding validity assumptions: https://gist.github.com/milseman/f32a81878efd2977508bab3c6fea5e32 |
Exhaustively testing this had uncovered a whole slew of related bugs and a need for more performance work. I'm using this as the parent bug for those. |
Environment
Xcode 10.2 beta 4
swift-5.0-DEVELOPMENT-SNAPSHOT-2019-03-10-a
Additional Detail from JIRA
md5: cdb7f07a9521853bd56a4e8007b5404c
Sub-Tasks:
Issue Description:
It does not crash with
NSString
basedString
.Reproducing code:
log:
The text was updated successfully, but these errors were encountered: