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

Don't check interpret_interner when accessing a static to fix miri mutable statics #49216

Merged
merged 2 commits into from
Mar 22, 2018

Conversation

bjorn3
Copy link
Member

@bjorn3 bjorn3 commented Mar 20, 2018

Mutable statics don't work in my PR to fix the standalone miri, as init_static didn't get called when the interpret_interner already contained a entry for the static, which is always immutable.

cc rust-lang/miri#364

@rust-highfive
Copy link
Collaborator

r? @estebank

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 20, 2018
@estebank
Copy link
Contributor

Test failure:

[00:42:07] ---- [ui] ui/const-eval/issue-47971.rs stdout ----
[00:42:07] 	
[00:42:07] error: test compilation failed although it shouldn't!
[00:42:07] status: exit code: 101
[00:42:07] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/const-eval/issue-47971.rs" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/const-eval/issue-47971.stage2-x86_64-unknown-linux-gnu" "-Crpath" "-O" "-Zmiri" "-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/ui/const-eval/issue-47971.stage2-x86_64-unknown-linux-gnu.aux" "-A" "unused"
[00:42:07] stdout:
[00:42:07] ------------------------------------------
[00:42:07] 
[00:42:07] ------------------------------------------
[00:42:07] stderr:
[00:42:07] ------------------------------------------
[00:42:07] {"message":"cyclic dependency detected","code":{"code":"E0391","explanation":"\nThis error indicates that some types or traits depend on each other\nand therefore cannot be constructed.\n\nThe following example contains a circular dependency between two traits:\n\n```compile_fail,E0391\ntrait FirstTrait : SecondTrait {\n\n}\n\ntrait SecondTrait : FirstTrait {\n\n}\n```\n"},"level":"error","spans":[{"file_name":"/checkout/src/test/ui/const-eval/issue-47971.rs","byte_start":614,"byte_end":616,"line_start":19,"line_end":19,"column_start":19,"column_end":21,"is_primary":true,"text":[{"text":"static T: S = S(g(&T), 0);","highlight_start":19,"highlight_end":21}],"label":"cyclic reference","suggested_replacement":null,"expansion":null}],"children":[{"message":"the cycle begins when const-evaluating `T`...","code":null,"level":"note","spans":[{"file_name":"/checkout/src/test/ui/const-eval/issue-47971.rs","byte_start":596,"byte_end":622,"line_start":19,"line_end":19,"column_start":1,"column_end":27,"is_primary":true,"text":[{"text":"static T: S = S(g(&T), 0);","highlight_start":1,"highlight_end":27}],"label":null,"suggested_replacement":null,"expansion":null}],"children":[],"rendered":null},{"message":"...which then again requires const-evaluating `T`, completing the cycle.","code":null,"level":"note","spans":[],"children":[],"rendered":null}],"rendered":"error[E0391]: cyclic dependency detected\n  --> /checkout/src/test/ui/const-eval/issue-47971.rs:19:19\n   |\nLL | static T: S = S(g(&T), 0);\n   |                   ^^ cyclic reference\n   |\nnote: the cycle begins when const-evaluating `T`...\n  --> /checkout/src/test/ui/const-eval/issue-47971.rs:19:1\n   |\nLL | static T: S = S(g(&T), 0);\n   | ^^^^^^^^^^^^^^^^^^^^^^^^^^\n   = note: ...which then again requires const-evaluating `T`, completing the cycle.\n\n"}
[00:42:07] {"message":"aborting due to previous error","code":null,"level":"error","spans":[],"children":[],"rendered":"error: aborting due to previous error\n\n"}
[00:42:07] {"message":"For more information about this error, try `rustc --explain E0391`.","code":null,"level":"","spans":[],"children":[],"rendered":"For more information about this error, try `rustc --explain E0391`.\n"}
[00:42:07] 
[00:42:07] ------------------------------------------
[00:42:07] 
[00:42:07] thread '[ui] ui/const-eval/issue-47971.rs' panicked at 'explicit panic', tools/compiletest/src/runtest.rs:2903:9
[00:42:07] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[00:42:07] 
[00:42:07] 
[00:42:07] failures:
[00:42:07]     [ui] ui/const-eval/issue-47971.rs
[00:42:07] 
[00:42:07] test result: FAILED. 1281 passed; 1 failed; 4 ignored; 0 measured; 0 filtered out
[00:42:07] 

@oli-obk
Copy link
Contributor

oli-obk commented Mar 21, 2018

I think you need to adjust https://github.com/rust-lang/rust/blob/master/src/librustc_mir/interpret/const_eval.rs#L343 to check whether the static has already been computed. If so, don't call tcx const_eval.

@bjorn3
Copy link
Member Author

bjorn3 commented Mar 21, 2018

@oli-obk the problem is that I use a custom init_static to call const_eval and copy it to a mutable alloc which is returned, however the second time the static Place is evaluated eval_place shortcuts and just returns the immutably cached static from const_eval without even calling init_static.

@oli-obk
Copy link
Contributor

oli-obk commented Mar 21, 2018

Oh I get that. I meant rustc's init static impl. The test failure is from that

@bjorn3
Copy link
Member Author

bjorn3 commented Mar 21, 2018

I get it

@bjorn3 bjorn3 force-pushed the patch-1 branch 2 times, most recently from 34ff964 to 5f73901 Compare March 21, 2018 13:40
@bjorn3
Copy link
Member Author

bjorn3 commented Mar 21, 2018

Travis is green

@estebank
Copy link
Contributor

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Mar 21, 2018

📌 Commit 5aa29c4 has been approved by estebank

@bors
Copy link
Contributor

bors commented Mar 21, 2018

🌲 The tree is currently closed for pull requests below priority 30, this pull request will be tested once the tree is reopened

@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 Mar 21, 2018
kennytm added a commit to kennytm/rust that referenced this pull request Mar 21, 2018
Don't check interpret_interner when accessing a static to fix miri mutable statics

Mutable statics don't work in my PR to fix the standalone [miri](https://github.com/solson/miri), as init_static didn't get called when the interpret_interner already contained a entry for the static, which is always immutable.

cc rust-lang/miri#364
kennytm added a commit to kennytm/rust that referenced this pull request Mar 22, 2018
Don't check interpret_interner when accessing a static to fix miri mutable statics

Mutable statics don't work in my PR to fix the standalone [miri](https://github.com/solson/miri), as init_static didn't get called when the interpret_interner already contained a entry for the static, which is always immutable.

cc rust-lang/miri#364
bors added a commit that referenced this pull request Mar 22, 2018
@alexcrichton alexcrichton merged commit 5aa29c4 into rust-lang:master Mar 22, 2018
@bjorn3 bjorn3 deleted the patch-1 branch March 23, 2018 06:28
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.

6 participants