From 997ad43bbf69d50d02b74c626cb4ea2d88df4e3d Mon Sep 17 00:00:00 2001 From: Martijn Dekker Date: Thu, 8 Apr 2021 00:10:07 +0100 Subject: [PATCH] Properly fix $LINENO crash on ARM (re: 23b7a163) and other bugs The typecast fix was insufficient, avoiding the crash only when compiling with optimisation disabled. The real problem is that put_lineno() was passed a misaligned pointer, and that the value didn't actually contain a double but a string. The bug occurred when restoring the LINENO value upon exiting a virtual subshell. Thanks to Harald van Dijk for figuring out the fix. src/cmd/ksh93/sh/subshell.c: nv_restore(): - When restoring a special variable as defined by nv_cover(), do not pass either the np->nvflag bits or NV_NOFREE. Why? * The np->nvflag bits are not needed. They are also harmful because they may include the NV_INTEGER bit. This is set when the value is numeric. However, nv_getval() always returns the value in string form, converting it if it is numeric. So the NV_INTEGER flag should never be passed to nv_putval() when it uses the result of nv_getval(). * According to nval.3, the NV_NOFREE flag stops nv_putval() from creating a copy of the value. But this should be unnecessary because the earlier _nv_unset(mp,NV_RDONLY|NV_CLONE) should ensure there is no previous value. In addition, the NV_NOFREE flag triggered another bug that caused the value of SECONDS to be corrupted upon restoring it when exiting a virtual subshell. - When restoring a regular variable, copy the entire nvalue union and not just the 'cp' member. In practice this worked because no current member of the nvalue union is larger than a pointer. However, there is no guarantee it will stay that way. src/cmd/ksh93/tests/leaks.sh: - Add disabled test for a memory leak that was discovered in the course of dealing with this bug. The fix doesn't introduce or influence it. It will have to be dealt with later. src/cmd/ksh93/tests/locale.sh: - Add test for restoring locale on leaving virtual subshell. https://github.com/ksh93/ksh/issues/253#issuecomment-815290154 src/cmd/ksh93/tests/variables.sh: - Test against corruption of SECONDS on leaving virtual subshell. https://github.com/ksh93/ksh/issues/253#issuecomment-815191052 Co-authored-by: Harald van Dijk Progresses: https://github.com/ksh93/ksh/issues/253 --- src/cmd/ksh93/sh/subshell.c | 4 ++-- src/cmd/ksh93/tests/leaks.sh | 12 ++++++++++++ src/cmd/ksh93/tests/locale.sh | 15 ++++++++++++++- src/cmd/ksh93/tests/variables.sh | 10 ++++++++++ 4 files changed, 38 insertions(+), 3 deletions(-) diff --git a/src/cmd/ksh93/sh/subshell.c b/src/cmd/ksh93/sh/subshell.c index d90a7f117..9c014ead5 100644 --- a/src/cmd/ksh93/sh/subshell.c +++ b/src/cmd/ksh93/sh/subshell.c @@ -392,9 +392,9 @@ static void nv_restore(struct subshell *sp) } mp->nvflag = np->nvflag|(flags&NV_MINIMAL); if(nv_cover(mp)) - nv_putval(mp, nv_getval(np),np->nvflag|NV_NOFREE|NV_RDONLY); + nv_putval(mp,nv_getval(np),NV_RDONLY); else - mp->nvalue.cp = np->nvalue.cp; + mp->nvalue = np->nvalue; if(nofree && np->nvfun && !np->nvfun->nofree) free((char*)np->nvfun); np->nvfun = 0; diff --git a/src/cmd/ksh93/tests/leaks.sh b/src/cmd/ksh93/tests/leaks.sh index b66f275e7..85a1481f7 100755 --- a/src/cmd/ksh93/tests/leaks.sh +++ b/src/cmd/ksh93/tests/leaks.sh @@ -439,5 +439,17 @@ after=$(getmem) err_exit_if_leak 'PWD and/or OLDPWD changed by cd' cd $original_pwd +# ====== +# https://github.com/ksh93/ksh/issues/253#issuecomment-815308466 +: <<'disabled' # TODO: known leak(s) (approx 1008 KiB after 16384 iterations) +before=$(getmem) +for ((i=0; i < N; i++)) +do + (SECONDS=1; LANG=C) +done +after=$(getmem) +err_exit_if_leak 'Variable with discipline function in subshell causes memory leak' +disabled + # ====== exit $((Errors<125?Errors:125)) diff --git a/src/cmd/ksh93/tests/locale.sh b/src/cmd/ksh93/tests/locale.sh index 645c718bf..c1043efe5 100755 --- a/src/cmd/ksh93/tests/locale.sh +++ b/src/cmd/ksh93/tests/locale.sh @@ -349,7 +349,7 @@ x=$(LC_ALL=debug $SHELL -c 'typeset -R10 x="a<2b|>c";print -r -- "${x}"') x=$(LC_ALL=debug $SHELL -c 'typeset -L10 x="a<2b|>c";print -r -- "${x}"') [[ $x == 'a<2b|>c ' ]] || err_exit 'typeset -L10 should end in three spaces' -if false && # Disable this test because it really test the OS-provided en_US.UTF-8 locale data, which may be broken. +if false && # Disable this test because it really tests the OS-provided en_US.UTF-8 locale data, which may be broken. $SHELL -c "export LC_ALL=en_US.UTF-8; c=$'\342\202\254'; [[ \${#c} == 1 ]]" 2>/dev/null then LC_ALL=en_US.UTF-8 unset i p1 p2 x @@ -378,5 +378,18 @@ then LC_ALL=en_US.UTF-8 fi +# ====== +# The locale should be restored along with locale variables when leaving a virtual subshell. +# https://github.com/ksh93/ksh/issues/253#issuecomment-815290154 +if ((SHOPT_MULTIBYTE)) +then + unset "${!LC_@}" + LANG=C.UTF-8 + exp=$'5\n10\n5' + got=$(eval 'var=äëïöü'; echo ${#var}; (LANG=C; echo ${#var}); echo ${#var}) + [[ $got == "$exp" ]] || err_exit "locale is not restored properly upon leaving virtual subshell" \ + "(expected $(printf %q "$exp"); got $(printf %q "$got"))" +fi + # ====== exit $((Errors<125?Errors:125)) diff --git a/src/cmd/ksh93/tests/variables.sh b/src/cmd/ksh93/tests/variables.sh index 3eb164d33..48427e9b4 100755 --- a/src/cmd/ksh93/tests/variables.sh +++ b/src/cmd/ksh93/tests/variables.sh @@ -1261,5 +1261,15 @@ got="$($SHELL -c 'PS4="${.sh.subshell}"; echo ${.sh.subshell}')" [[ "$exp" == "$got" ]] || err_exit "\${.sh.subshell} is wrongly unset in the \$PS4 prompt" \ "(expected $(printf %q "$exp"), got $(printf %q "$got"))" +# ====== +# Corruption of SECONDS on leaving virtual subshell +# https://github.com/ksh93/ksh/issues/253#issuecomment-815191052 +osec=$SECONDS +(SECONDS=20) +nsec=$SECONDS +if ((nsecosec+0.1)) +then err_exit "SECONDS corrupted after leaving virtual subshell (expected $osec, got $nsec)" +fi + # ====== exit $((Errors<125?Errors:125))