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

Fix the miri tool #49933

Merged
merged 1 commit into from
Apr 24, 2018
Merged

Fix the miri tool #49933

merged 1 commit into from
Apr 24, 2018

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Apr 13, 2018

r? @eddyb

cc @bjorn3

fixes #49777

@rust-highfive
Copy link
Collaborator

warning Warning warning

  • These commits modify submodules.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 13, 2018
@oli-obk oli-obk mentioned this pull request Apr 13, 2018
let discr_val = self.read_discriminant_value(place, ty)?;
let layout = self.layout_of(ty)?;
match layout.variants {
ty::layout::Variants::Single { index } => Ok(index),
Copy link
Member

Choose a reason for hiding this comment

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

Can this early-return index, without going through read_discriminant_value?

@@ -837,13 +817,39 @@ impl<'a, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M
}
}

/// obtains the discriminant from `ByVal` or `ByValPair` and forwards to
/// read_discriminant_value for `ByRef`
pub fn read_discriminant_value_as_variant_index(
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure the comment is correct. Also, read_discriminant_as_variant_index seems slightly better as a name?

layout::Variants::Tagged { .. } => {
if discr.ty.is_signed() {
let i = raw_discr.to_bytes()? as i128;
// going from layout discriminant type to typeck discriminant type
Copy link
Member

Choose a reason for hiding this comment

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

Please call the layout/codegen/memory counterpart "tag" not "discriminant".

let discr_ty = layout::Integer::from_attr(self.tcx.tcx, discr_ty);
let amt = 128 - discr_ty.size().bits();
let truncatee = sexted as u128;
(truncatee << amt) >> amt
Copy link
Member

Choose a reason for hiding this comment

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

Are there no helpers in miri to do "integer casts" (zext/sext/trunc) without repeating these bitops all over the place?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nope ^^ I'll take my own advice and open an easy issue

let discr_val = dest_ty.ty_adt_def().unwrap()
.discriminant_for_variant(*self.tcx, variant_index)
.val;

// raw discriminants for enums are isize or bigger during
// their computation, but later shrunk to the smallest possible
// representation
Copy link
Member

Choose a reason for hiding this comment

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

This should also use "discriminant" vs "tag" terminology. Also, I wish the discr field in Variants::Tagged was called tag.

@rust-highfive
Copy link
Collaborator

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.
Resolving deltas: 100% (614347/614347), completed with 4877 local objects.
---
[00:00:11] error: Server does not allow request for unadvertised object 3f1b2bdd6895f9aedb22601bd28bc690d5a55a1e
[00:00:11] Fetched in submodule path 'src/tools/miri', but it did not contain 3f1b2bdd6895f9aedb22601bd28bc690d5a55a1e. Direct fetching of that commit failed.
---
[00:00:13] error: Server does not allow request for unadvertised object 3f1b2bdd6895f9aedb22601bd28bc690d5a55a1e
[00:00:13] Fetched in submodule path 'src/tools/miri', but it did not contain 3f1b2bdd6895f9aedb22601bd28bc690d5a55a1e. Direct fetching of that commit failed.
---
[00:00:15] error: Server does not allow request for unadvertised object 3f1b2bdd6895f9aedb22601bd28bc690d5a55a1e
[00:00:15] Fetched in submodule path 'src/tools/miri', but it did not contain 3f1b2bdd6895f9aedb22601bd28bc690d5a55a1e. Direct fetching of that commit failed.
---
[00:00:17] error: Server does not allow request for unadvertised object 3f1b2bdd6895f9aedb22601bd28bc690d5a55a1e
[00:00:17] Fetched in submodule path 'src/tools/miri', but it did not contain 3f1b2bdd6895f9aedb22601bd28bc690d5a55a1e. Direct fetching of that commit failed.
---
[00:00:18] error: Server does not allow request for unadvertised object 3f1b2bdd6895f9aedb22601bd28bc690d5a55a1e
[00:00:18] Fetched in submodule path 'src/tools/miri', but it did not contain 3f1b2bdd6895f9aedb22601bd28bc690d5a55a1e. Direct fetching of that commit failed.
[00:00:18] The command has failed after 5 attempts.
---
$ ls -lat $HOME/Library/Logs/DiagnosticReports/
ls: cannot access /home/travis/Library/Logs/DiagnosticReports/: No such file or directory
travis_time:end:35898fee:start=1523630087062695198,finish=1523630087069131913,duration=6436715
travis_fold:end:after_failure.2
travis_fold:start:after_failure.3
travis_time:start:3b914e40
$ 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:3b914e40:start=1523630087075375511,finish=1523630087081524122,duration=6148611
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:0ad3767c
$ dmesg | grep -i kill
[   11.828926] init: failsafe main process (1096) kille

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 Author

oli-obk commented Apr 14, 2018

I created a long lived branch in the miri repo which we're going to merge, not rebase or force-push in any other way.

@bors
Copy link
Contributor

bors commented Apr 19, 2018

☔ The latest upstream changes (presumably #49949) made this pull request unmergeable. Please resolve the merge conflicts.

@bors
Copy link
Contributor

bors commented Apr 21, 2018

☔ The latest upstream changes (presumably #50056) made this pull request unmergeable. Please resolve the merge conflicts.

@eddyb
Copy link
Member

eddyb commented Apr 24, 2018

@bors r+

@bors
Copy link
Contributor

bors commented Apr 24, 2018

📌 Commit 3084c06 has been approved by eddyb

@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 Apr 24, 2018
@bors
Copy link
Contributor

bors commented Apr 24, 2018

⌛ Testing commit 3084c06 with merge f5203d1...

bors added a commit that referenced this pull request Apr 24, 2018
@bors
Copy link
Contributor

bors commented Apr 24, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: eddyb
Pushing f5203d1 to master...

@bors bors merged commit 3084c06 into rust-lang:master Apr 24, 2018
@kennytm-githubbot
Copy link

📣 Toolstate changed by #49933!

Tested on commit f5203d1.
Direct link to PR: #49933

🎉 miri on windows: build-fail → test-fail.
🎉 miri on linux: build-fail → test-fail.

kennytm-githubbot added a commit to rust-lang-nursery/rust-toolstate that referenced this pull request Apr 24, 2018
Tested on commit rust-lang/rust@f5203d1.
Direct link to PR: <rust-lang/rust#49933>

🎉 miri on windows: build-fail → test-fail.
🎉 miri on linux: build-fail → test-fail.
@oli-obk oli-obk deleted the miri_rustup branch June 15, 2020 15:26
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.

miri: invalid enum discriminant value read
5 participants