[SR-2790] Reject UnsafePointer initialization via implicit pointer conversion. #4326
Comments
|
Reproduced. It sounds like the C string is getting deallocated, which I wouldn't expect because I thought |
This works as expected; so I suspect that the bridge is autoreleasing the NSString conversion when calling cString |
works as well... |
|
Ooh, interesting. The NSString owns the cString result? It's not an independent entry? |
|
ASAN shows some interesting results here as well: |
|
Ahh, the problem is that String.cString(using:) returns an array, not a pointer. That array can be implicitly converted to a pointer, but (a) the pointer you get from that operation is temporary, and not considered valid beyond the particular call it is used in, and (b) nothing is keeping the Array alive. |
|
it is returning either an inner pointer or an inner pointer of an NSData that is created from the bytes. So I think this is not directly associated with NSString and perhaps a code-gen problem. I was able to break on the correct free call and backtrace to a swift_unknownRelease where it is calling free inappropriately before usage. |
|
@belkadan correct! the API differential is a problem since Foundation vends as a UnsafePointer and swift re-interprets it as an array of CChars. |
|
Ah, Swift isn't reinterpreting it; it's correctly calling its _persistCString helper to copy the contents into the array. But it's certainly a pitfall for users. |
|
@belkadan I think you're right that the root problem is the implicit conversion—the call to UnsafePointer<Int8>.init(_:) with the array is an error that we don't catch. Given that that kind of implicit conversion will always be an error, along with passing a variable via inout, is there any way we can warn or error on this kind of implicit casting? The pointer initializers should allow implicit casting between pointer types, but not the broader implicit casting that's there to support C bridging. They're currently deceptively tempting for developers, especially because they can "work" in some contexts, despite being undefined behavior. var number: Int8 = 0
var array: [Int8] = [1, 2, 3, 4, 5]
var ptr: UnsafeMutablePointer<Int8> = ...
// Can we block these first two?
let bad1 = UnsafeMutablePointer(&number)
let bad2 = UnsafePointer(array)
let okay = UnsafePointer(ptr)
// Tempting b/c it works:
bad1.pointee = 23
// number == 23cc @atrick |
|
Yeah, we've definitely talked about doing this. It's hard to come up with general rules (other than "only allow these conversions when calling imported things", which I think is too restrictive), but specifically rejecting the pointer construction case is reasonable. |
|
I've been working on a patch to reject such unsound pointer conversions through the introduction of an underscored Does this sound like a good direction in which to solve this issue? I'm hoping to open a WIP PR later today to see what kind of source breakage we get when running it through the compat suite. |
|
Opened a PR: apple/swift#20070 @belkadan Could you please run the source compat suite on it? |
|
Resolved by apple/swift#27695 we will now emit a warning for this (which will hopefully become an error in a future release). sclukey (JIRA User) please verify using the next available master snapshot from https://swift.org/download/#snapshots. |
|
The original bug as expressed here now results in a warning, but implicit conversions from array to pointer don't get the warning. I'm not sure I know why the following example should behave differently than the original (the following is equivalent to "bad2" from Nate's example above): var strings: [UnsafePointer<CChar>] = []
strings.append("swift".cString(using: .ascii)!)
print("\(strings[0])") |
Environment
macOS: 10.11.6 (15G1004)
Swift: 3.0 (swiftlang-800.0.46.2 clang-800.0.38)
Additional Detail from JIRA
md5: 12bfaf656d95062691f07a1deaa3aa0d
is duplicated by:
relates to:
Issue Description:
It seems perhaps that putting an UnsafePointer into an array somehow mangles the data. It also happens with UnsafeMutablePointer(mutating: ). I'm not sure whats going on so I'll just show an example.
I would expect the following code to print "swift", but instead it prints nonsense, generally two or three characters though: examples outputs include "P�", " -", "`[".
The text was updated successfully, but these errors were encountered: