Miri core engine: use throw_ub instead of throw_panic by RalfJung · Pull Request #66927 · rust-lang/rust

RalfJung · 2019-12-01T11:13:00Z

See #66902 for context: panicking is not really an "interpreter error", but just part of a normal Rust execution. This is a first step towards removing the InterpError::Panic variant: the core Miri engine does not use it any more.

ConstProp and ConstEval still use it, though. This will be addressed in future PRs.

From what I can tell, all the error messages this removes are actually duplicates.

r? @oli-obk @wesleywiser

wesleywiser · 2019-12-01T13:44:04Z

This is really great work cleaning up those error messages!

oli-obk · 2019-12-01T15:13:36Z

@bors r+

bors · 2019-12-01T15:13:38Z

📌 Commit 15f159a has been approved by oli-obk

bors · 2019-12-01T15:13:39Z

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

eddyb · 2019-12-02T13:43:08Z

 fn signed_offset<'tcx>(&self, val: u64, i: i64) -> InterpResult<'tcx, u64> {
 let (res, over) = self.overflowing_signed_offset(val, i128::from(i));
- if over { throw_panic!(Overflow(mir::BinOp::Add)) } else { Ok(res) }
+ if over { throw_ub!(PointerArithOverflow) } else { Ok(res) }


Ohh, so this is now specific enough to be equivalent to LLVM "inbounds GEP" overflowing, nice!

Yes that is the intention.

@oli-obk

Miri core engine: use throw_ub instead of throw_panic See rust-lang#66902 for context: panicking is not really an "interpreter error", but just part of a normal Rust execution. This is a first step towards removing the `InterpError::Panic` variant: the core Miri engine does not use it any more. ConstProp and ConstEval still use it, though. This will be addressed in future PRs. From what I can tell, all the error messages this removes are actually duplicates. r? @oli-obk @wesleywiser

RalfJung · 2019-12-05T09:58:08Z

@bors rollup=never
Let's not roll this up today as it'll likely break Miri again, and I'd like to have a nightly with Miri.

RalfJung · 2019-12-05T22:57:30Z

@bors rollup-

@oli-obk

Miri core engine: use throw_ub instead of throw_panic See rust-lang#66902 for context: panicking is not really an "interpreter error", but just part of a normal Rust execution. This is a first step towards removing the `InterpError::Panic` variant: the core Miri engine does not use it any more. ConstProp and ConstEval still use it, though. This will be addressed in future PRs. From what I can tell, all the error messages this removes are actually duplicates. r? @oli-obk @wesleywiser

bors · 2019-12-07T09:29:06Z

⌛ Testing commit 15f159a with merge 517286a2264b49996c5c79e6a224250e0dd6ffc5...

rust-highfive · 2019-12-07T11:24:30Z

The job i686-mingw-1 of your PR failed (pretty log, 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.

2019-12-07T09:30:37.4834926Z do so (now or later) by using -b with the checkout command again. Example:
2019-12-07T09:30:37.4834984Z 2019-12-07T09:30:37.4835696Z git checkout -b <new-branch-name>
2019-12-07T09:30:37.4835927Z 2019-12-07T09:30:37.4836002Z HEAD is now at 517286a22 Auto merge of #66927 - RalfJung:engines-dont-panic, r=oli-obk
2019-12-07T09:30:37.5214624Z ##[section]Starting: Setup environment
2019-12-07T09:30:37.5320147Z ==============================================================================
2019-12-07T09:30:37.5320241Z Task : Bash
2019-12-07T09:30:37.5320299Z Description : Run a Bash script on macOS, Linux, or Windows
---
2019-12-07T09:30:39.2429868Z BUILD_SOURCEBRANCHNAME=auto
2019-12-07T09:30:39.2429923Z BUILD_SOURCESDIRECTORY=D:\a\1\s
2019-12-07T09:30:39.2429997Z BUILD_SOURCEVERSION=517286a2264b49996c5c79e6a224250e0dd6ffc5
2019-12-07T09:30:39.2430077Z BUILD_SOURCEVERSIONAUTHOR=bors
2019-12-07T09:30:39.2430158Z BUILD_SOURCEVERSIONMESSAGE=Auto merge of #66927 - RalfJung:engines-dont-panic, r=oli-obk
2019-12-07T09:30:39.2430286Z CI_JOB_NAME=i686-mingw-1
2019-12-07T09:30:39.2430339Z COBERTURA_HOME=C:\cobertura-2.1.1
2019-12-07T09:30:39.2430419Z COMMONPROGRAMFILES=C:\Program Files\Common Files
2019-12-07T09:30:39.2430493Z COMMON_TESTRESULTSDIRECTORY=D:\a\1\TestResults
---
2019-12-07T09:30:39.2430781Z ChocolateyInstall=C:\ProgramData\chocolatey
2019-12-07T09:30:39.2430853Z ChromeWebDriver=C:\SeleniumWebDrivers\ChromeDriver
2019-12-07T09:30:39.2430918Z CommonProgramFiles(x86)=C:\Program Files (x86)\Common Files
2019-12-07T09:30:39.2431004Z CommonProgramW6432=C:\Program Files\Common Files
2019-12-07T09:30:39.2431089Z ConstProp and ConstEval still use it, though. This will be addressed in future PRs.
2019-12-07T09:30:39.2431228Z ENDPOINT_URL_SYSTEMVSSCONNECTION=https://dev.azure.com/rust-lang/
2019-12-07T09:30:39.2431288Z EXEPATH=C:\Program Files\Git\bin
2019-12-07T09:30:39.2431372Z From what I can tell, all the error messages this removes are actually duplicates.
2019-12-07T09:30:39.2431434Z GCM_INTERACTIVE=Never
---
2019-12-07T09:30:39.2441312Z SYSTEM_TEAMPROJECTID=e71b0ddf-dd27-435a-873c-e30f86eea377
2019-12-07T09:30:39.2441774Z SYSTEM_TIMELINEID=3f3e4b70-f97a-4e68-afaa-e8b2a8366681
2019-12-07T09:30:39.2441858Z SYSTEM_TOTALJOBSINPHASE=16
2019-12-07T09:30:39.2441944Z SYSTEM_WORKFOLDER=D:\a
2019-12-07T09:30:39.2442118Z See https://github.com/rust-lang/rust/issues/66902 for context: panicking is not really an "interpreter error", but just part of a normal Rust execution. This is a first step towards removing the `InterpError::Panic` variant: the core Miri engine does not use it any more.
2019-12-07T09:30:39.2442420Z TEMP=/tmp
2019-12-07T09:30:39.2442499Z TERM=cygwin
2019-12-07T09:30:39.2442579Z TF_BUILD=True
2019-12-07T09:30:39.2442644Z TMP=/tmp
---
2019-12-07T11:22:21.8187937Z 2019-12-07T11:22:21.8188064Z 2019-12-07T11:22:21.8906322Z failed to run: D:\a\1\s\build\bootstrap\debug\bootstrap test --exclude src/test/ui --exclude src/test/compile-fail
2019-12-07T11:22:21.8907085Z Build completed unsuccessfully in 1:42:05
2019-12-07T11:22:21.9561732Z make: *** [Makefile:89: ci-mingw-subset-1] Error 1
2019-12-07T11:22:22.0182798Z local time: Sat Dec 7 11:22:22 CUT 2019
2019-12-07T11:22:22.5221818Z network time: Sat, 07 Dec 2019 11:22:22 GMT
2019-12-07T11:22:22.5232211Z == end clock drift check ==
2019-12-07T11:22:22.6105885Z 2019-12-07T11:22:22.6105885Z 2019-12-07T11:22:23.0183985Z ##[error]Bash exited with code '2'.
2019-12-07T11:22:23.0802814Z ##[section]Starting: Checkout
2019-12-07T11:22:23.1583095Z ==============================================================================
2019-12-07T11:22:23.1583396Z Task : Get sources
2019-12-07T11:22:23.1583465Z Description : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.

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)

bors · 2019-12-07T11:24:39Z

💔 Test failed - checks-azure

wesleywiser · 2019-12-07T11:28:15Z

Actual error seems to be

Compiling compiler_builtins v0.1.18
error: failed to remove file `D:\a\1\s\build\i686-pc-windows-gnu\stage1-std\i686-pc-windows-gnu\release\deps\libcompiler_builtins-54fdf823ce6ac5da.rlib`
Caused by:
Access is denied. (os error 5)
warning: build failed, waiting for other jobs to finish...

Seems spurious?

oli-obk · 2019-12-07T11:30:29Z

@bors retry

bors · 2019-12-07T14:46:39Z

⌛ Testing commit 15f159a with merge 5c5c8eb...

@oli-obk

Miri core engine: use throw_ub instead of throw_panic See #66902 for context: panicking is not really an "interpreter error", but just part of a normal Rust execution. This is a first step towards removing the `InterpError::Panic` variant: the core Miri engine does not use it any more. ConstProp and ConstEval still use it, though. This will be addressed in future PRs. From what I can tell, all the error messages this removes are actually duplicates. r? @oli-obk @wesleywiser

bors · 2019-12-07T17:59:06Z

☀️ Test successful - checks-azure
Approved by: oli-obk
Pushing 5c5c8eb to master...

rust-highfive · 2019-12-07T18:00:31Z

📣 Toolstate changed by #66927!

Tested on commit 5c5c8eb.
Direct link to PR: #66927

💔 miri on windows: test-pass → test-fail (cc @oli-obk @eddyb @RalfJung, @rust-lang/infra).
💔 miri on linux: test-pass → test-fail (cc @oli-obk @eddyb @RalfJung, @rust-lang/infra).

@oli-obk

Tested on commit rust-lang/rust@5c5c8eb. Direct link to PR: <rust-lang/rust#66927> 💔 miri on windows: test-pass → test-fail (cc @oli-obk @eddyb @RalfJung, @rust-lang/infra). 💔 miri on linux: test-pass → test-fail (cc @oli-obk @eddyb @RalfJung, @rust-lang/infra).

Miri core engine: use throw_ub instead of throw_panic
a725a15

rust-highfive assigned oli-obk Dec 1, 2019

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 1, 2019

This was referenced Dec 1, 2019
ConstEval/ConstProp: get rid of Panic error kind #66902
Closed
Duplicated errors on UB inside a promoted #61821
Closed

RalfJung commented Dec 1, 2019
View reviewed changes

Comment threadsrc/test/ui/consts/array-literal-index-oob.stderr

This comment has been minimized.
Sign in to view

fix compile-fail tests
15f159a

RalfJung mentioned this pull request Dec 1, 2019
Describe promoted MIR quirks rust-lang/const-eval#32
Closed

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 Dec 1, 2019

eddyb reviewed Dec 2, 2019
View reviewed changes

Centril mentioned this pull request Dec 3, 2019
Rollup of 9 pull requests #66978
Closed

Centril mentioned this pull request Dec 7, 2019
Rollup of 7 pull requests #67111
Closed

bors removed the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Dec 7, 2019

bors added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 7, 2019

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 Dec 7, 2019

bors added the merged-by-bors This PR was explicitly merged by bors. label Dec 7, 2019

bors merged commit 15f159a into rust-lang:masterDec 7, 2019

rust-highfive mentioned this pull request Dec 7, 2019
miri no longer builds after rust-lang/rust#66927 #67124
Closed

RalfJung deleted the engines-dont-panic branch December 8, 2019 09:45

Uh oh!

Uh oh!

Conversation

RalfJung commented Dec 1, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

wesleywiser commented Dec 1, 2019

Uh oh!

oli-obk commented Dec 1, 2019

Uh oh!

bors commented Dec 1, 2019

Uh oh!

bors commented Dec 1, 2019

Uh oh!

eddybDec 2, 2019

Choose a reason for hiding this comment

Uh oh!

RalfJungDec 2, 2019

Choose a reason for hiding this comment

Uh oh!

RalfJung commented Dec 5, 2019

Uh oh!

RalfJung commented Dec 5, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bors commented Dec 7, 2019

Uh oh!

rust-highfive commented Dec 7, 2019

Uh oh!

bors commented Dec 7, 2019

Uh oh!

wesleywiser commented Dec 7, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oli-obk commented Dec 7, 2019

Uh oh!

bors commented Dec 7, 2019

Uh oh!

bors commented Dec 7, 2019

Uh oh!

rust-highfive commented Dec 7, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

RalfJung commented Dec 1, 2019•
edited
Loading

RalfJung commented Dec 5, 2019•
edited
Loading

wesleywiser commented Dec 7, 2019•
edited
Loading