Re: RFE: Fix the name collision issues and typing issues with namerefs, improve various issues for function libraries
On 6/14/17 12:04 PM, tetsu...@scope-eye.net wrote: > > This is relevant to my interests! > > So first off, the circular reference problem with "declare -n" > apparently doesn't exist in Korn Shell: If you "typeset -n x=$1", and > $1 is "x", it's not a circular reference. The nameref seems to take > the origin of the name into account when creating the nameref: If $1 > is used to provide the name for a nameref within a function, then the > nameref refers to a variable in the scope of the function's caller. > But if the name for the nameref is taken from another parameter, or a > string literal, then the variable is drawn from the scope of the > function itself. It seems a little "black magic" in a way but it has > the benefit of neatly matching some important common use-cases. I > think Bash should either adopt something similar, or provide another > mechanism to exclude function-local variables from nameref > construction. I've gotten a suggestion that function-scope circular references should always be resolved beginning at the previous scope, but I haven't done anything to implement that yet. There are a couple of problems that make it less clean to implement the Korn shell mechanism. First, declare is a shell builtin, which means that its arguments are expanded before it sees them. x=$1 and x=x both look the same to declare when it sees them. The second is dynamic scoping, which makes resolution tricker. The korn shell uses static scoping, so it only has to look at the current scope and the global scope (which makes the x=$1 case even more irregular). -- ``The lyf so short, the craft so long to lerne.'' - Chaucer ``Ars longa, vita brevis'' - Hippocrates Chet Ramey, UTech, CWRUc...@case.eduhttp://cnswww.cns.cwru.edu/~chet/
Re: Buffer corruption when the terminal is resized.
On 6/12/17 9:49 PM, Paul Peet wrote: > Yes, indeed, rewrapping the contents on resize breaks this assumption. > > Given that in a(n obviously non-representative) poll about 1.5 years > ago at https://opensource.com/life/15/11/top-open-source-terminal-emulators, > terminal emulators that now (not at the time of the poll) rewrap on > resize got around 50-60% of the votes, maybe it's time for > readline/bash to reconsider their assumptions. The problem with reconsidering this assumption is that it leaves you with no recourse but to move to a new line (and who knows, it may be a line containing newly-wrapped text) and just do a fresh redraw when you get a SIGWINCH. That's going to leave even more text on the screen than now, for gains only in pathological cases. -- ``The lyf so short, the craft so long to lerne.'' - Chaucer ``Ars longa, vita brevis'' - Hippocrates Chet Ramey, UTech, CWRUc...@case.eduhttp://cnswww.cns.cwru.edu/~chet/
Re: Issues with `rl_vi_change_char' function in combination with `_rl_vi_redoing = 1'
On 6/17/17 5:27 AM, Eduardo A. Bustamante López wrote: > # When bash is compiled with multibyte support, the following happens: > > - set -o vi > - Type: <ñ><.> > - Bash writes \261 to `mb[0]' and '\0' to `mb[1]' (ñ is U+00F1, or in > UTF-8, \303\261) This needs changes similar to _rl_vi_last_search_char. > - `mb' is of type signed char, and not really suitable to store a wide > character. This doesn't really matter, since that's not the intent. -- ``The lyf so short, the craft so long to lerne.'' - Chaucer ``Ars longa, vita brevis'' - Hippocrates Chet Ramey, UTech, CWRUc...@case.eduhttp://cnswww.cns.cwru.edu/~chet/
Issues with `rl_vi_change_char' function in combination with `_rl_vi_redoing = 1'
# When bash is compiled with multibyte support, the following happens:
- set -o vi
- Type: <ñ><.>
- Bash writes \261 to `mb[0]' and '\0' to `mb[1]' (ñ is U+00F1, or in
UTF-8, \303\261)
- `mb' is of type signed char, and not really suitable to store a wide
character.
dualbus@debian:~$ set -o vi
dualbus@debian:~$ �
bash: $'\261': command not found
# When bash is compiled w/o multibyte support, the following happens:
1948 int
1949 rl_vi_change_char (int count, int key)
1950 {
1951 int c;
1952 char mb[MB_LEN_MAX]; /* MB_LEN_MAX == 1 */
1953
1954 if (_rl_vi_redoing)
1955 {
1956 c = _rl_vi_last_replacement;
1957 mb[0] = c;
1958 mb[1] = '\0'; /* out of bounds write */
1959 }
Reproduce with:
- set -o vi
- Type: <.>
- Bash writes '\0' to `mb[1]', which is out of bounds.
dualbus@debian:~/src/gnu/bash-build$ ./bash
bash-4.4$ set -o vi
bash-4.4$ =
==7881==ERROR: AddressSanitizer: stack-buffer-overflow on address
0x7ffe8416f471 at pc 0x55e4ba75d81a bp 0x7ffe8416f420 sp 0x7ffe8416f418
WRITE of size 1 at 0x7ffe8416f471 thread T0
#0 0x55e4ba75d819 in rl_vi_change_char
../../../bash/lib/readline/vi_mode.c:1959
#1 0x55e4ba74fe3d in _rl_dispatch_subseq
../../../bash/lib/readline/readline.c:851
#2 0x55e4ba74fa18 in _rl_dispatch
../../../bash/lib/readline/readline.c:797
#3 0x55e4ba752a41 in rl_vi_redo ../../../bash/lib/readline/vi_mode.c:285
#4 0x55e4ba74fe3d in _rl_dispatch_subseq
../../../bash/lib/readline/readline.c:851
#5 0x55e4ba74fa18 in _rl_dispatch
../../../bash/lib/readline/readline.c:797
#6 0x55e4ba74f257 in readline_internal_char
../../../bash/lib/readline/readline.c:629
#7 0x55e4ba74f2e9 in readline_internal_charloop
../../../bash/lib/readline/readline.c:656
#8 0x55e4ba74f30d in readline_internal
../../../bash/lib/readline/readline.c:670
#9 0x55e4ba74e9c3 in readline ../../../bash/lib/readline/readline.c:374
#10 0x55e4ba5ffd01 in yy_readline_get ../bash/parse.y:1464
#11 0x55e4ba5ffed9 in yy_readline_get ../bash/parse.y:1495
#12 0x55e4ba5ffb64 in yy_getc ../bash/parse.y:1397
#13 0x55e4ba601ea4 in shell_getc ../bash/parse.y:2297
#14 0x55e4ba6048d0 in read_token ../bash/parse.y:3146
#15 0x55e4ba6032ad in yylex ../bash/parse.y:2683
#16 0x55e4ba5f7f31 in yyparse
/home/dualbus/src/gnu/bash-build/y.tab.c:1821
#17 0x55e4ba5f7102 in parse_command ../bash/eval.c:294
#18 0x55e4ba5f7357 in read_command ../bash/eval.c:338
#19 0x55e4ba5f6593 in reader_loop ../bash/eval.c:140
#20 0x55e4ba5f1d3d in main ../bash/shell.c:794
#21 0x7ff2e9a7d2b0 in __libc_start_main
(/lib/x86_64-linux-gnu/libc.so.6+0x202b0)
#22 0x55e4ba5f0939 in _start
(/home/dualbus/src/gnu/bash-build/bash+0x7b939)
Address 0x7ffe8416f471 is located in stack of thread T0 at offset 33 in frame
#0 0x55e4ba75d72f in rl_vi_change_char
../../../bash/lib/readline/vi_mode.c:1950
This frame has 1 object(s):
[32, 33) 'mb' <== Memory access at offset 33 overflows this variable
HINT: this may be a false positive if your program uses some custom stack
unwind mechanism or swapcontext
(longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow
../../../bash/lib/readline/vi_mode.c:1959 in rl_vi_change_char
Shadow bytes around the buggy address:
0x100050825e30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x100050825e40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x100050825e50: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x100050825e60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x100050825e70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x100050825e80: 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1[01]f4
0x100050825e90: f4 f4 f3 f3 f3 f3 00 00 00 00 00 00 00 00 00 00
0x100050825ea0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x100050825eb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x100050825ec0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
0x100050825ed0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
Addressable: 00
Partially addressable: 01 02 03 04 05 06 07
Heap left redzone: fa
Heap right redzone: fb
Freed heap region: fd
Stack left redzone: f1
Stack mid redzone: f2
Stack right redzone: f3
Stack partial redzone: f4
Stack after return: f5
Stack use after scope: f8
Global redzone: f9
Global init order: f6
Poisoned by user:f7
Container overflow: fc
Array cookie:ac
Intra object redzone:bb
ASan internal: fe
Left alloca redzone: ca
Right alloca redzone:cb
==7881==ABORTING
[PATCH] Bash does not build when --disable-multibyte due to missing HAVE_MULTIBYTE guard in display.c
dualbus@debian:~/src/gnu/bash$ git diff -- lib/readline/display.c diff --git a/lib/readline/display.c b/lib/readline/display.c index d5536211..1f281cd3 100644 --- a/lib/readline/display.c +++ b/lib/readline/display.c @@ -2127,7 +2127,7 @@ dumb_update: cpos_adjusted = 1; } -#if 1 +#ifdef HAVE_MULTIBYTE /* If we write a non-space into the last screen column, remove the note that we added a space to compensate for a multibyte double-width character that didn't fit, since -- Eduardo Bustamante https://dualbus.me/
Return code of coproc when NAME is not a legal_identifier
The manual is clear on this:
Since the coprocess is created as an asynchronous command, the coproc
command always returns success
I'd argue that it should return an error code for this particular case
though:
$ bash -c 'coproc % { :; }; echo $?'
bash: `%': not a valid identifier
0
The `execute_coproc' function _seems_ to try to return an error code for this
case (by the way, the value of `invert' is undefined at this point):
2301 /* XXX - expand coproc name without splitting -- bash-5.0 */
2302 /* could make this dependent on a shopt option */
2303 name = expand_string_unsplit_to_string (command->value.Coproc->name,
0);
2304 /* Optional check -- bash-5.0. */
2305 if (legal_identifier (name) == 0)
2306 {
2307 internal_error (_("`%s': not a valid identifier"), name);
2308 return (invert ? EXECUTION_SUCCESS : EXECUTION_FAILURE);
2309 }
2310 else
2311 {
2312 free (command->value.Coproc->name);
2313 command->value.Coproc->name = name;
2314 }
The following patch fixes this (coproc returns $? = 1 when `NAME' is not a
legal identifier), although I'm not convinced about the quality of this patch.
dualbus@debian:~/src/gnu/bash$ git diff
diff --git a/execute_cmd.c b/execute_cmd.c
index 0183a105..ab022bdb 100644
--- a/execute_cmd.c
+++ b/execute_cmd.c
@@ -597,8 +597,9 @@ execute_command_internal (command, asynchronous, pipe_in,
pipe_out,
return (execute_in_subshell (command, asynchronous, pipe_in, pipe_out,
fds_to_close));
#if defined (COPROCESS_SUPPORT)
- if (command->type == cm_coproc)
-return (execute_coproc (command, pipe_in, pipe_out, fds_to_close));
+ if (command->type == cm_coproc) {
+return (last_command_exit_value = execute_coproc (command, pipe_in,
pipe_out, fds_to_close));
+ }
#endif
user_subshell = command->type == cm_subshell || ((command->flags &
CMD_WANT_SUBSHELL) != 0);
@@ -2298,6 +2299,8 @@ execute_coproc (command, pipe_in, pipe_out, fds_to_close)
coproc_init (_coproc);
#endif
+ invert = (command->flags & CMD_INVERT_RETURN) != 0;
+
/* XXX - expand coproc name without splitting -- bash-5.0 */
/* could make this dependent on a shopt option */
name = expand_string_unsplit_to_string (command->value.Coproc->name, 0);
@@ -2313,7 +2316,6 @@ execute_coproc (command, pipe_in, pipe_out, fds_to_close)
command->value.Coproc->name = name;
}
- invert = (command->flags & CMD_INVERT_RETURN) != 0;
command_string_index = 0;
tcmd = make_command_string (command);
This came up as a warning from cppcheck:
[../bash/execute_cmd.c:2308]: (error) Uninitialized variable: invert
--
Eduardo Bustamante
https://dualbus.me/
