[m-rev.] for review: Handle exit codes of forked processes on windows safely.
Paul Bone
paul at bone.id.au
Wed Feb 18 11:57:13 AEDT 2015
Branches: version-14_01-branch, master
For review by anyone
---
Handle exit codes of forked processes on windows safely.
MSVC doesn't provide the WIFEXITED and other macros that help a programmer
determine if a child process exited with an error message or was killed by a
signal. In these cases Mercury assumes that positive numbers are exit codes
and negative numbers mean that a process was killed by a signal.
When "cl" exited with errors, returning the error code -2, mmc --make
thought that it was killed by a signal and failed to display the compilation
errors.
This patch removes the existing code that tries to detect signals when these
macros are not available. As far as I know platforms like windows don't
have a concept of SysV signals, therefore signals can only occur when using
cygwin or some other compatibility layer which will then provide the
WIFEXITED and other macros.
I considered using the windows API directly however it didn't (as far as I
could find) provide any concept of signals, so I rejected this idea.
library/io.m:
As above.
Also avoid the use of magic numbers such as 127 for failure.
Rename the handle_system_command_exit_status/1 function to
decode_system_command_exit_code,
compiler/process_util.m:
Conform to changes in io.m
compiler/module_cmds.m:
If a subprocess is killed by a signal then report this both to the
desired stream (the error stream for the subtask) and to standard
output.
After reporting the error then re-raise the signal, not before.
compiler/file_util.m:
Flush output after reporting an error to make sure that it is seen.
---
compiler/file_util.m | 1 +
compiler/module_cmds.m | 10 +++-
compiler/process_util.m | 6 +--
library/io.m | 126 +++++++++++++++++++++++++++---------------------
4 files changed, 82 insertions(+), 61 deletions(-)
diff --git a/compiler/file_util.m b/compiler/file_util.m
index f8085c8..f30588e 100644
--- a/compiler/file_util.m
+++ b/compiler/file_util.m
@@ -298,6 +298,7 @@ report_error(ErrorMessage, !IO) :-
io.write_string("Error: ", !IO),
io.write_string(ErrorMessage, !IO),
io.write_string("\n", !IO),
+ io.flush_output(!IO),
io.set_exit_status(1, !IO).
report_error_to_stream(Stream, ErrorMessage, !IO) :-
diff --git a/compiler/module_cmds.m b/compiler/module_cmds.m
index 99f3af6..3124c0a 100644
--- a/compiler/module_cmds.m
+++ b/compiler/module_cmds.m
@@ -588,11 +588,17 @@ invoke_system_command_maybe_filter_output(Globals, ErrorStream, Verbosity,
)
;
Result = ok(signalled(Signal)),
+ report_error_to_stream(ErrorStream, "system command received signal "
+ ++ int_to_string(Signal) ++ ".", !IO),
+ % Aleo report the error to standard output, because if we raise the
+ % signal this error may not ever been seen, the process stops and
+ % the user is confused.
+ report_error("system command received signal "
+ ++ int_to_string(Signal) ++ ".", !IO),
+
% Make sure the current process gets the signal. Some systems (e.g.
% Linux) ignore SIGINT during a call to system().
raise_signal(Signal, !IO),
- report_error_to_stream(ErrorStream, "system command received signal "
- ++ int_to_string(Signal) ++ ".", !IO),
CommandSucceeded = no
;
Result = error(Error),
diff --git a/compiler/process_util.m b/compiler/process_util.m
index a45b212..7cc50b3 100644
--- a/compiler/process_util.m
+++ b/compiler/process_util.m
@@ -333,7 +333,7 @@ call_in_forked_process_with_backup(P, AltP, Success, !IO) :-
(
MaybePid = yes(Pid),
do_wait(Pid, _, CallStatus, !IO),
- Status = io.handle_system_command_exit_status(CallStatus),
+ Status = decode_system_command_exit_code(CallStatus),
Success = (Status = ok(exited(0)) -> yes ; no)
;
MaybePid = no,
@@ -504,11 +504,11 @@ do_wait(_, _, _, _, _) :-
wait_pid(Pid, Status, !IO) :-
do_wait(Pid, _Pid, Status0, !IO),
- Status = io.handle_system_command_exit_status(Status0).
+ Status = decode_system_command_exit_code(Status0).
wait_any(Pid, Status, !IO) :-
do_wait(-1, Pid, Status0, !IO),
- Status = io.handle_system_command_exit_status(Status0).
+ Status = decode_system_command_exit_code(Status0).
%-----------------------------------------------------------------------------%
:- end_module libs.process_util.
diff --git a/library/io.m b/library/io.m
index 7ebf6ad..acc5a9b 100644
--- a/library/io.m
+++ b/library/io.m
@@ -1663,7 +1663,15 @@
% Interpret the child process exit status returned by
% system() or wait().
%
-:- func io.handle_system_command_exit_status(int) = io.res(io.system_result).
+ % Deprecated, renamed to decode_system_command_exit_code/1.
+ %
+:- pragma obsolete(handle_system_command_exit_status/1).
+:- func handle_system_command_exit_status(int) = io.res(io.system_result).
+
+ % Interpret the child process exit status returned by
+ % system() or wait().
+ %
+:- func decode_system_command_exit_code(int) = io.res(io.system_result).
%-----------------------------------------------------------------------------%
%-----------------------------------------------------------------------------%
@@ -1822,13 +1830,13 @@
:- pred io.read_byte_val(io.input_stream::in, int::out,
io::di, io::uo) is det.
- % io.call_system_code(Command, Status, Message, !IO):
+ % io.call_system_code(Command, Status, Success, Message, !IO):
%
% Invokes the operating system shell with the specified Command.
- % Returns Status = 127 and Message on failure. Otherwise returns
- % the raw exit status from the system() call.
+ % On success Success = yes and Status is valid. on failure Success = no and
+ % Message will contain the error message.
%
-:- pred io.call_system_code(string::in, int::out, string::out,
+:- pred io.call_system_code(string::in, int::out, bool::out, string::out,
io::di, io::uo) is det.
% io.getenv(Var, Value):
@@ -5590,10 +5598,12 @@ io.call_system(Command, Result, !IO) :-
).
io.call_system_return_signal(Command, Result, !IO) :-
- io.call_system_code(Command, Code, Msg, !IO),
- ( Code = 127 ->
+ io.call_system_code(Command, Code, Success, Msg, !IO),
+ (
+ Success = no,
Result = error(io_error(Msg))
;
+ Success = yes,
Result = io.handle_system_command_exit_status(Code)
).
@@ -9656,7 +9666,7 @@ io.close_binary_output(binary_output_stream(Stream), !IO) :-
").
:- pragma foreign_proc("C",
- io.call_system_code(Command::in, Status::out, Msg::out,
+ call_system_code(Command::in, Status::out, Success::out, Msg::out,
_IO0::di, _IO::uo),
[will_not_call_mercury, promise_pure, tabled_for_io, thread_safe,
does_not_affect_liveness, no_sharing],
@@ -9697,7 +9707,7 @@ io.close_binary_output(binary_output_stream(Stream), !IO) :-
if (err != 0) {
/* Spawn failed. */
- Status = 127;
+ Success = MR_NO;
ML_maybe_make_err_msg(MR_TRUE, errno,
""error invoking system command: "",
MR_ALLOC_ID, Msg);
@@ -9707,13 +9717,13 @@ io.close_binary_output(binary_output_stream(Stream), !IO) :-
err = waitpid(pid, &st, 0);
} while (err == -1 && MR_is_eintr(errno));
if (err == -1) {
- Status = 127;
+ Success = MR_NO;
ML_maybe_make_err_msg(MR_TRUE, errno,
""error invoking system command: "",
MR_ALLOC_ID, Msg);
} else {
Status = st;
- Msg = MR_make_string_const("""");
+ Success = MR_YES;
}
}
@@ -9726,24 +9736,19 @@ io.close_binary_output(binary_output_stream(Stream), !IO) :-
#endif
if (Status == -1) {
- /*
- ** Return values of 127 or -1 from system() indicate that
- ** the system call failed. Don't return -1, as -1 indicates
- ** that the system call was killed by signal number 1.
- */
- Status = 127;
+ Success = MR_NO;
ML_maybe_make_err_msg(MR_TRUE, errno,
""error invoking system command: "",
MR_ALLOC_ID, Msg);
} else {
- Msg = MR_make_string_const("""");
+ Success = MR_YES;
}
#endif /* !MR_THREAD_SAFE || !MR_HAVE_POSIX_SPAWN || !MR_HAVE_ENVIRON */
").
:- pragma foreign_proc("Erlang",
- io.call_system_code(Command::in, Status::out, Msg::out,
+ call_system_code(Command::in, Status::out, Success::out, Msg::out,
_IO0::di, _IO::uo),
[will_not_call_mercury, promise_pure, tabled_for_io, thread_safe,
does_not_affect_liveness],
@@ -9766,9 +9771,11 @@ io.close_binary_output(binary_output_stream(Stream), !IO) :-
{Status, []} = string:to_integer(Code),
case Status =:= 0 of
true ->
- Msg = <<>>;
+ Msg = <<>>,
+ Success = yes;
false ->
- Msg = <<""error invoking system command"">>
+ Msg = <<""error invoking system command"">>,
+ Success = no
end
").
@@ -9776,54 +9783,57 @@ io.progname(DefaultProgName::in, ProgName::out, IO::di, IO::uo) :-
% This is a fall-back for back-ends which don't support the C interface.
ProgName = DefaultProgName.
-io.handle_system_command_exit_status(Code0) = Status :-
- Code = io.handle_system_command_exit_code(Code0),
- ( Code = 127 ->
- Status = error(io_error("unknown result code from system command"))
- ; Code < 0 ->
- Status = ok(signalled(-Code))
+handle_system_command_exit_status(Code) =
+ decode_system_command_exit_code(Code).
+
+decode_system_command_exit_code(Code0) = Status :-
+ decode_system_command_exit_code(Code0, Exited, ExitCode, Signalled, Signal),
+ (
+ Exited = yes,
+ Status = ok(exited(ExitCode))
;
- Status = ok(exited(Code))
+ Exited = no,
+ (
+ Signalled = yes,
+ Status = ok(signalled(Signal))
+ ;
+ Signalled = no,
+ Status = error(io_error("unknown result code from system command"))
+ )
).
% Interpret the child process exit status returned by system() or wait():
- % return negative for `signalled', non-negative for `exited', or 127
- % for anything else (e.g. an error invoking the command).
%
-:- func io.handle_system_command_exit_code(int) = int.
+:- pred decode_system_command_exit_code(int::in, bool::out, int::out,
+ bool::out, int::out) is det.
-io.handle_system_command_exit_code(Status0::in) = (Status::out) :-
- % This is a fall-back for back-ends that don't support the C interface.
- ( (Status0 /\ 0xff) \= 0 ->
- % The process was killed by a signal.
- Status = -(Status0 /\ 0xff)
- ;
- % The process terminated normally.
- Status = (Status0 /\ 0xff00) >> 8
- ).
+% This is a fall-back for back-ends that don't support the C interface.
+decode_system_command_exit_code(Status, yes, Status, no, 0).
:- pragma foreign_proc("C",
- io.handle_system_command_exit_code(Status0::in) = (Status::out),
+ decode_system_command_exit_code(Status0::in, Exited::out, Status::out,
+ Signalled::out, Signal::out),
[will_not_call_mercury, thread_safe, promise_pure, does_not_affect_liveness,
no_sharing],
"
#if defined (WIFEXITED) && defined (WEXITSTATUS) && \
defined (WIFSIGNALED) && defined (WTERMSIG)
if (WIFEXITED(Status0)) {
+ Exited = MR_YES;
+ Signalled = MR_NO;
Status = WEXITSTATUS(Status0);
} else if (WIFSIGNALED(Status0)) {
- Status = -WTERMSIG(Status0);
+ Exited = MR_NO;
+ Signalled = MR_YES;
+ Signal = -WTERMSIG(Status0);
} else {
- Status = 127;
+ Exited = MR_NO;
+ Signalled = MR_NO;
}
#else
- if ((Status0 & 0xff) != 0) {
- /* the process was killed by a signal */
- Status = -(Status0 & 0xff);
- } else {
- /* the process terminated normally */
- Status = (Status0 & 0xff00) >> 8;
- }
+ Exited = MR_YES;
+ Status = Status0;
+ Signalled = MR_NO;
#endif
").
@@ -9855,7 +9865,7 @@ io.handle_system_command_exit_code(Status0::in) = (Status::out) :-
").
:- pragma foreign_proc("C#",
- io.call_system_code(Command::in, Status::out, Msg::out,
+ io.call_system_code(Command::in, Status::out, Success::out, Msg::out,
_IO0::di, _IO::uo),
[will_not_call_mercury, promise_pure, tabled_for_io],
"
@@ -9889,13 +9899,15 @@ io.handle_system_command_exit_code(Status0::in) = (Status::out) :-
process.WaitForExit();
Status = process.ExitCode;
Msg = """";
+ Success = mr_bool.YES;
// debugging...
// System.Console.Out.WriteLine(""[exitcode = "" + Status + ""]"");
}
catch (System.Exception e) {
- Status = 127;
+ Success = mr_bool.NO;
+ Status = 1;
Msg = e.Message;
// debugging...
@@ -9994,7 +10006,7 @@ command_line_argument(_, "") :-
").
:- pragma foreign_proc("Java",
- io.call_system_code(Command::in, Status::out, Msg::out,
+ io.call_system_code(Command::in, Status::out, Success::out, Msg::out,
_IO0::di, _IO::uo),
[will_not_call_mercury, promise_pure, tabled_for_io, may_not_duplicate],
"
@@ -10034,7 +10046,8 @@ command_line_argument(_, "") :-
stderr.start();
Status = process.waitFor();
- Msg = """";
+ Success = bool.YES;
+ Msg = null;
// The stdin StreamPipe is killed off after the Process is finished
// so as not to waste CPU cycles with a pointless thread.
@@ -10054,8 +10067,9 @@ command_line_argument(_, "") :-
throw stderr.exception;
}
} catch (java.lang.Exception e) {
- Status = 127;
- Msg = e.getMessage();
+ Status = 1;
+ Success = bool.NO;
+ Msg = e.getMessage();
}
").
--
2.1.4
More information about the reviews
mailing list