Skip to content

Commit

Permalink
improve error message of ExecError when called command exited with no…
Browse files Browse the repository at this point in the history
…n-zero status
  • Loading branch information
sekiguchi-nagisa committed May 20, 2024
1 parent 153bdc3 commit 54fc23c
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 18 deletions.
3 changes: 2 additions & 1 deletion src/constant.h
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,8 @@ constexpr const char *TYPE_FUNC = "Func";
constexpr unsigned int UDC_PARAM_ATTR = 0; // &&attr
constexpr unsigned int UDC_PARAM_REDIR = 1; // &&redir
constexpr unsigned int UDC_PARAM_ARGV = 2; // @
constexpr unsigned int UDC_PARAM_N = UDC_PARAM_ARGV + 1;
constexpr unsigned int UDC_PARAM_ARG0 = 3; // 0
constexpr unsigned int UDC_PARAM_N = UDC_PARAM_ARG0;

// ===== termination kind =====

Expand Down
26 changes: 16 additions & 10 deletions src/vm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -906,9 +906,11 @@ static void traceCmd(const ARState &state, const ArrayObject &argv) {
errno = 0; // ignore error
}

static bool checkCmdExecError(ARState &state, CmdCallAttr attr, int64_t status) {
static bool checkCmdExecError(ARState &state, StringRef cmdName, CmdCallAttr attr, int64_t status) {
if (status != 0 && hasFlag(attr, CmdCallAttr::RAISE) && state.has(RuntimeOption::ERR_RAISE)) {
std::string message = "command exits with non-zero status: `";
std::string message = "`";
appendAsPrintable(cmdName, SYS_LIMIT_ERROR_MSG_MAX, message);
message += "' command exits with non-zero status: `";
message += std::to_string(status);
message += "'";
raiseError(state, TYPE::ExecError, std::move(message), status);
Expand Down Expand Up @@ -940,13 +942,14 @@ bool VM::callCommand(ARState &state, const ResolvedCmd &cmd, ObjPtr<ArrayObject>
std::move(redirConfig), attr);
case ResolvedCmd::BUILTIN: {
errno = 0;
const auto cmdName = array.getValues()[0];
const int status = cmd.builtinCmd()(state, array);
flushStdFD();
if (state.hasError()) {
return false;
}
pushExitStatus(state, status); // set exit status before check ERR_RAISE
if (!checkCmdExecError(state, attr, status)) {
if (!checkCmdExecError(state, cmdName.asStrRef(), attr, status)) {
return false;
}
return true;
Expand All @@ -972,7 +975,7 @@ bool VM::callCommand(ARState &state, const ResolvedCmd &cmd, ObjPtr<ArrayObject>
const bool ret = forkAndExec(state, cmd.filePath(), array, std::move(redirConfig));
if (ret) {
const int status = state.getMaskedExitStatus();
if (!checkCmdExecError(state, attr, status)) {
if (!checkCmdExecError(state, array.getValues()[0].asStrRef(), attr, status)) {
return false;
}
}
Expand Down Expand Up @@ -1205,9 +1208,10 @@ int VM::builtinExec(ARState &state, ArrayObject &argvObj, Value &&redir) {

bool VM::returnFromUserDefinedCommand(ARState &state, int64_t status) {
const auto attr = static_cast<CmdCallAttr>(state.stack.getLocal(UDC_PARAM_ATTR).asNum());
const auto arg0 = state.stack.getLocal(UDC_PARAM_ARG0);
state.stack.unwind();
pushExitStatus(state, status); // set exit status before check ERR_RAISE
if (!checkCmdExecError(state, attr, status)) {
if (!checkCmdExecError(state, arg0.asStrRef(), attr, status)) {
return false;
}
assert(!state.stack.checkVMReturn());
Expand Down Expand Up @@ -2075,12 +2079,13 @@ bool VM::mainLoop(ARState &state) {
auto attr = static_cast<CmdCallAttr>(v);
Value redir = state.stack.getLocal(UDC_PARAM_REDIR);
auto argv = toObjPtr<ArrayObject>(state.stack.getLocal(UDC_PARAM_ARGV));
const auto arg0 = argv->getValues()[0];
const auto ret = builtinCommand(state, std::move(argv), std::move(redir), attr);
flushStdFD();
if (ret.kind == BuiltinCmdResult::CALL) {
TRY(ret.r);
} else {
TRY(checkCmdExecError(state, attr, ret.status));
TRY(checkCmdExecError(state, arg0.asStrRef(), attr, ret.status));
pushExitStatus(state, ret.status);
}
vmnext;
Expand All @@ -2106,9 +2111,10 @@ bool VM::mainLoop(ARState &state) {
auto v = state.stack.getLocal(UDC_PARAM_ATTR).asNum();
const auto attr = static_cast<CmdCallAttr>(v);
Value redir = state.stack.getLocal(UDC_PARAM_REDIR);
auto argv = state.stack.getLocal(UDC_PARAM_ARGV);
int status = builtinExec(state, typeAs<ArrayObject>(argv), std::move(redir));
TRY(checkCmdExecError(state, attr, status));
auto &argv = typeAs<ArrayObject>(state.stack.getLocal(UDC_PARAM_ARGV));
const auto arg0 = argv.getValues()[0];
int status = builtinExec(state, argv, std::move(redir));
TRY(checkCmdExecError(state, arg0.asStrRef(), attr, status));
pushExitStatus(state, status);
vmnext;
}
Expand Down Expand Up @@ -2154,7 +2160,7 @@ bool VM::mainLoop(ARState &state) {
Value arg0;
if (frame) {
unsigned int localOffset = frame->localVarOffset;
arg0 = state.stack.unsafeGetOperand(localOffset + UDC_PARAM_N);
arg0 = state.stack.unsafeGetOperand(localOffset + UDC_PARAM_ARG0);
} else {
arg0 = state.getGlobal(BuiltinVarOffset::POS_0);
}
Expand Down
10 changes: 7 additions & 3 deletions test/exec/cases/base/errraise4.ds
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,25 @@ var ex = 34 as Any
try { command -W; } catch e { $ex = $e; }
assert ($ex as ExecError).status() == 2
assert ($ex as ExecError).lineno() == 7
assert ($ex as ExecError).message() == "\`command' command exits with non-zero status: \`2'"

$ex = 34
try { command -V fjreifae; } catch e { $ex = $e; }
assert ($ex as ExecError).status() == 1
assert ($ex as ExecError).lineno() == 12
assert ($ex as ExecError).lineno() == 13
assert ($ex as ExecError).message() == "\`command' command exits with non-zero status: \`1'"

## exec
$ex = 45
try { exec -T; } catch e { $ex = $e; }
assert ($ex as ExecError).status() == 2
assert ($ex as ExecError).lineno() == 18
assert ($ex as ExecError).lineno() == 20
assert ($ex as ExecError).message() == "\`exec' command exits with non-zero status: \`2'"

$ex = 45
try { exec $'ls\x00'; } catch e { $ex = $e; }
assert ($ex as ExecError).status() == 1
assert ($ex as ExecError).lineno() == 23
assert ($ex as ExecError).lineno() == 26
assert ($ex as ExecError).message() == "\`exec' command exits with non-zero status: \`1'"

true
6 changes: 3 additions & 3 deletions test/exec/cases/base/errraise5.ds
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ try {
ls | for a in $STDIN {
sh -c 'exit 56'; echo $a; }
} catch e { $ex = $e; }
assert ($ex as ExecError).message() == "command exits with non-zero status: \`56'"
assert ($ex as ExecError).message() == "\`sh' command exits with non-zero status: \`56'"
assert ($ex as ExecError).lineno() == 9

assert $? == 0
Expand All @@ -23,7 +23,7 @@ try {
sh -c 'exit 156'; echo $a; } | {
false; }
} catch e { $ex = $e; }
assert ($ex as ExecError).message() == "command exits with non-zero status: \`1'"
assert ($ex as ExecError).message() == "\`false' command exits with non-zero status: \`1'"
assert ($ex as ExecError).lineno() == 24

assert $? == 0
Expand All @@ -40,7 +40,7 @@ try {
ff 67; echo $a; } | {
ff 145; }
} catch e { $ex = $e; }
assert ($ex as ExecError).message() == "command exits with non-zero status: \`145'"
assert ($ex as ExecError).message() == "\`ff' command exits with non-zero status: \`145'"
assert ($ex as ExecError).lineno() == 41

assert $? == 0
Expand Down
2 changes: 1 addition & 1 deletion test/exec/cases/output/try_finally_except1.ds
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ assert $({

# CHECKERR: [warning at subshell=1]
# CHECKERR: the following exception within finally/defer block is ignored
# CHECKERR: ExecError: command exits with non-zero status: `1'
# CHECKERR: ExecError: `false' command exits with non-zero status: `1'
# CHECKERR_RE: from .+/test/exec/cases/output/try_finally_except1\.ds:99 '<toplevel>\(\)'
# CHECKERR_RE: ^$
# CHECKERR: [runtime error at subshell=1]
Expand Down

0 comments on commit 54fc23c

Please sign in to comment.