-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Add debug assertions to write_bytes and copy* #58783
Conversation
r? @bluss (rust_highfive has picked a reviewer for you, use r? to override) |
r? @RalfJung |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Looks like something broke, we have a (questionably) invalid call to |
src/libcore/intrinsics.rs
Outdated
Ordering::Greater => dst_usize + size > src_usize, | ||
// src == dst | ||
Ordering::Equal => count != 0, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Uh, Miri will not like this... but yeah this seems right. Cc @oli-obk
Uh nice, maybe we found the first bug already? :D |
While building the compiler, I got an error on a |
Do you have a backtrace? |
Yes, but I just switched off my computer, so I'll add it tomorrow.
…On Wed, Feb 27, 2019, 19:04 Ralf Jung ***@***.***> wrote:
Do you have a backtrace?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#58783 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAwn2W5-CdEfn2au_y5xk_j5NQboOPZYks5vRsi4gaJpZM4bU5ug>
.
|
The stacktrace looks like this (trimmed to the relevant part):
|
That function calls |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
ping from triage @nitnelave you have to fix the failing tests |
The failure occurs in fn clone(&self) -> Vec<T> {
<[T]>::to_vec(&**self)
} I find it highly unlikely that there are indeed overlapping references here (and I think Miri would have found them), so my guess is that the check is wrong. OTOH, it looks all right... maybe try adding debug printing to If you do |
src/libcore/intrinsics.rs
Outdated
use crate::cmp::Ordering; | ||
let src_usize = src as usize; | ||
let dst_usize = dst as usize; | ||
let size = mem::size_of::<T>() * count; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you use checked arithmetic here? Overflow should always panic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, doing that.
No, I don't get failures on |
Well, then I guess you'll have to resort to printf debugging. :/ Let me know when you are stuck or need assistance. |
I think the failure from #58783 (comment) was caused by the error I pointed out in #58783 (comment) because before that was fixed there was a false positive from slices of ZSTs. I think we need a new backtrace. |
Co-Authored-By: nitnelave <nitnelave1@gmail.com>
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
☔ The latest upstream changes (presumably #60355) made this pull request unmergeable. Please resolve the merge conflicts. |
@nitnelave what is the status of this? Something went very wrong in the last rebase, you now have tons of submodule changes in this PR. |
Yes, I solved that but didn't push yet, I'm on holiday with little access
to my computer :)
…On Tue, Apr 30, 2019, 15:49 Ralf Jung ***@***.***> wrote:
@nitnelave <https://github.com/nitnelave> what is the status of this?
Something went very wrong in the last rebase, you now have tons of
submodule changes in this PR.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#58783 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAGCPWILJWVC2EYZN427A2LPS7TYBANCNFSM4G2TTOQA>
.
|
@nitnelave any updates on this? |
ping from triage @nitnelave |
Re-submitted as #62103. |
Sorry about that, life got in the way :( |
That's okay, and thanks for your work! I didn't have to do much any more. :) |
Add debug assertions to write_bytes and copy* Looks like @nitnelave went MIA in rust-lang#58783, so I am re-submitting their PR, tweaked just a bit. I took care to preserve commit authorship. Cc rust-lang#53871
Add debug assertions to write_bytes and copy* Looks like @nitnelave went MIA in rust-lang#58783, so I am re-submitting their PR, tweaked just a bit. I took care to preserve commit authorship. Cc rust-lang#53871
Add debug assertions to write_bytes and copy* Looks like @nitnelave went MIA in rust-lang#58783, so I am re-submitting their PR, tweaked just a bit. I took care to preserve commit authorship. Cc rust-lang#53871
Add debug assertions to write_bytes and copy* Looks like @nitnelave went MIA in rust-lang#58783, so I am re-submitting their PR, tweaked just a bit. I took care to preserve commit authorship. Cc rust-lang#53871
Add debug assertions to write_bytes and copy* Looks like @nitnelave went MIA in rust-lang#58783, so I am re-submitting their PR, tweaked just a bit. I took care to preserve commit authorship. Cc rust-lang#53871
Add debug assertions to write_bytes and copy* Looks like @nitnelave went MIA in rust-lang#58783, so I am re-submitting their PR, tweaked just a bit. I took care to preserve commit authorship. Cc rust-lang#53871
Add debug assertions to write_bytes and copy* Looks like @nitnelave went MIA in rust-lang#58783, so I am re-submitting their PR, tweaked just a bit. I took care to preserve commit authorship. Cc rust-lang#53871
Add debug assertions to write_bytes and copy* Looks like @nitnelave went MIA in rust-lang#58783, so I am re-submitting their PR, tweaked just a bit. I took care to preserve commit authorship. Cc rust-lang#53871
Add debug assertions to write_bytes and copy* Looks like @nitnelave went MIA in rust-lang#58783, so I am re-submitting their PR, tweaked just a bit. I took care to preserve commit authorship. Cc rust-lang#53871
Use resume_unwind instead of panic!() for nicer compiletest errors cc rust-lang/rust#58783 (comment)
This is a followup of #57997 to address #53871
We check that:
write_bytes
has an aligned pointercopy
has both aligned pointers and the end ofsrc
isn't indst
(otherwise we overwrite something before reading it)copy_nonoverlapping
has both aligned pointers andsrc
anddst
don't overlap.