fix: errors-suite fatal-error divergences (bad substitutions, POSIX fatality, builtin argument validation) - #661
Merged
Conversation
bash rejects text after ANY parameter reference that is not an operator
as a bad substitution -- ${x!y}, ${$NO_SUCH_VAR}, ${-3}, ${1x},
${@x} and the length forms ${#x%} / ${#x-y} -- where gnash silently
expanded the name and dropped the rest (issue #658, errors2/errors6).
Generalize the name[sub] trailing-junk check (issue #459) to every
parsed name. `~' joins the allowed operator set (it is the
undocumented case-invert operator, so ${a[0]~} was wrongly rejected
before), and `!' is exempt (${!@}/${!*} indirect through the
positional list).
Assigning to a readonly target through ${v:=val} / ${v=val} (or the
array-element form ${a[0]=x}) printed nothing and carried on. bash
treats it as an assignment error: the diagnostic is printed (Shell::set
/ array_set already do) and the command list is abandoned, fatal in
posix mode (errors7.sub's `( readonly v; : ${v:=val} )').
POSIX makes several error classes fatal to a non-interactive shell that gnash survived (errors3/4/7.sub): - a variable-assignment error -- standalone (`var=x' with var readonly), a for/select loop variable that cannot be assigned, or a temporary `var=x cmd' prefix -- exits the shell (status 1). The temporary form on a command with a word suppresses the command instead when it is not a special builtin (bash follows ksh93), discarding the rest of the list; outside posix mode the command still runs. - a redirection error on a SPECIAL builtin exits the shell, even on the LHS of `||' (errors3.sub), unless shielded by `command'. A DISCARD unwind that reaches the end of a subshell now exits it with plain failure -- bash's `(exit 42 43)' subshell reports 1 even though $? inside the shell after the discarded line reads 2.
bash runs the DEBUG trap before a [[ ]] conditional like any other leaf command; gnash skipped it there while firing for (( )) and simple commands (errors9.sub).
Match bash's argument validation for the special builtins and history (errors4/10.sub, errors.tests): - break/continue: a non-numeric count is FATAL to a non-interactive shell in BOTH modes (`break x' ends the script); extra arguments are `too many arguments', status 2, discarding the rest of the list. Both checks precede the loop-level check. - return: argument checks run FIRST (before can-only-return); a non-numeric status reports and returns from the function with 2 (fatal in posix); `--' is accepted before the status. - exit: extra arguments do NOT exit -- status 2, list discarded. - shift: extra arguments are status 2 with a discard; a non-numeric count is fatal in posix; an out-of-range count reports under posix (which implies shift_verbose -- errors8.sub's `command shift 12'). - history: the listing form takes at most one count (`history 10 42' is `too many arguments', status 2).
…bad one `export non-identifier' silently succeeded (status 0) where bash reports `export: `non-identifier': not a valid identifier' with status 1 -- readonly already validated. In POSIX mode an invalid identifier given to export/readonly is a special-builtin error, fatal to a non-interactive shell AT THE FIRST bad name (later names are not reported -- errors11.sub); a `command' prefix shields via posix_builtin_shield, and any other export/readonly failure is fatal in posix mode the same way.
Five error paths in run_builtin (`.'/source invalid option, missing filename, restricted; `command -p' and `exec' under restricted) returned a bare numeric constant from the bool-returning dispatch -- converting to `true' and leaving *status unwritten, so `. -x true || echo ok' never fired the ||. Report the status through *status (errors8.sub's `command . -x true' case).
…mantics Cover the issue #658 families in run_diff: trailing-text bad substitutions (with the `~' case-invert and ${!@}/${!*} exemptions), ${v:=val} readonly aborts, POSIX-fatal assignment/redirection errors (subshell-contained so exit codes compare), break/continue fatality, return/exit/shift argument validation, export identifier checks with posix first-name fatality and command shielding, `.' option-error status, and the DEBUG trap on [[ ]].
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes every family of issue #658 — the
errors.testsdivergences that became visible once the suite ran in a correctOLDPWDenvironment. The suite now scores 0 (byte-identical to bash 5.3); it was 125 before this PR and 126 at release 2.1.1.Closes #658
Changes (one commit per family)
fix(expand): trailing text after any parameter reference —${x!y},${$NO_SUCH_VAR},${-3},${1x},${@x},${#x%}are bad substitutions, generalizing thename[sub]junk check from Paired-quote detection in subscript parser spans past bracket boundaries #459. Fixes a latent bug (the~case-invert operator was missing from the allowed set, wrongly rejecting${a[0]~}) and exempts!(${!@}/${!*}indirect through the positionals).fix(expand):${v:=val}readonly store — now an assignment error: reported, list abandoned, fatal in posix.fix(executor): POSIX fatality — assignment errors (standalone, for/select loop variable,var=x cmdtemp prefix) and special-builtin redirection failures exit a non-interactive posix shell; the temp-prefix form on a non-special command suppresses the command with a discard instead (ksh93 emulation, as bash);commandshields; a DISCARD unwind leaving a subshell exits 1.fix(executor): DEBUG trap fires for[[ ]].fix(builtins): numeric arguments and argument counts — break/continue non-numeric counts are fatal in both modes; return validates before can-only-return and returns 2; exit with extra args doesn't exit (status 2, discard); shift posix behaviors; history count validation.fix(builtins): export validates bare names — was silently status 0; posix exits at the first bad name for export/readonly,command-shielded.fix(builtins):*statuson earlyrun_builtinreturns — five paths returned bare numerics from the bool dispatch, leaving*statusunwritten (the real cause of the errors8command . -xdivergence).test(harness): 41 new differential cases (525 total).Verification
errorsscoreboard: 125 → 0 with fresh oracles in a correct environment.read, and no regressions anywhere.run_diff525/525;ctest23/23.-c-mode discard/exit-code quirks documented and excluded from tests where bash itself is inconsistent).