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

PassMode::Cast results in LLVM IR with out of bounds loads #122617

Closed
erikdesjardins opened this issue Mar 17, 2024 · 3 comments · Fixed by #122619
Closed

PassMode::Cast results in LLVM IR with out of bounds loads #122617

erikdesjardins opened this issue Mar 17, 2024 · 3 comments · Fixed by #122619
Assignees
Labels
A-FFI Area: Foreign function interface (FFI) C-bug Category: This is a bug. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness P-high High priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@erikdesjardins
Copy link
Contributor

I tried this code:

// On a target that uses cast ABI, e.g. --target=aarch64-unknown-linux-gnu

#![crate_type = "lib"]

#[repr(C)]
pub struct TwoU16s {
    one: u16,
    two: u16,
}

extern "C" {
    pub fn rust_dbg_extern_identity_TwoU16s(v: TwoU16s) -> TwoU16s;
}

#[no_mangle]
pub unsafe fn foo() {
    let x = TwoU16s { one: 0, two: 0 };
    rust_dbg_extern_identity_TwoU16s(x);
}

I expected to see this happen: No UB.

Instead, this happened: https://godbolt.org/z/bd8MT4bEM

define void @foo() {
start:
  %0 = alloca i64, align 8
  %1 = alloca { i16, i16 }, align 2
  %2 = alloca { i16, i16 }, align 2
  %3 = getelementptr inbounds { i16, i16 }, ptr %2, i32 0, i32 0
  store i16 0, ptr %3, align 2
  %4 = getelementptr inbounds { i16, i16 }, ptr %2, i32 0, i32 1
  store i16 0, ptr %4, align 2
  %5 = load i64, ptr %2, align 2
  %6 = call i64 @rust_dbg_extern_identity_TwoU16s(i64 %5)
  call void @llvm.lifetime.start.p0(i64 4, ptr %1)
  call void @llvm.lifetime.start.p0(i64 4, ptr %0)
  store i64 %6, ptr %0, align 8
  call void @llvm.memcpy.p0.p0.i64(ptr align 2 %1, ptr align 8 %0, i64 4, i1 false)
  call void @llvm.lifetime.end.p0(i64 4, ptr %0)
  %7 = getelementptr inbounds { i16, i16 }, ptr %1, i32 0, i32 0
  %_1.0 = load i16, ptr %7, align 2, !noundef !2
  %8 = getelementptr inbounds { i16, i16 }, ptr %1, i32 0, i32 1
  %_1.1 = load i16, ptr %8, align 2, !noundef !2
  call void @llvm.lifetime.end.p0(i64 4, ptr %1)
  ret void
}

There are two problems here, first:

%2 = alloca { i16, i16 }, align 2
...
%5 = load i64, ptr %2, align 2

...we load 8 bytes from a 4 byte alloca. Alive2 believes this is UB: https://alive2.llvm.org/ce/z/eBaekz.

Second:

call void @llvm.lifetime.start.p0(i64 4, ptr %0)
store i64 %6, ptr %0, align 8
call void @llvm.lifetime.end.p0(i64 4, ptr %0)

...only 4 bytes of %0 are marked live, but we access 8 bytes. The langref is not clear about what the size arguments of lifetime intrinsics actually do, but this is probably UB. (Alive2 ignores the size arguments, so it can't detect this.)

@rustbot label I-unsound A-ffi


This was uncovered via #122053 (comment), where this issue is the root cause of a miscompilation. That comment contains a bit more context.

Meta

rustc --version --verbose:

rustc 1.76.0 (07dca489a 2024-02-04)
binary: rustc
commit-hash: 07dca489ac2d933c78d3c5158e3f43beefeb02ce
commit-date: 2024-02-04
host: x86_64-unknown-linux-gnu
release: 1.76.0
LLVM version: 17.0.6

(also reproducible on very recent master 766bdce)

@erikdesjardins erikdesjardins added the C-bug Category: This is a bug. label Mar 17, 2024
@rustbot rustbot added needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. A-FFI Area: Foreign function interface (FFI) I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Mar 17, 2024
@erikdesjardins
Copy link
Contributor Author

I will open a fix for this shortly.

@erikdesjardins erikdesjardins changed the title CastTarget ABIs result in LLVM IR with out of bounds loads PassMode::Cast results in LLVM IR with out of bounds loads Mar 17, 2024
@jieyouxu jieyouxu added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Mar 17, 2024
@apiraino
Copy link
Contributor

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-high

@rustbot rustbot added P-high High priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Mar 18, 2024
@erikdesjardins
Copy link
Contributor Author

Looks like this regressed in 1.51: https://godbolt.org/z/xqdEsdaoq

@bors bors closed this as completed in bc8415b Apr 3, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Apr 3, 2024
Rollup merge of rust-lang#122619 - erikdesjardins:cast, r=compiler-errors

Fix some unsoundness with PassMode::Cast ABI

Fixes rust-lang#122617

Reviewable commit-by-commit. More info in each commit message.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-FFI Area: Foreign function interface (FFI) C-bug Category: This is a bug. I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness P-high High priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants