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

Unsafe shenigans in constants can result in missing errors or ICE #49296

Closed
oli-obk opened this issue Mar 23, 2018 · 2 comments
Closed

Unsafe shenigans in constants can result in missing errors or ICE #49296

oli-obk opened this issue Mar 23, 2018 · 2 comments
Labels
A-const-fn Area: const fn foo(..) {..}. Pure functions which can be applied at compile time. A-diagnostics Area: Messages for errors, warnings, and lints C-bug Category: This is a bug. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@oli-obk
Copy link
Contributor

oli-obk commented Mar 23, 2018

#![feature(const_fn)]

const unsafe fn transmute<T: Copy, U: Copy>(t: T) -> U {
    union Transmute<T: Copy, U: Copy> {
        from: T,
        to: U,
    }

    Transmute { from: t }.to
}

const fn wat(x: u64) -> &'static u64 {
    unsafe { transmute(&x) }
}
const X: u64 = *wat(42);

fn main() {
    println!("{}", X);
}

produces

error[E0080]: constant evaluation error
  --> src/main.rs:18:20
   |
18 |     println!("{}", X);
   |                    ^ referenced constant has errors

without any additional info. If the value is not dereferenced, but instead fed to llvm, we get an ICE in trans:

#![feature(const_fn)]

const unsafe fn transmute<T: Copy, U: Copy>(t: T) -> U {
    union Transmute<T: Copy, U: Copy> {
        from: T,
        to: U,
    }

    Transmute { from: t }.to
}

const fn wat(x: u64) -> u64 {
    unsafe { transmute(&x) }
}
const X: u64 = wat(42);

fn main() {
    println!("{}", X);
}

results in

error: internal compiler error: librustc_mir/monomorphize/collector.rs:1131: alloc id without corresponding allocation: 39

Even though it should be reporting a dangling pointer const eval error

@oli-obk oli-obk added A-diagnostics Area: Messages for errors, warnings, and lints I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ A-const-fn Area: const fn foo(..) {..}. Pure functions which can be applied at compile time. labels Mar 23, 2018
@cuviper cuviper added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-bug Category: This is a bug. labels Mar 24, 2018
@memoryruins
Copy link
Contributor

Both samples now require #![feature(const_fn_union)]

The first reports a dereferenced dangling pointer

error: this constant cannot be used
  --> src/main.rs:16:1
   |
16 | const X: u64 = *wat(42);
   | ^^^^^^^^^^^^^^^--------^
   |                |
   |                dangling pointer was dereferenced

and the second sample no longer triggers an ICE

error[E0080]: this constant likely exhibits undefined behavior
  --> src/main.rs:16:1
   |
16 | const X: u64 = wat(42);
   | ^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a pointer, but expected initialized plain bits
   |
   = note: The rules on what exactly is undefined behavior aren't clear, so this check might be overzealous. Please open an issue on the rust compiler repository if you believe it should not be considered undefined behavior

error: aborting due to previous error

rustc 1.31.0-nightly (f99911a4a 2018-10-23)

@oli-obk
Copy link
Contributor Author

oli-obk commented Oct 25, 2018

Thanks for checking up on this!

So all we need is a ui test for the first example, because I don't think we have a regression test for that error. The second one definitely has a bunch of tests.

@oli-obk oli-obk added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. labels Oct 25, 2018
memoryruins added a commit to memoryruins/rust that referenced this issue Oct 29, 2018
kennytm added a commit to kennytm/rust that referenced this issue Oct 30, 2018
bors added a commit that referenced this issue Oct 30, 2018
Rollup of 12 pull requests

Successful merges:

 - #54885 (Don't lint 'unused_parens` on `if (break _) { .. }`)
 - #55205 (Improve a few cases of collecting to an FxHash(Map/Set))
 - #55450 (msp430: remove the whole Atomic* API)
 - #55459 (Add UI test for #49296)
 - #55472 (Use opt.take() instead of mem::replace(opt, None))
 - #55473 (Take advantage of impl Iterator in (transitive/elaborate)_bounds)
 - #55474 (Fix validation false positive)
 - #55476 (Change a flat_map with 0/1-element vecs to a filter_map)
 - #55487 (Adjust Ids of path segments in visibility modifiers)
 - #55493 (Doc fixes)
 - #55494 (borrowck=migrate must look at parents of closures)
 - #55496 (Update clippy)

Failed merges:

r? @ghost
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-fn Area: const fn foo(..) {..}. Pure functions which can be applied at compile time. A-diagnostics Area: Messages for errors, warnings, and lints C-bug Category: This is a bug. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants