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

Panic with Layout mismatch when copying! from macro #61663

Closed
mickvangelderen opened this issue Jun 8, 2019 · 16 comments · Fixed by #62055
Closed

Panic with Layout mismatch when copying! from macro #61663

mickvangelderen opened this issue Jun 8, 2019 · 16 comments · Fixed by #62055
Assignees
Labels
A-const-eval Area: constant evaluation (mir interpretation) C-bug Category: This is a bug. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ P-high High priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@mickvangelderen
Copy link
Contributor

The following code has the compiler (correctly) emit an error about how explicit enum tags with the default representation need to be of type isize. The compiler panics afterwards.

use std::convert::TryFrom;

pub trait Symbol<T> {
    const VALUE: T;
}

pub struct FALSE;

impl Symbol<i32> for FALSE {
    const VALUE: i32 = 0;
}

pub struct TRUE;

impl Symbol<i32> for TRUE {
    const VALUE: i32 = 1;
}

macro_rules! impl_symbol_enum {
    (
        $Raw: ident,
        $Enum: ident,
        $Error: ident { $(
            $Variant: ident = $Symbol: ident,
        )* }
    ) => {
        #[derive(Debug)]
        pub struct $Error($Raw);

        #[derive(Debug, Copy, Clone, Eq, PartialEq)]
        pub enum $Enum {
            $(
                $Variant = <$Symbol as Symbol<$Raw>>::VALUE,
            )*
        }

        impl TryFrom<$Raw> for $Enum {
            type Error = $Error;

            fn try_from(value: $Raw) -> Result<Self, Self::Error> {
                match value {
                    $(
                        valid if valid == <$Symbol as Symbol<$Raw>>::VALUE => Ok($Enum::$Variant),
                    )*
                    invalid => Err($Error(invalid)),
                }
            }
        }
    }
}

impl_symbol_enum! {
    i32,
    InvalidLinkStatus,
    LinkStatus {
        Unlinked = FALSE,
        Linked = TRUE,
    }
}

Output:

error[E0308]: mismatched types
  --> glw/src/glw.rs:35:28
   |
35 |                   $Variant = <$Symbol as Symbol<$Raw>>::VALUE,
   |                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected isize, found i32
...
54 | / impl_symbol_enum! {
55 | |     i32,
56 | |     InvalidLinkStatus,
57 | |     LinkStatus {
...  |
60 | |     }
61 | | }
   | |_- in this macro invocation
help: you can convert an `i32` to `isize` and panic if the converted value wouldn't fit
   |
35 |                 $Variant = <$Symbol as Symbol<$Raw>>::VALUE.try_into().unwrap(),
   |                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

thread 'rustc' panicked at 'Layout mismatch when copying!
src: OpTy {
    op: Indirect(
        MemPlace {
            ptr: AllocId(1).0x0,
            align: Align {
                pow2: 2,
            },
            meta: None,
        },
    ),
    layout: TyLayout {
        ty: i32,
        details: LayoutDetails {
            variants: Single {
                index: 0,
            },
            fields: Union(
                0,
            ),
            abi: Scalar(
                Scalar {
                    value: Int(
                        I32,
                        true,
                    ),
                    valid_range: 0..=4294967295,
                },
            ),
            align: AbiAndPrefAlign {
                abi: Align {
                    pow2: 2,
                },
                pref: Align {
                    pow2: 2,
                },
            },
            size: Size {
                raw: 4,
            },
        },
    },
}
dest: PlaceTy {
    place: Ptr(
        MemPlace {
            ptr: AllocId(0).0x0,
            align: Align {
                pow2: 3,
            },
            meta: None,
        },
    ),
    layout: TyLayout {
        ty: isize,
        details: LayoutDetails {
            variants: Single {
                index: 0,
            },
            fields: Union(
                0,
            ),
            abi: Scalar(
                Scalar {
                    value: Int(
                        I64,
                        true,
                    ),
                    valid_range: 0..=18446744073709551615,
                },
            ),
            align: AbiAndPrefAlign {
                abi: Align {
                    pow2: 3,
                },
                pref: Align {
                    pow2: 3,
                },
            },
            size: Size {
                raw: 8,
            },
        },
    },
}', src/librustc_mir/interpret/place.rs:826:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
error: aborting due to previous error

For more information about this error, try `rustc --explain E0308`.

error: internal compiler error: unexpected panic

note: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports

note: rustc 1.37.0-nightly (d132f544f 2019-06-07) running on x86_64-unknown-linux-gnu

note: compiler flags: -C debuginfo=2 -C incremental --crate-type lib

I tried to reduce the code more but this code does not cause a panic:

macro_rules! happy_compiler {
    ($T: ident, $v: expr) => {
        trait C<T> {
            const V: T;
        }

        impl C<$T> for M {
            const V: $T = $v;
        }

        enum E {
            V = <M as C<$T>>::V,
        }
    };
}

happy_compiler!(i32, 42);

I will happily move on after correcting my mistake but I thought I'd at least report this panic.

Meta

rustc --version --verbose:

rustc 1.37.0-nightly (d132f544f 2019-06-07)
binary: rustc
commit-hash: d132f544f9d74e3cc047ef211e57eae60b78e5c5
commit-date: 2019-06-07
host: x86_64-unknown-linux-gnu
release: 1.37.0-nightly
LLVM version: 8.0

Backtrace:

stack backtrace:
   0: backtrace::backtrace::libunwind::trace
             at /cargo/registry/src/git.luolix.top-1ecc6299db9ec823/backtrace-0.3.29/src/backtrace/libunwind.rs:88
   1: backtrace::backtrace::trace_unsynchronized
             at /cargo/registry/src/git.luolix.top-1ecc6299db9ec823/backtrace-0.3.29/src/backtrace/mod.rs:66
   2: std::sys_common::backtrace::_print
             at src/libstd/sys_common/backtrace.rs:47
   3: std::sys_common::backtrace::print
             at src/libstd/sys_common/backtrace.rs:36
   4: std::panicking::default_hook::{{closure}}
             at src/libstd/panicking.rs:198
   5: std::panicking::default_hook
             at src/libstd/panicking.rs:212
   6: rustc::util::common::panic_hook
   7: std::panicking::rust_panic_with_hook
             at src/libstd/panicking.rs:479
   8: std::panicking::continue_panic_fmt
             at src/libstd/panicking.rs:382
   9: std::panicking::begin_panic_fmt
             at src/libstd/panicking.rs:337
  10: rustc_mir::interpret::place::<impl rustc_mir::interpret::eval_context::InterpretCx<M>>::copy_op_no_validate
  11: rustc_mir::interpret::step::<impl rustc_mir::interpret::eval_context::InterpretCx<M>>::run
  12: rustc_mir::const_eval::eval_body_using_ecx
  13: rustc_mir::const_eval::const_eval_raw_provider
  14: rustc::ty::query::__query_compute::const_eval_raw
  15: rustc::ty::query::<impl rustc::ty::query::config::QueryAccessors for rustc::ty::query::queries::const_eval_raw>::compute
  16: rustc::dep_graph::graph::DepGraph::with_task_impl
  17: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt>::get_query
  18: rustc_mir::const_eval::const_eval_provider
  19: rustc::ty::query::__query_compute::const_eval
  20: rustc::ty::query::<impl rustc::ty::query::config::QueryAccessors for rustc::ty::query::queries::const_eval>::compute
  21: rustc::dep_graph::graph::DepGraph::with_task_impl
  22: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt>::get_query
  23: <rustc_typeck::collect::CollectItemTypesVisitor as rustc::hir::intravisit::Visitor>::visit_item
  24: rustc::hir::map::Map::visit_item_likes_in_module
  25: rustc_typeck::collect::collect_mod_item_types
  26: rustc::ty::query::__query_compute::collect_mod_item_types
  27: rustc::ty::query::<impl rustc::ty::query::config::QueryAccessors for rustc::ty::query::queries::collect_mod_item_types>::compute
  28: rustc::dep_graph::graph::DepGraph::with_task_impl
  29: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt>::get_query
  30: rustc_typeck::check_crate::{{closure}}::{{closure}}
  31: rustc::util::common::time
  32: rustc_typeck::check_crate
  33: rustc_interface::passes::analysis
  34: rustc::ty::query::__query_compute::analysis
  35: rustc::dep_graph::graph::DepGraph::with_task_impl
  36: rustc::ty::query::plumbing::<impl rustc::ty::context::TyCtxt>::get_query
  37: rustc::ty::context::tls::enter_global
  38: rustc_interface::passes::BoxedGlobalCtxt::access::{{closure}}
  39: rustc_interface::passes::create_global_ctxt::{{closure}}
  40: rustc_interface::interface::run_compiler_in_existing_thread_pool
  41: std::thread::local::LocalKey<T>::with
  42: scoped_tls::ScopedKey<T>::set
  43: syntax::with_globals
@jonas-schievink jonas-schievink added C-bug Category: This is a bug. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ I-nominated T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 8, 2019
@Centril Centril added the A-const-eval Area: constant evaluation (mir interpretation) label Jun 8, 2019
@Centril
Copy link
Contributor

Centril commented Jun 8, 2019

cc @oli-obk

@oli-obk
Copy link
Contributor

oli-obk commented Jun 8, 2019

🤣 this is a sanity check ensuring that const eval's type caching is sane. If there's a type mismatch, it won't be though. The trivial fix would be to use delay_span_bug, but that's just papering over the inconsistency.

cc @RalfJung

I am also wondering how this came to happen, const eval should not actually do anything if the typecktables of something has had errors. Or should we change this, in the spirit of not aborting compilation early, and try to evaluate things with broken types?

@RalfJung
Copy link
Member

RalfJung commented Jun 8, 2019

This reminds me of #61598. Looks like we execute CTFE on all sorts of invalid MIR? I guess that's the disadvantage of being so tightly integrated into the compiler. :/

A minimized examples that doesn't use macros would help me understand what that code even does.

@Centril Centril added the regression-from-stable-to-beta Performance or correctness regression from stable to beta. label Jun 8, 2019
@Centril
Copy link
Contributor

Centril commented Jun 8, 2019

Reduced:

struct F;
struct T;

impl F {
    const V: i32 = 0;
}

impl T {
    const V: i32 = 0;
}

macro_rules! mac {
    ($( $v: ident = $s: ident,)*) => {
        enum E {
            $( $v = $s::V, )*
        }
    }
}

mac! {
    A = F,
    B = T,
}

@Centril
Copy link
Contributor

Centril commented Jun 8, 2019

@RalfJung The macro seems to be important somehow; I cannot seem to get rid of it.

@RalfJung
Copy link
Member

RalfJung commented Jun 8, 2019

That is odd, to me that makes it sound like a bug in macro expansion instead of something we should fix in the Miri engine?

@Centril
Copy link
Contributor

Centril commented Jun 8, 2019

Yeah that's plausible.


If you change both i32 to isize it will rightly complain about having two discriminants 0. If you change one of the 0s to 1 it compiles. If you change one of the i32 to isize it doesn't ICE, if you change both i32 to usize it still ICEs.

@pnkfelix
Copy link
Member

triage: P-high. Leaving nominated and unassigned.

@pnkfelix pnkfelix added the P-high High priority label Jun 13, 2019
@pnkfelix
Copy link
Member

pnkfelix commented Jun 21, 2019

Bisected a bit.

This was injected between rustc 1.36.0-nightly (e305df1 2019-04-24) and rustc 1.36.0-nightly (3991285 2019-04-25).

Then I bisected the git log between those commits. Unfortunately rustup-install-toolchain-master had problems unpacking the archives for some of the commits in the series, but I was at least able to determine that the problem was injected by one of the following three PR's:

@pnkfelix
Copy link
Member

Bisection of the rollup indicates that the problem was injected by PR #59560.

@pnkfelix
Copy link
Member

Further bisection points to ff4d4b2 as the first bad commit

@pnkfelix
Copy link
Member

(surely subtyping requires that the layouts be compatible ... ?)

@pnkfelix
Copy link
Member

Assigning to @matthewjasper for initial investigation of a fix.

(The commit in question is also fixing an ICE, so I'm not currently suggesting that we back it out. But I do find the whole matter pretty curious, especially given the earlier comments saying that macros are somehow involved here...?)

Also assigning to self to make sure I don't lose track of this.

Removing nomination tag.

@matthewjasper
Copy link
Contributor

Relevant code:

rust/src/librustc/infer/mod.rs

Lines 1148 to 1161 in 38cd948

pub fn is_tainted_by_errors(&self) -> bool {
debug!(
"is_tainted_by_errors(err_count={}, err_count_on_creation={}, \
tainted_by_errors_flag={})",
self.tcx.sess.err_count(),
self.err_count_on_creation,
self.tainted_by_errors_flag.get()
);
if self.tcx.sess.err_count() > self.err_count_on_creation {
return true; // errors reported since this infcx was made
}
self.tainted_by_errors_flag.get()
}

This is broken because we de-duplicate identical errors. It seems like this should be exploitable in lots of other places.

Centril added a commit to Centril/rust that referenced this issue Jun 24, 2019
…=pnkfelix

Fix error counting

Count duplicate errors for `track_errors` and other error counting checks.
Add FIXMEs to make it clear that we should be moving away from this kind of logic.

Closes rust-lang#61663
@oli-obk

This comment has been minimized.

@pnkfelix

This comment has been minimized.

Centril added a commit to Centril/rust that referenced this issue Jun 25, 2019
…=pnkfelix

Fix error counting

Count duplicate errors for `track_errors` and other error counting checks.
Add FIXMEs to make it clear that we should be moving away from this kind of logic.

Closes rust-lang#61663
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-const-eval Area: constant evaluation (mir interpretation) C-bug Category: This is a bug. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ P-high High priority regression-from-stable-to-beta Performance or correctness regression from stable to beta. 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