Skip to content

Commit

Permalink
Fix undefined behaviour (re: 4592859, 23b7a16)
Browse files Browse the repository at this point in the history
The previous commit exposed undefined behaviour elsewhere that is
the result of old ksh bugs. My 2021 attempt to fix it in 23b7a16
was also wrong.

src/cmd/ksh93/sh/init.c: put_rand(), put_lineno(), put_lastarg():
- An incorrect typecast to double instead of Sfdouble_t (which
  translates to 'long double' on most systems, but not on macOS M1)
  caused arithmetic assignments to RANDOM, LINENO and _ to produce
  undefined results where sizeof(long double)!=sizeof(double). As
  ksh has been using Sfdouble_t for all arithmetic operations for
  years, fix the bug by typecasting to Sfdouble_t instead.

src/cmd/ksh93/tests/arith.sh:
- Fix the tests added in the previous commit to tolerate
  system-dependent rounding errors.
  • Loading branch information
McDutchie committed Jan 21, 2024
1 parent 4592859 commit b67c172
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 11 deletions.
8 changes: 5 additions & 3 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@ Uppercase BUG_* IDs are shell bug IDs as used by the Modernish shell library.
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.
shell's internal disciplines as well. For example, $RANDOM is no longer
spuriously read from while seeding it via an arithmetic expression.

- Fixed: assigning to LINENO, RANDOM, or _ via an arithmetic expression gave
undefined results on systems where sizeof(long double)!=sizeof(double).

2024-01-16:

Expand Down
8 changes: 4 additions & 4 deletions src/cmd/ksh93/sh/init.c
Original file line number Diff line number Diff line change
Expand Up @@ -673,7 +673,7 @@ static Sfdouble_t nget_seconds(Namval_t* np, Namfun_t *fp)
static void put_rand(Namval_t* np,const char *val,int flags,Namfun_t *fp)
{
struct rand *rp = (struct rand*)fp;
long n;
Sfdouble_t n;
sh_save_rand_seed(rp, 0);
if(!val)
{
Expand All @@ -684,7 +684,7 @@ static void put_rand(Namval_t* np,const char *val,int flags,Namfun_t *fp)
return;
}
if(flags&NV_INTEGER)
n = *(double*)val;
n = *(Sfdouble_t*)val;
else
n = sh_arith(val);
srand(rp->rand_seed = (unsigned int)n);
Expand Down Expand Up @@ -753,7 +753,7 @@ static void put_lineno(Namval_t* np,const char *val,int flags,Namfun_t *fp)
return;
}
if(flags&NV_INTEGER)
n = (Sfdouble_t)(*(double*)val);
n = *(Sfdouble_t*)val;
else
n = sh_arith(val);
sh.st.firstline += (int)(nget_lineno(np,fp) + 1 - n);
Expand All @@ -778,7 +778,7 @@ static void put_lastarg(Namval_t* np,const char *val,int flags,Namfun_t *fp)
{
if(flags&NV_INTEGER)
{
sfprintf(sh.strbuf,"%.*g",12,*((double*)val));
sfprintf(sh.strbuf,"%.*Lg",12,*((Sfdouble_t*)val));
val = sfstruse(sh.strbuf);
}
if(val)
Expand Down
20 changes: 16 additions & 4 deletions src/cmd/ksh93/tests/arith.sh
Original file line number Diff line number Diff line change
Expand Up @@ -1000,9 +1000,10 @@ got=$(PATH=/dev/null; typeset -i z; redirect 2>&1; z='add(2 , 3)'; echo $z)
# 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)"
x.getn() { .sh.value=987.65; }
let "got = x = 1234.56"
[[ $got == 1234.56* ]] || err_exit "arithmetic assignment triggers getn discipline (got $got)"
[[ $x == 987.65* ]] || err_exit "arithmetic comparison fails to trigger getn discipline (got $x)"
unset x
whence -q x.getn && err_exit "unset x fails to unset -f x.getn"

Expand All @@ -1015,7 +1016,8 @@ whence -q x.getn && err_exit "unset x fails to unset -f x.getn"
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"
let "got = x = 123.95"
if [[ $got != 123.95* ]] # ignore OS-dependent rounding error
then err_exit "arithmetic assignment does not return properly typecast value (-${sz}F, got $got)"
fi
done
Expand All @@ -1029,5 +1031,15 @@ if ! let "(got = RANDOM = 123.95) == 123"
then err_exit "arithmetic assignment to RANDOM does not return typecast of assigned value (got $got)"
fi

let "_ = 123.95, got = _"
if [[ $got != '123.95' ]]
then err_exit "arithmetic assignment to _ fails (got $got)"
fi

got=$(let "LINENO = 123"; print $LINENO )
if [[ $got != '122' ]] # TODO: should be 123
then err_exit "arithmetic assignment to LINENO fails (got $got)"
fi

# ======
exit $((Errors<125?Errors:125))

0 comments on commit b67c172

Please sign in to comment.