Re: [PATCH] multibyte_identifiers: opt-in UTF-8 variable names (UAX #31 + NFC) behind a root-controlled system switch

Hello,

v6 is attached, rebased onto devel as of 9c8a70bc 
(https://git.savannah.gnu.org/cgit/bash.git/commit/?id=9c8a70bc9e2572156fbece23e4153e68c267b510). 
It supersedes v5, the
last revision posted here.

Why that commit
---------------
9c8a70bc replaced the locale-dependent isalnum(3)/isalpha(3) tests in
legal_variable_starter and legal_variable_char with lookups in the syntax
table computed at build time. That is the natural foundation for this
patch, and I would rather build on it than beside it: the ASCII range is
now classified without consulting the locale, and this patch extends the
same idea past U+007F with a generated table instead of a run-time
library call. Nothing here modifies general.h, so the two do not overlap
-- v5 applied to devel unchanged, and the only adjustment v6 needed was
the one below.

One finding from the rebase
---------------------------
The ${#name} call site in subst.c passes a plain char to
legal_variable_starter. Since 9c8a70bc that expression indexes
sh_syntaxtab directly, so on a platform with signed char a byte >= 0x80
becomes a negative subscript; UBSan reports it as

     index -61 out of bounds for type 'int [256]'

for the first byte of a two-byte UTF-8 sequence. I could not get it to
misbehave visibly -- the bytes read out of bounds happened not to carry
CNAMESTART -- but it is a real out-of-bounds read, and it was harmless
before only because ISALPHA() went through isalnum(), where glibc uses a
table offset for exactly this case.

v6 casts at that call site, since the patch modifies that line anyway.
The same omission is in the macro definitions themselves, where
sh_syntaxtab[c] is indexed without a cast while shellmeta() and the new
ISNAME()/ISNAMESTART() use sh_syntaxtab[(unsigned char)c]. I have left
that alone -- it is not this patch's business, and I did not want to put
changes outside the HANDLE_MULTIBYTE paths into it again -- but it seemed
worth reporting either way. I checked the other thirteen call sites; they
all pass an unsigned char or cast already.

What else changed since v4
--------------------------
The UTS #39 general security profile I added in v3 turned out to be too
blunt, and Robert Elz found it. It is derived with NFKC in mind and
restricts everything NFKC would rewrite, while this patch requires NFC.
Applying it wholesale removed ordinary letters of scripts in daily use:
without U+0E33 THAI CHARACTER SARA AM, frequent Thai words could not be
used as names at all, and the same held for the Lao, Armenian, Arabic and
Kana equivalents.

The generator now re-admits a restricted character when it is NFC-stable,
has a compatibility mapping to two or more characters, and every
character of that mapping lies in its own block and is itself allowed.
That currently yields eleven characters, listed by name in the generator
with a self-check that fails if a UCD update changes the set. Fullwidth
forms, mathematical alphanumerics, the Latin digraphs and the Arabic and
Armenian presentation forms remain excluded.

The honest cost, which the manual now states: re-admitting those eleven
adds a spelling, since NFC does not unify a precomposed character with
its compatibility decomposition. It buys back the natural spelling
without buying uniqueness, because these scripts already admit several
identically rendering encodings that no normalization form unifies --
Thai permits either order of NIKHAHIT and a tone mark, and neither NFC
nor NFKC merges those. Excluding U+0E33 removed the ordinary way to write
a word while leaving the ambiguity it was meant to prevent.

Also in v6: tests/complete.right lists the new option, which appears in
shopt completions regardless of its state.

Verified
--------
Against a fresh checkout of 9c8a70bc: applies cleanly, builds without
warnings from the new files, `make regen-mbidtbl' reproduces mbidtbl.h
byte for byte, and the test suite passes with the switch on and with it
off. Behaviour spot checks: U+03BC, Thai คำ, ÄÖÜẞ and Straße accepted;
U+00B5, fullwidth Ａ and the Arabic ligature ﻻ rejected; with the switch
off, byte-for-byte the behaviour of an unpatched shell.

As before: happy to split the toggle_shopts return-value fix into a
separate preliminary patch, to add explicit HANDLE_MULTIBYTE guards where
the null macros are used now, or to drop the whole thing if you would
rather not carry it. The patch is also in a repository, with a changelog
of what each revision changed and why:

     https://github.com/Otzie2023/Bash-with-Unicode-Identifier

Regards,
Luca Rodenhäuser