Abort if a promoted fails to be const evaluable and its runtime checks didn't trigger#52571

[00:56:18] ---- [run-pass] run-pass/invalid_const_promotion.rs stdout ----
[00:56:18] 
[00:56:18] error: test run failed!
[00:56:18] status: exit code: 101
[00:56:18] command: "/node-v9.2.0-linux-x64/bin/node" "/checkout/src/etc/wasm32-shim.js" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/invalid_const_promotion/a.wasm"
[00:56:18] stdout:
[00:56:18] ------------------------------------------
[00:56:18] 
[00:56:18] ------------------------------------------
[00:56:18] stderr:
[00:56:18] ------------------------------------------
[00:56:18] RuntimeError: unreachable
[00:56:18]     at __rust_start_panic (wasm-function[131]:1)
[00:56:18]     at rust_panic.llvm.9502800419846304078 (wasm-function[127]:30)
[00:56:18]     at std::panicking::rust_panic_with_hook::h0d5a137ad2df1a11 (wasm-function[122]:441)
[00:56:18]     at std::panicking::continue_panic_fmt::hf8718f064fd9e77b (wasm-function[120]:120)
[00:56:18]     at rust_begin_unwind (wasm-function[119]:3)
[00:56:18]     at core::panicking::panic_fmt::hb53c33b6d4981cfb (wasm-function[228]:70)
[00:56:18]     at core::result::unwrap_failed::h0ceaf4312c8cc139 (wasm-function[9]:130)
[00:56:18]     at _$LT$core..result..Result$LT$T$C$$u20$E$GT$$GT$::unwrap::h68dfea1c9fcddfb1 (wasm-function[10]:34)
[00:56:18]     at invalid_const_promotion::main::h0e4b68e8564f7e57 (wasm-function[12]:179)
[00:56:18]     at std::rt::lang_start::_$u7b$$u7b$closure$u7d$$u7d$::hceea268ccf57703c (wasm-function[5]:17)
[00:56:18]     at std::sys_common::backtrace::__rust_begin_short_backtrace::ha35b6b1b512e8e7b (wasm-function[57]:8)
[00:56:18]     at std::panicking::try::do_call::h6e356fa82a1037a1 (.llvm.9502800419846304078) (wasm-function[118]:20)
[00:56:18]     at __rust_maybe_catch_panic (wasm-function[130]:5)
[00:56:18]     at std::rt::lang_start_internal::h21c3cbc5f89250ba (wasm-function[58]:104)
[00:56:18]     at main (wasm-function[13]:33)
[00:56:18]     at Object.<anonymous> (/checkout/src/etc/wasm32-shim.js:136:20)
[00:56:18]     at Module._compile (module.js:641:30)
[00:56:18]     at Object.Module._extensions..js (module.js:652:10)
[00:56:18]     at Module.load (module.js:560:32)
[00:56:18]     at tryModuleLoad (module.js:503:12)
[00:56:18] 
[00:56:18] ------------------------------------------
[00:56:18] 
[00:56:18] thread '[run-pass] run-pass/invalid_const_promotion.rs' panicked at 'explicit panic', tools/compiletest/src/runtest.rs:3137:9
[00:56:18] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[00:56:18] 
[00:56:18] 
[00:56:18] failures:
[00:56:18]     [run-pass] run-pass/invalid_const_promotion.rs

RalfJung · 2018-07-21T23:10:31Z

@bors r=nagisa

bors · 2018-07-21T23:10:32Z

📌 Commit 555a7b4 has been approved by nagisa

RalfJung · 2018-07-22T11:49:37Z

src/test/run-pass/invalid_const_promotion.rs

+        .stdout(Stdio::piped())
+        .stdin(Stdio::piped())
+        .arg("test").output().unwrap();
+    assert!(!p.status.success());


This test would also pass if the program SEGFAULTs (like some variants of this did) instead of SIGILLs... but I am not sure how to test for this properly, so whatever.^^

rust/src/test/run-pass/segfault-no-out-of-stack.rs

Lines 26 to 40 in a5c2d0f

#[cfg(unix)]

fn check_status(status: std::process::ExitStatus)

{

use libc;

use std::os::unix::process::ExitStatusExt;

assert!(status.signal() == Some(libc::SIGSEGV)

|| status.signal() == Some(libc::SIGBUS));

}

#[cfg(not(unix))]

fn check_status(status: std::process::ExitStatus)

{

assert!(!status.success());

}

So...? That's how this test could be improved, is what you are saying?

I mean we already have copy-pastable code that does the exact checking necessary here. We should avoid writing non-trivial tests by hand when there's already a good example.

kennytm · 2018-07-21T21:07:59Z

src/test/run-pass/invalid_const_promotion.rs

+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// ignore-wasm


Sorry it should be // ignore-wasm32 😭 (or maybe // ignore-emscripten to include asm.js too, I'm not sure if asm.js can run a program)

oli-obk · 2018-07-22T12:44:45Z

@bors r=nagisa

bors · 2018-07-22T12:44:45Z

📌 Commit 233a6e1 has been approved by nagisa

bors · 2018-07-23T03:06:14Z

⌛ Testing commit 233a6e1 with merge 210d61f...

@eddyb

Abort if a promoted fails to be const evaluable and its runtime checks didn't trigger r? @eddyb cc @RalfJung @nagisa cc #49760

bors · 2018-07-23T05:03:20Z

☀️ Test successful - status-appveyor, status-travis
Approved by: nagisa
Pushing 210d61f to master...

@nikomatsakis

[beta] Abort instead of UB if promotion fails original PR: #52571 (not beta approved yet) r? @nikomatsakis cc @RalfJung

pnkfelix · 2018-07-26T14:51:12Z

landed in beta in #52624

@nikomatsakis

[beta] Abort instead of UB if promotion fails original PR: rust-lang/rust#52571 (not beta approved yet) r? @nikomatsakis cc @RalfJung

rust-highfive assigned eddyb Jul 20, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 20, 2018

Abort instead of UB if promotion fails

7064f69

oli-obk force-pushed the promotion_abort branch from 2e9e953 to 7064f69 Compare July 20, 2018 17:00

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 Jul 20, 2018

nagisa added beta-nominated Nominated for backporting to the compiler in the beta channel. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 20, 2018

Add test

92fad0f

This comment has been minimized.

Sign in to view

Add trailing newline

297ad72

kennytm mentioned this pull request Jul 21, 2018

Rollup of 11 pull requests #52588

Closed

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 Jul 21, 2018

don't spawn processes on wasm

555a7b4

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 Jul 21, 2018

nagisa mentioned this pull request Jul 22, 2018

Promoteds are statics and statics have a place, not just a value #52597

Merged

RalfJung reviewed Jul 22, 2018

View reviewed changes

kennytm reviewed Jul 22, 2018

View reviewed changes

Use correct exclusion comment

233a6e1

oli-obk mentioned this pull request Jul 22, 2018

[beta] Abort instead of UB if promotion fails #52624

Merged

bors added a commit that referenced this pull request Jul 23, 2018

Auto merge of #52571 - oli-obk:promotion_abort, r=nagisa

210d61f

Abort if a promoted fails to be const evaluable and its runtime checks didn't trigger r? @eddyb cc @RalfJung @nagisa cc #49760

bors merged commit 233a6e1 into rust-lang:master Jul 23, 2018

bors added a commit that referenced this pull request Jul 24, 2018

Auto merge of #52624 - oli-obk:promotion_abort_backport, r=nikomatsakis

0aaa819

[beta] Abort instead of UB if promotion fails original PR: #52571 (not beta approved yet) r? @nikomatsakis cc @RalfJung

RalfJung mentioned this pull request Jul 26, 2018

Segfault or wrong values from const FOO: u32 = 0 - 1; println!("{}", FOO); #50814

Closed

pnkfelix added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Jul 26, 2018

oli-obk deleted the promotion_abort branch July 26, 2018 14:50

pnkfelix removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jul 26, 2018

pnkfelix mentioned this pull request Oct 9, 2018

error: internal compiler error: Accessing (*_310) with the kind Write(Move) shouldn't be possible #54597

Closed

	#[cfg(unix)]
	fn check_status(status: std::process::ExitStatus)
	{
	use libc;
	use std::os::unix::process::ExitStatusExt;

	assert!(status.signal() == Some(libc::SIGSEGV)
	\|\| status.signal() == Some(libc::SIGBUS));
	}

	#[cfg(not(unix))]
	fn check_status(status: std::process::ExitStatus)
	{
	assert!(!status.success());
	}

Uh oh!

Conversation

oli-obk commented Jul 20, 2018

Uh oh!

nagisa commented Jul 20, 2018

Uh oh!

bors commented Jul 20, 2018

Uh oh!

nagisa commented Jul 20, 2018

Uh oh!

oli-obk commented Jul 20, 2018

Uh oh!

bors commented Jul 20, 2018

Uh oh!

This comment has been minimized.

oli-obk commented Jul 20, 2018

Uh oh!

bors commented Jul 20, 2018

Uh oh!

kennytm commented Jul 21, 2018

Uh oh!

RalfJung commented Jul 21, 2018

Uh oh!

bors commented Jul 21, 2018

Uh oh!

RalfJung Jul 22, 2018

Choose a reason for hiding this comment

Uh oh!

eddyb Jul 29, 2018

Choose a reason for hiding this comment

Uh oh!

RalfJung Jul 29, 2018

Choose a reason for hiding this comment

Uh oh!

eddyb Jul 29, 2018

Choose a reason for hiding this comment

Uh oh!

kennytm Jul 21, 2018

Choose a reason for hiding this comment

Uh oh!

oli-obk commented Jul 22, 2018

Uh oh!

bors commented Jul 22, 2018

Uh oh!

bors commented Jul 23, 2018

Uh oh!

bors commented Jul 23, 2018

Uh oh!

pnkfelix commented Jul 26, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants