This sounds like an objection to UMRBP conforming to Collection<UInt8>. That doesn’t seem possible or advisable to change at this point.
2 Likes
Yes, if the point is to fill up an untyped buffer, then only UMRBP can be used to do that.
There's probably not complete alignment between how we think about safety. You're indisputably right that accessing memory through UMRBP is unsafe (for a compiler-centric definition of unsafe) because it gets rid of the safeguards that align the compiler's view of types with the developer's view of types, and now, if you slip, TBAA will break into your living room to rearrange your furniture. On the other hand, I don't think that I have (yet) seen that kind of mistake be the root of a security exploit, so for an exploit-centric definition of unsafe, it's not currently a top concern.
Unfortunately, UMRBP is all the types of unsafe, including the top-concern kinds of unsafe that are seen at the root of 70% of security bugs and which developers treat with dangerously insufficient awe.
If there was a version of UMRBP that was bounds-safe and lifetime-safe but still type-unsafe, I would probably not find anything to say about its use in standard API.
3 Likes
Swift raw pointers (like UMRBP) allow programmers to reinterpret memory as they wish without regard for TBAA. There can't be any TBAA if there's no pointer type. The only way you run afoul of TBAA in Swift is by misusing a memory binding API or unsafeBitcasting a typed pointer. This is not so in other "safe" languages. Raw pointers give Swift a safety advantage.
Lack of UMRBP bounds-checking in release builds is a mistake that we should correct.
4 Likes
This is somewhat far afield now, but...
Raw buffers (URBP, Foundation.Data) have no statically known in-memory type. Whenever you load a value, you need to choose how you want to interpret those bits. Conforming to a Collection of UInt8 provides a default view over that buffer as a "byte buffer". It does not say that the memory contains UInt8 typed things. Of course, if Swift had a Byte type, then that would have been the element type for a raw buffer.
This all "makes sense" because it's conventional to view a buffer of bits as a sequence of bytes, and it's conventional to view a byte as a UInt8 value. In fact it's reasonable to view a raw buffer as a collection of any trivially typed element. UInt8 doesn't need to be special. I'd like to move in that direction, but for now we just have the one Collection conformance.
I do understand the confusion. But it's unavoidable unless people understand that the type you load from a raw buffer doesn't need to be the type that was stored in that buffer.
UInt8 is the way to view a byte. That does not imply that a byte is a UInt8.
Collection of UInt8 is the way to view a byte buffer. That does not imply that the raw memory holds UInt8s.
It makes sense to pass a byte buffer to an algorithm that understands a Collection of UInt8s. The programmer has chosen to view the buffer that way.
That's not the same as advertising an algorithm that provides randomized bytes as generating a Collection of UInt8s. @scanon says it's worth making that distinction in this case. All I'm saying is that it is a valid distinction.
7 Likes
Yeah, this is something that we (Apple's standard library teams) very interested in doing. Raw-memory operations, while necessarily (type-)unsafe, should not also require discarding bounds-checking safety and lifetime safety.
7 Likes
That's great, and I look forward to seeing the fruits of that work.
Unfortunately, as it stands right now, the nature of this API is that the interface is type-safe but memory unsafe. It is simply not possible for a user to use it without taking on responsibility for bounds-checking and lifetime management.
We are all in this thread aware of the existing pitfalls that users will face when trying to hold this API. We know that the odds are 100% that at least one person will write var d = Data(repeating: 0, count 256); withUnsafeMutableBytes(of: &d) { rng.fill($0) } and get their foot blown off. None of this is your fault, and I think it's fine to say that it's not the responsibility of this PR to fix these issues. But it is a crying shame to repeatedly introduce unsafe APIs due to the absence of anything better, and it would be really nice if we made it possible for users to get what they want without having to risk getting their fingers chopped off.
3 Likes
how would library types (e.g. ByteBufferView) fill themselves from this API without providing a pointer view to storage?
This is best addressed by providing an extension in Foundation that does fill() for Data in the short term. In particular, even if we had a raw memory type that was bounds-checked and lifetime-managed, that bug would still exist. It’s a flaw in the API producing the buffer, not in the buffer type itself.
Longer term we fix it by providing a better alternative to withUnsafeBytes(of:) that people will reach for instead, but that's a separate proposal (blocked on non-copyable / non-escaping stuff).
3 Likes
Cory used a common mistake as an example. The correct way would be
var d = Data(repeating: 0, count: 256)
d.withUnsafeMutableBytes { rng.fill($0) }
The mistake passes MemoryLayout<Data>.size bytes to be overwritten by rng -- including the bits of a reference managed by the runtime. Obviously that results in nothing good.
1 Like
yes i understood what Cory was trying to illustrate, my question was in response to the criticism that UMRBP can escape and outlive the allocation it was originally taken from.
what i don’t understand is how library types could possibly provide a managed buffer view in the first place that is not UMRBP, without incurring reference counting overhead. i imagine the best we could do is keep a strong reference to the original instance alongside the buffer pointer.
struct ManagedBufferView<T>
{
let pointer:UnsafeMutableRawBufferPointer
let owner:T
}
but then we would need a way to prevent the buffer view itself from escaping, because it is still a violation of value semantics.
We could have a non-copyable buffer type that provides access methods that compile down to direct memory accesses. Using the same kind of closure-taking methods that we currently have in withUnsafeMutableBytes, but taking this new non-copyable type instead, we would have withMutableBytes. If such a type were provided by the standard library, it would absolutely make the choice of providing bounds-checking on all accesses by default. Such a closure would be a) unable to escape the storage, b) unable to access memory outside the bounds of the storage, c) thread-safe. The one remaining (and necessary) unsafety is the ability to load(as: T) -> T, which bypasses definite initialization.
The right way is for owner to be AnyObject? The code holding such a thing should not have to care about what kind of owner it's dealing with.
2 Likes
does this imply this feature is blocked on @noncopyable?
if history is an indicator, it will be a very, very long time before move only stuff actually becomes part of the language, and i don’t think everything else should block on it.
2 Likes
Interestingly, apple doc page only lists the deprecated flavours of those unsafe calls, those corresponding the first calls in the generated header:
@available(swift, deprecated: 5, message: "use `withUnsafeBytes<R>(_: (UnsafeRawBufferPointer) throws -> R) rethrows -> R` instead")
public func withUnsafeBytes<ResultType, ContentType>(_ body: (UnsafePointer<ContentType>) throws -> ResultType) rethrows -> ResultType
@available(swift, deprecated: 5, message: "use `withUnsafeMutableBytes<R>(_: (UnsafeMutableRawBufferPointer) throws -> R) rethrows -> R` instead")
public mutating func withUnsafeMutableBytes<ResultType, ContentType>(_ body: (UnsafeMutablePointer<ContentType>) throws -> ResultType) rethrows -> ResultType
@inlinable public func withUnsafeBytes<ResultType>(_ body: (UnsafeRawBufferPointer) throws -> ResultType) rethrows -> ResultType
@inlinable public mutating func withUnsafeMutableBytes<ResultType>(_ body: (UnsafeMutableRawBufferPointer) throws -> ResultType) rethrows -> ResultType
and there's no mention of the non deprecated alternatives (that share the same base name (which in itself is a constant source of confusion for me)).
actually after thinking this through a bit, i’d like to backtrack a bit, because i remembered we pass everything guaranteed by default now. and since our hypothetical withSafeBytes(_:) API is a closure API, this means that the code that creates the ManagedBufferView would also release it after the closure executes. so i imagine this would not actually incur much reference counting overhead. (i can only forsee that subsequent mutations would have to actually check isUniquelyReferenced(_:) at least once afterwards.)
If you search for "withUnsafe" on the page, you'll find get 6 hits including 2 for the non-deprecated versions. This is a very unfortunate state of affairs. Please report documentation bugs you find.
1 Like
You might be very, very pleasantly surprised.
6 Likes
No. If it has an owner, then it is safely copyable (copies retain the owner, and the role of the owner is to guarantee the lifetime of the paired pointer).
To bring this back on topic: what exactly is the practical difference between generating random bytes and generating random UInt8s (with no bounds). I understand that if there are bounds, we need to consider modulo bias and such, but why is this function okay for bytes but not for unbounded 8-bit unsigned integers?
I'll note that the current implementation returns a UInt64 which is essentially a fixed-size (8-byte) buffer filled it with random bytes.
no i believe @glessard brought up @noncopyable because if the buffer view escaped and someone tried to write to it, then that would also affect the owner, since they share storage. but if you wrote to self first that would “unsubscribe” it through COW, and that would be weird. but my thinking is now that it would not be weird enough to actually be a blocker by itself.
Such a view could require exclusive ownership of the owner in order to perform a write, and could be taught how to create new storage (although if it's doing that, the benefits of shared storage have gone, so it could just allocate its own buffer rather than asking the provider type how to do it).
I don't think that issue requires non-copyable types to solve. It makes one particular design possible (where the type eagerly copies to unique storage and gives it to the view, then the compiler prevents the view from escaping), but I'm not even sure that would be the best design. It's not COW; it's eager copying.
Anyway...