Comments (9)
I've localised the problem to this part of sh_fmtq()
:
Lines 360 to 381 in 1ca9286
(note: in the following
else
clause it does $'
...'
quoting which does not have a bug).
If we insert the following debug command on line 373 and recompile:
errormsg(SH_DICT,ERROR_warn(0),"[DEBUG] length == %d", cp - string - 1);
then the output of the test script above becomes:
test.ksh[2]: printf: warning: [DEBUG] length == 25
'Is shell-quoting garbled?'
test.ksh[4]: printf: warning: [DEBUG] length == 26
'Is shell-quoting garbled?
Note that the garbled string has its length counted wrong: one character more, not less. This means that stakwrite(string,c);
on line 376 writes one byte too many, which is the C string's terminating zero. This explains why the final quote doesn't appear; it is dutifully added, but after the erroneous zero.
(later edit: if a longer UTF-8 character is used, e.g. the four-byte '🇪🇺', then the wrong length in the second debug output correspondingly increases from 26 to 28 -- three bytes too many.)
So there is some problem with multibyte character processing. My working hypothesis is that the read -n1 g
command in the test script, which triggers the bug and which reads one byte (not one character), leaves some kind of inconsistent multibyte character processing state, as reading one byte is reading half of that multibyte character.
First suspect is the mbchar(cp)
call on line 360. Evidently this returns the current character and moves the character pointer (cp) to the next character in a multibyte-compatible way.
mbchar()
looks like a function, but is a macro. This macro and its friends are defined here:
ksh/src/lib/libast/include/ast.h
Lines 181 to 196 in 1ca9286
It looks like the behaviour of
mbchar()
and others depends on the result of mbwide()
, another macro which is defined as (mbmax()>1)
, and mbmax()
is another macro, defined as (ast.mb_cur_max)
. Aha! That's a global state variable. mb_cur_max, could that mean "multibyte current maximum"? Could this be what keeps track of the length of the current multibyte character and is left nonzero by read -n1
?
To put this to the test, insert ast.mb_cur_max = 0
on line 337 in string.c - right at the beginning of the sh_fmtq()
function. Sure enough, after doing that and recompiling, the bug vanishes.
This is a hack, though, not a proper fix. The real bug is in the multibyte macros, not in the shellquoting algorithm. More digging to follow…
from ksh.
One of those ast.h macros that is catching my attention is the one on line 190:
#define mbinit() (mbwide()?(*ast.mb_towc)((wchar_t*)0,(char*)0,mbmax()):0)
This macro's name suggests that is is meant to initialise the state of multibyte character processing. It is called in various places where multibyte characters are processed, but not in sh_fmtq()
.
However, unfortunately, putting a mbinit()
instead of ast.mb_sync = 0
on line 337 in sh/string.c makes the bug reappear. But what good is that whole mbinit()
thing then?
If we're currently in a multibyte locale (i.e.: mbwide()
is true a.k.a. ast.mb_cur_max > 1
), then mbinit()
calls the function pointed to by ast.mb_towc
with the first two parameters being null pointers and the third the current value of ast.mb_cur_max
. But what function is it calling?
The answer is in the set_ctype
discipline function quoted above, on line 2244 to be precise: in a UTF-8 locale, ast.mb_towc = utf8_mbtowc
. (Not sure what wc
is an acronym of... wordcode? wide characters?) Anyway, here is that utf8_mbtowc()
function:
ksh/src/lib/libast/comp/setlocale.c
Lines 594 to 638 in 2624b29
So what does this function actually do if it's called by
mbinit()
, with the first two pointer parameters being null? The answer is in lines 597, 603 and 604: if sp
, being equal to str
, being the second parameter, is null, then it simply does nothing at all and returns zero.
Which means that, in UTF-8 locales, invoking mbinit()
does nothing. It's a no-op.
Maybe this is the actual bug. This init routine should be resetting ast.mb_sync
to zero, as set_ctype()
does. If ast.mb_sync
is erroneously non-zero, then you can sort of see in the mbchar()
macro definition (which I still find very hard to understand) that too much gets added to the length. This is starting to make some kind of sense now.
from ksh.
Understood. Thank you for all the hard work on ksh.
from ksh.
My line 337 ast.mb_cur_max = 0
hack in the previous message is wrong. It introduces another bug:
$ cat > test2.ksh
LANG=C.UTF-8
var=múltîbytè
printf 'length of var: %d\n' ${#var}
printf '%q\n' 'Is shell-quoting garbled?'
printf 'length of var: %d\n' ${#var}
print -nr $'\303\274' | read -n1 g
printf '%q\n' 'Is shell-quoting garbled?'
$ arch/*/bin/ksh test2.ksh
length of var: 9
'Is shell-quoting garbled?'
length of var: 12
'Is shell-quoting garbled?'
Of course the length of the variable should be 9 in both cases. This means that ast.mb_cur_max
does not store any kind of temporary state and that setting it to zero actually disables multibyte processing altogether, which of course masks the shellquoting bug.
Grepping the code to find where ast.mb_cur_max
is set points us to this set_ctype()
function:
ksh/src/lib/libast/comp/setlocale.c
Lines 2218 to 2284 in 1ca9286
Interesting: this is the discipline function for the
LC_CTYPE
variable. So this initialises the character processing state depending on the locale that is being set. The first thing it does is set ast.mb_sync
to 0. Could that be what we need? — Sure enough, setting ast.mb_sync = 0
instead on line 337 in sh/string.c appears to fix the shellquoting bug without disabling multibyte character processing:
$ arch/*/bin/ksh test2.ksh
length of var: 9
'Is shell-quoting garbled?'
length of var: 9
'Is shell-quoting garbled?'
This also makes some sense of how those mb* macros are defined, if you read them carefully (good luck).
So this may be a better hack, but it's still a hack. Stay tuned.
from ksh.
So, it looks like the following patch makes the bug reliably go away without causing regressions when running bin/shtests --utf8
:
diff --git a/src/lib/libast/comp/setlocale.c b/src/lib/libast/comp/setlocale.c
index ba31fe9..65553f8 100644
--- a/src/lib/libast/comp/setlocale.c
+++ b/src/lib/libast/comp/setlocale.c
@@ -600,6 +600,8 @@ utf8_mbtowc(wchar_t* wp, const char* str, size_t n)
register int c;
register wchar_t w = 0;
+ if (!wp && !sp)
+ ast.mb_sync = 0; /* assume call from mbinit() macro: reset global multibyte sync state */
if (!sp || !n)
return 0;
if ((m = utf8tab[*sp]) > 0)
from ksh.
With the patch in the previous comment, it looks like sh_fmtq()
doesn't even need a new mbinit()
call/expansion for the bug to stay gone. So, apparently, this patch fixes one or more problems elsewhere in the code that left the global multibyte sync state inconsistent due to no-op mbinit()
calls, which is a good sign.
I think I'll add that mbinit()
call to sh_fmtq()
anwyay, though: it will probably make it more bug-proof even if I can't trigger any bugs now. (Compare sh_substitute()
in string.c lines 190-192, which does the same.)
from ksh.
FYI: I do not experience the missing closing single quote using ksh93v-.
$ echo $KSH_VERSION
Version ABIJM 93v- 2014-12-24
$ LANG=C.UTF-8
$ printf '%q\n' 'Is shell-quoting garbled?'
'Is shell-quoting garbled?'
$ print -nr $'\303\274' | read -n1 g
$ printf '%q\n' 'Is shell-quoting garbled?'
'Is shell-quoting garbled?'
$
from ksh.
Thanks @hyenias. The 93v- beta completely overhauled the whole multibyte processing system in a new version of the libast API, so it wasn't possible to backport a fix, nor was it feasible to backport that entire overhauled system as it's tied in with lots of other changes in the code.
from ksh.
Just to add to the record, I'd forgotten I'd found another trigger of this bug back in 2017: att#13 (comment)
$ ksh -c 'i=$IFS; IFS=é; set : :; echo "$*"; IFS=$i; trap "echo end" EXIT; trap' :?: trap -- 'echo end EXIT end
Note the missing quote after
trap -- 'echo end
. You get the same with the output ofexport -p
,alias
, etc.
I can happily confirm that this was fixed by 300cd19 as well (but of course IFS still doesn't actually handle multibyte characters).
from ksh.
Related Issues (20)
- Still problems building in strict C99 mode HOT 4
- can't expand a function name from an alias
- Regression in behavior of `exit` with no args in trap handler HOT 3
- date: wrong timezone for 1970 -> 1971 in Europe/London timezone HOT 4
- ksh remains in "noecho" mode forever HOT 4
- SHOPT_DYNAMIC=1 build leads to path.sh regression
- typo in 'getopts --man' HOT 7
- Subshell read command fails for 0 or 1 characters plus tab HOT 2
- `unsetref+=(foo)` segfaults HOT 6
- printf with %d: the man page should say that when ".." is used, the precision is 1 HOT 1
- RFE: meson build HOT 4
- getopts: not setting OPTARG when argument is both long form and declared optional HOT 4
- Got a "Memory fault" when trying to sum up to 1.000.000 using a recursive impl. of Gauss' algorithm HOT 2
- trap execution alters $? HOT 2
- Regression. Non determinsitic QA cycle (race condition) HOT 8
- parser confusions with braces inside ${ cmd; } form of command substitution HOT 1
- getopts OPTIND cf. OPTARG in functions
- free(): invalid pointer crash in path_pwd HOT 6
- stack overflow if cannot unlink $HOME/.sh_history HOT 1
- Negative integer literal with radix symbol should be formatted as positive with unary minus for integers defined with an explicit base. HOT 5
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from ksh.