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

Check array indices in constant propagation #51308

Merged
merged 3 commits into from
Jun 5, 2018

Conversation

fanzier
Copy link
Contributor

@fanzier fanzier commented Jun 2, 2018

Previously, uses of constant weren't correctly propagated.
This fixes #48920.

r? @oli-obk because you suggested it

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 2, 2018
@rust-highfive
Copy link
Collaborator

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.

[00:05:08] travis_fold:start:tidy
travis_time:start:tidy
tidy check
[00:05:09] tidy error: /checkout/src/test/ui/const-eval/index_out_of_bounds.rs: missing trailing newline
[00:05:09] tidy error: /checkout/src/librustc_mir/transform/const_prop.rs:244: line longer than 100 chars
[00:05:09] tidy error: /checkout/src/librustc_mir/transform/const_prop.rs:249: line longer than 100 chars
[00:05:09] tidy error: /checkout/src/librustc_mir/transform/const_prop.rs:253: line longer than 100 chars
[00:05:09] tidy error: /checkout/src/librustc_mir/transform/const_prop.rs:274: line longer than 100 chars
[00:05:09] tidy error: /checkout/src/librustc_mir/transform/const_prop.rs:549: line longer than 100 chars
[00:05:10] some tidy checks failed
[00:05:10] 
[00:05:10] 
[00:05:10] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor" "--quiet"
[00:05:10] 
[00:05:10] 
[00:05:10] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:05:10] Build completed unsuccessfully in 0:01:52
[00:05:10] Build completed unsuccessfully in 0:01:52
[00:05:10] Makefile:79: recipe for target 'tidy' failed
[00:05:10] make: *** [tidy] Error 1

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:0113d94a
$ 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)

Copy link
Contributor

@oli-obk oli-obk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My initial hope was that just propagating the Rvalue::Len would automatically cause the index out of bounds terminator to report its error.

Did you try to go down that route?

@fanzier
Copy link
Contributor Author

fanzier commented Jun 3, 2018

Not sure what you mean with Rvalue::Len because I don't think this is emitted for arrays (just for slices). In this example, there is only a comparison of two constants (the array size and the index) and an assertion that this comparison is as expected:

    bb5: {                              
        StorageDead(_4);                 // bb5[0]: scope 0 at src/main.rs:2:37: 2:38
        StorageLive(_6);                 // bb5[1]: scope 1 at src/main.rs:3:5: 3:9
        StorageLive(_7);                 // bb5[2]: scope 1 at src/main.rs:3:7: 3:8
        _7 = const 1usize;               // bb5[3]: scope 1 at src/main.rs:3:7: 3:8
                                         // ty::Const
                                         // + ty: usize
                                         // + val: Value(Scalar(Bits { defined: 64, bits: 1 }))
                                         // mir::Constant
                                         // + span: src/main.rs:3:7: 3:8
                                         // + ty: usize
                                         // + literal: const 1usize
        _8 = const 1usize;               // bb5[4]: scope 1 at src/main.rs:3:5: 3:9
                                         // ty::Const
                                         // + ty: usize
                                         // + val: Value(Scalar(Bits { defined: 64, bits: 1 }))
                                         // mir::Constant
                                         // + span: src/main.rs:3:5: 3:9
                                         // + ty: usize
                                         // + literal: const 1usize
        _9 = Lt(_7, _8);                 // bb5[5]: scope 1 at src/main.rs:3:5: 3:9
        assert(move _9, "index out of bounds: the len is move _8 but the index is _7") -> bb6; // bb5[6]: scope 1 at src/main.rs:3:5: 3:9
    }

So I'll just look into what exactly has to be propagated here and adjust the PR. I think I understand what kind of approach you're thinking of.

@oli-obk
Copy link
Contributor

oli-obk commented Jun 3, 2018

So I'll just look into what exactly has to be propagated here and adjust the PR. I think I understand what kind of approach you're thinking of.

oh... that makes a lot of sense. So we're just failing badly at propagating those constants?

@fanzier
Copy link
Contributor Author

fanzier commented Jun 3, 2018

Yes, exactly, this code in const_prop.rs was the problem:

            // No need to overwrite an already evaluated constant
            Rvalue::Use(Operand::Constant(box Constant {
                literal: Literal::Value {
                    value: &ty::Const {
                        val: ConstVal::Value(_),
                        ..
                    },
                },
                ..
            })) => None,

Removing this makes array index checks work. I'll do slices in a separate PR. Will update the commit in a minute.

@fanzier fanzier force-pushed the const-prop-array-bounds-check branch from f34283a to 42bd288 Compare June 3, 2018 19:01
@rust-highfive
Copy link
Collaborator

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.
[00:45:13] ...........................................................................i........................
[00:45:18] ....................................................................................................
[00:45:23] ....................................................................................................
[00:45:29] ....................................................................................................
[00:45:34] ........i.................iiiiiiiii...................................................
[00:45:34] 
[00:45:34] travis_fold:start:test_ui_nll
travis_time:start:test_ui_nll
Check compiletest suite=ui mode=ui compare_mode=nll (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
---
[00:46:23] ...........................................................................i........................
[00:46:28] ....................................................................................................
[00:46:32] ....................................................................................................
[00:46:38] ....................................................................................................
[00:46:42] ........i.................iiiiiiiii...................................................
[00:46:42] 
[00:46:42]  finished in 68.693
[00:46:42] travis_fold:end:test_ui_nll

---
[00:55:53] ....................................................................................................
[00:55:58] ....................................................................................................
[00:56:03] ....................................................................................................
[00:56:08] ....................................................................................................
[00:56:15] .......................................................F...FF.........................i.............
[00:56:26] ..............................................................................i.....................
[00:56:31] .......................i............................................................................
[00:56:35] ..............................................................................................i.....
[00:56:40] ....................................................................................................
---
[00:58:03] failures:
[00:58:03] 
[00:58:03] ---- [compile-fail] compile-fail/const-err-early.rs stdout ----
[00:58:03] 
[00:58:03] error: /checkout/src/test/compile-fail/const-err-early.rs:22: unexpected error: '22:1: 22:28: this constant cannot be used [const_err]'
[00:58:03] 
[00:58:03] error: 1 unexpected errors found, 0 expected errors not found
[00:58:03] status: exit code: 101
[00:58:03] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/compile-fail/const-err-early.rs" "--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/const-err-early/a" "-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/const-err-early/auxiliary" "-A" "unused"
[00:58:03] unexpected errors (from JSON output): [
[00:58:03]     Error {
[00:58:03]         line_num: 22,
[00:58:03]         kind: Some(
[00:58:03]         ),
[00:58:03]         ),
[00:58:03]         msg: "22:1: 22:28: this constant cannot be used [const_err]"
[00:58:03] ]
[00:58:03] 
[00:58:03] thread '[compile-fail] compile-fail/const-err-early.rs' panicked at 'explicit panic', tools/compiletest/src/runtest.rs:1284:13
[00:58:03] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[00:58:03] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[00:58:03] 
[00:58:03] ---- [compile-fail] compile-fail/const-err3.rs stdout ----
[00:58:03] 
[00:58:03] error: /checkout/src/test/compile-fail/const-err3.rs:25: unexpected error: '25:14: 25:22: index out of bounds: the len is 1 but the index is 1 [const_err]'
[00:58:03] 
[00:58:03] error: 1 unexpected errors found, 0 expected errors not found
[00:58:03] status: exit code: 101
[00:58:03] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/compile-fail/const-err3.rs" "--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/const-err3/a" "-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/const-err3/auxiliary" "-A" "unused"
[00:58:03] unexpected errors (from JSON output): [
[00:58:03]     Error {
[00:58:03]         line_num: 25,
[00:58:03]         kind: Some(
[00:58:03]         ),
[00:58:03]         ),
[00:58:03]         msg: "25:14: 25:22: index out of bounds: the len is 1 but the index is 1 [const_err]"
[00:58:03] ]
[00:58:03] 
[00:58:03] thread '[compile-fail] compile-fail/const-err3.rs' panicked at 'explicit panic', tools/compiletest/src/runtest.rs:1284:13
[00:58:03] 
[00:58:03] 
[00:58:03] ---- [compile-fail] compile-fail/const-err2.rs stdout ----
[00:58:03] 
[00:58:03] error: /checkout/src/test/compile-fail/const-err2.rs:33: unexpected error: '33:14: 33:22: index out of bounds: the len is 1 but the index is 1 [const_err]'
[00:58:03] 
[00:58:03] error: 1 unexpected errors found, 0 expected errors not found
[00:58:03] status: exit code: 101
[00:58:03] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/compile-fail/const-err2.rs" "--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/const-err2/a" "-Crpath" "-O" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-O" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/compile-fail/const-err2/auxiliary" "-A" "unused"
[00:58:03] unexpected errors (from JSON output): [
[00:58:03]     Error {
[00:58:03]         line_num: 33,
[00:58:03]         kind: Some(
[00:58:03]         ),
[00:58:03]         ),
[00:58:03]         msg: "33:14: 33:22: index out of bounds: the len is 1 but the index is 1 [const_err]"
[00:58:03] ]
[00:58:03] 
[00:58:03] thread '[compile-fail] compile-fail/const-err2.rs' panicked at 'explicit panic', tools/compiletest/src/runtest.rs:1284:13
[00:58:03] 
---
[00:58:03] 
[00:58:03] thread 'main' panicked at 'Some tests failed', tools/compiletest/src/main.rs:498:22
[00:58:03] 
[00:58:03] 
[00:58:03] 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"
[00:58:03] 
[00:58:03] 
[00:58:03] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[00:58:03] Build completed unsuccessfully in 0:15:00
[00:58:03] Build completed unsuccessfully in 0:15:00
[00:58:03] Makefile:58: recipe for target 'check' failed
[00:58:03] make: *** [check] Error 1

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:03f3ee51
$ 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)

@rust-highfive
Copy link
Collaborator

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.
[00:48:51] ...........................................................................i........................
[00:48:56] ....................................................................................................
[00:49:02] ....................................................................................................
[00:49:08] ....................................................................................................
[00:49:13] ........i.................iiiiiiiii...................................................
[00:49:13] 
[00:49:13] travis_fold:start:test_ui_nll
travis_time:start:test_ui_nll
Check compiletest suite=ui mode=ui compare_mode=nll (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
---
[00:50:06] ...........................................................................i........................
[00:50:11] ....................................................................................................
[00:50:16] ....................................................................................................
[00:50:22] ....................................................................................................
[00:50:26] ........i.................iiiiiiiii...................................................
[00:50:26] 
[00:50:26]  finished in 73.493
[00:50:26] travis_fold:end:test_ui_nll

---
travis_time:start:test_run-fail
Check compiletest suite=run-fail mode=run-fail (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
[01:02:27] 
[01:02:27] running 142 tests
[01:02:38] .........................................................................FF.F.......................
[01:02:43]    |
[01:02:43] 16 |     C[10]
[01:02:43]    |     ^^^^^
[01:02:43]    |
[01:02:43]    |
[01:02:43]    = note: #[deny(const_err)] on by default
[01:02:43] error: aborting due to previous error
[01:02:43] 
[01:02:43] 
[01:02:43] ------------------------------------------
[01:02:43] ------------------------------------------
[01:02:43] 
[01:02:43] thread '[run-fail] run-fail/mir_indexing_oob_1.rs' panicked at 'explicit panic', tools/compiletest/src/runtest.rs:3096:9
[01:02:43] 
[01:02:43] ---- [run-fail] run-fail/mir_indexing_oob_2.rs stdout ----
[01:02:43] 
[01:02:43] error: compilation failed!
[01:02:43] status: exit code: 101
[01:02:43] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/run-fail/mir_indexing_oob_2.rs" "--target=x86_64-unknown-linux-gnu" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-fail/mir_indexing_oob_2/a" "-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/run-fail/mir_indexing_oob_2/auxiliary"
[01:02:43] ------------------------------------------
[01:02:43] 
[01:02:43] ------------------------------------------
[01:02:43] stderr:
[01:02:43] stderr:
[01:02:43] ------------------------------------------
[01:02:43] error: index out of bounds: the len is 5 but the index is 10
[01:02:43]   --> /checkout/src/test/run-fail/mir_indexing_oob_2.rs:16:5
[01:02:43] 16 |     C[10]
[01:02:43]    |     ^^^^^
[01:02:43]    |
[01:02:43]    |
[01:02:43]    = note: #[deny(const_err)] on by default
[01:02:43] error: abor04 ./obj/build/x86_64-unknown-linux-gnu/test/run-pass
34588 ./obj/build/x86_64-unknown-linux-gnu/native/jemalloc/lib
34372 ./obj/build/x86_64-unknown-linux-gnu/doc/core/arch
33884 ./src/llvm-emscripten/lib/Target

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)

@oli-obk
Copy link
Contributor

oli-obk commented Jun 4, 2018

You should #![allow(const_err)] in the failing run-fail tests, because while we know at compile-time that they are going to fail, the tests are supposed to test that the panic actually occurs

@fanzier fanzier force-pushed the const-prop-array-bounds-check branch from c4fb5d7 to 4db265b Compare June 4, 2018 12:06
@fanzier
Copy link
Contributor Author

fanzier commented Jun 4, 2018

Thanks, Travis is passing now

@oli-obk
Copy link
Contributor

oli-obk commented Jun 4, 2018

@bors r+

Awesome! Thanks

@bors
Copy link
Contributor

bors commented Jun 4, 2018

📌 Commit 4db265b has been approved by oli-obk

@bors 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 Jun 4, 2018
@bors
Copy link
Contributor

bors commented Jun 5, 2018

🔒 Merge conflict

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jun 5, 2018
@fanzier fanzier force-pushed the const-prop-array-bounds-check branch from 4db265b to 9600489 Compare June 5, 2018 12:16
@fanzier
Copy link
Contributor Author

fanzier commented Jun 5, 2018

Rebased but didn't find any conflicts (?)

@oli-obk
Copy link
Contributor

oli-obk commented Jun 5, 2018

yea bors is a little confused lately

@bors r+

@bors
Copy link
Contributor

bors commented Jun 5, 2018

📌 Commit 9600489 has been approved by oli-obk

@bors 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 5, 2018
Mark-Simulacrum added a commit to Mark-Simulacrum/rust that referenced this pull request Jun 5, 2018
…ck, r=oli-obk

Check array indices in constant propagation

Previously, uses of constant weren't correctly propagated.
This fixes rust-lang#48920.

r? @oli-obk because you suggested it
bors added a commit that referenced this pull request Jun 5, 2018
Rollup of 7 pull requests

Successful merges:

 - #50852 (Add doc comment to hiding portions of code example)
 - #51183 (Update rustdoc book to suggest using Termination trait instead of hidden ‘foo’ function)
 - #51255 (Fix confusing error message for sub_instant)
 - #51256 (Fix crate-name option in rustdoc)
 - #51308 (Check array indices in constant propagation)
 - #51343 (test: Ignore some problematic tests on sparc and sparc64)
 - #51358 (Tests that #39963 is fixed on MIR borrowck)

Failed merges:
@bors bors merged commit 9600489 into rust-lang:master Jun 5, 2018
@leonardo-m
Copy link

Is code like this supposed to give an error in the last Nightly?

fn main() {
    let a: [i32; 1] = [0; 1];
    a[1];
}

@oli-obk
Copy link
Contributor

oli-obk commented Jun 9, 2018

It's just a lint and not an error, but yes indeed it should

@leonardo-m
Copy link

If I compile with "rustc test.rs" I see:

error: index out of bounds: the len is 1 but the index is 1
 --> test.rs:3:5
  |
3 |     a[1];
  |     ^^^^
  |
  = note: #[deny(const_err)] on by default

If I compile with "rustc --emit=metadata test.rs", I see no errors. Is this good?

@oli-obk
Copy link
Contributor

oli-obk commented Jun 10, 2018

Ugh.... that is weird. Open an issue?

@leonardo-m
Copy link

Opened as #51491

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing index out of bounds error
5 participants