typeck: Fix ICE with struct update syntax #50643

dlrobertson · 2018-05-11T04:19:23Z

If check_expr_struct_fields fails, do not continue to record update.
If we continue to record update, the struct may cause us to ICE later
on indexing a field that may or may not exist.

Fixes: #50618

rust-highfive · 2018-05-11T04:19:27Z

r? @eddyb

(rust_highfive has picked a reviewer for you, use r? to override)

rust-highfive · 2018-05-11T05:27:46Z

The job x86_64-gnu-llvm-3.9 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.

Check compiletest suite=compile-fail mode=compile-fail (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
[01:00:21] 
[01:00:21] running 2311 tests
[01:00:26] ....................................................................................................
[01:00:32] ................................................................F...................................
[01:00:38] .......................................F..............F.............................................
[01:00:51] ....................i............................................................ii.iii.............
[01:00:58] ....................................................................................................
[01:01:05] ..........i..............................i..........................................................
[01:01:11] ....................................................................................................
---
[01:03:20] failures:
[01:03:20] 
[01:03:20] ---- [compile-fail] compile-fail/borrowck/borrowck-field-sensitivity.rs stdout ----
[01:03:20]  
[01:03:20] error: /checkout/src/test/compile-fail/borrowck/borrowck-field-sensitivity.rs:24: expected error not found: use of moved value: `*x.b`
[01:03:20] 
[01:03:20] error: /checkout/src/test/compile-fail/borrowck/borrowck-field-sensitivity.rs:37: expected error not found: use of moved value: `x.b`
[01:03:20] 
[01:03:20] error: /checkout/src/test/compile-fail/borrowck/borrowck-field-sensitivity.rs:51: expected error not found: cannot move out of `x.b` because it is borrowed
[01:03:20] 
[01:03:20] error: /checkout/src/test/compile-fail/borrowck/borrowck-field-sensitivity.rs:72: expected error not found: use of moved value: `x.b`
[01:03:20] 
[01:03:20] error: /checkout/src/test/compile-fail/borrowck/borrowck-field-sensitivity.rs:78: expected error not found: use of moved value: `x.b`
[01:03:20] 
[01:03:20] error: /checkout/src/test/compile-fail/borrowck/borrowck-field-sensitivity.rs:84: expected error not found: use of moved value: `x.b`
[01:03:20] 
[01:03:20] error: 0 unexpected errors found, 6 expected errors not found
[01:03:20] status: exit code: 101
[01:03:20] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/compile-fail/borrowck/borrowck-field-sensitivity.rs" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/compile-fail" "--target=x86_64-unknown-linux-gnu" "--error-forma "use of moved value: `x.b`"
[01:03:20]     Error {
[01:03:20]         line_num: 84,
[01:03:20]         kind: Some(
[01:03:20]             Error
[01:03:20]             Error
[01:03:20]         ),
[01:03:20]         msg: "use of moved value: `x.b`"
[01:03:20] ]
[01:03:20] 
[01:03:20] thread '[compile-fail] compile-fail/borrowck/borrowck-field-sensitivity.rs' panicked at 'explicit panic', tools/compiletest/src/runtest.rs:1312:13
[01:03:20] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[01:03:20] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[01:03:20] 
[01:03:20] ---- [compile-fail] compile-fail/borrowck/borrowck-struct-update-with-dtor.rs stdout ----
[01:03:20]  
[01:03:20] error in revision `ast`: /checkout/src/test/compile-fail/borrowck/borrowck-struct-update-with-dtor.rs:31: expected error not found: cannot move out of type `T`, which implements the `Drop` trait
[01:03:20] 
[01:03:20] error in revision `ast`: 0 unexpected errors found, 1 expected errors not found
[01:03:20] status: exit code: 101
[01:03:20] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/compile-fail/borrowck/borrowck-struct-update-with-dtor.rs" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/compile-fail" "--target=x86_64-unknown-linux-gnu" "--cfg" "ast" "--error-format" "json" "-Zui-testing" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/compile-fail/borrowck/borrowck-struct-update-with-dtor.stage2-x86_64-unknown-linux-gnu" "-Crpath" "-O" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/compile-fail/borrowck/borrowck-struct-update-with-dtor.stage2-x86_64-unknown-linux-gnu.aux" "-A" "unused"
[01:03:20] not found errors (from test file): [
[01:03:20]     Error {
[01:03:20]         line_num: 31,
[01:03:20]         kind: Some(
[01:03:20]         ),
[01:03:20]         ),
[01:03:20]         msg: "cannot move out of type `T`, which implements the `Drop` trait"
[01:03:20] ]
[01:03:20] 
[01:03:20] thread '[compile-fail] compile-fail/borrowck/borrowck-struct-update-with-dtor.rs' panicked at 'explicit panic', tools/compiletest/src/runtest.rs:1312:13
[01:03:20] 
[01:03:20] 
[01:03:20] ---- [compile-fail] compile-fail/borrowck/borrowck-use-mut-borrow.rs stdout ----
[01:03:20]  
[01:03:20] error: /checkout/src/test/compile-fail/borrowck/borrowck-use-mut-borrow.rs:49: expected error not found: cannot use `x.a` because it was mutably borrowed
[01:03:20] 
[01:03:20] error: /checkout/src/test/compile-fail/borrowck/borrowck-use-mut-borrow.rs:57: expected error not found: cannot use `x.a` because it was mutably borrowed
[01:03:20] 
[01:03:20] error: 0 unexpected errors found, 2 expected errors not found
[01:03:20] status: exit code: 101
[01:03:20] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/compile-fail/borrowck/borrowck-use-mut-borrow.rs" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/compile-fail" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/compile-fail/borrowck/borrowck-use-mut-borrow.stage2-x86_64-unknown-linux-gnu" "-Crpath" "-O" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/compile-fail/borrowck/borrowck-use-mut-borrow.stage2-x86_64-unknown-linux-gnu.aux" "-A" "unused"
[01:03:20] not found errors (from test file): [
[01:03:20]     Error {
[01:03:20]         line_num: 49,
[01:03:20]         kind: Some(
[01:03:20]         ),
[01:03:20]         ),
[01:03:20]         msg: "cannot use `x.a` because it was mutably borrowed"
[01:03:20]     Error {
[01:03:20]         line_num: 57,
[01:03:20]         kind: Some(
[01:03:20]             Error
[01:03:20]             Error
[01:03:20]         ),
[01:03:20]         msg: "cannot use `x.a` because it was mutably borrowed"
[01:03:20] ]
[01:03:20] 
[01:03:20] thread '[compile-fail] compile-fail/borrowck/borrowck-use-mut-borrow.rs' panicked at 'explicit panic', tools/compiletest/src/runtest.rs:1312:13
[01:03:20] 
---
[01:03:20] test result: FAILED. 2293 passed; 3 failed; 15 ignored; 0 measured; 0 filtered out
[01:03:20] 
[01:03:20] 
[01:03:20] 
[01:03:20] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/compiletest" "--compile-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib" "--run-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "--rustc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "--src-base" "/checkout/src/test/compile-fail" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/compile-fail" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--mode" "compile-fail" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-3.9/bin/FileCheck" "--host-rustcflags" "-Crpath -O -Zunstable-options " "--target-rustcflags" "-Crpath -O -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python2.7" "--lldb-python" "/usr/bin/python2.7" "--gdb" "/usr/bin/gdb" "--quiet" "--llvm-version" "3.9.1\n" "--system-llvm" "--cc" "" "--cxx" "" "--cflags" "" "--llvm-components" "" "--llvm-cxxflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"
[01:03:20] 
[01:03:20] 
[01:03:20] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[01:03:20] Build completed unsuccessfully in 0:17:05
[01:03:20] Build completed unsuccessfully in 0:17:05
[01:03:20] Makefile:58: recipe for target 'check' failed
[01:03:20] make: *** [check] Error 1

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:008c38f0
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

pietroalbini · 2018-05-11T09:11:57Z

Nominating for beta, this fixes a regression.

oli-obk

Please add the test case from the issue as a ui test with a // compile-pass comment to ensure that we don't regress this in the future

oli-obk · 2018-05-11T09:16:10Z

src/librustc/middle/expr_use_visitor.rs

+                    // If typeck failed we cannot trust this expr to be valid.
+                    // Finding a index for a field cause us to panic if the
+                    // structure does not have the field.
+                    let is_mentioned = if !self.tcx().sess.has_errors() {


This check aborts in more cases than necessary (as shown by the failing tests).

You replace the any-closure's body with

let hir_id = self.tcx().hir.node_to_hir_id(f.id); self.mc.tables.field_indices().get(hir_id).cloned().map_or(true, |idx| ixd == f_index)

Which is essentially the body of the field_index function, but without the panic. Definitely keep the above comment and maybe add a note as to why we can't use the field_index function.

dlrobertson · 2018-05-12T02:15:57Z

@oli-obk I should have commented on the PR as well. I posted on the issue that after looking at this again, I'm not sure that this is the correct approach. I think the real problem is here. Which is the reason why the example only hits the ICE if struct update syntax is used. I think a better fix would be to only use the base_expr if check_expr_struct_fields doesn't hit an error.

Please add the test case from the issue as a ui test with a // compile-pass comment to ensure that we don't regress this in the future

Will do.

dlrobertson · 2018-05-12T02:16:55Z

I have a working example of this that doesn't hit the ICE. I'm just running some tests to make sure I didn't break anything else 😄

If check_expr_struct_fields fails, do not continue to record update. If we continue to record update, the struct may cause us to ICE later on indexing a field that may or may not exist.

dlrobertson · 2018-05-12T02:58:14Z

Updated to fix the problem in typeck and added a test

oli-obk · 2018-05-12T08:14:28Z

Nice! The new fix is much cleaner, I agree

@bors r+

bors · 2018-05-12T08:14:29Z

📌 Commit f6a46cf has been approved by oli-obk

kennytm · 2018-05-13T08:16:41Z

@bors p=3

bors · 2018-05-13T09:16:19Z

⌛ Testing commit f6a46cf with merge 0d48c96...

typeck: Fix ICE with struct update syntax If check_expr_struct_fields fails, do not continue to record update. If we continue to record update, the struct may cause us to ICE later on indexing a field that may or may not exist. Fixes: #50618

bors · 2018-05-13T11:32:15Z

☀️ Test successful - status-appveyor, status-travis
Approved by: oli-obk
Pushing 0d48c96 to master...

@alexcrichton

[beta] Process backports * #50622: rustc: leave space for fields of uninhabited types to allow partial initialization. * #50643: typeck: Fix ICE with struct update syntax r? @alexcrichton

rust-highfive assigned eddyb May 11, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 11, 2018

dlrobertson force-pushed the fix_ice branch from e587174 to 7d36eb1 Compare May 11, 2018 04:22

pietroalbini added the beta-nominated Nominated for backporting to the compiler in the beta channel. label May 11, 2018

oli-obk requested changes May 11, 2018

View reviewed changes

oli-obk assigned oli-obk and unassigned eddyb May 11, 2018

nagisa added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label May 11, 2018

dlrobertson changed the title ~~middle: Do not index struct fields if typck failed~~ typeck: Fix ICE with struct update syntax May 12, 2018

typeck: Fix ICE with struct update syntax

f6a46cf

If check_expr_struct_fields fails, do not continue to record update. If we continue to record update, the struct may cause us to ICE later on indexing a field that may or may not exist.

dlrobertson force-pushed the fix_ice branch from 7d36eb1 to f6a46cf Compare May 12, 2018 02:57

oli-obk approved these changes May 12, 2018

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 12, 2018

bors merged commit f6a46cf into rust-lang:master May 13, 2018

pietroalbini mentioned this pull request May 13, 2018

[beta] Process backports #50708

Merged

alexcrichton added beta-accepted Accepted for backporting to the compiler in the beta channel. and removed beta-nominated Nominated for backporting to the compiler in the beta channel. labels May 13, 2018

dlrobertson deleted the fix_ice branch May 13, 2018 17:27

kennytm mentioned this pull request May 27, 2018

Invaldily matching a struct from winnit makes rustc panic #51102

Closed

dwrensha mentioned this pull request Feb 25, 2020

internal compiler error: no type for node HirId ... #69378

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

typeck: Fix ICE with struct update syntax #50643

typeck: Fix ICE with struct update syntax #50643

dlrobertson commented May 11, 2018 •

edited

Loading

rust-highfive commented May 11, 2018

rust-highfive commented May 11, 2018

pietroalbini commented May 11, 2018

oli-obk left a comment

oli-obk May 11, 2018

dlrobertson commented May 12, 2018

dlrobertson commented May 12, 2018

dlrobertson commented May 12, 2018

oli-obk commented May 12, 2018

bors commented May 12, 2018

kennytm commented May 13, 2018

bors commented May 13, 2018

bors commented May 13, 2018

typeck: Fix ICE with struct update syntax #50643

typeck: Fix ICE with struct update syntax #50643

Conversation

dlrobertson commented May 11, 2018 • edited Loading

rust-highfive commented May 11, 2018

rust-highfive commented May 11, 2018

pietroalbini commented May 11, 2018

oli-obk left a comment

Choose a reason for hiding this comment

oli-obk May 11, 2018

Choose a reason for hiding this comment

dlrobertson commented May 12, 2018

dlrobertson commented May 12, 2018

dlrobertson commented May 12, 2018

oli-obk commented May 12, 2018

bors commented May 12, 2018

kennytm commented May 13, 2018

bors commented May 13, 2018

bors commented May 13, 2018

dlrobertson commented May 11, 2018 •

edited

Loading