Repository navigation
Miscompilation on release profile when std::ptr::read is used to cast byte primitive to some tuples in unreachable code paths #127286
Copy link
Copy link
Closed
Labels
A-LLVMArea: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.C-bugCategory: This is a bug.Category: This is a bug.I-unsoundIssue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/SoundnessIssue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/SoundnessP-highHigh priorityHigh priorityT-compilerRelevant to the compiler team, which will review and decide on the PR/issue.Relevant to the compiler team, which will review and decide on the PR/issue.llvm-fixed-upstreamIssue expected to be fixed by the next major LLVM upgrade, or backported fixesIssue expected to be fixed by the next major LLVM upgrade, or backported fixes
Description
Activity
- addedneeds-triageThis issue may need triage. Remove when done. See docs forge.rust-lang.org/release/issue-triagingThis issue may need triage. Remove when done. See docs forge.rust-lang.org/release/issue-triaging
on Jul 3, 2024 - addedA-LLVMArea: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.Relevant to the compiler team, which will review and decide on the PR/issue.I-unsoundIssue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/SoundnessIssue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundnessand removedneeds-triageThis issue may need triage. Remove when done. See docs forge.rust-lang.org/release/issue-triagingThis issue may need triage. Remove when done. See docs forge.rust-lang.org/release/issue-triaging
on Jul 3, 2024 - addedI-prioritizeIssue needs a team member to assess the impact. Will be replaced by P-{low,medium,high,critical}Issue needs a team member to assess the impact. Will be replaced by P-{low,medium,high,critical}
on Jul 3, 2024 From a brief glance, this looks like a SROA bug to me: https://llvm.godbolt.org/z/o1Poz5vzn We should not be folding to poison.
(Alive2 doesn't flag this, but I believe that's a bug in alive2. Filed AliveToolkit/alive2#1067.)
Reacted by Ben Kimock, Jubilee, Andrey Zheleznov, scottmcm and dianqkIf this showed up in 1.70, it was probably exposed by #109035 since that changed
ptr::readfrom memcpy to a normal load.(EDIT: doh, you'd already bisected to the rollup with that PR.)
Reacted by Ben KimockUpstream issue: llvm/llvm-project#97702
Reacted by apiraino and Andrey ZheleznovWG-prioritization assigning priority (Zulip discussion).
@rustbot label -I-prioritize +P-high
- addedP-highHigh priorityHigh priorityand removedI-prioritizeIssue needs a team member to assess the impact. Will be replaced by P-{low,medium,high,critical}Issue needs a team member to assess the impact. Will be replaced by P-{low,medium,high,critical}
on Jul 4, 2024 - addedllvm-fixed-upstreamIssue expected to be fixed by the next major LLVM upgrade, or backported fixesIssue expected to be fixed by the next major LLVM upgrade, or backported fixes
on Jul 4, 2024 - added a commit that references this issue
on Jul 4, 2024 - added a commit that references this issue
on Jul 6, 2024 - added 2 commits that reference this issue
on Aug 13, 2024
Metadata
Metadata
Assignees
Labels
A-LLVMArea: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues.C-bugCategory: This is a bug.Category: This is a bug.I-unsoundIssue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/SoundnessIssue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/SoundnessP-highHigh priorityHigh priorityT-compilerRelevant to the compiler team, which will review and decide on the PR/issue.Relevant to the compiler team, which will review and decide on the PR/issue.llvm-fixed-upstreamIssue expected to be fixed by the next major LLVM upgrade, or backported fixesIssue expected to be fixed by the next major LLVM upgrade, or backported fixes
std::ptr::readwhich is used to castu8,i8orboolto two-element tuples withu8,i8,u16ori16types leads to incorrect code generation when located in unreachable code paths (and not supposed to be called).Replacing
std::ptr::readtotransmute_copyfixes the issue. Making conditions more simple can also fix the issue. Building with debug profile fixes the issue. Regressed since 1.70.0 (see Affected Rust versions section below).This issue can lead to UB in creates which use
std::ptr::readwith generics for runtime casting/specialization.Reproducible example
Started with:
cargo run --releaseExpected: no output
Result:
assertion failedReproducible example variations
False conditions with which the issue is reproduced (failure):
if core::any::TypeId::of::<u8>() == core::any::TypeId::of::<(u8, u8)>() {,if core::any::TypeId::of::<u8>() == core::any::TypeId::of::<String>() {,if core::any::type_name::<u8>() == core::any::type_name::<u64>() {,if core::hint::black_box(false) {(but generate less optimized assembly),False conditions with which the issue is NOT reproduced (ok):
if false {,if core::convert::identity(false) {,Сasts (actually unreachable) with which the issue is reproduced (failure):
Ok(unsafe { std::ptr::read(&value as *const u8 as *const (u8, u8)) })Ok(unsafe { std::ptr::read(&value as *const u8 as *const (i8, u16)) })Casts (actually unreachable) with which the issue is NOT reproduced (ok):
Ok(unsafe { std::ptr::read(&value as *const u8 as *const (u8, u8, u8)) })Ok(unsafe { std::ptr::read(&value as *const u8 as *const u16) })Ok(unsafe { core::mem::transmute_copy::<u8, (u8, u8)>(&value) })Cloned implementation of
transmute_copywithassert!removed will also leadto the error, i.e. it's the assert that helps to avoid the issue when using
transmute_copy:Affected Rust versions
rustc 1.79.0 (129f3b996 2024-06-10)) - reproducible.rustc 1.80.0-beta.4 (64a1fe671 2024-06-21))- reproducible.rustc 1.81.0-nightly (6292b2af6 2024-07-02)) - reproducible.Reproducible in Rust playground as well: https://play.rust-lang.org/?version=stable&mode=release&edition=2021&gist=7a2847de3ac47d87a157a8644e7fd668
Quick bisection between 1.69.0 and 1.70.0 with MRE:
Known affected crates
castaway
Casting some byte primitives to some two-element tuples unexpectedly succeeds on release builds and definitely leads to UB.
These assertions will success on debug builds, but will fail on release builds:
Castaway crate uses
std::ptr::readto transmute between types when type ids are matched.The source and target cast types which leads to issue
Unexpected cast success is reproduced for casts from
T1to(T2, T3)where
T1isu8,i8orbool,T2isu8,i8,u16ori16,T3isu8,i8,u16ori16,or where
T1isu16,i16,T2isu8,i8,u16ori16,T3isu8,i8,u16ori16but has different byte size thanT2Check code: