arith: avoid assignment triggering get/getn discipline

An assignment should trigger the 'set' discipline, not the 'get'.
Yet, when the assignment is in an arithmetic expression:

  $ x.set() { echo SET; }; x.get() { echo GET; }; ((x=1))
  SET
  GET

That is clearly incorrect, inconsistent and counter to the
documentation. For a simple shell assignment 'x=1' instead of
'((x=1))', only SET is triggered.

The inconsistency also applies to internal shell disciplines. This
can be demonstrated using the RANDOM reproducible pseudorandom
sequence:

  $ ksh -c 'RANDOM=123; echo $RANDOM $RANDOM $RANDOM $RANDOM'
  29031 19003 403 6924
  $ ksh -c '((RANDOM=123)); echo $RANDOM $RANDOM $RANDOM $RANDOM'
  19003 403 6924 25974

Note the second output is the same as the first one starting from
the second number, meaning RANDOM was incorrectly read from while
the arithmetic command was assigning the seed value to it. The
second output should be identical to the first.

Another reproducer of the same problem:
  $ (( r = (RANDOM = -1) )); echo $r
  2100
The return value r of the "RANDOM = -1" assignment should be -1.
No reading from RANDOM should happen here.

Analysis: Since shell arithmetic uses C syntax, an assignment also
has a return value, i.e., the value assigned to the variable. In
the arith() function in arith.c, case ASSIGN, there is a
'r=nv_getnum(np)' call that retrieves this value. However, calling
nv_getnum() also triggers a getn/get discipline, which is the bug.

Simply replacing this call with "r = n;" (make the return value the
assigned value) fixes the bug, but does cause a regression:

arith.sh[119]: FAIL: typecast not working in arithmetic evaluation

Archaeological research shows that this test was introduced, along
with the nv_getnum() call, sometime between 1995 and 1999. The
following is most likely the corresponding entry in RELEASE, as the
example in it closely matches the arith.sh regression test:

96-07-31  A bug in right to left arithmetic assignment for which
	  the arithmetic expression (( y = x = 1.5 )) did not
	  yield 1 for y when x was declared typeset -i was fixed.

In other words, if x is an integer, the return value of the
assignment is expected to be typecast to integer before being
assigned to y. So that was the purpose of the added nv_getnum()
call: it was a lazy way of doing the correct typecast for the
variable's type -- unfortunately with side effects.

src/cmd/ksh93/include/nval.h:
- Add nv_isnum() macro determining if a shell variable has numeric
  type attributes, and returning their bit mask if so.

src/cmd/ksh93/sh/arith.c: arith(): case ASSIGN:
- Replace the offending nv_getnum() call with a simple "r = n;" by
  default, fixing the bug.
- To avoid the regression, if nv_isnum() yields attribute bits for
  the variable, then do "r = n" with a direct typecast to the
  correct type, depending on the shell variable's attribute bits.

src/cmd/ksh93/sh.1:
- Rewrite the $RANDOM description to match reality. (re: af6a32d)

NEWS

@@ -2,6 +2,16 @@ This documents significant changes in the 1.0 branch of ksh 93u+m.
 For full details, see the git log at: https://github.com/ksh93/ksh/tree/1.0
 Uppercase BUG_* IDs are shell bug IDs as used by the Modernish shell library.
 
+2024-01-21:
+
+- A bug was fixed that caused both the 'set' and 'get'/'getn' discipline
+  functions of a variable to be triggered when performing an assignment in
+  an arithmetic expression; only the 'set' discipline is now triggered when
+  assigning (as was always the documented behaviour). This applies to the
+  shell's internal disciplines as well. For example, seeding the $RANDOM
+  generator via an arithmetic expression assignment now no longer takes the
+  first value away from the reproducible pseudorandom sequence.
+
 2024-01-16:
 
 - Fixed a parsing bug in array subscripts containing '=' in combination with

src/cmd/ksh93/COMPATIBILITY

@@ -1,4 +1,4 @@
-		ksh 93u+m/1.0.5 vs. ksh 93u+
+		ksh 93u+m/1.0.9 vs. ksh 93u+
 
 The following is a list of changes between ksh 93u+ 2012-08-01 and the new
 ksh 93u+m reboot that could cause incompatibilities in rare corner cases.
@@ -180,17 +180,25 @@ For more details, see the NEWS file and for complete details, see the git log.
 33.	The -e option of test/[/[[ no longer returns true for pseudodevices
 	that are only supported by the shell and do not in fact exist in the
 	file system, such as /dev/tcp/*.
+	(Post-1.0 fix introduced in ksh 93u+m/1.0.4)
 
 34.	Single and double quotes now work like backslashes to quote '!', '^'
 	and '-' in bracket expressions in shell glob patterns, e.g., the glob
 	pattern ["!a-z"] now matches !, a, - or z and is no longer misparsed
 	as [!a-z]. This restores compatibility with ksh88 and non-ksh shells.
+	(Post-1.0 fix introduced in ksh 93u+m/1.0.4)
 
 35.	In the ${parameter/pattern/string} search-and-replace expansion, an
 	anchored empty pattern will now match the beginning or the end of the
 	string, so that ${parameter/#/string} will prefix the string to the
 	parameter value and ${parameter/%/string} will append it. Previously,
 	these operations were no-ops (equivalent to ${parameter}).
+	(Post-1.0 fix introduced in ksh 93u+m/1.0.5)
+
+36.	An assignment within an arithmetic expression no longer triggers both
+	of the variable's 'set' and 'get' discipline functions (if set). Only
+	the 'set' discipline is now triggered, as it was always documented.
+	(Post-1.0 fix introduced in ksh 93u+m/1.0.9)
 
 ____________________________________________________________________________
 

src/cmd/ksh93/include/nval.h

@@ -2,7 +2,7 @@
 *                                                                      *
 *               This software is part of the ast package               *
 *          Copyright (c) 1982-2012 AT&T Intellectual Property          *
-*          Copyright (c) 2020-2022 Contributors to ksh 93u+m           *
+*          Copyright (c) 2020-2024 Contributors to ksh 93u+m           *
 *                      and is licensed under the                       *
 *                 Eclipse Public License, Version 2.0                  *
 *                                                                      *
@@ -196,12 +196,15 @@ struct Namval
 #define NV_UINT16P	(NV_LJUST|NV_UNSIGN|NV_SHORT|NV_INTEGER)
 #define NV_UINT16	(NV_UNSIGN|NV_SHORT|NV_INTEGER)
 #define NV_INT32	(NV_INTEGER)
-#define NV_UNT32	(NV_UNSIGN|NV_INTEGER)
+#define NV_UINT32	(NV_UNSIGN|NV_INTEGER)
 #define NV_INT64	(NV_LONG|NV_INTEGER)
 #define NV_UINT64	(NV_UNSIGN|NV_LONG|NV_INTEGER)
 #define NV_FLOAT	(NV_SHORT|NV_DOUBLE)
 #define NV_LDOUBLE	(NV_LONG|NV_DOUBLE)
 
+/* check/isolate all the bit flags used for numeric types */
+#define nv_isnum(np)	(nv_isattr(np,NV_INTEGER)?nv_isattr(np,NV_DOUBLE|NV_INTEGER|NV_LJUST|NV_LONG|NV_SHORT|NV_UNSIGN):0)
+
 /* name-value pair macros */
 #define nv_isattr(np,f)		((np)->nvflag & (f))
 #define nv_onattr(n,f)		((n)->nvflag |= (f))

src/cmd/ksh93/include/version.h

@@ -18,7 +18,7 @@
 
 #define SH_RELEASE_FORK	"93u+m"		/* only change if you develop a new ksh93 fork */
 #define SH_RELEASE_SVER	"1.0.9-beta"	/* semantic version number: https://semver.org */
-#define SH_RELEASE_DATE	"2024-01-16"	/* must be in this format for $((.sh.version)) */
+#define SH_RELEASE_DATE	"2024-01-21"	/* must be in this format for $((.sh.version)) */
 #define SH_RELEASE_CPYR	"(c) 2020-2024 Contributors to ksh " SH_RELEASE_FORK
 
 /* Scripts sometimes field-split ${.sh.version}, so don't change amount of whitespace. */

src/cmd/ksh93/sh.1

@@ -1943,11 +1943,16 @@ command.
 .TP
 .B
 .SM RANDOM
-Each time this variable is referenced, a random integer,
-uniformly distributed between 0 and 32767, is generated.
-The sequence of random numbers can be initialized by assigning
-a numeric value to
+Each time this variable is referenced, a pseudorandom integer is generated,
+uniformly distributed between 0 and 32767 (the 16-bit unsigned integer range)
+except that the same number is never repeated twice in a row.
+The sequence of pseudorandom numbers is reproducible
+and can be initialized to a fixed starting point by assigning
+a numeric seed value to
 .BR RANDOM .
+Each time a new shell or subshell environment is entered (see
+.I Subshells\^
+above), the sequence is automatically reset to a different point.
 .TP
 .B
 .SM REPLY

src/cmd/ksh93/sh/arith.c

@@ -2,7 +2,7 @@
 *                                                                      *
 *               This software is part of the ast package               *
 *          Copyright (c) 1982-2012 AT&T Intellectual Property          *
-*          Copyright (c) 2020-2023 Contributors to ksh 93u+m           *
+*          Copyright (c) 2020-2024 Contributors to ksh 93u+m           *
 *                      and is licensed under the                       *
 *                 Eclipse Public License, Version 2.0                  *
 *                                                                      *
@@ -227,6 +227,7 @@ static Sfdouble_t arith(const char **ptr, struct lval *lvalue, int type, Sfdoubl
 	    case ASSIGN:
 	    {
 		Namval_t *np;
+		unsigned short attr;
 		if (lvalue->sub && lvalue->nosub > 0) /* indexed array ARITH_ASSIGNOP */
 		{
 			np = (Namval_t*)lvalue->sub; /* use saved subscript reference instead of last worked value */
@@ -253,8 +254,37 @@ static Sfdouble_t arith(const char **ptr, struct lval *lvalue, int type, Sfdoubl
 		if(lvalue->eflag)
 			lvalue->ptr = nv_hasdisc(np,&ENUM_disc);
 		lvalue->eflag = 0;
-		r=nv_getnum(np);
 		lvalue->value = (char*)np;
+		/*
+		 * The result (r) of an assignment is its value (n), cast to the type of the variable
+		 * assigned to. We cannot simply reobtain the value with nv_getnum() to effectuate the
+		 * typecast, because that may trigger a getnum discipline function with side effects.
+		 * The order of checks below is essential due to how the bit masks work (see nval.h).
+		 */
+		attr = nv_isnum(np);
+		if(!attr || (attr & NV_LDOUBLE)==NV_LDOUBLE)	/* long float */
+			r = n;
+		else if((attr & NV_FLOAT)==NV_FLOAT)		/* short float */
+			r = (float)n;
+		else if((attr & NV_DOUBLE)==NV_DOUBLE)		/* normal float */
+			r = (double)n;
+		else if((attr & NV_UINT64)==NV_UINT64)		/* long unsigned integer */
+			r = (uintmax_t)n;
+		else if((attr & NV_UINT16)==NV_UINT16)		/* short unsigned integer */
+			r = (uint16_t)n;
+		else if((attr & NV_UINT32)==NV_UINT32)		/* normal unsigned integer */
+			r = (uint32_t)n;
+		else if((attr & NV_INT64)==NV_INT64)		/* long signed integer */
+			r = (intmax_t)n;
+		else if((attr & NV_INT16)==NV_INT16)		/* short signed integer */
+			r = (int16_t)n;
+		else if((attr & NV_INT32)==NV_INT32)		/* normal signed integer */
+			r = (int32_t)n;
+#if _AST_release
+		else	r = n;					/* should never happen */
+#else
+		else	abort();
+#endif
 		break;
 	    }
 	    case LOOKUP:

src/cmd/ksh93/tests/arith.sh

@@ -2,7 +2,7 @@
 #                                                                      #
 #               This software is part of the ast package               #
 #          Copyright (c) 1982-2012 AT&T Intellectual Property          #
-#          Copyright (c) 2020-2023 Contributors to ksh 93u+m           #
+#          Copyright (c) 2020-2024 Contributors to ksh 93u+m           #
 #                      and is licensed under the                       #
 #                 Eclipse Public License, Version 2.0                  #
 #                                                                      #
@@ -995,5 +995,39 @@ function .sh.math.add x y { .sh.value=x+y; }
 got=$(PATH=/dev/null; typeset -i z; redirect 2>&1; z='add(2 , 3)'; echo $z)
 [[ e=$? -eq 0 && $got == '5' ]] || err_exit ".sh.math.* function parsing: got status $e and $(printf %q "$got")"
 
+# ======
+# arithmetic assignments should not trigger getn disciplines, but the return
+# value should still be cast to the type of the variable that is assigned to
+
+float x
+x.getn() { .sh.value=999.99; }
+let "(got = x = 1234.56) == 1234.56" || err_exit "arithmetic assignment triggers getn discipline (got $got)"
+let "(got = x) == 999.99" || err_exit "arithmetic comparison fails to trigger getn discipline (got $got)"
+unset x
+whence -q x.getn && err_exit "unset x fails to unset -f x.getn"
+
+(
+	ulimit -c 0  # fork
+	Errors=0
+	for sz in '' s l
+	do	typeset -${sz}i x=0
+		if	! let "(got = x = 123.95) == 123"
+		then	err_exit "arithmetic assignment does not return properly typecast value (-${sz}i, got $got)"
+		fi
+		typeset -${sz}F x=0
+		if	! let "(got = x = 123.95) == 123.95"
+		then	err_exit "arithmetic assignment does not return properly typecast value (-${sz}F, got $got)"
+		fi
+	done
+)
+if	let "(e = $?) > 128"
+then	err_exit "typeset crashed the shell (got status $e/SIG$(kill -l "$e"))"
+else	let "Errors += e"
+fi
+
+if	! let "(got = RANDOM = 123.95) == 123"
+then	err_exit "arithmetic assignment to RANDOM does not return typecast of assigned value (got $got)"
+fi
+
 # ======
 exit $((Errors<125?Errors:125))

src/cmd/ksh93/tests/variables.sh

@@ -2,7 +2,7 @@
 #                                                                      #
 #               This software is part of the ast package               #
 #          Copyright (c) 1982-2012 AT&T Intellectual Property          #
-#          Copyright (c) 2020-2023 Contributors to ksh 93u+m           #
+#          Copyright (c) 2020-2024 Contributors to ksh 93u+m           #
 #                      and is licensed under the                       #
 #                 Eclipse Public License, Version 2.0                  #
 #                                                                      #
@@ -125,6 +125,12 @@ got=$(RANDOM=1; print $RANDOM; :& print $RANDOM)
 [[ $got == "$exp" ]] || err_exit "Background job influences reproducible $RANDOM sequence" \
 	"(expected $(printf %q "$exp"), got $(printf %q "$got"))"
 
+# Seeding with an arithmetic expression should be identical to seeding using a shell assignment
+RANDOM=12345; exp="$RANDOM $RANDOM $RANDOM $RANDOM"
+let "RANDOM = 12345"; got="$RANDOM $RANDOM $RANDOM $RANDOM"
+[[ $got == "$exp" ]] || err_exit "Seeding RANDOM using arithmetic expression fails" \
+	"(expected $(printf %q "$exp"), got $(printf %q "$got"))"
+
 # SECONDS
 float secElapsed=0.0 secSleep=0.001
 let SECONDS=$secElapsed