Fix ICE on offsetted ZST pointer #147576
Fix ICE on offsetted ZST pointer #147576
Conversation
|
Some changes occurred in compiler/rustc_codegen_gcc Some changes occurred in compiler/rustc_codegen_cranelift cc @bjorn3 |
rustbot
added
A-LLVM
|
r? @nnethercote rustbot has assigned @nnethercote. Use |
This comment has been minimized.
This comment has been minimized.
| const A: *const () = (&() as *const ()).wrapping_byte_add(2); | ||
| const B: *const () = (&Foo as *const _ as *const ()).wrapping_byte_add(2); | ||
| const C: *const () = (&Foo as *const _ as *const ()).wrapping_byte_add(usize::MAX); | ||
|
|
There was a problem hiding this comment.
I don't understand this at all. Can you explain a little? Perhaps this test needs an explanatory comment at the top?
There was a problem hiding this comment.
I extended with a longer comment, but without a bit more detail on what you're confused by it's hard to know whether it helps. Let me know.
There was a problem hiding this comment.
Thanks, that helps... but I still don't understand where the values of 3, 3, and 6 come from.
There was a problem hiding this comment.
- A 1-aligned ZST (1-ZST) is placed at
0x1. Then offsetting that by 2 results in 3. Foois a 4-aligned ZST, so is placed at0x4. +2 = 6Foois a 4-aligned ZST, so is placed at0x4. +usize::MAX = -1 (same bit pattern) = 3
There was a problem hiding this comment.
Please add that to the comment! Also, why is Foo a 4-ZST? Add that information too, and then r=me, thanks.
There was a problem hiding this comment.
I'll skip adding why Foo is a 4-ZST -- it's likely obvious if you saw the full file, it has a repr(align(4)) on it. If you mean why 4 (and not some other number), there's not a particular meaning behind 4. Documented that on Foo itself.
Mark-Simulacrum
force-pushed
the
fix-offset-zst
branch
3 times, most recently
from
403865a to
060e453
Compare
A grep for `const_usize.*align` found the same code copied to rustc_codegen_gcc but I don't see other cases where we get this wrong.
Mark-Simulacrum
force-pushed
the
fix-offset-zst
branch
from
060e453 to
7a513dd
Compare
|
@bors r=nnethercote |
bors
added
S-waiting-on-bors
…nnethercote Fix ICE on offsetted ZST pointer I'm not sure this is the *right* fix, but it's simple enough and does roughly what I'd expect. Like with the previous optimization to codegen usize rather than a zero-sized static, there's no guarantee that we continue returning a particular value from the offsetting. A grep for `const_usize.*align` found the same code copied to rustc_codegen_gcc and cranelift but a quick skim didn't find other cases of similar 'optimization'. That said, I'm not convinced I caught everything, it's not trivial to search for this. Closes rust-lang#147516
5 participants
I'm not sure this is the right fix, but it's simple enough and does roughly what I'd expect. Like with the previous optimization to codegen usize rather than a zero-sized static, there's no guarantee that we continue returning a particular value from the offsetting.
A grep for
const_usize.*alignfound the same code copied to rustc_codegen_gcc and cranelift but a quick skim didn't find other cases of similar 'optimization'. That said, I'm not convinced I caught everything, it's not trivial to search for this.Closes #147516