From 304b1e938cd0b41b6f691d11242f602b58791c85 Mon Sep 17 00:00:00 2001 From: Chet Ramey Date: Mon, 11 Oct 2021 10:38:16 -0400 Subject: [PATCH] preserve traps across a failed exec; small changes to how completion options interact; fix a bug with arithmetic for commands and LINENO --- CWRU/CWRU.chlog | 51 +++++++++++++++++++++++++++++ builtins/exec.def | 7 ++-- doc/bash.1 | 8 +++-- lib/readline/complete.c | 5 +-- lib/readline/terminal.c | 6 +++- parse.y | 4 ++- subst.c | 7 ++-- tests/RUN-ONE-TEST | 2 +- tests/array.right | 2 ++ tests/array15.sub | 15 +++++++++ tests/exec.right | 13 ++++++-- tests/exec3.sub | 37 +++++++++++++++++++-- tests/new-exp16.sub | 14 ++++++++ trap.c | 72 +++++++++++++++++++++++++++++++++++++++++ trap.h | 1 + 15 files changed, 228 insertions(+), 16 deletions(-) diff --git a/CWRU/CWRU.chlog b/CWRU/CWRU.chlog index 31c3fb53..7f909099 100644 --- a/CWRU/CWRU.chlog +++ b/CWRU/CWRU.chlog @@ -2228,3 +2228,54 @@ arrayfunc.c we don't inadvertently pass a value with bit 2 set, which would cause an off-by-one error in subscript parsing + 10/8 + ---- +trap.c + - restore_traps: inverse of reset_signal_handlers. This understands + how reset_signal_handlers changes the signal disposition while + leaving the trap string in place, and knows how to restore flags + and state based on that preserved trap string and whether or not + the signal is "special" to the shell. + +builtins/exec.def + - exec_builtin: instead of using restore_original_signals to completely + cancel the traps, call reset_signal_handlers so the trap strings are + preserved. Then if the exec fails (and we're not exiting) we can + look at the trap strings and see how to restore the trap state. + This calls restore_traps after reinitializing the shell signal + handlers, using the trap strings saved by reset_signal_handlers. + Fixes issue with not exiting after a failed exec clearing the EXIT + trap reported by Mark March , using the + approach suggested by Robert Elz + +subst.c + - expand_declaration_argument: when parsing options that modify + attributes that affect how the value is handled (i, c, u, etc.), + make sure to create an option string and call make_internal_declare + with options that start with a `+' so the attribute is off when + the assignment is performed and changes how the value is expanded. + From a report by Léa Gris + +lib/readline/complete.c + - rl_display_match_list: even if _rl_completion_prefix_display_length + is set to a non-zero value, pass the common prefix length to + fnprint if we've turned on colored completion prefixes; passes + through to fnprint via print_filename + - fnprint: add the ellipsis if prefix_bytes exceeds the + _rl_completion_prefix_display_length, add explicit check for + prefix_bytes being longer since print_filename passes it through + if colored-completion-prefix is set. This means that while + completion-prefix-display-length still has precedence over + colored-completion-prefix, it doesn't override it if both are set + and the common prefix length is shorter than + completion-prefix-display-length. From a report by + Christoph Anton Mitterer + + 10/10 + ----- +parse.y + - parse_dparen: if the last token is FOR, increment word_top and assign + word_lineno like for other for loops. Fixes bug with LINENO after + arithmetic for commands reported by + Tom Coleman + diff --git a/builtins/exec.def b/builtins/exec.def index cc10278a..f654b151 100644 --- a/builtins/exec.def +++ b/builtins/exec.def @@ -215,7 +215,7 @@ exec_builtin (list) maybe_save_shell_history (); #endif /* HISTORY */ - restore_original_signals (); + reset_signal_handlers (); /* leave trap strings in place */ #if defined (JOB_CONTROL) orig_job_control = job_control; /* XXX - was also interactive_shell */ @@ -261,8 +261,11 @@ failed_exec: if (env && env != export_env) strvec_dispose (env); - initialize_traps (); + /* If we're not exiting after the exec fails, we restore the shell signal + handlers and then modify the signal dispositions based on the trap strings + before the failed exec. */ initialize_signals (1); + restore_traps (); #if defined (JOB_CONTROL) if (orig_job_control) diff --git a/doc/bash.1 b/doc/bash.1 index 216b1b44..7f226c05 100644 --- a/doc/bash.1 +++ b/doc/bash.1 @@ -5,12 +5,12 @@ .\" Case Western Reserve University .\" chet.ramey@case.edu .\" -.\" Last Change: Fri Oct 1 14:20:30 EDT 2021 +.\" Last Change: Fri Oct 8 14:10:30 EDT 2021 .\" .\" bash_builtins, strip all but Built-Ins section .if \n(zZ=1 .ig zZ .if \n(zY=1 .ig zY -.TH BASH 1 "2021 October 1" "GNU Bash 5.1" +.TH BASH 1 "2021 October 8" "GNU Bash 5.1" .\" .\" There's some problem with having a `@' .\" in a tagged paragraph with the BSD man macros. @@ -11403,6 +11403,10 @@ The individual per-interactive-shell startup file .FN ~/.bash_logout The individual login shell cleanup file, executed when a login shell exits .TP +.FN ~/.bash_history +The default value of \fBHISTFILE\fP, the file in which bash saves the +command history +.TP .FN ~/.inputrc Individual \fIreadline\fP initialization file .PD diff --git a/lib/readline/complete.c b/lib/readline/complete.c index 3f80c1cf..1e4dfa3b 100644 --- a/lib/readline/complete.c +++ b/lib/readline/complete.c @@ -835,7 +835,8 @@ fnprint (const char *to_print, int prefix_bytes, const char *real_pathname) colored_stat_start (real_pathname); #endif - if (prefix_bytes && _rl_completion_prefix_display_length > 0) + if (prefix_bytes && _rl_completion_prefix_display_length > 0 && + prefix_bytes > _rl_completion_prefix_display_length) { char ellipsis; @@ -1548,7 +1549,7 @@ rl_display_match_list (char **matches, int len, int max) if (common_length > _rl_completion_prefix_display_length && common_length > ELLIPSIS_LEN) max -= common_length - ELLIPSIS_LEN; - else + else if (_rl_colored_completion_prefix <= 0) common_length = sind = 0; } #if defined (COLOR_SUPPORT) diff --git a/lib/readline/terminal.c b/lib/readline/terminal.c index 0a46b93e..5b4ef1bd 100644 --- a/lib/readline/terminal.c +++ b/lib/readline/terminal.c @@ -386,8 +386,12 @@ _rl_sigwinch_resize_terminal (void) void rl_resize_terminal (void) { + int width, height; + + width = _rl_screenwidth; + height = _rl_screenheight; _rl_get_screen_size (fileno (rl_instream), 1); - if (_rl_echoing_p) + if (_rl_echoing_p && (width != _rl_screenwidth || height != _rl_screenheight)) { if (CUSTOM_REDISPLAY_FUNC ()) rl_forced_update_display (); diff --git a/parse.y b/parse.y index 00b98b5b..6e1770ab 100644 --- a/parse.y +++ b/parse.y @@ -4348,7 +4348,9 @@ parse_dparen (c) #if defined (ARITH_FOR_COMMAND) if (last_read_token == FOR) { - arith_for_lineno = line_number; + if (word_top < MAX_CASE_NEST) + word_top++; + arith_for_lineno = word_lineno[word_top] = line_number; cmdtyp = parse_arith_cmd (&wval, 0); if (cmdtyp == 1) { diff --git a/subst.c b/subst.c index 89b4d48b..10b74960 100644 --- a/subst.c +++ b/subst.c @@ -12132,10 +12132,13 @@ expand_declaration_argument (tlist, wcmd) memset (omap, '\0', sizeof (omap)); for (l = wcmd->next; l != tlist; l = l->next) { - if (l->word->word[0] != '-') + int optchar; + + if (l->word->word[0] != '-' && l->word->word[0] != '+') break; /* non-option argument */ if (l->word->word[0] == '-' && l->word->word[1] == '-' && l->word->word[2] == 0) break; /* -- signals end of options */ + optchar = l->word->word[0]; for (oind = 1; l->word->word[oind]; oind++) switch (l->word->word[oind]) { @@ -12147,7 +12150,7 @@ expand_declaration_argument (tlist, wcmd) case 'c': omap[l->word->word[oind]] = 1; if (opti == 0) - opts[opti++] = '-'; + opts[opti++] = optchar; break; default: break; diff --git a/tests/RUN-ONE-TEST b/tests/RUN-ONE-TEST index 0b063810..c8bef8dd 100755 --- a/tests/RUN-ONE-TEST +++ b/tests/RUN-ONE-TEST @@ -1,4 +1,4 @@ -BUILD_DIR=/usr/local/build/chet/bash/bash-current +BUILD_DIR=/usr/local/build/bash/bash-current THIS_SH=$BUILD_DIR/bash PATH=$PATH:$BUILD_DIR diff --git a/tests/array.right b/tests/array.right index 8ad96aef..03f6c61b 100644 --- a/tests/array.right +++ b/tests/array.right @@ -381,6 +381,8 @@ strlen(4four) = 5 1 2 0 3 1 2 0 3 1 2 0 3 +declare -ai arr=([0]="2" [1]="4" [2]="6") +declare -a arr=([0]="hello" [1]="world") foo index 1: ok foo index 2: ok foo: implicit reference to element 0: ok diff --git a/tests/array15.sub b/tests/array15.sub index 47796b95..12f5391e 100644 --- a/tests/array15.sub +++ b/tests/array15.sub @@ -35,3 +35,18 @@ func() func echo "${foo[@]}" + +unset foo + +# test options to declare that disable attributes that affect how values +# are expanded +# +# we already handle options that set attributes specially, so we should +# handle attributes that unset those attributes specially as well + +unset arr +declare -i -a arr=(1+1 2+2 3+3) +declare -p arr + +declare +i arr=(hello world) +declare -p arr diff --git a/tests/exec.right b/tests/exec.right index 0a249dda..8664e97f 100644 --- a/tests/exec.right +++ b/tests/exec.right @@ -20,8 +20,17 @@ after exec1.sub: one two three 126 0 this is bashenv -./exec3.sub: line 3: /tmp/bash-notthere: No such file or directory -127 +trap -- 'echo EXIT' EXIT +trap -- '' SIGTERM +trap -- 'echo USR1' SIGUSR1 +USR1 +./exec3.sub: line 27: /tmp/bash-notthere: No such file or directory +./exec3.sub: after failed exec: 127 +trap -- 'echo EXIT' EXIT +trap -- '' SIGTERM +trap -- 'echo USR1' SIGUSR1 +USR1 +EXIT ./execscript: line 71: notthere: No such file or directory 127 ./execscript: line 74: notthere: No such file or directory diff --git a/tests/exec3.sub b/tests/exec3.sub index 4f2f8e21..81b53b72 100644 --- a/tests/exec3.sub +++ b/tests/exec3.sub @@ -1,6 +1,37 @@ +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# +# test the behavior of `execfail' not exiting an interactive shell +# added tests for changes in 10/2021 for preserving the traps across a failed +# exec + shopt -s execfail -exec /tmp/bash-notthere -# make sure we're still around -echo $? +trap 'echo EXIT' EXIT +trap 'echo USR1' USR1 +trap '' TERM +trap +kill -s USR1 $$ # should run the trap + +exec /tmp/bash-notthere + +# make sure we're still around +echo $0: after failed exec: $? + +trap +kill -s USR1 $$ # should run the trap +kill -s TERM $$ # should still be ignored + +# this should run the exit trap +exit 0 diff --git a/tests/new-exp16.sub b/tests/new-exp16.sub index db89a317..f871f708 100644 --- a/tests/new-exp16.sub +++ b/tests/new-exp16.sub @@ -1,3 +1,17 @@ +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# + HOME=/homes/chet string=abcdefg set -- a b c diff --git a/trap.c b/trap.c index b4bb76b0..1a84aa15 100644 --- a/trap.c +++ b/trap.c @@ -83,6 +83,7 @@ static void free_trap_string (int); static void reset_signal (int); static void restore_signal (int); static void reset_or_restore_signal_handlers (sh_resetsig_func_t *); +static void reinit_trap (int); static void trap_if_untrapped (int, char *); @@ -1365,6 +1366,77 @@ restore_original_signals () reset_or_restore_signal_handlers (restore_signal); } +/* Change the flags associated with signal SIG without changing the trap + string. The string is TRAP_LIST[SIG] if we need it. */ +static void +reinit_trap (sig) + int sig; +{ + sigmodes[sig] |= SIG_TRAPPED; + if (trap_list[sig] == (char *)IGNORE_SIG) + sigmodes[sig] |= SIG_IGNORED; + else + sigmodes[sig] &= ~SIG_IGNORED; + if (sigmodes[sig] & SIG_INPROGRESS) + sigmodes[sig] |= SIG_CHANGED; +} + +/* Undo the effects of reset_signal_handlers(), which unsets the traps but + leaves the trap strings in place. This understands how reset_signal_handlers + works. */ +void +restore_traps () +{ + char *trapstr; + int i; + + /* Take care of the exit trap first. If TRAP_LIST[0] is non-null, the trap + has been set. */ + trapstr = trap_list[EXIT_TRAP]; + if (trapstr) + reinit_trap (EXIT_TRAP); + + /* Then DEBUG, RETURN, and ERROR. TRAP_LIST[N] == 0 if these signals are + not trapped. This knows what reset_signal_handlers does for these traps */ + trapstr = trap_list[DEBUG_TRAP]; + if (trapstr && function_trace_mode == 0) + reinit_trap (DEBUG_TRAP); + trapstr = trap_list[RETURN_TRAP]; + if (trapstr && function_trace_mode == 0) + reinit_trap (RETURN_TRAP); + trapstr = trap_list[ERROR_TRAP]; + if (trapstr && error_trace_mode == 0) + reinit_trap (ERROR_TRAP); + + /* And finally all the `real' signals. reset_signal_handlers just changes the + signal handler for these signals, leaving the trap value in place. We + intuit what to do based on that value. We assume that signals marked as + SIG_SPECIAL are reinitialized by initialize_signals (), so we don't + change the signal handler unless the signal is supposed to be ignored. */ + for (i = 1; i < NSIG; i++) + { + trapstr = trap_list[i]; + if (sigmodes[i] & SIG_SPECIAL) + { + if (trapstr && trapstr != (char *)DEFAULT_SIG) + reinit_trap (i); + if (trapstr == (char *)IGNORE_SIG && (sigmodes[i] & SIG_NO_TRAP) == 0) + set_signal_handler (i, SIG_IGN); + } + else if (trapstr == (char *)IGNORE_SIG) + { + reinit_trap (i); + if ((sigmodes[i] & SIG_NO_TRAP) == 0) + set_signal_handler (i, SIG_IGN); + } + else if (trapstr != (char *)DEFAULT_SIG) + /* set_signal duplicates the string argument before freeing it. */ + set_signal (i, trapstr); + + pending_traps[i] = 0; /* XXX */ + } +} + /* If a trap handler exists for signal SIG, then call it; otherwise just return failure. Returns 1 if it called the trap handler. */ int diff --git a/trap.h b/trap.h index ef64a680..62d7ee9e 100644 --- a/trap.h +++ b/trap.h @@ -98,6 +98,7 @@ extern void run_return_trap PARAMS((void)); extern void free_trap_strings PARAMS((void)); extern void reset_signal_handlers PARAMS((void)); extern void restore_original_signals PARAMS((void)); +extern void restore_traps PARAMS((void)); extern void get_original_signal PARAMS((int)); extern void get_all_original_signals PARAMS((void));