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

ICE on invalid input: fn f(){; #74120

Closed
xobs opened this issue Jul 7, 2020 · 6 comments · Fixed by #74204
Closed

ICE on invalid input: fn f(){; #74120

xobs opened this issue Jul 7, 2020 · 6 comments · Fixed by #74204
Assignees
Labels
A-HIR Area: The high-level intermediate representation (HIR) C-bug Category: This is a bug. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@xobs
Copy link
Contributor

xobs commented Jul 7, 2020

Code

extern "C" {
    fn f(){;
    fn g();
}

Meta

rustc --version --verbose:

rustc 1.44.1 (c7087fe00 2020-06-17)
binary: rustc
commit-hash: c7087fe00d2ba919df1d813c040a5d47e43b0fe7
commit-date: 2020-06-17
host: x86_64-pc-windows-msvc
release: 1.44.1
LLVM version: 9.0

Also occurs on nightly at https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=b87d0d902d835c40f300071f4976ee62

Error output

   Compiling playground v0.0.1 (/playground)
error: this file contains an unclosed delimiter
 --> src/lib.rs:4:3
  |
1 | extern "C" {
  |            - unclosed delimiter
2 |     fn f(){;
  |           - this delimiter might not be properly closed...
3 |     fn g();
4 | }
  | - ^
  | |
  | ...as it matches this but it has different indentation

error: incorrect function inside `extern` block
 --> src/lib.rs:2:8
  |
1 |   extern "C" {
  |   ---------- `extern` blocks define existing foreign functions and functions inside of them cannot have a body
2 |       fn f(){;
  |  ________^__-
  | |        |
  | |        cannot have a body
3 | |     fn g();
4 | | }
  | |_- help: remove the invalid body: `;`
  |
  = help: you might have meant to write a function accessible through FFI, which can be done by writing `extern fn` outside of the `extern` block
  = note: for more information, visit https://doc.rust-lang.org/std/keyword.extern.html

error: free function without a body
 --> src/lib.rs:3:5
  |
3 |     fn g();
  |     ^^^^^^-
  |           |
  |           help: provide a definition for the function: `{ <body> }`

warning: unnecessary trailing semicolon
 --> src/lib.rs:2:12
  |
2 |     fn f(){;
  |            ^ help: remove this semicolon
  |
  = note: `#[warn(redundant_semicolons)]` on by default

thread 'rustc' panicked at 'called `Option::unwrap()` on a `None` value', src/librustc_middle/hir/map/collector.rs:155:31
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

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.44.1 (c7087fe00 2020-06-17) running on x86_64-unknown-linux-gnu

note: compiler flags: -C codegen-units=1 -C debuginfo=2 --crate-type lib

note: some of the compiler flags provided by cargo are hidden

error: aborting due to 3 previous errors; 1 warning emitted

error: could not compile `playground`.

To learn more, run the command again with --verbose.


Backtrace

thread 'rustc' panicked at 'called `Option::unwrap()` on a `None` value', src\librustc_middle\hir\map\collector.rs:155:31
stack backtrace:
   0:     0x7fff11089bdf - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::h72e337a2cec56afc
   1:     0x7fff110b747b - core::fmt::write::h301a11378dba5c10
   2:     0x7fff1107a3ac - <std::io::IoSliceMut as core::fmt::Debug>::fmt::h9e4d180d084072c2
   3:     0x7fff1108f1fc - std::panicking::take_hook::h1017141a1333a573
   4:     0x7fff1108ee3f - std::panicking::take_hook::h1017141a1333a573
   5:     0x7ffeb1e9d61a - rustc_driver::report_ice::h3f0dba12da2a5809
   6:     0x7fff1108fa98 - std::panicking::rust_panic_with_hook::h43d41cd5d682c22d
   7:     0x7fff1108f5ef - rust_begin_unwind
   8:     0x7fff110b3ed0 - core::panicking::panic_fmt::h7623003454911b6c
   9:     0x7fff110b3e1c - core::panicking::panic::hb5a07a26965dd7fe
  10:     0x7ffeb578da46 - ZN12rustc_middle2ty111_DERIVE_rustc_data_structures_stable_hasher_HashStable_rustc_middle_ich_StableHashingContext_ctx_FOR_SymbolName159_$LT$impl$u20$rustc_data_structures..stable_hasher..HashStable$LT$rustc_middle..ich..hcx..StableHashingContext$GT$$u20$
  11:     0x7ffeb57bdd69 - <rustc_middle::hir::map::Map as rustc_hir::intravisit::Map>::impl_item::h29a5725cef6a6bc2
  12:     0x7ffeb56476e6 - rustc_middle::dep_graph::<impl rustc_query_system::dep_graph::DepKind for rustc_middle::dep_graph::dep_node::DepKind>::debug_node::ha0b2381c3958984f
  13:     0x7ffeb5ac3f6e - <rustc_middle::ty::sty::TypeAndMut as core::fmt::Debug>::fmt::h14510a0c43efcc1f
  14:     0x7ffeb5a111aa - <rustc_middle::ty::sty::TypeAndMut as core::fmt::Debug>::fmt::h14510a0c43efcc1f
  15:     0x7ffeb56c7a01 - <rustc_middle::ty::cast::CastKind as core::fmt::Debug>::fmt::h90b142d735531df6
  16:     0x7ffeb56476e6 - rustc_middle::dep_graph::<impl rustc_query_system::dep_graph::DepKind for rustc_middle::dep_graph::dep_node::DepKind>::debug_node::ha0b2381c3958984f
  17:     0x7ffeb5ab16ad - <rustc_middle::ty::sty::TypeAndMut as core::fmt::Debug>::fmt::h14510a0c43efcc1f
  18:     0x7ffeb59e634e - <rustc_middle::ty::sty::TypeAndMut as core::fmt::Debug>::fmt::h14510a0c43efcc1f
  19:     0x7ffeb57bb305 - rustc_middle::hir::map::Map::expect_item::h4affa69e9dd72ff8
  20:     0x7ffeb463f102 - <rustc_passes::upvars::CaptureCollector as rustc_hir::intravisit::Visitor>::visit_expr::hd6e55112cd082a96
  21:     0x7ffeb45b2994 - rustc_passes::hir_id_validator::check_crate::hc754549cd5db125d
  22:     0x7ffeb2198782 - rustc_interface::passes::QueryContext::print_stats::h6d9e7b3318b13f88
  23:     0x7ffeb1eb265b - rustc_errors::snippet::MultilineAnnotation::increase_depth::hc7cbc289ff11bb9b
  24:     0x7ffeb202abb6 - rustc_driver::pretty::print_after_hir_lowering::h0533888ff42872ae
  25:     0x7ffeb2017a43 - rustc_driver::pretty::print_after_hir_lowering::h0533888ff42872ae
  26:     0x7ffeb1eb7192 - rustc_errors::snippet::MultilineAnnotation::increase_depth::hc7cbc289ff11bb9b
  27:     0x7ffeb205976e - rustc_driver::pretty::print_after_hir_lowering::h0533888ff42872ae
  28:     0x7ffeb202cccd - rustc_driver::pretty::print_after_hir_lowering::h0533888ff42872ae
  29:     0x7ffeb1ea647e - <rustc_middle::ty::instance::InstanceDef as rustc_middle::ty::query::keys::Key>::query_crate::hfbc139b7b5e8d887
  30:     0x7ffeb1ea3099 - <rustc_middle::ty::instance::InstanceDef as rustc_middle::ty::query::keys::Key>::query_crate::hfbc139b7b5e8d887
  31:     0x7ffeb1eab934 - rustc_errors::snippet::MultilineAnnotation::increase_depth::hc7cbc289ff11bb9b
  32:     0x7ffeb2031193 - rustc_driver::pretty::print_after_hir_lowering::h0533888ff42872ae
  33:     0x7fff1109f222 - std::sys::windows::thread::Thread::new::he0eea534f2b6df4d
  34:     0x7fff4f357bd4 - BaseThreadInitThunk
  35:     0x7fff4f64ce51 - RtlUserThreadStart

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.44.1 (c7087fe00 2020-06-17) running on x86_64-pc-windows-msvc

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

note: some of the compiler flags provided by cargo are hidden

query stack during panic:
#0 [index_hir] index HIR
#1 [hir_owner] HIR owner of `{{misc}}#0`
#2 [analysis] running analysis passes on this crate
end of query stack

This issue has been assigned to @ayazhafiz via this comment.

@xobs xobs added C-bug Category: This is a bug. 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. labels Jul 7, 2020
@jonas-schievink jonas-schievink added A-HIR Area: The high-level intermediate representation (HIR) I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Jul 7, 2020
@ratijas
Copy link
Contributor

ratijas commented Jul 7, 2020

Semicolon is not the main problem here. Standalone semicolons are allowed as items inside function body.

Second function definition fn g(); would be OK if not for the extern "C" block — AFAIK only function declarations are allowed there, so fn f() {..} is not valid syntax.

@spastorino spastorino added P-medium Medium priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Jul 7, 2020
@spastorino
Copy link
Member

Prioritized as P-medium as per Zulip discussion

@ayazhafiz
Copy link
Contributor

@rustbot claim

@rustbot rustbot self-assigned this Jul 8, 2020
@ayazhafiz
Copy link
Contributor

A possibly clearer repro: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=40cd60c27e0927eba22d2e9667f091e2

extern "C" {
    fn f() {
        fn g() {}
    }
}

@JohnTitor

This comment has been minimized.

@JohnTitor
Copy link
Member

The previous issue has been fixed, let's try again (sorry for the noise!).
@rustbot glacier "https://gist.github.com/fa3a4448239b08769f0c581dd1588f36"

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Aug 16, 2020
Don't visit foreign function bodies when lowering ast to hir

Previously the existence of bodies inside a foreign function block would
cause a panic in the hir `NodeCollector` during its collection of crate
bodies to compute a crate hash:

https://github.com/rust-lang/rust/blob/e59b08e62ea691916d2f063cac5aab4634128022/src/librustc_middle/hir/map/collector.rs#L154-L158

The collector walks the hir tree and creates a map of hir nodes, then
attaching bodies in the crate to their owner in the map. For a code like

```rust
extern "C" {
    fn f() {
        fn g() {}
    }
}
```

The crate bodies include the body of the function `g`. But foreign
functions cannot have bodies, and while the parser AST permits a foreign
function to have a body, the hir doesn't. This means that the body of
`f` is not present in the hir, and so neither is `g`. So when the
`NodeCollector` finishes the walking the hir, it has no record of `g`,
cannot find an owner for the body of `g` it sees in the crate bodies,
and blows up.

Why do the crate bodies include the body of `g`? The AST walker has a
need a for walking function bodies, and FFIs share the same AST node as
functions in other contexts.

There are at least two options to fix this:

- Don't unwrap the map entry for an hir node in the `NodeCollector`
- Modifier the ast->hir lowering visitor to ignore foreign function
  blocks

I don't think the first is preferrable, since we want to know when we
can't find a body for an hir node that we thought had one (dropping this
information may lead to an invalid hash). So this commit implements the
second option.

Closes rust-lang#74120
tmandry added a commit to tmandry/rust that referenced this issue Aug 16, 2020
Don't visit foreign function bodies when lowering ast to hir

Previously the existence of bodies inside a foreign function block would
cause a panic in the hir `NodeCollector` during its collection of crate
bodies to compute a crate hash:

https://github.com/rust-lang/rust/blob/e59b08e62ea691916d2f063cac5aab4634128022/src/librustc_middle/hir/map/collector.rs#L154-L158

The collector walks the hir tree and creates a map of hir nodes, then
attaching bodies in the crate to their owner in the map. For a code like

```rust
extern "C" {
    fn f() {
        fn g() {}
    }
}
```

The crate bodies include the body of the function `g`. But foreign
functions cannot have bodies, and while the parser AST permits a foreign
function to have a body, the hir doesn't. This means that the body of
`f` is not present in the hir, and so neither is `g`. So when the
`NodeCollector` finishes the walking the hir, it has no record of `g`,
cannot find an owner for the body of `g` it sees in the crate bodies,
and blows up.

Why do the crate bodies include the body of `g`? The AST walker has a
need a for walking function bodies, and FFIs share the same AST node as
functions in other contexts.

There are at least two options to fix this:

- Don't unwrap the map entry for an hir node in the `NodeCollector`
- Modifier the ast->hir lowering visitor to ignore foreign function
  blocks

I don't think the first is preferrable, since we want to know when we
can't find a body for an hir node that we thought had one (dropping this
information may lead to an invalid hash). So this commit implements the
second option.

Closes rust-lang#74120
@bors bors closed this as completed in 2303939 Aug 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-HIR Area: The high-level intermediate representation (HIR) C-bug Category: This is a bug. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ P-medium Medium 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.

7 participants