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

Fix copy_from_slice #58810

Closed
wants to merge 1 commit into from
Closed

Fix copy_from_slice #58810

wants to merge 1 commit into from

Conversation

nitnelave
Copy link
Contributor

There is no guarantee that the slices are not overlapping.

This was discovered due to #58783

There is no guarantee that the slices are not overlapping.
@rust-highfive
Copy link
Collaborator

r? @dtolnay

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 28, 2019
@nitnelave
Copy link
Contributor Author

r? @RalfJung

@rust-highfive rust-highfive assigned RalfJung and unassigned dtolnay Feb 28, 2019
@nitnelave
Copy link
Contributor Author

I'd like to add a test, but I'm not sure where.

@RalfJung
Copy link
Member

A shared slice may never overlap with a mutably borrowed one. So this change does not seem right to me.

@nitnelave
Copy link
Contributor Author

nitnelave commented Feb 28, 2019 via email

@RalfJung
Copy link
Member

If the caller is unsafe, it's possible, no? But in that case, who bears the responsibility?

The caller. You must not create such overlapping references, period -- even if you don't use them, that's already UB.

@Dylan-DPC-zz
Copy link

ping from triage @nitnelave @RalfJung any updates on this?

@nitnelave
Copy link
Contributor Author

Sorry, I need to debug this, but it's trickier than expected (and I didn't have time recently). I'll try to have another swing at it some time this week.

@Dylan-DPC-zz
Copy link

sure @nitnelave no issues :)

@Dylan-DPC-zz Dylan-DPC-zz 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 Mar 18, 2019
@RalfJung
Copy link
Member

RalfJung commented Mar 18, 2019

I think this PR should be closed. The change is incorrect (or rather, unnecessary: we may assume here that the slices don't alias, and there is nothing gained by using copy instead of copy_nonoverlapping). Further investigation is needed in #58783 to determine what causes the assertion failure.

@nitnelave
Copy link
Contributor Author

fair enough, I'll open a new one when I have a better fix :)

@nitnelave nitnelave closed this Mar 18, 2019
@Dylan-DPC-zz Dylan-DPC-zz removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Mar 18, 2019
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

Successfully merging this pull request may close these issues.

5 participants