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

possible ASAN miscompile #121028

Closed
krasimirgg opened this issue Feb 13, 2024 · 9 comments · Fixed by #127168
Closed

possible ASAN miscompile #121028

krasimirgg opened this issue Feb 13, 2024 · 9 comments · Fixed by #127168
Labels
A-codegen Area: Code generation A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-sanitizers Area: Sanitizers for correctness and code quality C-bug Category: This is a bug. C-optimization Category: An issue highlighting optimization opportunities or PRs implementing such I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness requires-nightly This issue requires a nightly compiler in some way. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@krasimirgg
Copy link
Contributor

krasimirgg commented Feb 13, 2024

I tried this code with ASAN nightly rust (https://godbolt.org/z/KE6P9xYsY):

// Doesn't fail without -Copt-level=0

#[repr(C)] // doesn't fail without this
#[derive(Clone, Debug)]
pub enum A {
    A(i8),
}

#[repr(C)]  // doesn't fail without this
pub enum B {
    B(A),
}

// doesn't fail if extern "C" is dropped
pub extern "C" fn f(b: B) -> A {
    let a = match b {
        B::B(a) => Some(a),
    };
    a.unwrap()
}

fn main() {
    let b = B::B(A::A(3));
    // doesn't fail if the body of f is inlined.
    let a = f(b);
    println!("a: {:?}", &a);
}

I expected to see this happen: program executes successfully.

Instead, this happened: program failed with ASAN warning.

Meta

Requires nightly with -Zsanitizer=address --target x86_64-unknown-linux-gnu -Copt-level=0: https://godbolt.org/z/KE6P9xYsY.

It looks this is not a recent regression, it's present in rust from 2021.

% export dt=2021-06-10 ; rustup install nightly-$dt &&  rustc +nightly-$dt  -Zsanitizer=address --target x86_64-unknown-linux-gnu -Copt-level=0 -g main.rs && ./main
@krasimirgg krasimirgg added the C-bug Category: This is a bug. label Feb 13, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Feb 13, 2024
@krasimirgg
Copy link
Contributor Author

@cuviper could this be a dup of #75839?

@nikic nikic added the A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. label Feb 13, 2024
@cuviper
Copy link
Member

cuviper commented Feb 14, 2024

I don't know -- the symptoms look different, but UB could have the same root cause.

It looks this is not a recent regression, it's present in rust from 2021.

% export dt=2021-06-10

Is that a specific bisected date, or just one that you happened to try?

@krasimirgg
Copy link
Contributor Author

I don't know -- the symptoms look different, but UB could have the same root cause.

It looks this is not a recent regression, it's present in rust from 2021.

% export dt=2021-06-10

Is that a specific bisected date, or just one that you happened to try?
Just one I happened to try.

@nikic
Copy link
Contributor

nikic commented Feb 14, 2024

This does look the same as #75839 to me. We really shouldn't be generating aggregate loads like that.

@nikic
Copy link
Contributor

nikic commented Feb 14, 2024

I think the load is generated here:

let ty = bx.cast_backend_type(cast_ty);
bx.load(ty, llslot, self.fn_abi.ret.layout.align.abi)

@nikic
Copy link
Contributor

nikic commented Feb 14, 2024

Eh, I meant this one:

let llty = bx.cast_backend_type(ty);
llval = bx.load(llty, llval, align.min(arg.layout.align.abi));

The above is for returns rather than argument passing -- though it probably has the same/similar issue.

@jieyouxu
Copy link
Member

jieyouxu commented Feb 14, 2024

@rustbot label +requires-nightly +A-sanitizers +T-compiler +C-optimization -needs-triage

@rustbot rustbot added A-sanitizers Area: Sanitizers for correctness and code quality requires-nightly This issue requires a nightly compiler in some way. C-optimization Category: An issue highlighting optimization opportunities or PRs implementing such 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 Feb 14, 2024
@mmastrac
Copy link

Rust lint output from this testcase:

$ rustc -Cpasses=lint /tmp/test.rs
Undefined behavior: Buffer overflow
  %15 = load { i64, i32 }, ptr %8, align 4

@jieyouxu jieyouxu added A-codegen Area: Code generation I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness labels Apr 13, 2024
@DianQK
Copy link
Member

DianQK commented Jun 30, 2024

Duplicate of #75839.

bors added a commit to rust-lang-ci/rust that referenced this issue Jul 2, 2024
Use the aligned size for alloca at args/ret when the pass mode is cast

Fixes rust-lang#75839. Fixes rust-lang#121028.

The `load` and `store` instructions in LLVM access the aligned size. For example, `load { i64, i32 }` accesses 16 bytes on x86_64: https://alive2.llvm.org/ce/z/n8CHAp.

BTW, this example is expected to be optimized to immediate UB by Alive2: https://rust.godbolt.org/z/b7xK7hv1c and https://alive2.llvm.org/ce/z/vZDtZH.

r? compiler
@bors bors closed this as completed in 33b0238 Jul 2, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jul 2, 2024
Rollup merge of rust-lang#127168 - DianQK:cast-size, r=workingjubilee

Use the aligned size for alloca at args/ret when the pass mode is cast

Fixes rust-lang#75839. Fixes rust-lang#121028.

The `load` and `store` instructions in LLVM access the aligned size. For example, `load { i64, i32 }` accesses 16 bytes on x86_64: https://alive2.llvm.org/ce/z/n8CHAp.

BTW, this example is expected to be optimized to immediate UB by Alive2: https://rust.godbolt.org/z/b7xK7hv1c and https://alive2.llvm.org/ce/z/vZDtZH.

r? compiler
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-codegen Area: Code generation A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-sanitizers Area: Sanitizers for correctness and code quality C-bug Category: This is a bug. C-optimization Category: An issue highlighting optimization opportunities or PRs implementing such I-unsound Issue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/Soundness requires-nightly This issue requires a nightly compiler in some way. 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.

7 participants