From c03e4b999f4a47e60d125e6e4e1aec3a31218e57 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Thu, 8 Oct 2026 00:25:03 +0000 Subject: [PATCH] fix(interpreter): a failed arithmetic expansion does not truncate the script `echo $((1/0))` reported the error and then stopped the script: the failure reused the abort path `set -u` needs. It now ends only the enclosing subshell, as bash does, so the command fails with status 1 and the next one runs. Claude-Session: https://claude.ai/code/session_019aFikmptPc91Fj4N2iDXQA --- crates/bashkit/src/interpreter/mod.rs | 46 ++++++++++++++----- .../spec_cases/bash/arith-errors.test.sh | 11 +++++ knowledge/log.md | 1 + 3 files changed, 47 insertions(+), 11 deletions(-) diff --git a/crates/bashkit/src/interpreter/mod.rs b/crates/bashkit/src/interpreter/mod.rs index d0af87736..eaaaf6341 100644 --- a/crates/bashkit/src/interpreter/mod.rs +++ b/crates/bashkit/src/interpreter/mod.rs @@ -1438,6 +1438,9 @@ pub struct Interpreter { /// `unset LINENO` makes it an ordinary variable for the rest of the /// shell's life, like bash. lineno_unset: bool, + /// The `Return(1)` just produced came from a failed `$(( ))`, which ends a + /// subshell but not the script itself. + arith_failure_pending: bool, /// HTTP client for network builtins (curl, wget) #[cfg(feature = "http_client")] http_client: Option>, @@ -2046,6 +2049,7 @@ impl Interpreter { jobs: jobs::new_shared_job_table(), current_line: 1, lineno_unset: false, + arith_failure_pending: false, #[cfg(feature = "http_client")] http_client: None, #[cfg(feature = "git")] @@ -2189,6 +2193,7 @@ impl Interpreter { fs: Arc::clone(&self.fs), env: self.env.clone(), scoped: self.scoped.clone(), + arith_failure_pending: self.arith_failure_pending, flags: self.flags, cwd: self.cwd.clone(), last_exit_code: self.last_exit_code, @@ -3354,6 +3359,14 @@ impl Interpreter { break; } } else { + // A failed `$(( ))` fails its command and nothing more: + // the script keeps going, as it does in bash. + if top_level + && result.control_flow == ControlFlow::Return(1) + && std::mem::take(&mut self.arith_failure_pending) + { + continue; + } stopped = true; break; } @@ -4174,12 +4187,12 @@ impl Interpreter { // the right side must NOT be expanded (to avoid set -u errors). let result = self.evaluate_conditional_words(words).await?; // If a nounset error occurred during evaluation, propagate it. - if let Some(err_msg) = self.take_expansion_error() { + if let Some((err_msg, control_flow)) = self.take_expansion_failure() { self.last_exit_code = 1; return Ok(ExecResult { stderr: err_msg.into(), exit_code: 1, - control_flow: ControlFlow::Return(1), + control_flow, ..Default::default() }); } @@ -6777,10 +6790,21 @@ impl Interpreter { /// The pending expansion error, if any: an arithmetic failure first, then a /// `set -u` failure. Clears both. - fn take_expansion_error(&mut self) -> Option { - let arith = self.take_arith_error(""); - let nounset = self.nounset_error.take(); - arith.or(nounset) + /// + /// Both abort the enclosing subshell, as bash does — `( echo a; echo + /// $((1/0)); echo b )` stops after `a`. They differ at the outermost + /// level of a script: `set -u` ends it, while an arithmetic failure only + /// fails its own command and the script runs on. `arith_failure_pending` + /// tells the two apart there. + fn take_expansion_failure(&mut self) -> Option<(String, ControlFlow)> { + if let Some(msg) = self.take_arith_error("") { + self.nounset_error = None; + self.arith_failure_pending = true; + return Some((msg, ControlFlow::Return(1))); + } + self.nounset_error + .take() + .map(|msg| (msg, ControlFlow::Return(1))) } /// Take the pending arithmetic error, formatted with `prefix`. @@ -7023,14 +7047,14 @@ impl Interpreter { } }; - if let Some(err_msg) = self.take_expansion_error() { + if let Some((err_msg, control_flow)) = self.take_expansion_failure() { self.last_exit_code = 1; self.discard_deferred_proc_subs_from(deferred_proc_sub_start); return Ok(ExecResult { stdout: crate::StreamData::new(), stderr: err_msg.into(), exit_code: 1, - control_flow: ControlFlow::Return(1), + control_flow, ..Default::default() }); } @@ -7275,14 +7299,14 @@ impl Interpreter { self.insert_variable_checked("_".to_string(), name.to_string()); } - // Check for nounset error from argument expansion - if let Some(err_msg) = self.take_expansion_error() { + // Check for an arithmetic or `set -u` failure during expansion. + if let Some((err_msg, control_flow)) = self.take_expansion_failure() { self.last_exit_code = 1; return Ok(ExecResult { stdout: crate::StreamData::new(), stderr: err_msg.into(), exit_code: 1, - control_flow: ControlFlow::Return(1), + control_flow, ..Default::default() }); } diff --git a/crates/bashkit/tests/spec_cases/bash/arith-errors.test.sh b/crates/bashkit/tests/spec_cases/bash/arith-errors.test.sh index 62fa1dbb4..a3b0d05f7 100644 --- a/crates/bashkit/tests/spec_cases/bash/arith-errors.test.sh +++ b/crates/bashkit/tests/spec_cases/bash/arith-errors.test.sh @@ -1,3 +1,14 @@ +### arith_division_by_zero_aborts_the_subshell_but_not_the_script +echo before +echo $((1/0)) 2>/dev/null +echo "rc=$?" +echo after +### expect +before +rc=1 +after +### end + ### arith_division_by_zero_aborts_and_fails ( echo before; echo $((1/0)); echo never ) 2>/dev/null echo "rc=$?" diff --git a/knowledge/log.md b/knowledge/log.md index 9526bbb06..f8405baa1 100644 --- a/knowledge/log.md +++ b/knowledge/log.md @@ -2,6 +2,7 @@ ## 2026-10-07 +* **Fix**: a failed `$(( ))` fails its command without truncating the script. `echo $((1/0)); echo "rc=$?"` reported the error and then stopped, because the failure reused `set -u`'s abort path; it now ends only the enclosing subshell, as bash does, so `( echo a; echo $((1/0)); echo b )` still stops after `a` while a script carries on. * **Fix**: `grep` follows GNU grep 3.11. Prefixes print as `FILE:LINE:BYTE:` (context lines with `-`), group separators appear between non-adjacent groups and files whenever a context option is given (`-NUM`, `--group-separator`, `--no-group-separator`), trailing context after `-m` is printed in full, `-o` skips empty matches and is POSIX leftmost-longest (`grep -oE 'ab|abcd'` prints `abcd`), `-b -o` gives each match's offset. New `grep_pattern` translates GNU BRE/ERE (brackets, intervals, `\< \> \b \w`, literal leading `*` in BRE, dropped leading ERE operator with GNU's warning, glibc error messages) onto awk's leftmost-longest `AwkRegex`; back-references work in BRE and ERE via fancy-regex (TM-DOS-025 cap). Options parse like getopt_long (unique prefixes such as `--no-gr`, `-T`, `-d`, `--label`, `-` for stdin, `-R` follows symlinks), an empty `-f` file matches nothing, a directory operand without `-r` reports `Is a directory`, invalid UTF-8 output turns the file binary, `\r` is line data. Debian-oracle grep score 60.5% to 96.8% (185 cases); grep's probes in the regex set all pass except two raw-`\x01` patterns the shell drops. Remaining misses are readdir order (L-GREP-003); L-GREP-002 lists the other gaps. See foundations/builtins "grep". * **Fix**: jq `implode` maps invalid code points to U+FFFD, `strptime`/`fromdate` fail with jq's `date "X" does not match format "F"`, and `@base64d` accepts unpadded input and non-zero trailing bits with jq's error texts.