debug_assert a few more raw pointer methods #69208
Conversation
|
r? @cramertj (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
I believe we're already doing this elsewhere and these are consistent with that, so @bors r+ rollup |
|
📌 Commit bec5d37 has been approved by |
bors
added
S-waiting-on-bors
…acrum debug_assert a few more raw pointer methods Fixes rust-lang#53871
…acrum debug_assert a few more raw pointer methods Fixes rust-lang#53871
…acrum debug_assert a few more raw pointer methods Fixes rust-lang#53871
|
This likely caused the failure: #69219 (comment) |
|
Hm, PR CI passed and I don't see what the error message would have to do with this PR. So, doesn't seem likely to me... but I have seen stranger things. Why do you think it was this one? |
|
I suspect this and #68767, but it's a tweak for macOS so I've commented here. Hm so likely later one caused the failure (i.e. this PR is unrelated)? |
…acrum debug_assert a few more raw pointer methods Fixes rust-lang#53871
|
Okay, next rollup contains this, if the failure isn't shown, I'll visit later one (sorry for the noise!). |
|
No you were right, I can reproduce the failure locally. @bors r- I am at a loss though about what could be causing this... |
bors
added
S-waiting-on-author
|
Perhaps some function isn't getting inlined as much and we're spilling more to the stack? It would be helpful to get llvm ir before/after this PR, I think. @bors try so we'll have public artifacts |
|
⌛ Trying commit bec5d37 with merge 87552e5967d74a9c38c6b3261a821204d68e6761... |
|
💔 Test failed - checks-azure |
|
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 |
bors
added
S-waiting-on-review
|
@eddyb I have enabled debug assertions locally. That was my first suspicion, but that's not it -- or at least, it is not alone sufficient. Maybe "debug assertions + 32bit", or so. Now it happened on wasm32, it seams? I am somewhat out of my depth to debug this, I am afraid. We could just add |
src/test/ui/issues/issue-40883.rs
Outdated
| // give space for 2 copies of `Big` + 128 "misc" bytes. | ||
| if stack_usage > mem::size_of::<Big>() * 2 + 128 { | ||
| // give space for 2 copies of `Big` + 256 "misc" bytes. | ||
| if stack_usage > mem::size_of::<Big>() * 2 + 256 { |
There was a problem hiding this comment.
Hmm why would this change? I would hope your additions don't actually need format_args! but it's possible debug_assert! is inefficient?
src/libcore/ptr/mod.rs
Outdated
| @@ -795,6 +801,7 @@ pub unsafe fn read_unaligned<T>(src: *const T) -> T { | |||
| #[inline] | |||
| #[stable(feature = "rust1", since = "1.0.0")] | |||
| pub unsafe fn write<T>(dst: *mut T, src: T) { | |||
| debug_assert!(is_aligned_and_not_null(dst), "attempt to write to unaligned or null pointer"); | |||
There was a problem hiding this comment.
I wonder if this is the problem, maybe try landing swap_nonoverlapping and this separately.
Maybe some uses of ptr::write want to use intrinsics::move_val_init directly without creating a raw pointer? This is so important, IIRC we lower intrinsics::move_val_init in MIR building directly.
RalfJung
force-pushed
the
debug-assert
branch
from
7a6b451
to
8d3b306
Compare
|
All right, I removed the assertion from Since this is a subset of the previous changes: @bors r=Mark-Simulacrum rollup- |
|
📌 Commit 8d3b306 has been approved by |
bors
added
S-waiting-on-bors
|
@bors p=0 |
|
☀️ Test successful - checks-azure |
debug-assert ptr sanity in ptr::write This is a re-submission of the parts that we removed from rust-lang#69208 due to ["interesting" test failures](rust-lang#69208 (comment)). Fixes rust-lang#53871 r? @Mark-Simulacrum @eddyb
Makes progress for #53871