From f00bcb069bfba85601f5fe670de6beac72072046 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Thu, 8 Oct 2026 03:25:42 +0000 Subject: [PATCH 1/3] fix(interpreter): scope ERR and RETURN traps like bash ERR fires after the failing simple command, multi-command pipeline, ( ), [[ ]] or (( )) itself, only when a handler was set before the command ran. Functions inherit ERR only under set -E and RETURN only under set -T; subshells, command substitutions and pipeline stages keep ERR listed but dormant without set -E. RETURN now runs when a function or sourced file finishes, and trap handlers count $LINENO from the triggering line. --- crates/bashkit/src/builtins/trap.rs | 17 +- crates/bashkit/src/interpreter/mod.rs | 220 +++++++++++++++--- .../spec_cases/bash/bashbox-err-trap.test.sh | 10 - .../bash/bashbox-interpreter-builtins.test.sh | 13 -- 4 files changed, 196 insertions(+), 64 deletions(-) diff --git a/crates/bashkit/src/builtins/trap.rs b/crates/bashkit/src/builtins/trap.rs index 51e19aeba..794bf6532 100644 --- a/crates/bashkit/src/builtins/trap.rs +++ b/crates/bashkit/src/builtins/trap.rs @@ -157,14 +157,19 @@ impl Builtin for Trap { let mut stderr = String::new(); for spec in specs { match canonical(spec) { - Some((_, key)) => match handler { - Some(cmd) => { - shell.traps.insert(key, cmd.clone()); + Some((_, key)) => { + if key == "ERR" { + *shell.err_trap_dormant = false; } - None => { - shell.traps.remove(&key); + match handler { + Some(cmd) => { + shell.traps.insert(key, cmd.clone()); + } + None => { + shell.traps.remove(&key); + } } - }, + } None => stderr.push_str(&invalid_spec(spec)), } } diff --git a/crates/bashkit/src/interpreter/mod.rs b/crates/bashkit/src/interpreter/mod.rs index 5f5d1602d..aa2060ba6 100644 --- a/crates/bashkit/src/interpreter/mod.rs +++ b/crates/bashkit/src/interpreter/mod.rs @@ -339,6 +339,9 @@ pub(crate) struct ShellRef<'a> { pub(crate) aliases: &'a mut HashMap, /// Direct mutable access to trap handlers. pub(crate) traps: &'a mut HashMap, + /// Set while the ERR handler is inherited for display only (subshell + /// without `set -E`); `trap` clears it when it sets or resets ERR. + pub(crate) err_trap_dormant: &'a mut bool, /// Variable attribute table (readonly/integer/lower/upper). Mutable so /// `readonly`/`declare`/`unset` builtins can update attributes without /// re-allocating `_READONLY_X`-style marker strings. @@ -1228,6 +1231,8 @@ bitflags::bitflags! { const PIPEFAIL = 0b0000_0001_0000_0000; // set -o pipefail / SHOPT_pipefail const EXPAND_ALIAS = 0b0000_0010_0000_0000; // shopt expand_aliases const KEYWORD = 0b0000_0100_0000_0000; // set -k / SHOPT_k + const ERRTRACE = 0b0000_1000_0000_0000; // set -E / SHOPT_E + const FUNCTRACE = 0b0001_0000_0000_0000; // set -T / SHOPT_T } } @@ -1246,6 +1251,8 @@ impl BashFlags { "SHOPT_pipefail" => Some(Self::PIPEFAIL), "SHOPT_expand_aliases" => Some(Self::EXPAND_ALIAS), "SHOPT_k" => Some(Self::KEYWORD), + "SHOPT_E" => Some(Self::ERRTRACE), + "SHOPT_T" => Some(Self::FUNCTRACE), _ => None, } } @@ -1323,6 +1330,8 @@ struct SubshellSnapshot { last_bg_pid: Option, seconds_base: (crate::time_compat::Instant, i64), bash_subshell: u32, + err_trap_dormant: bool, + line_base: usize, } /// Interpreter state. @@ -1389,6 +1398,9 @@ pub struct Interpreter { jobs: SharedJobTable, /// Current line number for $LINENO current_line: usize, + /// Added to command line numbers: a trap handler counts its lines from + /// the line of the command that triggered it (bash). + line_base: usize, /// `unset LINENO` makes it an ordinary variable for the rest of the /// shell's life, like bash. lineno_unset: bool, @@ -1518,6 +1530,14 @@ pub struct Interpreter { /// True while executing a trap handler. Suppresses recursive DEBUG trap /// invocation to prevent amplification attacks (TM-DOS-035). in_trap: bool, + /// The ERR trap is inherited for display only: a subshell, command + /// substitution or pipeline stage without `set -E` keeps the parent's + /// handler visible to `trap -p` but does not run it (bash). `trap` + /// setting or resetting ERR clears it. + err_trap_dormant: bool, + /// The next `execute_command` is a multi-command pipeline stage: its own + /// status never fires ERR (the pipeline as a whole does). + err_trap_skip_stage: bool, /// Depth of if/while/until condition evaluation. /// Important decision: condition context is tracked as interpreter state so /// nested AND-OR lists can suppress ERR traps without weakening top-level @@ -1995,6 +2015,7 @@ impl Interpreter { ), jobs: jobs::new_shared_job_table(), current_line: 1, + line_base: 0, lineno_unset: false, source_depth: 0, #[cfg(feature = "http_client")] @@ -2043,6 +2064,8 @@ impl Interpreter { cancelled: Arc::new(AtomicBool::new(false)), hooks: Arc::new(crate::hooks::Hooks::default()), in_trap: false, + err_trap_dormant: false, + err_trap_skip_stage: false, condition_sequence_depth: 0, deferred_proc_subs: Vec::new(), proc_sub_paths: HashSet::new(), @@ -2168,6 +2191,7 @@ impl Interpreter { execution_budget: self.execution_budget.clone(), jobs: self.jobs.fork(), current_line: self.current_line, + line_base: self.line_base, lineno_unset: self.lineno_unset, #[cfg(feature = "http_client")] http_client: self.http_client.clone(), @@ -2213,6 +2237,10 @@ impl Interpreter { cancelled: Arc::clone(&self.cancelled), hooks: Arc::clone(&self.hooks), in_trap: false, + // A job or pipeline stage is a subshell: ERR stays dormant there + // unless `set -E`. + err_trap_dormant: self.err_trap_dormant || !self.flags.contains(BashFlags::ERRTRACE), + err_trap_skip_stage: false, condition_sequence_depth: self.condition_sequence_depth, deferred_proc_subs: Vec::new(), proc_sub_paths: HashSet::new(), @@ -2470,6 +2498,9 @@ impl Interpreter { // handler is awaited; clear the re-entrancy guard before each exec so // one cancelled script cannot suppress traps in the next script. self.in_trap = false; + self.err_trap_dormant = false; + self.err_trap_skip_stage = false; + self.line_base = 0; self.condition_sequence_depth = 0; self.loop_depth = 0; self.return_depth = 0; @@ -3371,18 +3402,8 @@ impl Interpreter { } } - // Run ERR trap on non-zero exit (unless in conditional chain). - // Lists are suppressed here because execute_list already fired - // the ERR trap for the failing subcommand; firing again would - // double-invoke the trap (e.g. `set -e; trap 'f' ERR; false`). - if exit_code != 0 { - let suppressed = matches!(command, Command::List(_)) - || matches!(command, Command::Pipeline(p) if p.negated) - || result.errexit_suppressed; - if !suppressed { - self.run_err_trap(&mut stdout, &mut stderr).await; - } - } + // The ERR trap already ran in `execute_command`, after the + // failing simple command, pipeline or `( )` itself. // errexit (set -e): stop on non-zero exit unless the callee marks // the status as suppressed (for example, a short-circuited AND-OR @@ -3528,9 +3549,15 @@ impl Interpreter { command: &'a Command, ) -> std::pin::Pin> + Send + 'a>> { Box::pin(async move { + // Whether ERR may fire after this command: bash decides before + // running it, so a trap the command itself sets (`f` setting + // ERR inside) does not fire for that same command. + let skip_stage = std::mem::take(&mut self.err_trap_skip_stage); + let err_armed = !skip_stage && self.err_trap_armed() && Self::fires_err_trap(command); + let emit_before = self.output_emit_count; self.check_cancelled()?; // Update current line for $LINENO - self.current_line = Self::command_line(command); + self.current_line = self.line_base + Self::command_line(command); // `set -n`: read commands without running them for the rest of // this exec (bash does the same in scripts; nothing can undo it, @@ -3650,10 +3677,57 @@ impl Interpreter { Ok(ExecResult::ok(String::new())) } }; - self.abort_line_on_error(result) + let mut result = self.abort_line_on_error(result); + if err_armed + && let Ok(r) = &mut result + && r.exit_code != 0 + && r.control_flow == ControlFlow::None + && !r.errexit_suppressed + && !self.is_in_condition_sequence() + { + // `$LINENO` in the handler is the failing command's line. + self.current_line = self.line_base + Self::command_line(command); + Box::pin(self.run_err_trap_after(r, emit_before)).await; + } + result }) } + /// An ERR trap is set and live (not dormant in a subshell). + fn err_trap_armed(&self) -> bool { + !self.err_trap_dormant && !self.in_trap && self.scoped.traps.contains_key("ERR") + } + + /// Commands whose own failure fires ERR, like bash: simple commands + /// (function calls included), multi-command pipelines, `( )`, `[[ ]]` + /// and `(( ))`. Lists and other compounds do not; the failing command + /// inside them fires instead. + fn fires_err_trap(command: &Command) -> bool { + match command { + Command::Simple(_) => true, + Command::Pipeline(p) => !p.negated && p.commands.len() > 1, + Command::Compound(c, _) => matches!( + c, + CompoundCommand::Subshell(_) + | CompoundCommand::Arithmetic(_) + | CompoundCommand::Conditional(_) + ), + Command::List(_) | Command::Function(_) => false, + } + } + + /// Run the ERR trap after a failing command, its output following the + /// command's own (streamed first, so neither is dropped). + async fn run_err_trap_after(&mut self, result: &mut ExecResult, emit_before: u64) { + self.maybe_emit_output(&result.stdout, &result.stderr, emit_before); + self.last_exit_code = result.exit_code; + let mut stdout = crate::StreamData::new(); + let mut stderr = crate::StreamData::new(); + self.run_err_trap(&mut stdout, &mut stderr).await; + result.stdout.append(&stdout); + result.stderr.append(&stderr); + } + /// Turn a line-abort error (or an arithmetic error recorded by a /// read-only evaluation site) into `ControlFlow::Abort`, keeping the /// output produced so far. @@ -3731,6 +3805,7 @@ impl Interpreter { let snap = self.snapshot_subshell_state(); let saved_call_stack = self.call_stack.clone(); self.bash_subshell += 1; + self.enter_subshell_err_scope(); let saved_exit = self.last_exit_code; let saved_coproc = self.coproc_buffers.clone(); @@ -5576,6 +5651,7 @@ impl Interpreter { /// keep only exported scalars in `variables`. The caller is expected to /// have just taken a snapshot to undo this on return. See issue #1777. fn reset_state_for_child_shell(&mut self) { + self.line_base = 0; let exported_names: Vec = self .scoped .var_attrs @@ -5882,7 +5958,10 @@ impl Interpreter { Ok((scope, stdin)) => { let result = match command { Command::Simple(simple) => self.execute_simple_command(simple, stdin).await, - _ => self.execute_command(command).await, + _ => { + self.err_trap_skip_stage = count > 1; + self.execute_command(command).await + } }; self.exit_pipeline_stage(scope, result) } @@ -6069,6 +6148,8 @@ impl Interpreter { sink.write(out.as_bytes()); })); child.pipe_out = Some(Arc::clone(&pipe)); + // The pipeline as a whole fires ERR, not its stages. + child.err_trap_skip_stage = true; let write_end = pipe::WriteEnd(Arc::clone(&pipe)); let read_end = input.take().map(pipe::ReadEnd); let command = command.clone(); @@ -6123,7 +6204,10 @@ impl Interpreter { let (scope, stdin) = self.enter_pipeline_stage(last, None, !lastpipe)?; let result = match last { Command::Simple(simple) => self.execute_simple_command(simple, stdin).await, - _ => self.execute_command(last).await, + _ => { + self.err_trap_skip_stage = true; + self.execute_command(last).await + } }; self.exit_pipeline_stage(scope, result) }); @@ -6255,6 +6339,9 @@ impl Interpreter { { self.bash_subshell += 1; } + if subshell { + self.enter_subshell_err_scope(); + } let mut scope = PipelineStageScope { saved, prev_pipeline_stdin: None, @@ -6444,15 +6531,6 @@ impl Interpreter { ..Default::default() }); } - - // Check if first command in a semicolon-separated list failed => ERR trap - let first_op_is_semicolon = list - .rest - .first() - .is_some_and(|(op, _)| matches!(op, ListOperator::Semicolon)); - if exit_code != 0 && first_op_is_semicolon && !self.is_in_condition_sequence() { - self.run_err_trap(&mut stdout, &mut stderr).await; - } } for (i, (op, cmd)) in list.rest.iter().enumerate() { @@ -6538,14 +6616,6 @@ impl Interpreter { ..Default::default() }); } - - // ERR trap follows the same AND-OR suppression as errexit. - if exit_code != 0 - && !exit_code_from_conditional_context - && !self.is_in_condition_sequence() - { - self.run_err_trap(&mut stdout, &mut stderr).await; - } } } } @@ -7662,6 +7732,7 @@ impl Interpreter { functions: &self.scoped.functions, aliases: Arc::make_mut(&mut self.scoped.aliases), traps: Arc::make_mut(&mut self.scoped.traps), + err_trap_dormant: &mut self.err_trap_dormant, var_attrs: Arc::make_mut(&mut self.scoped.var_attrs), namerefs: Arc::make_mut(&mut self.scoped.namerefs), dir_stack: Arc::make_mut(&mut self.scoped.dir_stack), @@ -7732,6 +7803,7 @@ impl Interpreter { functions: &self.scoped.functions, aliases: Arc::make_mut(&mut self.scoped.aliases), traps: Arc::make_mut(&mut self.scoped.traps), + err_trap_dormant: &mut self.err_trap_dormant, var_attrs: Arc::make_mut(&mut self.scoped.var_attrs), namerefs: Arc::make_mut(&mut self.scoped.namerefs), dir_stack: Arc::make_mut(&mut self.scoped.dir_stack), @@ -8552,7 +8624,11 @@ impl Interpreter { // Use execute_script_body (not execute) to preserve depth counters. self.return_depth += 1; self.source_depth += 1; - let exec_result = self.execute_script_body(&script, false, true).await; + let emit_before = self.output_emit_count; + let mut exec_result = self.execute_script_body(&script, false, true).await; + if let Ok(r) = &mut exec_result { + Box::pin(self.run_return_trap(r, emit_before)).await; + } self.return_depth -= 1; self.source_depth -= 1; @@ -9030,9 +9106,19 @@ impl Interpreter { // Execute function body. Always restore call state even on error. // The body starts outside any loop (bash resets `loop_level`). let saved_loop_depth = std::mem::replace(&mut self.loop_depth, 0); + // Body lines count from the definition, not a calling trap handler. + let saved_line_base = std::mem::replace(&mut self.line_base, 0); + let saved_traps = self.enter_function_traps(); + let emit_before = self.output_emit_count; self.return_depth += 1; - let result = self.execute_command(&func_def.body).await; + let mut result = self.execute_command(&func_def.body).await; + if let Ok(r) = &mut result { + // Still in the function: the handler sees its locals. + Box::pin(self.run_return_trap(r, emit_before)).await; + } self.return_depth -= 1; + self.leave_function_traps(saved_traps); + self.line_base = saved_line_base; self.loop_depth = saved_loop_depth; if let Some(cb) = saved_callback { self.output_callback = Some(cb); @@ -10242,6 +10328,16 @@ impl Interpreter { last_bg_pid: self.last_bg_pid.clone(), seconds_base: self.seconds_base, bash_subshell: self.bash_subshell, + err_trap_dormant: self.err_trap_dormant, + line_base: self.line_base, + } + } + + /// Entering a subshell environment (`( )`, `$( )`, a pipeline stage): + /// without `set -E` the ERR trap stays listed but no longer runs. + fn enter_subshell_err_scope(&mut self) { + if !self.flags.contains(BashFlags::ERRTRACE) { + self.err_trap_dormant = true; } } @@ -10259,6 +10355,8 @@ impl Interpreter { self.last_bg_pid = snap.last_bg_pid; self.seconds_base = snap.seconds_base; self.bash_subshell = snap.bash_subshell; + self.err_trap_dormant = snap.err_trap_dormant; + self.line_base = snap.line_base; } /// Perform the redirections of a null command (no command word) and @@ -10440,6 +10538,7 @@ impl Interpreter { // mutable state so mutations don't leak to the parent. let snapshot = self.snapshot_subshell_state(); self.bash_subshell += 1; + self.enter_subshell_err_scope(); // THREAT[TM-DOS-111]: Expansion happens before top-level output caps, // so reserve every byte before growing the substitution buffer. let mut stdout = BudgetedString::new(Some(&self.execution_budget))?; @@ -11562,8 +11661,10 @@ impl Interpreter { }; let was_in_trap = self.in_trap; self.in_trap = true; + let saved_line_base = std::mem::replace(&mut self.line_base, self.current_line.saturating_sub(1)); let emit_before = self.output_emit_count; let result = self.execute_command_sequence(&trap_script.commands).await; + self.line_base = saved_line_base; self.in_trap = was_in_trap; let Ok(trap_result) = result else { return None; @@ -11574,6 +11675,55 @@ impl Interpreter { Some((trap_result.control_flow, trap_result.exit_code)) } + /// Function entry: like bash, the body does not inherit the ERR trap + /// without `set -E`, nor the RETURN trap without `set -T`. Returns the + /// handlers set aside, for `leave_function_traps`. + fn enter_function_traps(&mut self) -> (Option, Option) { + let take = |this: &mut Self, key: &str, inherit: BashFlags| { + if this.flags.contains(inherit) || !this.scoped.traps.contains_key(key) { + None + } else { + this.traps_mut().remove(key) + } + }; + let err = take(self, "ERR", BashFlags::ERRTRACE); + let ret = take(self, "RETURN", BashFlags::FUNCTRACE); + (err, ret) + } + + /// Function return: a handler set aside on entry comes back unless the + /// body set its own, which then stays (bash `trap_if_untrapped`). + fn leave_function_traps(&mut self, saved: (Option, Option)) { + for (key, handler) in [("ERR", saved.0), ("RETURN", saved.1)] { + if let Some(handler) = handler + && !self.scoped.traps.contains_key(key) + { + self.traps_mut().insert(key.to_string(), handler); + } + } + } + + /// Run the RETURN trap as a function or sourced file finishes, after + /// its output. The handler keeps `$?`; an `exit` in it ends the shell. + async fn run_return_trap(&mut self, result: &mut ExecResult, emit_before: u64) { + if self.in_trap { + return; + } + let Some(trap_cmd) = self.scoped.traps.get("RETURN").cloned() else { + return; + }; + self.maybe_emit_output(&result.stdout, &result.stderr, emit_before); + let saved_exit = self.last_exit_code; + let flow = self + .run_trap_command(&trap_cmd, &mut result.stdout, &mut result.stderr) + .await; + self.last_exit_code = saved_exit; + if let Some((flow @ ControlFlow::Exit(code), _)) = flow { + result.control_flow = flow; + result.exit_code = code; + } + } + /// The trap set for `signal`, under any of the spellings `trap` accepts /// (`TERM`, `SIGTERM`, `15`). fn signal_trap(&self, signal: i32) -> Option { diff --git a/crates/bashkit/tests/spec_cases/bash/bashbox-err-trap.test.sh b/crates/bashkit/tests/spec_cases/bash/bashbox-err-trap.test.sh index afc0282ad..41f59923f 100644 --- a/crates/bashkit/tests/spec_cases/bash/bashbox-err-trap.test.sh +++ b/crates/bashkit/tests/spec_cases/bash/bashbox-err-trap.test.sh @@ -4,7 +4,6 @@ ### bashbox_err_trap_functions_do_not_inherit_the_err_trap # functions do not inherit the ERR trap -### skip: TODO bashbox corpus gap, bashkit output differs from real bash trap 'echo err' ERR; f(){ false; echo in; }; f; echo out ### expect in @@ -21,7 +20,6 @@ end ### bashbox_err_trap_set_e_passes_it_to_functions # set -E passes it to functions -### skip: TODO bashbox corpus gap, bashkit output differs from real bash set -E; trap 'echo err $LINENO' ERR; f(){ false; }; f; echo $-; set +E; f; echo end ### expect err 1 @@ -33,7 +31,6 @@ end ### bashbox_err_trap_a_trap_set_in_a_function_stays # a trap set in a function stays -### skip: TODO bashbox corpus gap, bashkit output differs from real bash f(){ trap 'echo ferr $LINENO' ERR; false; } f f @@ -49,7 +46,6 @@ end ### bashbox_err_trap_the_call_fires_only_a_trap_set_before_it # the call fires only a trap set before it -### skip: TODO bashbox corpus gap, bashkit output differs from real bash f(){ trap 'echo t' ERR; return 1; }; f; false; echo end ### expect t @@ -76,7 +72,6 @@ end ### bashbox_err_trap_tested_function_calls_do_not_fire_it # tested function calls do not fire it -### skip: TODO bashbox corpus gap, bashkit output differs from real bash trap 'echo err' ERR; f(){ false; echo in; }; f || true; f && true; ! f; if f; then :; fi; echo end ### expect in @@ -107,7 +102,6 @@ end ### bashbox_err_trap_a_failing_subshell_fires_it_once # a failing subshell fires it once -### skip: TODO bashbox corpus gap, bashkit output differs from real bash trap 'echo err' ERR; (false); (false; true); echo end ### expect err @@ -124,7 +118,6 @@ end ### bashbox_err_trap_command_substitution_drops_the_err_trap # command substitution drops the ERR trap -### skip: TODO bashbox corpus gap, bashkit output differs from real bash trap 'echo err' ERR; x=$(false; echo in); echo "[$x]"; f(){ false; }; y=$(f); echo "[$y]" ### expect [in] @@ -151,7 +144,6 @@ end ### bashbox_err_trap_errexit_ignores_conditions # errexit ignores conditions -### skip: TODO bashbox corpus gap, bashkit output differs from real bash set -e; if false; then :; fi; while false; do :; done; f(){ false; echo in; }; f || true; f && true; ! f; echo end ### expect in @@ -162,7 +154,6 @@ end ### bashbox_err_trap_set_t_passes_the_return_trap_to_functions # set -T passes the RETURN trap to functions -### skip: TODO bashbox corpus gap, bashkit output differs from real bash set -T; echo $-; trap 'echo r' RETURN; g(){ :; }; f(){ g; }; f; echo end ### expect hBTc @@ -180,7 +171,6 @@ end ### bashbox_err_trap_set_o_lists_the_options # set -o lists the options -### skip: TODO bashbox corpus gap, bashkit output differs from real bash set -o | grep -E "errtrace|functrace"; set -E; set +o | grep errtrace ### expect errtrace off diff --git a/crates/bashkit/tests/spec_cases/bash/bashbox-interpreter-builtins.test.sh b/crates/bashkit/tests/spec_cases/bash/bashbox-interpreter-builtins.test.sh index e56258158..879809b86 100644 --- a/crates/bashkit/tests/spec_cases/bash/bashbox-interpreter-builtins.test.sh +++ b/crates/bashkit/tests/spec_cases/bash/bashbox-interpreter-builtins.test.sh @@ -193,7 +193,6 @@ after 1 ### bashbox_interpreter_builtins_fires_after_the_body_of_the_function_that_set_it # fires after the body of the function that set it -### skip: TODO bashbox corpus gap, bashkit output differs from real bash f(){ trap 'echo ret' RETURN; echo in; }; f; echo after ### expect in @@ -211,7 +210,6 @@ end ### bashbox_interpreter_builtins_stays_set_but_other_functions_do_not_see_it # stays set but other functions do not see it -### skip: TODO bashbox corpus gap, bashkit output differs from real bash f(){ trap 'echo ret' RETURN; }; g(){ echo g; }; f; echo top; g; h(){ g; }; h ### expect ret @@ -222,7 +220,6 @@ g ### bashbox_interpreter_builtins_sees_the_function_locals # sees the function locals -### skip: TODO bashbox corpus gap, bashkit output differs from real bash f(){ local v=loc; trap 'echo $v' RETURN; }; f ### expect loc @@ -237,7 +234,6 @@ cat < missing.txt; echo $? ### bashbox_interpreter_builtins_creates_the_file # <> creates the file -### skip: TODO bashbox corpus gap, bashkit output differs from real bash cat <> rw.txt; echo $?; test -f rw.txt && echo created ### expect 0 @@ -270,7 +266,6 @@ e ### bashbox_interpreter_builtins_and_write_and_append_both_streams # &> and &>> write and append both streams -### skip: TODO bashbox corpus gap, bashkit output differs from real bash { echo a; echo b >&2; } &> all.txt; { echo c; } &>> all.txt; cat all.txt ### expect a @@ -335,7 +330,6 @@ a=(); echo ${#a[@]}; a=(1 2 3); a=(); echo ${#a[@]} ### bashbox_interpreter_builtins_to_a_readonly_variable_fails # to a readonly variable fails -### skip: TODO bashbox corpus gap, bashkit output differs from real bash readonly r=1 r=2 echo $? @@ -359,7 +353,6 @@ x=0; echo $((0 && (x=5))) $x $((1 || (x=6))) $x $((1 && 2)) $((0 || 0)) $((0 || ### bashbox_interpreter_builtins_unary_operators # unary operators -### skip: TODO bashbox corpus gap, bashkit output differs from real bash echo $((-(2+3))) $((+4)) $((!0)) $((!5)) $((~0)) $((++5)) $((--5)) ### expect -5 4 1 0 -1 5 5 @@ -404,7 +397,6 @@ a=(1 2 3); echo $(("1"+1)) $(( $(echo 2) * 3 )) $(( ${#a[@]} + 1 )) ### bashbox_interpreter_builtins_associative_array_elements # associative array elements -### skip: TODO bashbox corpus gap, bashkit output differs from real bash declare -A m; m[foo]=5; echo $((m[foo]*2)); (( m[bar]=3 )); echo ${m[bar]} ### expect 10 @@ -430,7 +422,6 @@ for ((i=$(echo 1); i<3; i++)); do echo $i; done ### bashbox_interpreter_builtins_case # /= -### skip: TODO bashbox corpus gap, bashkit output differs from real bash x=5; ((x/=0)); echo $? $x ### expect 1 5 @@ -438,7 +429,6 @@ x=5; ((x/=0)); echo $? $x ### bashbox_interpreter_builtins_case_2 # %= -### skip: TODO bashbox corpus gap, bashkit output differs from real bash x=5; ((x%=0)); echo $? $x ### expect 1 5 @@ -460,7 +450,6 @@ x=5; ((x%=0)); echo $? $x ### bashbox_interpreter_builtins_string_comparisons # string comparisons -### skip: TODO bashbox corpus gap, bashkit output differs from real bash [[ abc != a* ]]; echo $?; [[ abc != x* ]]; echo $?; [[ a < b ]]; echo $?; [[ b > a ]]; echo $?; [[ b < a ]]; echo $? ### expect 1 @@ -506,7 +495,6 @@ re="(b)(c)"; [[ abc =~ $re ]]; echo $? ${BASH_REMATCH[0]} ${BASH_REMATCH[2]}; [[ ### bashbox_interpreter_builtins_variable_is_set # variable is set -### skip: TODO bashbox corpus gap, bashkit output differs from real bash v=1; a=(1 2); [[ -v v ]]; echo $?; [[ -v a ]]; echo $?; [[ -v nope ]]; echo $? ### expect 0 @@ -524,7 +512,6 @@ nl ### bashbox_interpreter_builtins_unset_in_its_own_function_keeps_a_local_hiding_the_global # unset in its own function keeps a local hiding the global -### skip: TODO bashbox corpus gap, bashkit output differs from real bash f(){ local x=1; unset x; echo "[${x-unset}]"; x=2; echo $x; }; x=g; f; echo $x ### expect [unset] From 9bd5ebe9ea77d7943d46a342daf0101894fe6379 Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Thu, 8 Oct 2026 03:35:38 +0000 Subject: [PATCH 2/3] fix(interpreter): N>&word redirects and for (( )) substitutions N>&- closes the descriptor for the command (stderr vanishes, stdout writes fail), >&file and 1>&file send both streams to the file like &>file, 2>&file is an ambiguous redirect, and 2>&$fd duplicates the expanded descriptor instead of writing a file named after it. A failed &>/>& open drops the command's output. for (( )) headers run their $(...) each time an expression is evaluated. --- crates/bashkit/src/interpreter/mod.rs | 24 ++- crates/bashkit/src/interpreter/redirection.rs | 200 +++++++++++++++--- crates/bashkit/src/parser/lexer.rs | 5 +- crates/bashkit/src/parser/mod.rs | 26 +++ crates/bashkit/src/parser/tokens.rs | 4 + .../bash/bashbox-interpreter-builtins.test.sh | 4 - 6 files changed, 220 insertions(+), 43 deletions(-) diff --git a/crates/bashkit/src/interpreter/mod.rs b/crates/bashkit/src/interpreter/mod.rs index aa2060ba6..e232cc839 100644 --- a/crates/bashkit/src/interpreter/mod.rs +++ b/crates/bashkit/src/interpreter/mod.rs @@ -4304,7 +4304,8 @@ impl Interpreter { // Execute initialization if !arith_for.init.is_empty() { - self.execute_arithmetic_with_side_effects(&arith_for.init); + let init = self.arith_for_expr(&arith_for.init).await?; + self.execute_arithmetic_with_side_effects(&init); } self.counters.enter_loop(); @@ -4315,7 +4316,8 @@ impl Interpreter { // Check condition (if empty, always true) if !arith_for.condition.is_empty() { - let cond_result = self.evaluate_arithmetic(&arith_for.condition); + let condition = self.arith_for_expr(&arith_for.condition).await?; + let cond_result = self.evaluate_arithmetic(&condition); if cond_result == 0 { break; } @@ -4341,7 +4343,8 @@ impl Interpreter { // Execute step if !arith_for.step.is_empty() { - self.execute_arithmetic_with_side_effects(&arith_for.step); + let step = self.arith_for_expr(&arith_for.step).await?; + self.execute_arithmetic_with_side_effects(&step); } } @@ -4768,6 +4771,18 @@ impl Interpreter { } } + /// A `for ((init; cond; step))` expression with its `$(...)` run, each + /// time it is evaluated, as `((...))` does. + async fn arith_for_expr<'e>(&mut self, expr: &'e str) -> Result> { + if expr.contains("$(") { + Ok(Box::pin(self.expand_command_subs_in_arithmetic(expr)) + .await? + .into()) + } else { + Ok(expr.into()) + } + } + /// Execute arithmetic expression with side effects (assignments, ++, --) fn execute_arithmetic_with_side_effects(&mut self, expr: &str) -> i64 { self.evaluate_arithmetic_with_assign(expr) @@ -11661,7 +11676,8 @@ impl Interpreter { }; let was_in_trap = self.in_trap; self.in_trap = true; - let saved_line_base = std::mem::replace(&mut self.line_base, self.current_line.saturating_sub(1)); + let saved_line_base = + std::mem::replace(&mut self.line_base, self.current_line.saturating_sub(1)); let emit_before = self.output_emit_count; let result = self.execute_command_sequence(&trap_script.commands).await; self.line_base = saved_line_base; diff --git a/crates/bashkit/src/interpreter/redirection.rs b/crates/bashkit/src/interpreter/redirection.rs index 4faf1cb8b..156211b8d 100644 --- a/crates/bashkit/src/interpreter/redirection.rs +++ b/crates/bashkit/src/interpreter/redirection.rs @@ -239,9 +239,20 @@ impl Interpreter { } } + // `N>&-` among other redirects (`2>&1 1>&-`) depends on their order, + // which only the fd-table path tracks. + let has_close = redirects.len() > 1 + && redirects.iter().any(|r| { + r.kind == RedirectKind::DupOutput + && matches!(r.target.parts.as_slice(), [WordPart::Literal(t)] if t == "-") + }); + // `N>file` (N>=3) opens fd N without touching stdout; only the // fd-table path keeps fd N separate from fd 1. - if (has_dup_output && has_file_redirect) || has_high_fd_file_redirect(redirects) { + if (has_dup_output && has_file_redirect) + || has_close + || has_high_fd_file_redirect(redirects) + { return self.apply_redirections_fd_table(result, redirects).await; } @@ -352,40 +363,29 @@ impl Interpreter { } RedirectKind::OutputBoth => { let target_path = self.expand_word(&redirect.target).await?; - let path = self.resolve_path(&target_path); - if let Some(target_fd) = dev_fd_alias(&path) { - // `&> /dev/fd/N` == `>&N 2>&N`, resolved against the - // original descriptors. - if target_fd == 2 { - let mut combined = std::mem::take(&mut result.stdout); - combined.append(&result.stderr); - result.stderr = combined; - } else { - self.dup_output_fast(&mut result, 1, target_fd).await?; - self.dup_output_fast(&mut result, 2, target_fd).await?; - } - } else if is_dev_null(&path) { - result.stdout = crate::StreamData::new(); - result.stderr = crate::StreamData::new(); - } else { - let mut combined = result.stdout.as_bytes().to_vec(); - combined.extend_from_slice(result.stderr.as_bytes()); - if let Err(e) = self.fs.write_file(&path, &combined).await { - result.stderr = self - .redirect_error(&target_path, &io_error_reason(&e)) - .into(); - result.exit_code = 1; - return Ok(result); - } - result.stdout = crate::StreamData::new(); - result.stderr = crate::StreamData::new(); + if !self.output_both_fast(&mut result, &target_path).await? { + return Ok(result); } } RedirectKind::DupOutput => { let target = self.expand_word(&redirect.target).await?; - let target_fd: i32 = target.parse().unwrap_or(1); let src_fd = redirect.fd.unwrap_or(1); - self.dup_output_fast(&mut result, src_fd, target_fd).await?; + match DupTarget::parse(&target) { + DupTarget::Fd(target_fd) => { + self.dup_output_fast(&mut result, src_fd, target_fd).await?; + } + DupTarget::Close => self.close_output_fast(&mut result, src_fd), + // `>&file` / `1>&file`: bash's spelling of `&>file`. + DupTarget::File if src_fd == 1 => { + if !self.output_both_fast(&mut result, &target).await? { + return Ok(result); + } + } + DupTarget::File => { + self.ambiguous_dup_target(&mut result, &target); + return Ok(result); + } + } } RedirectKind::Input | RedirectKind::HereString @@ -398,6 +398,82 @@ impl Interpreter { Ok(result) } + /// `&>file` on the fast path: both streams go to the file. Returns + /// false when the file could not be opened (the command's output is + /// dropped and the error reported, as bash never runs it). + async fn output_both_fast( + &mut self, + result: &mut ExecResult, + target_path: &str, + ) -> Result { + let path = self.resolve_path(target_path); + if let Some(target_fd) = dev_fd_alias(&path) { + // `&> /dev/fd/N` == `>&N 2>&N`, resolved against the + // original descriptors. + if target_fd == 2 { + let mut combined = std::mem::take(&mut result.stdout); + combined.append(&result.stderr); + result.stderr = combined; + } else { + self.dup_output_fast(result, 1, target_fd).await?; + self.dup_output_fast(result, 2, target_fd).await?; + } + } else if is_dev_null(&path) { + result.stdout = crate::StreamData::new(); + result.stderr = crate::StreamData::new(); + } else { + let mut combined = result.stdout.as_bytes().to_vec(); + combined.extend_from_slice(result.stderr.as_bytes()); + if let Err(e) = self.fs.write_file(&path, &combined).await { + result.stdout = crate::StreamData::new(); + result.stderr = self + .redirect_error(target_path, &io_error_reason(&e)) + .into(); + result.exit_code = 1; + return Ok(false); + } + result.stdout = crate::StreamData::new(); + result.stderr = crate::StreamData::new(); + } + Ok(true) + } + + /// `N>&-` on the fast path: fd N is closed for the command. Writes to + /// a closed stderr vanish; output to a closed stdout fails the command + /// with a write error, as in bash. + // WTF: bash names the writing command (`echo: write error`); the + // redirect is applied after the command ran, so the name is not known. + fn close_output_fast(&self, result: &mut ExecResult, src_fd: i32) { + match src_fd { + 1 => { + if !std::mem::take(&mut result.stdout).is_empty() { + result + .stderr + .append(&crate::StreamData::from(self.closed_stdout_error())); + result.exit_code = 1; + } + } + 2 => result.stderr = crate::StreamData::new(), + // Closing fd 3+ for one command routes nothing. + _ => {} + } + } + + fn closed_stdout_error(&self) -> String { + format!( + "bash: line {}: write error: Bad file descriptor\n", + self.current_line + ) + } + + /// `N>&word` with N other than 1 and a word that is neither a number + /// nor `-`: bash refuses it without running the command. + fn ambiguous_dup_target(&self, result: &mut ExecResult, word: &str) { + result.stdout = crate::StreamData::new(); + result.stderr = self.redirect_error(word, "ambiguous redirect").into(); + result.exit_code = 1; + } + /// `src_fd>&target_fd` on the fast (single-pass) redirect path. async fn dup_output_fast( &mut self, @@ -583,9 +659,38 @@ impl Interpreter { } RedirectKind::DupOutput => { let target = self.expand_word(&redirect.target).await?; - let target_fd: i32 = target.parse().unwrap_or(1); let src_fd = redirect.fd.unwrap_or(1); - self.dup_output_fd_table(src_fd, target_fd, &mut fd1, &mut fd2); + match DupTarget::parse(&target) { + DupTarget::Fd(target_fd) => { + self.dup_output_fd_table(src_fd, target_fd, &mut fd1, &mut fd2); + } + DupTarget::Close => match src_fd { + 1 => fd1 = FdTarget::Closed, + 2 => fd2 = FdTarget::Closed, + n => self.pending_fd_targets.push((n, FdTarget::Closed)), + }, + // `>&file`: both streams to the file, like `&>file`. + DupTarget::File if src_fd == 1 => { + if target.ends_with('/') { + self.clear_pending_fd_redirect_state(); + self.rejects_directory_target(&target, &mut result); + return Ok(result); + } + let path = self.resolve_path(&target); + let file = if is_dev_null(&path) { + FdTarget::DevNull + } else { + FdTarget::WriteFile(path, target) + }; + fd1 = file.clone(); + fd2 = file; + } + DupTarget::File => { + self.clear_pending_fd_redirect_state(); + self.ambiguous_dup_target(&mut result, &target); + return Ok(result); + } + } } RedirectKind::Input | RedirectKind::HereString @@ -597,7 +702,14 @@ impl Interpreter { // Route stdout/stderr/fd3+ to their targets (non-async to avoid state machine bloat) let orig_stdout = std::mem::take(&mut result.stdout); - let orig_stderr = std::mem::take(&mut result.stderr); + let mut orig_stderr = std::mem::take(&mut result.stderr); + if matches!(fd1, FdTarget::Closed) && !orig_stdout.is_empty() { + // Writing to the closed stdout fails; the report goes to fd 2. + let mut report = crate::StreamData::from(self.closed_stdout_error()); + report.append(&orig_stderr); + orig_stderr = report; + result.exit_code = 1; + } let (new_stdout, mut new_stderr, file_writes) = route_fd_table_content( &orig_stdout, &orig_stderr, @@ -723,6 +835,28 @@ pub(super) fn push_redirect_scope_fds(scope: &mut Vec, redirects: &[Redirec } } +/// What the word of `N>&word` names once expanded. +enum DupTarget { + /// A descriptor number: duplicate it. + Fd(i32), + /// `-`: close fd N. + Close, + /// Anything else: a file (`>&file` means `&>file`). + File, +} + +impl DupTarget { + fn parse(word: &str) -> Self { + if word == "-" { + Self::Close + } else if !word.is_empty() && word.bytes().all(|b| b.is_ascii_digit()) { + word.parse().map_or(Self::File, Self::Fd) + } else { + Self::File + } + } +} + fn bad_fd_error(word: &str) -> ExecResult { ExecResult::err(format!("bash: {word}: Bad file descriptor\n"), 1) } diff --git a/crates/bashkit/src/parser/lexer.rs b/crates/bashkit/src/parser/lexer.rs index 2ce770053..8850f057e 100644 --- a/crates/bashkit/src/parser/lexer.rs +++ b/crates/bashkit/src/parser/lexer.rs @@ -479,8 +479,9 @@ impl<'a> Lexer<'a> { return Some(Token::DupFdCloseOut(fd)); } if target_str.is_empty() { - // Just N>& without target - treat as DupOutput with fd - return Some(Token::RedirectFd(fd)); + // `N>&word`: a descriptor, `-` or (for fd 1) a file, known + // only once the word is expanded. + return Some(Token::DupFdWord(fd)); } let target_fd: i32 = target_str.parse().unwrap_or(1); diff --git a/crates/bashkit/src/parser/mod.rs b/crates/bashkit/src/parser/mod.rs index 7c541cde6..6f50d7a63 100644 --- a/crates/bashkit/src/parser/mod.rs +++ b/crates/bashkit/src/parser/mod.rs @@ -691,6 +691,19 @@ impl<'a> Parser<'a> { heredoc_delim: None, }); } + Some(tokens::Token::DupFdWord(fd)) => { + let fd = *fd; + self.advance(); + if let Ok(target) = self.expect_word() { + redirects.push(Redirect { + fd: Some(fd), + fd_var: None, + kind: RedirectKind::DupOutput, + target, + heredoc_delim: None, + }); + } + } Some(tokens::Token::DupInput) => { self.advance(); if let Ok(target) = self.expect_word() { @@ -2883,6 +2896,7 @@ impl<'a> Parser<'a> { | tokens::Token::RedirectFdAppend(_) | tokens::Token::DupFd(..) | tokens::Token::DupFdCloseOut(_) + | tokens::Token::DupFdWord(_) | tokens::Token::DupInput | tokens::Token::DupFdIn(..) | tokens::Token::DupFdClose(_) @@ -3171,6 +3185,18 @@ impl<'a> Parser<'a> { heredoc_delim: None, }); } + Some(tokens::Token::DupFdWord(fd)) => { + let fd = *fd; + self.advance(); + let target = self.expect_word()?; + redirects.push(Redirect { + fd: Some(fd), + fd_var: None, + kind: RedirectKind::DupOutput, + target, + heredoc_delim: None, + }); + } Some(tokens::Token::DupInput) => { let fd_var = Self::pop_fd_var(words); self.advance(); diff --git a/crates/bashkit/src/parser/tokens.rs b/crates/bashkit/src/parser/tokens.rs index 0cd2f25eb..e805111ec 100644 --- a/crates/bashkit/src/parser/tokens.rs +++ b/crates/bashkit/src/parser/tokens.rs @@ -142,6 +142,10 @@ pub enum Token { /// Duplicate fd to another (e.g., 2>&1) DupFd(i32, i32), + /// `N>&` followed by a word that is not a digit or `-` (`2>&$fd`, + /// `1>&file`): the word decides at expansion time. + DupFdWord(i32), + /// Close output fd (e.g., 4>&-) DupFdCloseOut(i32), diff --git a/crates/bashkit/tests/spec_cases/bash/bashbox-interpreter-builtins.test.sh b/crates/bashkit/tests/spec_cases/bash/bashbox-interpreter-builtins.test.sh index 879809b86..89c96b27d 100644 --- a/crates/bashkit/tests/spec_cases/bash/bashbox-interpreter-builtins.test.sh +++ b/crates/bashkit/tests/spec_cases/bash/bashbox-interpreter-builtins.test.sh @@ -249,7 +249,6 @@ err ### bashbox_interpreter_builtins_closing_stderr # closing stderr -### skip: TODO bashbox corpus gap, bashkit output differs from real bash ls /nope 2>&-; echo y ### expect y @@ -257,7 +256,6 @@ y ### bashbox_interpreter_builtins_file_sends_both_streams_to_the_file # >&file sends both streams to the file -### skip: TODO bashbox corpus gap, bashkit output differs from real bash { echo z; echo e >&2; } >& both.txt; cat both.txt ### expect z @@ -289,7 +287,6 @@ echo a > /no/such/dir/f; echo $? ### bashbox_interpreter_builtins_a_missing_directory_fails_2 # a missing directory fails >& -### skip: TODO bashbox corpus gap, bashkit output differs from real bash echo z >& /no/such/dir/f; echo $? ### expect 1 @@ -413,7 +410,6 @@ declare -A m; m[foo]=5; echo $((m[foo]*2)); (( m[bar]=3 )); echo ${m[bar]} ### bashbox_interpreter_builtins_c_style_for_header_with_a_substitution # c-style for header with a substitution -### skip: TODO bashbox corpus gap, bashkit output differs from real bash for ((i=$(echo 1); i<3; i++)); do echo $i; done ### expect 1 From 06d4460fbc168283bddf11a4496acaac26a031bf Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Thu, 8 Oct 2026 03:44:18 +0000 Subject: [PATCH 3/3] test(interpreter): trap scoping and N>&word spec cases; record in knowledge --- crates/bashkit/src/interpreter/redirection.rs | 16 +-- .../spec_cases/bash/pipes-redirects.test.sh | 45 ++++++ .../tests/spec_cases/bash/trap-scope.test.sh | 136 ++++++++++++++++++ knowledge/foundations/builtins.md | 17 +++ knowledge/log.md | 1 + knowledge/operations/limitations.md | 2 +- 6 files changed, 208 insertions(+), 9 deletions(-) create mode 100644 crates/bashkit/tests/spec_cases/bash/trap-scope.test.sh diff --git a/crates/bashkit/src/interpreter/redirection.rs b/crates/bashkit/src/interpreter/redirection.rs index 156211b8d..5e8d109e9 100644 --- a/crates/bashkit/src/interpreter/redirection.rs +++ b/crates/bashkit/src/interpreter/redirection.rs @@ -445,16 +445,16 @@ impl Interpreter { // redirect is applied after the command ran, so the name is not known. fn close_output_fast(&self, result: &mut ExecResult, src_fd: i32) { match src_fd { - 1 => { - if !std::mem::take(&mut result.stdout).is_empty() { - result - .stderr - .append(&crate::StreamData::from(self.closed_stdout_error())); - result.exit_code = 1; - } + 1 if !result.stdout.is_empty() => { + result.stdout = crate::StreamData::new(); + result + .stderr + .append(&crate::StreamData::from(self.closed_stdout_error())); + result.exit_code = 1; } 2 => result.stderr = crate::StreamData::new(), - // Closing fd 3+ for one command routes nothing. + // Nothing written to stdout; closing fd 3+ for one command + // routes nothing. _ => {} } } diff --git a/crates/bashkit/tests/spec_cases/bash/pipes-redirects.test.sh b/crates/bashkit/tests/spec_cases/bash/pipes-redirects.test.sh index b595d878e..d1cf74938 100644 --- a/crates/bashkit/tests/spec_cases/bash/pipes-redirects.test.sh +++ b/crates/bashkit/tests/spec_cases/bash/pipes-redirects.test.sh @@ -370,3 +370,48 @@ echo "${PIPESTATUS[*]}" got y 141 0 ### end + + +### dup_word_to_fd1_writes_both_streams +# 1>&file is &>file +{ echo o; echo e >&2; } 1>&both.txt +cat both.txt +### expect +o +e +### end + +### dup_word_to_fd2_is_ambiguous +# 2>&file is an ambiguous redirect and the command does not run +echo b 2>&out.txt +echo "status $?" +test -e out.txt || echo "no file" +### expect +status 1 +no file +### end + +### dup_expanded_fd +# 2>&$fd duplicates the descriptor the word expands to +fd=1 +{ echo e >&2; } 2>&$fd | tr e E +### expect +E +### end + +### close_stdout_after_dup +# 2>&1 1>&- sends stderr to the old stdout and drops stdout +f() { echo out; echo err >&2; } +f 2>&1 1>&- | sed 's/^.*write error/write error/' +### expect +write error: Bad file descriptor +err +### end + +### failed_output_both_runs_nothing +# &> into a missing directory drops the output and fails +echo z &> /no/such/dir/f +echo "status $?" +### expect +status 1 +### end diff --git a/crates/bashkit/tests/spec_cases/bash/trap-scope.test.sh b/crates/bashkit/tests/spec_cases/bash/trap-scope.test.sh new file mode 100644 index 000000000..80e21ff9b --- /dev/null +++ b/crates/bashkit/tests/spec_cases/bash/trap-scope.test.sh @@ -0,0 +1,136 @@ +# ERR and RETURN trap scoping (bash 5.2 semantics): which commands fire ERR, +# inheritance by functions/subshells (set -E / set -T), RETURN after functions +# and sourced files. + +### err_trap_compound_commands_do_not_fire_it +# the failing command inside a group, loop or case fires ERR, not the compound +trap 'echo err' ERR +{ false; echo x; } +for i in 1; do false; done +case a in a) false;; esac +if true; then false; fi +echo end +### expect +err +x +err +err +err +end +### end + +### err_trap_test_commands_and_pipelines_fire_it +# [[ ]], (( )) and a failing pipeline fire ERR once; ! and && heads do not +trap 'echo err' ERR +[[ a = b ]] +((0)) +true | false +false | true +! false +false && true +true && false +echo end +### expect +err +err +err +err +end +### end + +### err_trap_eval_fires_inside_and_for_itself +# eval fires for the failing command it runs and for its own status +trap 'echo err' ERR +eval false +eval 'false; true' +echo end +### expect +err +err +err +end +### end + +### err_trap_pipeline_stage_is_a_subshell +# a pipeline stage keeps the ERR trap dormant unless set -E +trap 'echo err' ERR +{ false; echo x; } | cat +set -E +{ false; echo y; } | cat +### expect +x +err +y +### end + +### err_trap_listed_in_a_subshell +# a subshell lists the inherited ERR trap without running it; one it sets runs +trap 'echo err' ERR +(trap -p ERR; false; echo in) +x=$(trap -p ERR) +echo "[$x]" +(trap 'echo sub' ERR; false; echo z) +### expect +trap -- 'echo err' ERR +in +[trap -- 'echo err' ERR] +sub +z +### end + +### err_trap_function_restores_the_callers_trap +# a function that resets ERR gets the caller's back; one that sets it keeps its own +trap 'echo A' ERR +f() { trap - ERR; return 1; } +f +trap -p ERR +g() { trap 'echo B' ERR; return 1; } +g +trap -p ERR +### expect +A +trap -- 'echo A' ERR +B +trap -- 'echo B' ERR +### end + +### err_trap_lineno_is_the_failing_line +# $LINENO in the handler is the line of the failing command or call +trap 'echo "err $LINENO"' ERR +f() { + false +} +f +false +### expect +err 5 +err 6 +### end + +### return_trap_runs_after_source +# the RETURN trap runs when a sourced file finishes, and in functions only with set -T +echo 'echo in' > ret_src.sh +trap 'echo "ret ${FUNCNAME[0]:-top}"' RETURN +. ./ret_src.sh +f() { . ./ret_src.sh; } +f +set -T +f +### expect +in +ret top +in +in +ret f +ret f +### end + +### return_trap_keeps_status +# the RETURN handler sees $? and leaves the function status alone +f() { trap 'echo "ret $?"' RETURN; false; } +f +echo "status $?" +### expect +ret 1 +status 1 +### end diff --git a/knowledge/foundations/builtins.md b/knowledge/foundations/builtins.md index 876a4023a..08be3b4f9 100644 --- a/knowledge/foundations/builtins.md +++ b/knowledge/foundations/builtins.md @@ -253,6 +253,23 @@ Internal builtins that need interpreter state receive it via `Context.shell`: `DEBUG`, `ERR`, `RETURN`), accepts `0`/`2`/`int`/`SIGINT`, rejects unknown specs (status 1) and lists in bash order (EXIT, signals by number, DEBUG, ERR, RETURN) with `sh_single_quote` quoting, as `alias` does. +- ERR fires in `execute_command`, after the failing command itself, for + the commands bash fires it for: simple commands (function calls and + `eval` included), multi-command pipelines, `( )`, `[[ ]]` and `(( ))`. + Lists, groups, loops, `if`/`case` never fire it themselves; the failing + command inside does. Whether it may fire is decided before the command + runs (bash's `was_error_trap`), so a function that sets ERR does not fire + it for its own call. Condition contexts (`if`/`while` tests, non-final + `&&`/`||` elements, `!`) suppress it like errexit. `$LINENO` in any trap + handler counts from the triggering line (`Interpreter::line_base`). +- ERR and RETURN scoping: without `set -E` a function body runs with ERR + removed, and a subshell, `$( )` or pipeline stage keeps ERR listed for + `trap -p` but dormant (`err_trap_dormant`, cleared when `trap` sets or + resets ERR there). Without `set -T` a function body runs with RETURN + removed. On return the caller's handler comes back only if the body left + that trap unset (bash `trap_if_untrapped`), so a trap a function sets + stays. RETURN runs when a function body or sourced file finishes, before + locals are popped, keeping `$?` and the function's status. - `hash` keeps no table (every lookup walks the virtual PATH): a bare `hash` reports `hash table empty`, `hash NAME` only checks that NAME resolves. - `builtin NAME`, like `type`/`command -v`, treats a registered command that diff --git a/knowledge/log.md b/knowledge/log.md index 6d189ba8a..2ef1ff04f 100644 --- a/knowledge/log.md +++ b/knowledge/log.md @@ -97,6 +97,7 @@ * **Fix**: Named file descriptors follow bash. `{var}>file` (and `<`, `>>`, `<>`, `<<<`, `<<`, `>&N`, `<&N`) allocates the lowest free fd >= 10 into `var` (local, array element; readonly fails) on any command, simple or compound, and the fd stays open as bash never undoes `{var}` redirections; `{var}>&-` closes it and a closed fd is reused. `>&N`/`<&N` to a closed fd is `Bad file descriptor` (status 1), a dup to a word is `ambiguous redirect`. The lexer accepts multi-digit fds (`10>x`), `<>` and `N<<f` in a loop is capped by `max_file_descriptors` (TM-DOS-063 tests updated: a prior `$v` value is ignored). Gaps in [Limitations](operations/limitations.md) ("Named fds / `<>`"). * **Fix**: xargs runs its command with stdin at EOF (`printf '' | xargs wc -l` prints 0); an empty pipe now reaches compound commands and functions as stdin at EOF; `printf %q` leaves printable non-ASCII unescaped under UTF-8. * **Fix**: Interpreter-core parity from the BashBox corpus (bash-verified cases unskipped). `break`/`continue` never leave a caller's loop and warn outside one; `return` outside a function fails with 2, `return` with no argument returns `$?`, and `return` ends a sourced file; `${x!}`/`${}` report "bad substitution" and `${1:=x}` "cannot assign in this way", abandoning the line; ERR/DEBUG traps keep `$?`; `trap` canonicalizes specs and lists in bash order with re-readable quoting (as do `alias` and a bare `set`); `getopts` always writes `OPTIND` (initialized to 1); `unset` parses options only before operands and refuses readonly elements; `a[-5]=x` past the start is a bad subscript; `[[ -eq ]]` operands are arithmetic; an invalid `=~` regex returns 2; `set -k`; `read -d ''`; `dirs`/`pushd` abbreviate `$HOME` as `~`; `hash`, `builtin`, `source` error statuses. `<>` and ENOTDIR under a regular file stay open. See [Builtins](foundations/builtins.md#control-flow-and-shell-state-builtins). +* **Fix**: ERR and RETURN traps follow bash's scoping. ERR fires once, after the failing simple command, multi-command pipeline, `( )`, `[[ ]]` or `(( ))` (not after lists or compound commands, not for a function call that set the trap itself); functions inherit ERR only under `set -E` and RETURN only under `set -T`; subshells, `$( )` and pipeline stages list ERR but do not run it without `set -E`; RETURN runs after a function or sourced file; handler `$LINENO` counts from the failing line. `N>&word` resolves at expansion: `N>&-` closes the fd for the command, `>&file`/`1>&file` mean `&>file`, `2>&file` is an ambiguous redirect, `2>&$fd` dups the expanded fd (it wrote a file named after it). `for ((...))` headers run their `$(...)`. All 27 skipped `bashbox-interpreter-builtins`/`bashbox-err-trap` cases pass; new `trap-scope.test.sh`. See [Builtins](foundations/builtins.md#control-flow-and-shell-state-builtins). ## 2026-10-06 diff --git a/knowledge/operations/limitations.md b/knowledge/operations/limitations.md index 9d0713ddf..1fda40cae 100644 --- a/knowledge/operations/limitations.md +++ b/knowledge/operations/limitations.md @@ -108,7 +108,7 @@ Target: IEEE 1003.1-2024 Shell Command Language. | `bash`/`sh` | `-c`, `-n`, `-e`, `-x`, `-u`, `-f`, `-o option`, script files, stdin, `--version`, `--help` | Login shell | | Expansion errors | `set -u` unbound variables stop the shell | `$((1/0))` evaluates to 0 and `${x!}` expands empty; bash reports them and drops the rest of the line the failing command ends on (`bashbox_line_number_an_expansion_error_drops_the_rest_of_the_line_it_ends_on`, skipped) | | Process substitution | `<(cmd)` and `>(cmd)` as filenames, nesting, redirections | The path is `/dev/fd/proc_sub_`, not bash's `/dev/fd/63`. Several interpreters can share one filesystem, so a per-shell descending number would let one tenant's substitution overwrite another's. A script that parses the number sees a different one (`scripts/bash-oracle/cases/process-substitution-dev-fd.sh`) | -| Named fds / `<>` | `{var}>file`, `{var}file`, `{var}>>file`, `{var}<<&N`/`<&N` allocate the lowest free fd >= 10 on any command (persisting, as in bash); `{var}>&-` closes; `>&N` with N closed is `Bad file descriptor`; `N<>file`, `N<<file` has no shared read/write offset (writes append); `3&1`, `( ... ) 3>&1`) is dropped; a `Bad file descriptor` message ignores redirect order (`2>&1 >&7`) | +| Named fds / `<>` | `{var}>file`, `{var}file`, `{var}>>file`, `{var}<<&N`/`<&N` allocate the lowest free fd >= 10 on any command (persisting, as in bash); `{var}>&-` closes; `>&N` with N closed is `Bad file descriptor`; `N<>file`, `N<<file` has no shared read/write offset (writes append); `3&1`, `( ... ) 3>&1`) is dropped; a `Bad file descriptor` message ignores redirect order (`2>&1 >&7`); output to a stdout closed by the command's own `>&-` reports `write error: Bad file descriptor` without naming the command, since the redirect is applied after it ran | | Brace expansion | Runs before other expansions on unquoted literal text; lists, nesting, ranges with step and zero padding, quoted/escaped braces and commas | Works on parsed words, not raw text: `$v{1,2}` expands `$v` then appends `1`/`2` (bash reads `$v1`, `$v2`); write `${v}` | ## Builtins