Skip to content

Commit

Permalink
Fix corner case bug involving 'eval' and return < 0 (re: b90b8cd)
Browse files Browse the repository at this point in the history
Reproducer:

    $ foo() { return -1; }
    $ eval foo
    -ksh: eval: not found

ksh tries to run 'eval' as an external command. No such external
command generally exists on $PATH, hence the error message.

Analysis:

Built-ins may have the BLT_EXIT attribute (see data/builtins.c)
which has two functions:
1)  To avoid trimming a built-in's exit status to the least
    significant eight bits (xec.c, lines 1278-1279).
2)  To fall back to an external command by the same name if the
    built-in returns an exit status < 0 (xec.c, lines 1338-1345).
    This of course depends on the exit status not being trimmed.

As of the referenced commit which made 'return' accept a negative
exit status, feature 2 triggers this bug. 'eval' has the BLT_EXIT
attribute, and it needs it beecause of feature 1, but feature 2 is
not desired here.

Feature 2 also unused, and (as far as I can tell) always has been.
No regular built-in falls back to an external command. Some libcmd
built-ins (date, getconf, uname) can fall back to their external
counterparts, but they use the sh_run() interface to do so.

Since feature 2 is unused except for triggering this bug, the
simplest fix is to remove it. This commit does that.
  • Loading branch information
McDutchie committed Nov 24, 2024
1 parent 8ee1efd commit 6e30fe1
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 9 deletions.
2 changes: 1 addition & 1 deletion src/cmd/ksh93/include/version.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

#define SH_RELEASE_FORK "93u+m" /* only change if you develop a new ksh93 fork */
#define SH_RELEASE_SVER "1.1.0-alpha" /* semantic version number: https://semver.org */
#define SH_RELEASE_DATE "2024-11-17" /* must be in this format for $((.sh.version)) */
#define SH_RELEASE_DATE "2024-11-24" /* must be in this format for $((.sh.version)) */
#define SH_RELEASE_CPYR "(c) 2020-2024 Contributors to ksh " SH_RELEASE_FORK

/* Scripts sometimes field-split ${.sh.version}, so don't change amount of whitespace. */
Expand Down
9 changes: 1 addition & 8 deletions src/cmd/ksh93/sh/xec.c
Original file line number Diff line number Diff line change
Expand Up @@ -1335,14 +1335,7 @@ int sh_exec(const Shnode_t *t, int flags)
sh.redir0 = 0;
if(jmpval)
siglongjmp(*sh.jmplist,jmpval);
if(sh.exitval >=0)
goto setexit;
/*
* If a built-in sets sh.exitval < 0 (must be allowed by BLT_EXIT attribute),
* then fall through to TFORK which runs the external command by that name
*/
np = 0;
type=0;
goto setexit;
}
/* check for functions */
if(!command && np && nv_isattr(np,NV_FUNCTION))
Expand Down
10 changes: 10 additions & 0 deletions src/cmd/ksh93/tests/return.sh
Original file line number Diff line number Diff line change
Expand Up @@ -279,5 +279,15 @@ trap - USR1
unset -f f
[[ $(<out) == 0 ]] || err_exit "default return status in traps is broken (expected 0, got $(<out))"

# ======
print 'print "BUG TRIGGERED"' >eval
chmod +x eval
PATH=$PWD:$PATH
foo() { return -1; }
got=$(eval foo)
[[ -z $got ]] || err_exit "negative return status from 'eval' triggers external 'eval' command (got $(printf %q "$got"))"
PATH=${PATH%"$PWD:"}
unset -f foo

# ======
exit $((Errors<125?Errors:125))

0 comments on commit 6e30fe1

Please sign in to comment.