Skip to content

Commit 86787f8

Browse files
borsbors
committed
auto merge of #10109 : pcmattman/rust/pass-nonzero-exit-status-on-termination-by-signal, r=alexcrichton
The UvProcess exit callback is called with a zero exit status and non-zero termination signal when a child is terminated by a signal. If a parent checks only the exit status (for example, only checks the return value from `wait()`), it may believe the process completed successfully when it actually failed. Helpers for common use-cases are in `std::rt::io::process`. Should resolve #10062.
2 parents c0b7972 + f698dec commit 86787f8

File tree

15 files changed

+202
-100
lines changed

15 files changed

+202
-100
lines changed

‎src/compiletest/procsrv.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
use std::os;
1212
use std::run;
1313
use std::str;
14+
use std::rt::io::process::ProcessExit;
1415

1516
#[cfg(target_os = "win32")]
1617
fn target_env(lib_path: &str, prog: &str) -> ~[(~str,~str)] {
@@ -39,7 +40,7 @@ fn target_env(_lib_path: &str, _prog: &str) -> ~[(~str,~str)] {
3940
os::env()
4041
}
4142

42-
pub struct Result {status: int, out: ~str, err: ~str}
43+
pub struct Result {status: ProcessExit, out: ~str, err: ~str}
4344

4445
pub fn run(lib_path: &str,
4546
prog: &str,

‎src/compiletest/runtest.rs‎

Lines changed: 32 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ use util::logv;
2323
use std::rt::io;
2424
use std::rt::io::fs;
2525
use std::rt::io::File;
26+
use std::rt::io::process;
27+
use std::rt::io::process::ProcessExit;
2628
use std::os;
2729
use std::str;
2830
use std::vec;
@@ -60,7 +62,7 @@ pub fn run_metrics(config: config, testfile: ~str, mm: &mut MetricMap) {
6062
fn run_cfail_test(config: &config, props: &TestProps, testfile: &Path) {
6163
let ProcRes = compile_test(config, props, testfile);
6264

63-
if ProcRes.status == 0 {
65+
if ProcRes.status.success() {
6466
fatal_ProcRes(~"compile-fail test compiled successfully!", &ProcRes);
6567
}
6668
@@ -81,7 +83,7 @@ fn run_rfail_test(config: &config, props: &TestProps, testfile: &Path) {
8183
let ProcRes = if !config.jit {
8284
let ProcRes = compile_test(config, props, testfile);
8385

84-
if ProcRes.status != 0 {
86+
if !ProcRes.status.success() {
8587
fatal_ProcRes(~"compilation failed!", &ProcRes);
8688
}
8789
@@ -92,7 +94,7 @@ fn run_rfail_test(config: &config, props: &TestProps, testfile: &Path) {
9294
9395
// The value our Makefile configures valgrind to return on failure
9496
static VALGRIND_ERR: int = 100;
95-
if ProcRes.status == VALGRIND_ERR {
97+
if ProcRes.status.matches_exit_status(VALGRIND_ERR) {
9698
fatal_ProcRes(~"run-fail test isn't valgrind-clean!", &ProcRes);
9799
}
98100
@@ -115,10 +117,9 @@ fn run_rfail_test(config: &config, props: &TestProps, testfile: &Path) {
115117
fn check_correct_failure_status(ProcRes: &ProcRes) {
116118
// The value the rust runtime returns on failure
117119
static RUST_ERR: int = 101;
118-
if ProcRes.status != RUST_ERR {
120+
if !ProcRes.status.matches_exit_status(RUST_ERR) {
119121
fatal_ProcRes(
120-
format!("failure produced the wrong error code: {}",
121-
ProcRes.status),
122+
format!("failure produced the wrong error: {}", ProcRes.status),
122123
ProcRes);
123124
}
124125
}
@@ -127,19 +128,19 @@ fn run_rpass_test(config: &config, props: &TestProps, testfile: &Path) {
127128
if !config.jit {
128129
let mut ProcRes = compile_test(config, props, testfile);
129130

130-
if ProcRes.status != 0 {
131+
if !ProcRes.status.success() {
131132
fatal_ProcRes(~"compilation failed!", &ProcRes);
132133
}
133134
134135
ProcRes = exec_compiled_test(config, props, testfile);
135136
136-
if ProcRes.status != 0 {
137+
if !ProcRes.status.success() {
137138
fatal_ProcRes(~"test run failed!", &ProcRes);
138139
}
139140
} else {
140141
let ProcRes = jit_test(config, props, testfile);
141142
142-
if ProcRes.status != 0 { fatal_ProcRes(~"jit failed!", &ProcRes); }
143+
if !ProcRes.status.success() { fatal_ProcRes(~"jit failed!", &ProcRes); }
143144
}
144145
}
145146
@@ -160,7 +161,7 @@ fn run_pretty_test(config: &config, props: &TestProps, testfile: &Path) {
160161
logv(config, format!("pretty-printing round {}", round));
161162
let ProcRes = print_source(config, testfile, srcs[round].clone());
162163

163-
if ProcRes.status != 0 {
164+
if !ProcRes.status.success() {
164165
fatal_ProcRes(format!("pretty-printing failed in round {}", round),
165166
&ProcRes);
166167
}
@@ -192,7 +193,7 @@ fn run_pretty_test(config: &config, props: &TestProps, testfile: &Path) {
192193
// Finally, let's make sure it actually appears to remain valid code
193194
let ProcRes = typecheck_source(config, props, testfile, actual);
194195

195-
if ProcRes.status != 0 {
196+
if !ProcRes.status.success() {
196197
fatal_ProcRes(~"pretty-printed source does not typecheck", &ProcRes);
197198
}
198199

@@ -264,7 +265,7 @@ fn run_debuginfo_test(config: &config, props: &TestProps, testfile: &Path) {
264265

265266
// compile test file (it shoud have 'compile-flags:-g' in the header)
266267
let mut ProcRes = compile_test(config, props, testfile);
267-
if ProcRes.status != 0 {
268+
if !ProcRes.status.success() {
268269
fatal_ProcRes(~"compilation failed!", &ProcRes);
269270
}
270271

@@ -375,7 +376,7 @@ fn run_debuginfo_test(config: &config, props: &TestProps, testfile: &Path) {
375376
}
376377
}
377378
378-
if ProcRes.status != 0 {
379+
if !ProcRes.status.success() {
379380
fatal(~"gdb failed to execute");
380381
}
381382
let num_check_lines = check_lines.len();
@@ -431,7 +432,7 @@ fn check_error_patterns(props: &TestProps,
431432
}
432433
}
433434
434-
if ProcRes.status == 0 {
435+
if ProcRes.status.success() {
435436
fatal(~"process did not return an error status");
436437
}
437438
@@ -473,7 +474,7 @@ fn check_expected_errors(expected_errors: ~[errors::ExpectedError],
473474
let mut found_flags = vec::from_elem(
474475
expected_errors.len(), false);
475476
476-
if ProcRes.status == 0 {
477+
if ProcRes.status.success() {
477478
fatal(~"process did not return an error status");
478479
}
479480
@@ -625,7 +626,7 @@ fn scan_string(haystack: &str, needle: &str, idx: &mut uint) -> bool {
625626
626627
struct ProcArgs {prog: ~str, args: ~[~str]}
627628
628-
struct ProcRes {status: int, stdout: ~str, stderr: ~str, cmdline: ~str}
629+
struct ProcRes {status: ProcessExit, stdout: ~str, stderr: ~str, cmdline: ~str}
629630
630631
fn compile_test(config: &config, props: &TestProps,
631632
testfile: &Path) -> ProcRes {
@@ -692,7 +693,7 @@ fn compose_and_run_compiler(
692693
|a,b| make_lib_name(a, b, testfile), &abs_ab);
693694
let auxres = compose_and_run(config, &abs_ab, aux_args, ~[],
694695
config.compile_lib_path, None);
695-
if auxres.status != 0 {
696+
if !auxres.status.success() {
696697
fatal_ProcRes(
697698
format!("auxiliary build of {} failed to compile: ",
698699
abs_ab.display()),
@@ -966,7 +967,12 @@ fn _arm_exec_compiled_test(config: &config, props: &TestProps,
966967
967968
dump_output(config, testfile, stdout_out, stderr_out);
968969
969-
ProcRes {status: exitcode, stdout: stdout_out, stderr: stderr_out, cmdline: cmdline }
970+
ProcRes {
971+
status: process::ExitStatus(exitcode),
972+
stdout: stdout_out,
973+
stderr: stderr_out,
974+
cmdline: cmdline
975+
}
970976
}
971977
972978
fn _dummy_exec_compiled_test(config: &config, props: &TestProps,
@@ -976,9 +982,9 @@ fn _dummy_exec_compiled_test(config: &config, props: &TestProps,
976982
let cmdline = make_cmdline("", args.prog, args.args);
977983
978984
match config.mode {
979-
mode_run_fail => ProcRes {status: 101, stdout: ~"",
985+
mode_run_fail => ProcRes {status: process::ExitStatus(101), stdout: ~"",
980986
stderr: ~"", cmdline: cmdline},
981-
_ => ProcRes {status: 0, stdout: ~"",
987+
_ => ProcRes {status: process::ExitStatus(0), stdout: ~"",
982988
stderr: ~"", cmdline: cmdline}
983989
}
984990
}
@@ -1099,33 +1105,33 @@ fn run_codegen_test(config: &config, props: &TestProps,
10991105
}
11001106
11011107
let mut ProcRes = compile_test_and_save_bitcode(config, props, testfile);
1102-
if ProcRes.status != 0 {
1108+
if !ProcRes.status.success() {
11031109
fatal_ProcRes(~"compilation failed!", &ProcRes);
11041110
}
11051111
11061112
ProcRes = extract_function_from_bitcode(config, props, "test", testfile, "");
1107-
if ProcRes.status != 0 {
1113+
if !ProcRes.status.success() {
11081114
fatal_ProcRes(~"extracting 'test' function failed", &ProcRes);
11091115
}
11101116
11111117
ProcRes = disassemble_extract(config, props, testfile, "");
1112-
if ProcRes.status != 0 {
1118+
if !ProcRes.status.success() {
11131119
fatal_ProcRes(~"disassembling extract failed", &ProcRes);
11141120
}
11151121
11161122
11171123
let mut ProcRes = compile_cc_with_clang_and_save_bitcode(config, props, testfile);
1118-
if ProcRes.status != 0 {
1124+
if !ProcRes.status.success() {
11191125
fatal_ProcRes(~"compilation failed!", &ProcRes);
11201126
}
11211127
11221128
ProcRes = extract_function_from_bitcode(config, props, "test", testfile, "clang");
1123-
if ProcRes.status != 0 {
1129+
if !ProcRes.status.success() {
11241130
fatal_ProcRes(~"extracting 'test' function failed", &ProcRes);
11251131
}
11261132
11271133
ProcRes = disassemble_extract(config, props, testfile, "clang");
1128-
if ProcRes.status != 0 {
1134+
if !ProcRes.status.success() {
11291135
fatal_ProcRes(~"disassembling extract failed", &ProcRes);
11301136
}
11311137

‎src/librustc/back/link.rs‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -378,9 +378,8 @@ pub mod write {
378378

379379
let prog = run::process_output(cc_prog, cc_args);
380380

381-
if prog.status != 0 {
382-
sess.err(format!("building with `{}` failed with code {}",
383-
cc_prog, prog.status));
381+
if !prog.status.success() {
382+
sess.err(format!("linking with `{}` failed: {}", cc_prog, prog.status));
384383
sess.note(format!("{} arguments: {}",
385384
cc_prog, cc_args.connect(" ")));
386385
sess.note(str::from_utf8(prog.error + prog.output));
@@ -947,11 +946,11 @@ pub fn link_binary(sess: Session,
947946

948947
// We run 'cc' here
949948
let prog = run::process_output(cc_prog, cc_args);
950-
if 0 != prog.status {
951-
sess.err(format!("linking with `{}` failed with code {}",
952-
cc_prog, prog.status));
949+
950+
if !prog.status.success() {
951+
sess.err(format!("linking with `{}` failed: {}", cc_prog, prog.status));
953952
sess.note(format!("{} arguments: {}",
954-
cc_prog, cc_args.connect(" ")));
953+
cc_prog, cc_args.connect(" ")));
955954
sess.note(str::from_utf8(prog.error + prog.output));
956955
sess.abort_if_errors();
957956
}

‎src/librustpkg/api.rs‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -159,17 +159,16 @@ pub fn build_library_in_workspace(exec: &mut workcache::Exec,
159159

160160
let all_args = flags + absolute_paths + cc_args +
161161
~[~"-o", out_name.as_str().unwrap().to_owned()];
162-
let exit_code = run::process_status(tool, all_args);
163-
if exit_code != 0 {
164-
command_failed.raise((tool.to_owned(), all_args, exit_code))
165-
}
166-
else {
162+
let exit_process = run::process_status(tool, all_args);
163+
if exit_process.success() {
167164
let out_name_str = out_name.as_str().unwrap().to_owned();
168165
exec.discover_output("binary",
169166
out_name_str,
170167
digest_only_date(&out_name));
171168
context.add_library_path(out_name.dir_path());
172169
out_name_str
170+
} else {
171+
command_failed.raise((tool.to_owned(), all_args, exit_process))
173172
}
174173
}
175174

‎src/librustpkg/conditions.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
pub use std::path::Path;
1414
pub use package_id::PkgId;
1515
pub use std::rt::io::FileStat;
16+
pub use std::rt::io::process::ProcessExit;
1617

1718
condition! {
1819
pub bad_path: (Path, ~str) -> Path;
@@ -57,5 +58,5 @@ condition! {
5758
condition! {
5859
// str is output of applying the command (first component)
5960
// to the args (second component)
60-
pub command_failed: (~str, ~[~str], int) -> ~str;
61+
pub command_failed: (~str, ~[~str], ProcessExit) -> ~str;
6162
}

‎src/librustpkg/lib.rs‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ extern mod rustc;
2626
extern mod syntax;
2727

2828
use std::{os, result, run, str, task};
29+
use std::rt::io::process;
2930
use std::hashmap::HashSet;
3031
use std::rt::io;
3132
use std::rt::io::fs;
@@ -164,14 +165,14 @@ impl<'self> PkgScript<'self> {
164165
/// is the command to pass to it (e.g., "build", "clean", "install")
165166
/// Returns a pair of an exit code and list of configs (obtained by
166167
/// calling the package script's configs() function if it exists
167-
fn run_custom(exe: &Path, sysroot: &Path) -> (~[~str], int) {
168+
fn run_custom(exe: &Path, sysroot: &Path) -> (~[~str], process::ProcessExit) {
168169
debug!("Running program: {} {} {}", exe.as_str().unwrap().to_owned(),
169170
sysroot.display(), "install");
170171
// FIXME #7401 should support commands besides `install`
171172
// FIXME (#9639): This needs to handle non-utf8 paths
172173
let status = run::process_status(exe.as_str().unwrap(),
173174
[sysroot.as_str().unwrap().to_owned(), ~"install"]);
174-
if status != 0 {
175+
if !status.success() {
175176
debug!("run_custom: first pkg command failed with {:?}", status);
176177
(~[], status)
177178
}
@@ -486,7 +487,7 @@ impl CtxMethods for BuildContext {
486487
// We always *run* the package script
487488
let (cfgs, hook_result) = PkgScript::run_custom(&Path::new(pkg_exe), &sysroot);
488489
debug!("Command return code = {:?}", hook_result);
489-
if hook_result != 0 {
490+
if !hook_result.success() {
490491
fail!("Error running custom build command")
491492
}
492493
custom = true;
@@ -697,7 +698,7 @@ impl CtxMethods for BuildContext {
697698
debug!("test: test_exec = {}", test_exec.display());
698699
// FIXME (#9639): This needs to handle non-utf8 paths
699700
let status = run::process_status(test_exec.as_str().unwrap(), [~"--test"]);
700-
if status != 0 {
701+
if !status.success() {
701702
fail!("Some tests failed");
702703
}
703704
}

‎src/librustpkg/source_control.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ pub fn safe_git_clone(source: &Path, v: &Version, target: &Path) -> CloneResult
3636
let outp = run::process_output("git", [~"clone",
3737
source.as_str().unwrap().to_owned(),
3838
target.as_str().unwrap().to_owned()]);
39-
if outp.status != 0 {
39+
if !outp.status.success() {
4040
println(str::from_utf8_owned(outp.output.clone()));
4141
println(str::from_utf8_owned(outp.error));
4242
return DirToUse(target.clone());
@@ -52,7 +52,7 @@ pub fn safe_git_clone(source: &Path, v: &Version, target: &Path) -> CloneResult
5252
[format!("--work-tree={}", target.as_str().unwrap().to_owned()),
5353
format!("--git-dir={}", git_dir.as_str().unwrap().to_owned()),
5454
~"checkout", format!("{}", *s)]);
55-
if outp.status != 0 {
55+
if !outp.status.success() {
5656
println(str::from_utf8_owned(outp.output.clone()));
5757
println(str::from_utf8_owned(outp.error));
5858
return DirToUse(target.clone());
@@ -73,7 +73,7 @@ pub fn safe_git_clone(source: &Path, v: &Version, target: &Path) -> CloneResult
7373
format!("--git-dir={}", git_dir.as_str().unwrap().to_owned()),
7474
~"pull", ~"--no-edit", source.as_str().unwrap().to_owned()];
7575
let outp = run::process_output("git", args);
76-
assert!(outp.status == 0);
76+
assert!(outp.status.success());
7777
}
7878
CheckedOutSources
7979
} else {
@@ -110,7 +110,7 @@ pub fn git_clone_url(source: &str, target: &Path, v: &Version) {
110110
// FIXME (#9639): This needs to handle non-utf8 paths
111111
let outp = run::process_output("git", [~"clone", source.to_owned(),
112112
target.as_str().unwrap().to_owned()]);
113-
if outp.status != 0 {
113+
if !outp.status.success() {
114114
debug!("{}", str::from_utf8_owned(outp.output.clone()));
115115
debug!("{}", str::from_utf8_owned(outp.error));
116116
cond.raise((source.to_owned(), target.clone()))
@@ -120,7 +120,7 @@ pub fn git_clone_url(source: &str, target: &Path, v: &Version) {
120120
&ExactRevision(ref s) | &Tagged(ref s) => {
121121
let outp = process_output_in_cwd("git", [~"checkout", s.to_owned()],
122122
target);
123-
if outp.status != 0 {
123+
if !outp.status.success() {
124124
debug!("{}", str::from_utf8_owned(outp.output.clone()));
125125
debug!("{}", str::from_utf8_owned(outp.error));
126126
cond.raise((source.to_owned(), target.clone()))

0 commit comments

Comments
 (0)