Skip to content
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

Unexpectedly found nil while unwrapping an Optional value #86

Open
pnewell opened this issue Sep 4, 2024 · 8 comments
Open

Unexpectedly found nil while unwrapping an Optional value #86

pnewell opened this issue Sep 4, 2024 · 8 comments

Comments

@pnewell
Copy link

pnewell commented Sep 4, 2024

I am trying to run the Tests in Xcode 16 and getting an error in Helpers, line 57.

Thread 2: Fatal error: Unexpectedly found nil while unwrapping an Optional value
@kirilltitov
Copy link
Owner

Hm. That's interesting. I have never seen this conversion crashed, every byte should be a valid ASCII symbol... Like, once I literally tried every single possible byte, and it worked just fine, that's why I was so certain on force unwrap.

A few questions for you:

  1. Name of failed test?
    1.1 A stack trace maybe?
    1.2 MAYBE an input that caused the crash?
  2. Toolchain/Swift version?

I believe Xcode 16 is still in beta, I really don't want to install it just now. I'll try running the tests in Linux dev snapshots of Swift 6.

@dimitribouniol
Copy link
Contributor

It’s easy to drop the optional entirely in this case with a different initializer: String(decoding: self, as: UTF8.self)

@pnewell
Copy link
Author

pnewell commented Sep 6, 2024

@kirilltitov It happens on a lot of tests, even testConnect(). Looks like for testConnect() it happens from:

        self.log("Getting key '\(keyBytes.string.safe)'")

Line 19, Transaction+Internal+Async.swift

If I swap it with the line @dimitribouniol recommended, it does seem to fix a lot of the tests but then I get a new error in Helpers line 89, for test "testGetRange()", when parseKeyValues() gets called:

Thread 4: Fatal error: self must be a properly aligned pointer for types Pointee and T

@pnewell
Copy link
Author

pnewell commented Sep 6, 2024

Yah, so with the change @dimitribouniol recommended, all tests pass except for two, testGetRange() and testUnpackSanity()

Edit: testUnpackSanity() fixed, has same String issue on line 87 of Test+Unpack

@kirilltitov
Copy link
Owner

kirilltitov commented Sep 7, 2024

No, I believe decoder init won't help. First, it's throwing, so you still have to do something in case of error; second — if you feed invalid unicode sequence to it, it WILL throw.

Now, I managed to reproduce the issue with most recent nightly docker image of swift (tag swiftlang/swift:nightly-rhel-ubi9, digest 6868dfa0c56ec6914f6d01b161290ea4d43ce62c8b766bf201a9bfa1ec495adc), and indeed, the new Foundation now returns nil after 127 base ASCII bytes (as it kinda LEGALLY should) (and it does exactly that for .utf8, which is a bit uncanny), check this out:

for i in UInt8.min ... UInt8.max {
  print([i: String(bytes: [i], encoding: .ascii) ?? "UNKNOWN"])
}

.isoLatin1 won't work, it crashes even earlier, on zero octet (which is really weird, as it should be extended ASCII), and .isoLatin2 doesn't work at all.

It makes SOME-ish sense (I mean, vanilla ASCII is 0-127), but I don't know what to do now. I need this naive conversion, if not here, but in many other packages.

I'll do some research (there's a hope it's just a Foundation bug), so let's just wait for a bit. I'll be in touch.

@kirilltitov
Copy link
Owner

kirilltitov commented Sep 7, 2024

Funny thing is I have a next-gen branch of this repo (next major version), there's no such helper at all there, so theoretically it won't be a problem at all if I find some time to get it done :)

@dimitribouniol
Copy link
Contributor

No, I believe decoder init won't help. First, it's throwing, so you still have to do something in case of error; second — if you feed invalid unicode sequence to it, it WILL throw.

I double checked because this statement surprised me, but it most definitely isn't throwing: https://github.com/swiftlang/swift/blob/adc80ec3adf65330aab87fee0d3b48b33e1af903/stdlib/public/core/String.swift#L435-L447
image

Double checking the implementation, it'll repair the utf8 sequence if an invalid code point is encountered, and won't trap.

@kirilltitov
Copy link
Owner

Oh lord, I'm so sorry, totally forgot about this issue, I was terribly sick at the hospital at that moment. Will sort it out soon, don't worry

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants