arithmetic: fix divide by negative on ARM systems

*** BUG 1 ***

Léonard Oest O'Leary (@leo-ard) reports:
> Doing an arithmetic expansion with a division where the dividend
> is a negative number returns a "division by zero error" :
>
>     > ksh
>     $ echo $(( 10 / -10 ))
>     ksh:  10 / -10 : divide by zero
>     $ echo $(( 10 / -1 ))
>     ksh:  10 / -1 : divide by zero
>     $ echo $(( 10 / -2 ))
>     ksh:  10 / -2 : divide by zero
>
> Tested with version sh (AT&T Research) 93u+ 2012-08-01 of ksh.
> Running on Macos M2 14.1.1 (23B81)

Turns out this bug is reproducible on all ARM systems. It was
inherited from 93u+ 2012-08-01. Testing my stash of compiled
historic AT&T ksh versions (on a Debian arm64 VM) shows that the
bug was introduced in 93q 2005-01-31 and fixed in 93v- 2013-06-28.

Analysis: All shell arithmetic is internally done with the
maximum-length float type that the system supports (Sfdouble_t).
Since comparison with floats is inexact, ksh converts the numnber
to Sfulong_t, the largest unsigned integer type supported by the
system, before comparing it to zero and throwing a "divide by
zero". In streval.c:

354:	else if((Sfulong_t)(num)==0)
355:		arith_error(e_divzero,ep->expr,ep->emode);

However, converting a negative float value to an unsigned integer
type is undefined behaviour in the C standard. On (at least) ARM
systems and IBM POWER systems [*1], the result is always zero in
that case, which causes this bug.

src/cmd/ksh93/sh/streval.c: arith_exec():
- Backport the 93v- 2013-06-28 fix: make sure that the number is
  not negative before typecasting it and compating it to zero.

*** BUG 2 ***

I introduced a similar bug with the same undefined behaviour myself
in 4592859 (see there for explanation). On ARM systems:

    $ typeset -ui i
    $ echo $((i = -5))
    0
    $ echo $i
    4294967291

The assignment worked fine (wrapped around as expected for an
unsigned integer), but the value of the assignment was 0 and should
have been the same as the variable's new value.

So lines 272, 274 and 276 in arith.c have undefined behaviour for a
negative n (which is of the Sfdouble_t float type):

271: else if((attr & NV_UINT64)==NV_UINT64) /* long unsigned integer */
272: 	r = (uintmax_t)n;
273: else if((attr & NV_UINT16)==NV_UINT16) /* short unsigned integer */
274: 	r = (uint16_t)n;
275: else if((attr & NV_UINT32)==NV_UINT32) /* normal unsigned integer */
276: 	r = (uint32_t)n;

src/cmd/ksh93/sh/arith.c: arith():
- Before doing those typecasts, check if n is negative. If so,
  invert the sign using the unary minus (yielding a positive
  float), do the typecast, and then do another unary minus
  operation on the resulting positive unsigned integer value --
  which is well-defined behaviour in C [*2] and yields a positive
  wrapped-around unsigned integer value, which is fine to
  implicitly typecast back to r's float type.

Resolves: #770

[*1] https://www.ibm.com/support/pages/assigning-negative-float-value-unsigned-integral-type-yields-undefined-value-stored-type
[*2] https://stackoverflow.com/questions/8026694/c-unary-minus-operator-behavior-with-unsigned-operands

NEWS

@@ -2,6 +2,18 @@ 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-07-24:
+
+- Fixed a serious bug in the arithmetic subsystem that was triggered on
+  non-Intel processors (such as ARM): any division of an integer by a
+  negative integer threw a spurious "divide by zero" error. Bug introduced
+  in ksh 93q 2005-01-31.
+
+- Similarly fixed: the return value of the assignment of a negative value to
+  an unsigned integer in an arithmetic expression was always 0 on ARM
+  processors (instead of being equal to the assigned wrapped-around value).
+  Bug introduced by the discipline fix on 2024-01-21.
+
 2024-07-21:
 
 - Fixed a long-standing parsing bug: a '((' that was unmatched by

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.10-beta"	/* semantic version number: https://semver.org */
-#define SH_RELEASE_DATE	"2024-07-21"	/* must be in this format for $((.sh.version)) */
+#define SH_RELEASE_DATE	"2024-07-24"	/* 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/arith.c

@@ -268,12 +268,14 @@ static Sfdouble_t arith(const char **ptr, struct lval *lvalue, int type, Sfdoubl
 			r = (float)n;
 		else if((attr & NV_DOUBLE)==NV_DOUBLE)		/* normal float */
 			r = (double)n;
+		/* Avoid typecasting a negative float (Sfdouble_t) to an
+		 * unsigned integer (uint*_t), which is undefined behaviour */
 		else if((attr & NV_UINT64)==NV_UINT64)		/* long unsigned integer */
-			r = (uintmax_t)n;
+			r = n < 0 ? -((uintmax_t)(-n)) : (uintmax_t)n;
 		else if((attr & NV_UINT16)==NV_UINT16)		/* short unsigned integer */
-			r = (uint16_t)n;
+			r = n < 0 ? -((uint16_t)(-n)) : (uint16_t)n;
 		else if((attr & NV_UINT32)==NV_UINT32)		/* normal unsigned integer */
-			r = (uint32_t)n;
+			r = n < 0 ? -((uint32_t)(-n)) : (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 */

src/cmd/ksh93/sh/streval.c

@@ -351,7 +351,9 @@ Sfdouble_t	arith_exec(Arith_t *ep)
 				num = sp[-1]/num;
 				type = 1;
 			}
-			else if((Sfulong_t)(num)==0)
+			/* Avoid typecasting a negative float (Sfdouble_t) to an
+			 * unsigned integer (Sfulong_t), which is undefined behaviour */
+			else if((Sfulong_t)(num < 0 ? -num : num)==0)
 				arith_error(e_divzero,ep->expr,ep->emode);
 			else if(type==2 || tp[-1]==2)
 				num = U2F((Sfulong_t)(sp[-1]) / (Sfulong_t)(num));

src/cmd/ksh93/tests/arith.sh

@@ -1123,5 +1123,19 @@ got=$( ( ulimit -c 0; set +x; eval '{ (( $(( 1 )) )); }' ) 2>&1 )
 	"(expected status 0 and ''," \
 	"got status $e$( ((e>128)) && print -n /SIG && kill -l "$e") and $(printf %q "$got"))"
 
+# ======
+# On ARM systems, division by negative threw an incorrect "divide by zero" error!
+# https://github.com/ksh93/ksh/issues/770
+got=$(let "10 / -10 == -1 && 10 / -1 == -10 && 10 / -2 == -5" 2>&1) || err_exit "division by negative (got $(printf %q "$got"))"
+# Also, from 2024-01-21 to 2024-07-24, the return value of assigning negative to unsigned int was 0 on ARM
+if	! exp=$(PATH=/opt/ast/bin:$PATH; getconf UINT_MAX 2>/dev/null)
+then	warning "getconf UINT_MAX failed; skipping issue 770 test"
+else	typeset -ui i
+	got=$((i = -1))
+	[[ $i == "$exp" ]] || err_exit "unsigned int wrap-around of -1 (expected '$exp', got '$i')"
+	[[ $got == "$exp" ]] || err_exit "return value of assigning -1 to unsigned int (expected '$exp', got '$got')"
+	unset i
+fi
+
 # ======
 exit $((Errors<125?Errors:125))