`ptr::copy_nonoverlapping` is not memcpy #113347

workingjubilee · 2023-07-05T00:50:42Z

A C implementation may treat memcpy calls Rust expects ptr::copy_nonoverlapping to handle correctly as UB. Stop confusing the matter.

r? @RalfJung

A C implementation may treat memcpy calls Rust expects `ptr::copy_nonoverlapping` to handle correctly as UB. Stop confusing the matter.

library/core/src/intrinsics.rs

workingjubilee · 2023-07-05T01:23:18Z

Also of interest:

Document the assumptions we make about the C standard library, that go beyond what C requires unsafe-code-guidelines#426
Miri's memcpy shim accepts programs that violate the C spec miri#2966

library/core/src/intrinsics.rs

RalfJung · 2023-07-05T11:48:33Z

Similar comments appear in copy and write_bytes (saying they are like memmove and memset), so those should also be updated. It's not just the intrinsics, we also have this here:

rust/library/core/src/ptr/mut_ptr.rs

Line 1452 in beb0e73

/// Invokes memset on the specified pointer, setting `count * size_of::<T>()`

workingjubilee · 2023-07-05T14:36:02Z

Oh, lovely.

library/core/src/intrinsics.rs

Co-authored-by: Ralf Jung <post@ralfj.de>

These intrinsics have a divergent meaning from C's own, so stop trying to tell people that they are the same.

Most docs are not actually referring to the symbol memcpy, so do not refer to that symbol.

workingjubilee · 2023-07-05T16:42:23Z

Note that I am leaving the aliases in place quite on-purpose: if someone is determined to search for that name, finding these functions and reading them is fine by me.

library/core/src/intrinsics.rs

RalfJung · 2023-07-05T16:58:52Z

library/core/src/intrinsics.rs

-/// `copy` is semantically equivalent to C's [`memmove`], but with the argument
-/// order swapped. Copying takes place as if the bytes were copied from `src`
-/// to a temporary array and then copied from the array to `dst`.
+/// `copy` may temporarily use additional space to copy bytes from `src` into an array, before


Doesn't it just copy forwards/backwards depending on which region is "lower"? I don't think you ever need a temporary array?

arguably a register is in fact a "temporary array", but you're right, I think.

RalfJung · 2023-07-05T17:00:00Z

This reference to memset seems to still be around?

rust/library/core/src/ptr/mut_ptr.rs

Line 1452 in beb0e73

/// Invokes memset on the specified pointer, setting `count * size_of::<T>()`

workingjubilee · 2023-07-05T17:00:28Z

!@#$%^&*

Co-authored-by: Ralf Jung <post@ralfj.de>

RalfJung · 2023-07-05T17:10:44Z

LGTM apart from that last open thread, feel free to resolve either way.

But let's also get more eyes on this since we are removing the documentation's references to the C function equivalents entirely:
r? libs-api

workingjubilee · 2023-07-05T17:29:34Z

Note that I am happy to readd documentation like what I removed if insisted-upon, but instead explaining more clearly that it is a false friend. The element vs. byte confusion, in particular, is something I will be landing a fix for today in another Rust-based software repo. It is really bad out there!

the8472 · 2023-07-05T19:21:27Z

The comment is there to some extent to tell people that if they're coming from C-land and would normally use memcpy/memmove/memset then these are the rusty replacements.
If the comment has to say "like memcpy, but different", that's fine.

RalfJung · 2023-07-05T19:30:18Z

(Based on some discussion I had off-thread with @workingjubilee)

The thing is, there are many languages people come from, not just C. So should a comparison with C really be but front and center, before even our own list of safety requirements?

the8472 · 2023-07-05T19:42:35Z

We do give C a bit of a special treatment here and there, since it's mentioned in the #[doc(alias=...)] attribute which directs people to the function if they're searching for that. It could probably be buried further down. After they have been directed there it would be good to to explicitly explain how it differs from memcpy.

workingjubilee · 2023-10-08T23:48:13Z

Alright, I think that's reasonable.
@rustbot author

joshtriplett · 2024-02-11T02:46:50Z

r? libs

Dylan-DPC · 2024-07-29T13:26:49Z

@workingjubilee any updates on this? thanks

workingjubilee · 2024-10-11T18:23:40Z

Somewhat hilariously, this PR has become much more incorrect, as it is almost exactly memcpy now that WG14 has declared an intent to validate memcpy(NULL, NULL, 0) in C2y.

Dylan-DPC · 2024-10-12T09:11:16Z

Thanks :D. Closing this pr then :)

ptr::copy_nonoverlapping is not memcpy

73163f8

A C implementation may treat memcpy calls Rust expects `ptr::copy_nonoverlapping` to handle correctly as UB. Stop confusing the matter.

rustbot assigned RalfJung Jul 5, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jul 5, 2023

workingjubilee commented Jul 5, 2023

View reviewed changes

library/core/src/intrinsics.rs Outdated Show resolved Hide resolved

library/core/src/intrinsics.rs Outdated Show resolved Hide resolved

RalfJung reviewed Jul 5, 2023

View reviewed changes

library/core/src/intrinsics.rs Show resolved Hide resolved

Nestle paragraphs in copy_nonoverlapping

cb42825

RalfJung reviewed Jul 5, 2023

View reviewed changes

library/core/src/intrinsics.rs Outdated Show resolved Hide resolved

workingjubilee and others added 3 commits July 5, 2023 09:00

Erase redundancy about 1+ bytes

313ab9c

Co-authored-by: Ralf Jung <post@ralfj.de>

Describe write_bytes and copy sans C

8a7f449

These intrinsics have a divergent meaning from C's own, so stop trying to tell people that they are the same.

Remove scattered refs to C fn

3cb3b0e

Most docs are not actually referring to the symbol memcpy, so do not refer to that symbol.

RalfJung reviewed Jul 5, 2023

View reviewed changes

library/core/src/intrinsics.rs Outdated Show resolved Hide resolved

RalfJung reviewed Jul 5, 2023

View reviewed changes

workingjubilee and others added 2 commits July 5, 2023 10:01

Swat one last memset

24fe39f

Avoid winking description of non-overlap

9c35d94

Co-authored-by: Ralf Jung <post@ralfj.de>

rustbot assigned joshtriplett and unassigned RalfJung Jul 5, 2023

RalfJung mentioned this pull request Jul 5, 2023

Document the assumptions we make about the C standard library, that go beyond what C requires rust-lang/unsafe-code-guidelines#426

Closed

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 8, 2023

rustbot added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Feb 11, 2024

rustbot assigned cuviper and unassigned joshtriplett Feb 11, 2024

Dylan-DPC closed this Oct 12, 2024

Dylan-DPC removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Oct 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`ptr::copy_nonoverlapping` is not memcpy #113347

`ptr::copy_nonoverlapping` is not memcpy #113347

workingjubilee commented Jul 5, 2023

workingjubilee commented Jul 5, 2023 •

edited

Loading

RalfJung commented Jul 5, 2023 •

edited

Loading

workingjubilee commented Jul 5, 2023

workingjubilee commented Jul 5, 2023

RalfJung Jul 5, 2023

workingjubilee Jul 5, 2023

RalfJung commented Jul 5, 2023

workingjubilee commented Jul 5, 2023

RalfJung commented Jul 5, 2023 •

edited by workingjubilee

Loading

workingjubilee commented Jul 5, 2023 •

edited

Loading

the8472 commented Jul 5, 2023

RalfJung commented Jul 5, 2023

the8472 commented Jul 5, 2023 •

edited

Loading

workingjubilee commented Oct 8, 2023

joshtriplett commented Feb 11, 2024

Dylan-DPC commented Jul 29, 2024

workingjubilee commented Oct 11, 2024

Dylan-DPC commented Oct 12, 2024

ptr::copy_nonoverlapping is not memcpy #113347

ptr::copy_nonoverlapping is not memcpy #113347

Conversation

workingjubilee commented Jul 5, 2023

workingjubilee commented Jul 5, 2023 • edited Loading

RalfJung commented Jul 5, 2023 • edited Loading

workingjubilee commented Jul 5, 2023

workingjubilee commented Jul 5, 2023

RalfJung Jul 5, 2023

Choose a reason for hiding this comment

workingjubilee Jul 5, 2023

Choose a reason for hiding this comment

RalfJung commented Jul 5, 2023

workingjubilee commented Jul 5, 2023

RalfJung commented Jul 5, 2023 • edited by workingjubilee Loading

workingjubilee commented Jul 5, 2023 • edited Loading

the8472 commented Jul 5, 2023

RalfJung commented Jul 5, 2023

the8472 commented Jul 5, 2023 • edited Loading

workingjubilee commented Oct 8, 2023

joshtriplett commented Feb 11, 2024

Dylan-DPC commented Jul 29, 2024

workingjubilee commented Oct 11, 2024

Dylan-DPC commented Oct 12, 2024

`ptr::copy_nonoverlapping` is not memcpy #113347

`ptr::copy_nonoverlapping` is not memcpy #113347

workingjubilee commented Jul 5, 2023 •

edited

Loading

RalfJung commented Jul 5, 2023 •

edited

Loading

RalfJung commented Jul 5, 2023 •

edited by workingjubilee

Loading

workingjubilee commented Jul 5, 2023 •

edited

Loading

the8472 commented Jul 5, 2023 •

edited

Loading