Make test(1) always use the POSIX "number of args" evaluation rules
when they apply.
Only fall back to the old expression evaluation when there are more
than 4 args, or when the args given cannot work as a test expression
using the POSIX rules. That is when the result is unspecified.
Also fix old bug where a string of whitespace is considered to be a
valid number (at least one digit is needed amongst it somewhere...)
XXX pullup -8
the option when a pipeline is created that controls the way the exit
status of the pipeline is calculated. Previously it was the state of
the option when the exit status of the pipeline was collected.
This makes no difference at all for foreground pipelines (there is
no way to change the option between starting and completing the
pipeline) but it does for asynchronous (background) pipelines.
This was always the right way to implement it - it was originally
done the other way as I could not find any other shell implemented
this way - they all seemed to do it our previous way, and I could
not see a good reason to be the sole different shell.
However, now I know that ksh93 works as we will now work, and I
am told that if the option is added to the FreeBSD shell (apparently
the code exists, uncommitted) it will be the same.
Save more characters of command in non-interactive jobs, in case of
core dumps and similar (16 effective chars was a few too little).
Arrange for number to increase if command buffer size increases.
Add a paragraph (briefer than previously posted to mailing lists)
to explain that there is no guarantee that the results of a command
substitution will be available before all commands started by the
cmdsub have completed.
Include the original proposed text (much longer) as *roff comments, so
it will at least be available to those who browse the man page sources.
While here, clean up the existing text about command substitutions to
make it a little more accurate (and to advise against using the `` form).
Deal with the new shell internal exit reason EXEXIT in the case of
a shell which has vfork()'d. It takes a peculiar set of circumstances
to get into a situation where this is ever relevant, but it can be
done. See the PR for details.
we had incorrect usage of setstackmark()/popstackmark()
There was an ancient idiom (imported from CSRG in 1993) where code
can do:
setstackmark(&smark); loop until whatever condition {
/* do lots of code */ popstackmark(&smark);
} popstackmark(&smark);
The 1st (inner) popstackmark() resets the stack, conserving memory,
The 2nd one is needed just in case the "whatever condition" was never
true, and the first one was never executed.
This is (was) safe as all popstackmark() did was reset the stack.
That could be done over and over again with no harm.
That is, until 2000 when a fix from FreeBSD for another problem was
imported. That connected all the stack marks as a list (so they can be
located). That caused the problem, as the idiom was not changed, now
there is this list of marks, and popstackmark() was removing an entry.
It rarely (never?) caused any problems as the idiom was rarely used
(the shell used to do loops like above, mostly, without the inner
popstackmark()). Further, the stack mark list is only ever used when
a memory block is realloc'd.
That is, until last weekend - with the recent set of changes.
Part of that copied code from FreeBSD introduced the idiom above
into more functions - functions used much more, and with a greater
possibility of stack marks being set on blocks that are realloc'd
and so cause the problem. In the FreeBSD code, they changed the idiom,
and always do a setstackmark() immediately after the inner popstackmark().
But not for reasons related to a list of stack marks, as in the
intervening period, FreeBSD deleted that, but for another reason.
We do not have their issue, and I did not believe that their
updated idiom was needed (I did some analysis of exactly this issue -
just missed the important part!), and just continued using the old one.
Hence Patrick's core dump....
The solution used here is to split popstackmark() into 2 halves,
popstackmark() continues to do what it has (recently) done,
but is now implemented as a call of (a new func) rststackmark()
which does all the original work of popstackmark - but not removing
the entry from the stack mark list (which remains in popstackmark()).
Then in the idiom above, the inner popstackmark() turns into a call of
rststackmark() so the stack is reset, but the stack mark list is
unchanged. Tail recursion elimination makes this essentially free.
Import a whole set of tree evaluation enhancements from FreeBSD.
With these, before forking, the shell predicts (often) when all it will
have to do after forking (in the parent) is wait for the child and then
exit with the status from the child, and in such a case simply does not
fork, but rather allows the child to take over the parent's role.
This turns out to handle the particular test case from PR bin/48875 in
such a way that it works as hoped, rather than as it did (the delay there
was caused by an extra copy of the shell hanging around waiting for the
background child to complete ... and keeping the command substitution
stdout open, so the "real" parent had to wait in case more output appeared).
As part of doing this, redirection processing for compound commands gets
moved out of evalsubshell() and into a new evalredir(), which allows us
to properly handle errors occurring while performing those redirects,
and not mishandle (as in simply forget) fd's which had been moved out
of the way temporarily.
evaltree() has its degree of recursion reduced by making it loop to
handle the subsequent operation: that is instead of (for any binop
like ';' '&&' (etc)) where it used to
evaltree(node->left);
evaltree(node->right);
return;
it now does (kind of)
next = node;
while ((node = next) != NULL) {
next = NULL;
if (node is a binary op) {
evaltree(node->left);
if appropriate /* if && test for success, etc */
next = node->right;
continue;
}
/* similar for loops, etc */
}
which can be a good saving, as while the left side (now) tends to be
(usually) a simple (or simpleish) command, the right side can be many
commands (in a command sequence like a; b; c; d; ... the node at the
top of the tree will now have "a" as its left node, and the tree for
b; c; d; ... as its right node - until now everything was evaluated
recursively so it made no difference, and the tree was constructed
the other way).
if/while/... statements are done similarly, recurse to evaluate the
condition, then if the (or one of the) body parts is to be evaluated,
set next to that, and loop (previously it recursed).
There is more to do in this area (particularly in the way that case
statements are processed - we can avoid recursion there as well) but
that can wait for another day.
While doing all of this we keep much better track of when the shell is
just going to exit once the current tree is evaluated (with a new
predicate at_eof() to tell us that we have, for sure, reached the end
of the input stream, that is, this shell will, for certain, not be reading
more command input) and use that info to avoid unneeded forks. For that
we also need another new predicate (have_traps()) to determine of there
are any caught traps which might occur - if there are, we need to remain
to (potentially) handle them, so these optimisations will not occur (to
make the issue in PR 48875 appear again, run the same code, but with a
trap set to execute some code when a signal (or EXIT) occurs - note that
the trap must be set in the appropriate level of sub-shell to have this
effect, any caught traps are cleared in a subshell whenever one is created).
There is still work to be done to handle traps properly, whatever
weirdness they do (some of which is related to some of this.)
These changes do not need man page updates, but 48875 does - an update
to sh.1 will be forthcoming once it is decided what it should say...
Once again, all the heavy lifting for this set of changes comes directly
(with thanks) from the FreeBSD shell.
XXX pullup-8 (but not very soon)
Revert the changes that were made 19 May 2016 (principally eval.c 1.125)
and the bug fixes in subsequent days (eval.c 1.126 and 1.127) and also
update some newer code that was added more recently which acted in
accordance with those changes (make that code be as it would have been
if the changes now being reverted had never been made).
While the changes made did solve the problem, in a sense, they were
never correct (see the PR for some discussion) and it had always been
intended that they be reverted. However, in practical sh code, no
issues were reported - until just recently - so nothing was done,
until now...
After this commit, the validate_fn_redirects test case of the sh ATF
test t_redir will fail. In particular, the subtest of that test
case which is described in the source (of the test) as:
This one is the real test for PR bin/48875
will fail.
Alternative changes, not to "fix" the problem in the PR, but to
often avoid it will be coming very soon - after which that ATF
test will succeed again.
XXX pullup-8
previous rev) the two values (node name, and node number) were
arbitrarily printed in different formats and orders (depending
upon my mood at the time I guess...) The new macros will standardise
that usage (in the debug output) once some use of them actually begins.
When the macros were added, I arbitrarily copied the format of one
use I was looking at at that instant (the one which inspired the change),
but after gazing at DEBUG mode output over the intervening time, I
have concluded that I did not pick the easiest to read/follow format.
So, even before they are used, change the style... Also, conform
to standard PRIxxxx macro style by omitting the leading '%'.
NFC (since they aren't used at all, anywhere, yet, not even the
possibility of anything changing!)
This generates nodenames.h which is a file that used to begin
#ifdef DEBUG
(line 1) and end with
#endif
(last line) with no intervening (matching) #else ... ie: for DEBUG use only.
That led to situations where non-debug code would like to make use
of the info provided, if DEBUG was enabled, needed to add #ifdef DEBUG
at the point of use.
Avoid that by providing new macros that are always defined (DEBUG or not,
so now we have a #else) which allow code to be written to make use of
the extra DEBUG info, if it is available, or not, if not.
While here, add double-include protection on the generated .h file
(just being cautious - nothing is ever going to cause it to get
included anywhere twice - or it shouldn't) and add the traditional
comments on the #else and #endif stuff (which is also really useless
as no-one is really expected to ever read the generated file). Never mind.
Nothing yet (elsewhere in the sh source) uses the new macros, so there's
even less chance of this changing anything than there would otherwise be.
Fix "command not found" handling so that the error message
goes to stderr (after any redirections are applied).
More importantly, in
foo > /tmp/junk
/tmp/junk should be created, before any attempt is made
to execute (the assumed non-existing) "foo".
All this was always true for any command (not found command)
containing a / in its name
foo/bar >/tmp/junk 2>>/tmp/errs
would have created /tmp/junk, then complained (in /tmp/errs)
about foo/bar not being found. Now that happens for ordinary
commands as well.
The fix (which I found when I saw differences between our
code and FreeBSD's, where, for the benefit of PR 42184,
this has been fixed, sometime in the past 9 years) is
frighteningly simple. Simply do not short circuit execution
(or print any error) when the initial lookup fails to
find the command - it will fail anyway when we actually
try running it. The cost is a (seemingly unnecessary,
except that it really is) fork in this case.
This is what I had been planning, but I expected it would
be much more difficult than it turned out....
XXX pullup-8
1. Make command -pv (and -pV) work (which is not as easy as the PR
suggests it might be (the "check and cause error" was there because
it did not work, not in order to prevent it from working).
2. Stop -v and -V being both used (that makes no sense).
3. Stop the "type" builtin inheriting the args (-pvV) that "command" has
(which it did, as when -v -or -V is used with command, it and type are
implemented using the same code).
4. make "command -v word" DTRT for sh keywords (was treating them as an error).
5. Require at least one arg for "command -[vV]" or "type" else usage & error.
Strictly this should also apply to "command" and "command -p" (no -v)
but that's handled elsewhere, so perhaps some other time. Perhaps
"command -v" (and -V) should be limited to 1 command name (where "type"
can have many) as in the POSIX definitions, but I don't think that matters.
6. With "command -V alias", (or "type alias" which is the same thing),
(but not "command -v alias") alter the output format, so we get
ll is an alias for: ls -al
instead of the old
ll is an alias for
ls -al
(and note there was a space, for some reason, after "for")
That is, unless the alias value contains any \n characters, in which
case (something approximating) the old multi-line format is retained.
Also note: that if code wants to parse/use the value of an alias, it
should be using the output of "alias name", not command or type.
Note that none of the above affects "command [-p] cmd" (no -v or -V options)
only "command -[vV]" and "type".
Note also that the changes to eval.[ch] are merely to make syspath()
visible in exec.c rather than static in eval.c
Attempt to correctly deal with \ (both when it is a literal,
in appropriate cases, and when it appears as CTLESC when it was
detected as a quoting character during parsing).
In a pattern, in sh, no quoted character can ever be anything other
than a literal character. This is quite different than regular
expressions, and even different than other uses of glob matching,
where shell quoting is not an issue.
In something like
ls ?\*.c
the ? is a meta-character, the * is a literal (it was quoted). This
is nothing new, sh has handled that properly for ever.
But the same happens with
VAR='?\*.c'
and
ls $VAR
which has not always been handled correctly. Of course, in
ls "$VAR"
nothing in VAR is a meta-character (the entire expansion is quoted)
so even the '\' must match literally (or more accurately, no matching
happens - VAR simply contains an "unusual" filename). But if it had
been
ls *"$VAR"
then we would be looking for filenames that end with the literal 5
characters that make up $VAR.
The same kinds of things are requires of matching patterns in case
statements, and sub-strings with the % and # operators in variable
expansions.
While here, the final remnant of the ancient !! pattern matching
hack has been removed (the code that actually implemented it was
long gone, but one small piece remained, not doing any real harm,
but potentially wasting time - if someone gave a pattern which would
once have invoked that hack.)
This is more or less the same patch as provided in the PR
(just 11 years later, so changed a bit) by woods@...
Since there is no known way to actually cause the reported crash,
we may never know if this change actually fixes anything. But
even if it doesn't it certainly cannot hurt.
There is a potential race which could possibly explain the issue
(see commentary in the PR) which is not easy to avoid - if that is
the actual cause, this should provide a defence, if not really a fix.
possibilities that we do not currently handle all that well.
This mostly means (for now) making sure that quoted pattern
magic characters (as well as quoted sh syntax magic chars)
are properly marked, so they remain known as being quoted,
and do not turn into pattern magic. Also, make sure that an
unquoted \ in a pattern always quotes whatever comes next
(which, unlike in regular expressions, includes inside []
matches),
Mostly use number() (no longer implemented using atoi()) when an
unsigned integer is required, but use strtoXXX() when a conversion
is wanted, without the possibility or error (like setting OPTIND
and RANDOM). Always init OPTIND to 1 when sh starts (overriding
anything in environ.)
that is longer than we can handle the same way we treat an unknown
class name (as a valid char class which contains nothing, so never
matches). Previously a "too long" class name invalidated the
class, so [:very-long-name:] would match any of '[' ':' 'v' ...
(note: "very-long-name" is not long enough to trigger this, but you
get the idea!)
However, the name itself has a restricted syntax ([[:***:]] is not a
character class, it is a match for one of a '[' ':' or '*', followed by
a ']') which we did not implement - check the syntax of the name before
treating it as a character class (but we do add '_' to alphanumerics
as legal class name characters).
expansion, case patterrns, etc) do not force '[' to be a member of
every class.
Before this fix, try:
case [ in [[:alpha:]]) echo Huh\?;; esac
XXX pullup-8 (Perhaps -7 as well, though that shell version has
much more relevant bugs than this one.) This bug is not in -6 as
that has no charclass support.
itself, or some other function which is still active.
This was a long known bug (fixed ages ago in the FreeBSD sh) which
hadn't been fixed as in practice, the situation that causes the
problem simply doesn't arise .. ASAN found it in the sh dotcmd
tests which do have this odd "feature" in the way they are written
(but where it never caused a problem, as the tests are so simple
that no mem is ever allocated between when the old version of the
function was deleted, and when it finished executing, so its code
all remained intact, despite having been freed.)
The fix is taken from the FreeBSD sh.
XXX -- pullup-8 (after a while to ensure no other problems arise).
The current implementation of operations - + * / % could cause Undefined
Behavior and in narrow cases (INT64_MIN / -1 and INT64_MIN % -1) SIGFPE
and crash duping core.
Detected with MKSANITIZER enabled for the Undefined Behavior variation:
# eval expr '4611686018427387904 + 4611686018427387904'
/public/src.git/bin/expr/expr.y:315:12: runtime error: signed integer overflow: 4611686018427387904 + 4611686018427387904 cannot be represented in type 'long'
All bin/t_expr ATF tests pass now in a sanitized userland.
Sponsored by <The NetBSD Foundation>
UBSan can detect that during switching a login to root there is unportable
left shift operation:
$ su -
Password:
/public/src.git/bin/ksh/eval.c:598:13: runtime error: left shift of 1073741824 by 1 places cannot be represented in type 'int'
#
Sponsored by <The NetBSD Foundation>
