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

Internal Compiler Error when using extern types #126814

Closed
lolbinarycat opened this issue Jun 21, 2024 · 7 comments · Fixed by #126833
Closed

Internal Compiler Error when using extern types #126814

lolbinarycat opened this issue Jun 21, 2024 · 7 comments · Fixed by #126833
Labels
C-bug Category: This is a bug. F-extern_types `#![feature(extern_types)]` 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

@lolbinarycat
Copy link
Contributor

code that caused the ICE: https://codeberg.org/binarycat/counted_str/src/commit/e04084bfcb9f33dcf5fc44ffb84ca3684e3496b6

backtrace: rustc-ice-2024-06-21T21_39_57-71546.txt

@lolbinarycat
Copy link
Contributor Author

it looks like the problem arises when transmuting in as_counted_str, i'll try to do a minimal repo.

@lolbinarycat
Copy link
Contributor Author

minimal repro found

#![feature(extern_types)]
#![allow(dead_code)]

extern {
    type Opaque;
}

struct ThinDst {
    x: u8,
    tail: Opaque,
}

const fn t<const N: usize>(x: &[u8; N]) -> &ThinDst {
    unsafe { std::mem::transmute(x.as_ptr()) }
}

const C1: &ThinDst = t(b"d");

fn main() {} // required because "miri build" does not exist

@saethlin saethlin transferred this issue from rust-lang/miri Jun 21, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jun 21, 2024
@saethlin
Copy link
Member

I've transferred this issue because it's not a bug in Miri, it's a bug in the compiler overall. Miri is the compiler, until main starts. But in your repro, there's nothing in main. So this is just compiler.

@saethlin saethlin 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. F-extern_types `#![feature(extern_types)]` C-bug Category: This is a bug. labels Jun 21, 2024
@saethlin
Copy link
Member

Smaller:

#![feature(extern_types)]

extern {
    type Opaque;
}

struct ThinDst {
    x: u8,
    tail: Opaque,
}

const C1: &ThinDst = std::mem::transmute(b"d".as_ptr());

@RalfJung
Copy link
Member

ICE message (which as cut off from the original backtrace -- please pots the full error message, not just a pat of it :)

error: internal compiler error: compiler/rustc_const_eval/src/interpret/validity.rs:1039:17: Unexpected error during validation: `extern type` does not have a known offset

We should really reject extern types being used like that, they violate fundamental language invariants...

@RalfJung
Copy link
Member

Note that this is not a good idea IMO:

// use extern types to satisfy stacked borrow rules.
#[cfg(miri)]
extern {
	type TrailingData;
}

Using extern types basically intimidates Miri into not checking your code, but that means Miri may miss bugs.

A better fix is to check your code with -Zmiri-tree-borrows, which has proper support for references that only cover the "header" of a struct. Tree Borrows should still find all UB bugs that can affect your code today, but in the future it's likely we are going to pick aliasing rules that are stricter than Tree Borrows (and weaker than Stacked Borrows).

@lolbinarycat
Copy link
Contributor Author

Note that this is not a good idea IMO:

// use extern types to satisfy stacked borrow rules.
#[cfg(miri)]
extern {
	type TrailingData;
}

Using extern types basically intimidates Miri into not checking your code, but that means Miri may miss bugs.

A better fix is to check your code with -Zmiri-tree-borrows, which has proper support for references that only cover the "header" of a struct. Tree Borrows should still find all UB bugs that can affect your code today, but in the future it's likely we are going to pick aliasing rules that are stricter than Tree Borrows (and weaker than Stacked Borrows).

I used extern types because it's what URLO suggested to accurately represent my semantics. it's a DST that exists behind a thin pointer. unfortunatly the "unknown alignment" part isn't really what i wanted. apparently tree borrows are weak enough that it allows writing to a zero-sized field past the end of a struct, so long as its backed by a larger allocation.

compiler-errors added a commit to compiler-errors/rust that referenced this issue Jun 23, 2024
…compiler-errors

don't ICE when encountering an extern type field during validation

"extern type" is a pain that keeps on giving...

Fixes rust-lang#126814

r? `@oli-obk`
compiler-errors added a commit to compiler-errors/rust that referenced this issue Jun 23, 2024
…compiler-errors

don't ICE when encountering an extern type field during validation

"extern type" is a pain that keeps on giving...

Fixes rust-lang#126814

r? ``@oli-obk``
@saethlin saethlin removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jun 23, 2024
@bors bors closed this as completed in a9959bd Jun 23, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jun 23, 2024
Rollup merge of rust-lang#126833 - RalfJung:extern-type-field-ice, r=compiler-errors

don't ICE when encountering an extern type field during validation

"extern type" is a pain that keeps on giving...

Fixes rust-lang#126814

r? ```@oli-obk```
github-actions bot pushed a commit to rust-lang/miri that referenced this issue Jun 24, 2024
…errors

don't ICE when encountering an extern type field during validation

"extern type" is a pain that keeps on giving...

Fixes rust-lang/rust#126814

r? ```@oli-obk```
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jun 28, 2024
core: avoid `extern type`s in formatting infrastructure

`@RalfJung` [said](https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/Use.20of.20.60extern.20type.60.20in.20formatting.20machinery/near/446552837):

>How attached are y'all to using `extern type` in the formatting machinery?
Seems like this was introduced a [long time ago](rust-lang@34ef8f5). However, it's also [not really compatible with Stacked Borrows](rust-lang/unsafe-code-guidelines#256), and only works currently because we effectively treat references-to-extern-type almost like raw pointers in Stacked Borrows -- which of course is unsound, it's not how LLVM works. I was planning to make Miri emit a warning when this happens to avoid cases like [this](rust-lang#126814 (comment)) where people use extern type specifically to silence Miri without realizing what happens. but with the formatting machinery using  extern type, this warning would just show up everywhere...
>
> The "proper" way to do this in Stacked Borrows is to use raw pointers (or `NonNull`).

This PR does just that.

r? `@RalfJung`
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jun 28, 2024
core: avoid `extern type`s in formatting infrastructure

``@RalfJung`` [said](https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/Use.20of.20.60extern.20type.60.20in.20formatting.20machinery/near/446552837):

>How attached are y'all to using `extern type` in the formatting machinery?
Seems like this was introduced a [long time ago](rust-lang@34ef8f5). However, it's also [not really compatible with Stacked Borrows](rust-lang/unsafe-code-guidelines#256), and only works currently because we effectively treat references-to-extern-type almost like raw pointers in Stacked Borrows -- which of course is unsound, it's not how LLVM works. I was planning to make Miri emit a warning when this happens to avoid cases like [this](rust-lang#126814 (comment)) where people use extern type specifically to silence Miri without realizing what happens. but with the formatting machinery using  extern type, this warning would just show up everywhere...
>
> The "proper" way to do this in Stacked Borrows is to use raw pointers (or `NonNull`).

This PR does just that.

r? ``@RalfJung``
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Jun 28, 2024
core: avoid `extern type`s in formatting infrastructure

```@RalfJung``` [said](https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/Use.20of.20.60extern.20type.60.20in.20formatting.20machinery/near/446552837):

>How attached are y'all to using `extern type` in the formatting machinery?
Seems like this was introduced a [long time ago](rust-lang@34ef8f5). However, it's also [not really compatible with Stacked Borrows](rust-lang/unsafe-code-guidelines#256), and only works currently because we effectively treat references-to-extern-type almost like raw pointers in Stacked Borrows -- which of course is unsound, it's not how LLVM works. I was planning to make Miri emit a warning when this happens to avoid cases like [this](rust-lang#126814 (comment)) where people use extern type specifically to silence Miri without realizing what happens. but with the formatting machinery using  extern type, this warning would just show up everywhere...
>
> The "proper" way to do this in Stacked Borrows is to use raw pointers (or `NonNull`).

This PR does just that.

r? ```@RalfJung```
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Jun 28, 2024
Rollup merge of rust-lang#126956 - joboet:fmt_no_extern_ty, r=RalfJung

core: avoid `extern type`s in formatting infrastructure

```@RalfJung``` [said](https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/Use.20of.20.60extern.20type.60.20in.20formatting.20machinery/near/446552837):

>How attached are y'all to using `extern type` in the formatting machinery?
Seems like this was introduced a [long time ago](rust-lang@34ef8f5). However, it's also [not really compatible with Stacked Borrows](rust-lang/unsafe-code-guidelines#256), and only works currently because we effectively treat references-to-extern-type almost like raw pointers in Stacked Borrows -- which of course is unsound, it's not how LLVM works. I was planning to make Miri emit a warning when this happens to avoid cases like [this](rust-lang#126814 (comment)) where people use extern type specifically to silence Miri without realizing what happens. but with the formatting machinery using  extern type, this warning would just show up everywhere...
>
> The "proper" way to do this in Stacked Borrows is to use raw pointers (or `NonNull`).

This PR does just that.

r? ```@RalfJung```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. F-extern_types `#![feature(extern_types)]` 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

Successfully merging a pull request may close this issue.

4 participants