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

Jump to bottom

[SR-7726] Data.withUnsafe(Mutable)Bytes should use bindMemory(to:) instead of assumingMemoryBound(to:) #3702

Closed
hamishknight opened this issue May 18, 2018 · 4 comments
Closed

[SR-7726] Data.withUnsafe(Mutable)Bytes should use bindMemory(to:) instead of assumingMemoryBound(to:) #3702

hamishknight opened this issue May 18, 2018 · 4 comments
Labels
bug Foundation

Comments

@hamishknight
Copy link

Previous ID SR-7726
Radar rdar://problem/40417943
Original Reporter @hamishknight
Type Bug
Status Closed
Resolution Won't Do
Additional Detail from JIRA
Votes 0
Component/s Foundation
Labels Bug
Assignee None
Priority Medium

md5: 1b93fb05d5fdda8846f4a50088a5f7b1

relates to:

  • SR-7849 DataStorage's get(:) method should use load(fromByteOffset:as:)
  • SR-7850 DataStorage's set(:) method should use storeBytes(of:toByteOffset:as:)
  • SR-7851 _DataStorage's enumerateBytes method should use bindMemory instead of assumingMemoryBound(to:)

Issue Description:

Currently the implementation (core libs foundation) of Data's withUnsafeBytes and withUnsafeMutableBytes methods uses assumingMemoryBound(to:) in order to obtain a typed pointer to the underlying memory – however this produces undefined behaviour if the type T is unrelated to the type that the memory is already bound to.

We should switch back to using bindMemory(to:) in order to avoid this undefined behaviour.

See https://forums.swift.org/t/how-to-use-data-withunsafebytes-in-a-well-defined-manner/12811 for more context.
@belkadan
Copy link

cc @atrick

@swift-ci create

@atrick
Copy link
Member

atrick commented Jul 7, 2019

The version of withUnsafe[Mutable]Bytes that uses assumingMemoryBound to produce a typed pointer was deprecated in Swift 5.

@belkadan
Copy link

belkadan commented Jul 8, 2019

We don't get to remove it, though, so we should still fix the implementation, no?

@atrick
Copy link
Member

atrick commented Jul 8, 2019

Switching to bindMemory was not a great solution anyway. It added a code motion barrier in one direction but not the other.

I added a note to SR-11087 to fix this the right way once we have stdlib/compiler support.

Although fixing the implementation doesn't change the fact that we don't want anyone using this Data API. The issue was that this Data API hides a fundamentally unsafe operation and masquerades as a safe operation. If the user happens to know the type of the buffer that was passed into Data, then they should explicitly state that by using the Unsafe API. We could also eventually provide a typed Data-like buffer that has unique ownership of its memory.

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
@shahmishal shahmishal transferred this issue from apple/swift May 5, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Foundation
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@belkadan @atrick @hamishknight