Fix miri Discriminant() for non-ADT #49540

bjorn3 · 2018-03-31T11:58:38Z

oli-obk · 2018-03-31T12:42:11Z

src/librustc_mir/interpret/eval_context.rs

+                    }
+                    layout::Variants::Tagged { ref variants, .. } |
+                    layout::Variants::NicheFilling { ref variants, .. } => {
+                        if variants.get(discr_val as usize).is_none() {


Discriminants and variant indices don't match. You also need to adjust read_discriminant_value to return the variant index (so a usize) and adjust all calls to it to stop doing weird magic. In case of tagged encoding you always need to iterate like the old code did, but we should be doing so inside read_discriminant_value

I just noticed it too

TimNN · 2018-03-31T17:33:41Z

Your PR failed on Travis. 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.

Resolving deltas: 100% (613923/613923), completed with 4858 local objects.
---
[00:00:42] configure: rust.quiet-tests     := True
---
[00:35:49] .........................................................................i..........................
[00:35:55] ................iF..................................................................................
---
[00:36:29] ............................................................................................i.......
[00:36:35] ................................................................i...................................
---
[00:36:56] Makefile:58: recipe for target 'check' failed
[00:36:56] make: *** [check] Error 1
---
$ find $HOME/Library/Logs/DiagnosticReports -type f -name '*.crash' -not -name '*.stage2-*.crash' -not -name 'com.apple.CoreSimulator.CoreSimulatorService-*.crash' -exec printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" {} \; -exec head -750 {} \; -exec echo travis_fold":"end:crashlog \; || true
find: `/home/travis/Library/Logs/DiagnosticReports': No such file or directory
travis_time:end:090e6c4d:start=1522517618136701569,finish=1522517618143477157,duration=6775588
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:0112bb8b
$ dmesg | grep -i kill
[   10.604492] init: failsafe main process (1095) killed by TERM signal

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.

oli-obk

Just a nit.

Also please squash the commits down to a single one

oli-obk · 2018-03-31T19:25:20Z

src/librustc_mir/interpret/eval_context.rs

+            layout::Variants::Tagged { ref discr, .. } => {
+                if let ty::TyAdt(adt_def, _) = ty.sty {
+                    let raw_discr = raw_discr.to_bytes()?;
+                    println!("Read discriminant {}, valid discriminants {:?}", raw_discr, adt_def.discriminants(*self.tcx).collect::<Vec<_>>());


change this to a trace! call

bjorn3 · 2018-04-01T16:33:15Z

This is ready. Locally I have one miri run-pass test failing with a different error 🎊

oli-obk · 2018-04-01T17:28:39Z

@bors r+

bors · 2018-04-01T17:28:40Z

📌 Commit 7a28ffc has been approved by oli-obk

bors · 2018-04-01T18:58:17Z

⌛ Testing commit 7a28ffc with merge 7586c9471cb9f02357aa9cde09a92363205daa77...

bors · 2018-04-01T21:59:35Z

💔 Test failed - status-travis

TimNN · 2018-04-01T21:59:37Z

Your PR failed on Travis. 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.

The job exceeded the maximum time limit for jobs, and has been terminated.

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.

TimNN · 2018-04-01T22:00:32Z

Your PR failed on Travis. 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.

The job exceeded the maximum time limit for jobs, and has been terminated.

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.

bjorn3 · 2018-04-03T15:39:12Z

ping @oli-obk test timeout

oli-obk · 2018-04-03T16:51:35Z

@bors retry

…-obk Fix miri Discriminant() for non-ADT Fixes rust-lang#49327

Rollup of 25 pull requests Successful merges: - #49179 (Handle future deprecation annotations ) - #49512 (Add support for variant and types fields for intra links) - #49515 (fix targetted value background) - #49516 (Add missing anchor for union type fields) - #49532 (Add test for rustdoc ignore test) - #49533 (Add #[must_use] to a few standard library methods) - #49540 (Fix miri Discriminant() for non-ADT) - #49559 (Introduce Vec::resize_with method (see #41758)) - #49570 (avoid IdxSets containing garbage above the universe length) - #49577 (Stabilize String::replace_range) - #49599 (Fix typo) - #49603 (Fix url for intra link provided method) - #49607 (Stabilize iterator methods in 1.27) - #49609 (run-pass/attr-stmt-expr: expand test cases) - #49612 (Fix "since" version for getpid feature.) - #49618 (Fix build error when compiling libcore for 16bit targets) - #49619 (tweak core::fmt docs) - #49637 (Stabilize parent_id()) - #49639 (Update Cargo) - #49628 (Re-write the documentation index) - #49594 (Add some performance guidance to std::fs and std::io docs) - #49625 (miri: add public alloc_kind accessor) - #49634 (Add a test for the fix to issue #43058) - #49641 (Regression test for #46314) - #49547 (Unignore borrowck test) Failed merges:

rust-highfive assigned petrochenkov Mar 31, 2018

bjorn3 mentioned this pull request Mar 31, 2018

Rustup rust-lang/miri#364

Closed

oli-obk reviewed Mar 31, 2018

View reviewed changes

bjorn3 changed the title ~~Fix miri Discriminant() for non-ADT~~ [WIP] Fix miri Discriminant() for non-ADT Mar 31, 2018

oli-obk requested changes Mar 31, 2018

View reviewed changes

bjorn3 force-pushed the fix_miri_discriminant branch from 8267032 to cd7fafe Compare April 1, 2018 08:45

bjorn3 changed the title ~~[WIP] Fix miri Discriminant() for non-ADT~~ Fix miri Discriminant() for non-ADT Apr 1, 2018

Fix miri Discriminant() for non-ADT

7a28ffc

bjorn3 force-pushed the fix_miri_discriminant branch from cd7fafe to 7a28ffc Compare April 1, 2018 16:34

oli-obk approved these changes Apr 1, 2018

View reviewed changes

bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Apr 1, 2018

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

petrochenkov assigned oli-obk and unassigned petrochenkov Apr 1, 2018

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 Apr 3, 2018

kennytm added a commit to kennytm/rust that referenced this pull request Apr 4, 2018

Rollup merge of rust-lang#49540 - bjorn3:fix_miri_discriminant, r=oli…

b5ceda8

…-obk Fix miri Discriminant() for non-ADT Fixes rust-lang#49327

kennytm mentioned this pull request Apr 4, 2018

Rollup of 25 pull requests #49642

Merged

alexcrichton merged commit 7a28ffc into rust-lang:master Apr 5, 2018

bjorn3 deleted the fix_miri_discriminant branch April 5, 2018 17:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix miri Discriminant() for non-ADT #49540

Fix miri Discriminant() for non-ADT #49540

bjorn3 commented Mar 31, 2018

oli-obk Mar 31, 2018

bjorn3 Mar 31, 2018

TimNN commented Mar 31, 2018

oli-obk left a comment

oli-obk Mar 31, 2018

bjorn3 commented Apr 1, 2018

oli-obk commented Apr 1, 2018

bors commented Apr 1, 2018

bors commented Apr 1, 2018

bors commented Apr 1, 2018

TimNN commented Apr 1, 2018

TimNN commented Apr 1, 2018

bjorn3 commented Apr 3, 2018

oli-obk commented Apr 3, 2018

Fix miri Discriminant() for non-ADT #49540

Fix miri Discriminant() for non-ADT #49540

Conversation

bjorn3 commented Mar 31, 2018

oli-obk Mar 31, 2018

Choose a reason for hiding this comment

bjorn3 Mar 31, 2018

Choose a reason for hiding this comment

TimNN commented Mar 31, 2018

oli-obk left a comment

Choose a reason for hiding this comment

oli-obk Mar 31, 2018

Choose a reason for hiding this comment

bjorn3 commented Apr 1, 2018

oli-obk commented Apr 1, 2018

bors commented Apr 1, 2018

bors commented Apr 1, 2018

bors commented Apr 1, 2018

TimNN commented Apr 1, 2018

TimNN commented Apr 1, 2018

bjorn3 commented Apr 3, 2018

oli-obk commented Apr 3, 2018