Fix representation of negative non-base-10 numbers

Simple reproducer:

    $ typeset -si16 n=-12
    $ echo $n
    16#fffffffffffffff4

Expected output: -16#c

The base-16 representation is incorrectly treated as unsigned
(and also as long integer -li, though it was declared as -si
-- e.g. 'typeset -sui 16 n=-12' correctly outputs 16#fff4).

This behaviour is clearly incorrect in two ways. Both the
signedness and the type should match in the output.

With less obvious cases, this can be very confusing, e.g.:

   $ typeset -li20 n=-20#j12
   $ echo $n
   20#b53bjh07be4cjje

Expected output: -20#j12

Analysis: nv_getval(), the central function to render a variable's
value as a string, calls libast's fmtbase() to render numbers of
any type (src/lib/libast/string/fmtbase.c). When the base is 10
and the prefix is not shown, this simply calls fmtint (fmtint.c)
which is a performance-optimised function to render base-10
integers, and all is well.

But if a base is explicitly specified (b != 0), then if the base is
10, fmtint() is always called with unsign==1, or if the base is
other than 10, fmtbase() always tells sfsprintf() to treat the
value as unsigned (using the %u formatter). This is the problem. It
was clearly done this way on purpose, but it's also clearly wrong.

I also made the following observations:

(1) ksh is the only user of the fmtbase() function; even in the old
    complete AST toolset, nothing else calls fmtbase(). So we don't
    need to worry about backward compatibility here.

(2) fmtint() has only ever been called via fmtbase(), though it is
    publicly declared in ast.h.

(3) fmtbase() is called in 10 places in the ksh code. Only one of
    them, the one in nv_getval(), possibly involves a non-10 base.
    The rest of them just use fmtint() via fmtbase().

(4) fmtbase() is inefficient. It uses a string buffer returned by
    fmtbuf(), not an Sfio stream buffer. This is efficient for
    fmtint() which stores output directly in memory. But for a
    non-10 base, fmtbase() calls sfsprintf() (see sfio/sfprintf.c)
    which opens a temporary Sfio stream, calls sfprintf(), copies
    the Sfio stream buffer contents to the string buffer, then
    closes the stream. It should be faster to use Sfio directly.

So, with all that taken into consideration, this commit simplifies
the eight fmtbase() calls with a known base of 10 to equivalent
fmtint() calls, and handles the ninth case directly in nv_getval()
using the sh.strbuf stream buffer pre-opened in init.c. Handling
signedness correctly in nv_getval() is trivial, because sfprintf()
does the correct thing if the correct d or u formatter is used.

With these changes, the incorrect fmtbase() function is now unused,
and is therefore deleted. Relevant documentation updates are made.

Thanks to Daniel Douglas (@ormaaj) for the bug report.
Resolves: #696

NEWS

@@ -2,6 +2,13 @@ This documents significant changes in the dev branch of ksh 93u+m.
 For full details, see the git log at: https://github.com/ksh93/ksh
 Uppercase BUG_* IDs are shell bug IDs as used by the Modernish shell library.
 
+2024-02-03:
+
+- Fixed: the arithmetic representation of negative integers with a base other
+  than 10 is no longer incorrectly treated as unsigned long. For example,
+	typeset -i16 n=-12; echo $n
+  now correctly outputs '-16#c' and no longer ouputs '16#fffffffffffffff4'.
+
 2024-01-27:
 
 - Fixed: tilde expansion discipline functions (see 2021-03-16) were not

src/cmd/ksh93/COMPATIBILITY

@@ -212,6 +212,12 @@ For more details, see the NEWS file and for complete details, see the git log.
 	the 'set' discipline is now triggered, as it was always documented.
 	(Post-1.0 fix introduced in ksh 93u+m/1.0.9)
 
+37.	Non-base-10 signed negative integer values are now output as negative
+	numbers and no longer incorrectly treated as unsigned long integers
+	regardless of their type length. For example, 'typeset -i16 n=-12;
+	echo $n' now correctly prints -16#c instead of 16#fffffffffffffff4.
+	(Post-1.0 fix introduced in ksh 93u+m/1.0.9)
+
 ____________________________________________________________________________
 
 		KSH-93 VS. KSH-88

src/cmd/ksh93/edit/completion.c

@@ -652,7 +652,7 @@ int ed_fulledit(Edit_t *ep)
 		hist_flush(sh.hist_ptr);
 	}
 	cp = strcopy((char*)ep->e_inbuf,e_runvi);
-	cp = strcopy(cp, fmtbase((intmax_t)ep->e_hline,10,0));
+	cp = strcopy(cp, fmtint(ep->e_hline,1));
 #if SHOPT_VSH
 	ep->e_eol = ((unsigned char*)cp - (unsigned char*)ep->e_inbuf)-(sh_isoption(SH_VI)!=0);
 #else

src/cmd/ksh93/edit/emacs.c

@@ -1291,7 +1291,7 @@ static void xcommands(Emacs_t *ep,int count)
 			}
 			return;
 
-#	define itos(i)	fmtbase((intmax_t)(i),0,0)	/* want signed conversion */
+#	define itos(i)	fmtint(i,0)	/* want signed conversion */
 
 		case cntl('H'):		/* ^X^H show history info */
 			{

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.1.0-alpha"	/* semantic version number: https://semver.org */
-#define SH_RELEASE_DATE	"2024-01-27"	/* must be in this format for $((.sh.version)) */
+#define SH_RELEASE_DATE	"2024-02-03"	/* 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

@@ -3178,20 +3178,22 @@ Integer constants follow the ANSI C programming language
 integer constant conventions although only single byte
 character constants are recognized and character casts
 are not recognized.
-In addition constants can be of the form
-\*(OK\f2base\f3#\^\f1\*(CK\f2n\^\fP
+.PP
+In addition, integer constants can be of the form
+\f2base\f3#\^\f1\f2n\^\fP
+or (for negative numbers)
+\f3-\^\f2base\f3#\^\f1\f2n\^\fP,
 where
 .I base\^
 is a decimal number between two and sixty-four
-representing the arithmetic base
-and
+representing the arithmetic base, and
 .I n\^
-is a number in that base.
+is an unsigned integer in that base.
 The digits above 9 are represented
 by the lower case letters, the upper case letters,
 .BR @ ,
 and
-.B _
+.BR _ ,
 respectively.
 For bases less than or equal to 36, upper and lower case
 characters can be used interchangeably.

src/cmd/ksh93/sh/args.c

@@ -751,7 +751,7 @@ struct argnod *sh_argprocsub(struct argnod *argp)
 	chmod(sh.fifo,S_IRUSR|S_IWUSR);	/* mkfifo + chmod works regardless of umask */
 	sfputr(sh.stk,sh.fifo,0);
 #endif /* SHOPT_DEVFD */
-	sfputr(sh.stk,fmtbase((intmax_t)pv[fd],10,0),0);
+	sfputr(sh.stk,fmtint(pv[fd],1),0);
 	ap = (struct argnod*)stkfreeze(sh.stk,0);
 	sh.inpipe = sh.outpipe = 0;
 	/* turn off job control */

src/cmd/ksh93/sh/array.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                  *
 *                                                                      *
@@ -807,7 +807,7 @@ static struct index_array *array_grow(Namval_t *np, struct index_array *arp,int
 	int newsize = arsize(arp,maxi+1);
 	if (maxi >= ARRAY_MAX)
 	{
-		errormsg(SH_DICT,ERROR_exit(1),e_subscript, fmtbase((intmax_t)maxi,10,0));
+		errormsg(SH_DICT,ERROR_exit(1),e_subscript,fmtint(maxi,1));
 		UNREACHABLE();
 	}
 	i = (newsize-1)*sizeof(union Value)+newsize;

src/cmd/ksh93/sh/init.c

@@ -713,7 +713,7 @@ static Sfdouble_t nget_rand(Namval_t* np, Namfun_t *fp)
 static char* get_rand(Namval_t* np, Namfun_t *fp)
 {
 	intmax_t n = (intmax_t)nget_rand(np,fp);
-	return fmtbase(n, 10, 0);
+	return fmtint(n,1);
 }
 
 void sh_reseed_rand(struct rand *rp)
@@ -752,7 +752,7 @@ static Sfdouble_t nget_srand(Namval_t* np, Namfun_t *fp)
 static char* get_srand(Namval_t* np, Namfun_t *fp)
 {
 	intmax_t n = (intmax_t)(sh.srand_upper_bound ? arc4random_uniform(sh.srand_upper_bound) : arc4random());
-	return fmtbase(n, 10, 0);
+	return fmtint(n,1);
 }
 
 /*
@@ -791,7 +791,7 @@ static void put_lineno(Namval_t* np,const char *val,int flags,Namfun_t *fp)
 static char* get_lineno(Namval_t* np, Namfun_t *fp)
 {
 	intmax_t n = (intmax_t)nget_lineno(np,fp);
-	return fmtbase(n, 10, 0);
+	return fmtint(n,1);
 }
 
 static char* get_lastarg(Namval_t* np, Namfun_t *fp)

src/cmd/ksh93/sh/io.c

@@ -2256,7 +2256,7 @@ static void	sftrack(Sfio_t* sp, int flag, void* data)
 #ifdef DEBUG
 	if(flag==SF_READ || flag==SF_WRITE)
 	{
-		char *z = fmtbase((intmax_t)sh.current_pid,0,0);
+		char *z = fmtint(sh.current_pid,0);
 		write(ERRIO,z,strlen(z));
 		write(ERRIO,": ",2);
 		write(ERRIO,"attempt to ",11);

src/cmd/ksh93/sh/macro.c

@@ -83,7 +83,7 @@ typedef struct  _mac_
 #define isescchar(s)	((s)>S_QUOTE)
 #define isqescchar(s)	((s)>=S_QUOTE)
 #define isbracechar(c)	((c)==RBRACE || (_c_=sh_lexstates[ST_BRACE][c])==S_MOD1 ||_c_==S_MOD2)
-#define ltos(x)		fmtbase((intmax_t)(x),0,0)
+#define ltos(x)		fmtint(x,0)
 
 /* type of macro expansions */
 #define M_BRACE		1	/* ${var}	*/

src/cmd/ksh93/sh/name.c

@@ -2722,6 +2722,7 @@ char *nv_getval(Namval_t *np)
 	if(numeric)
 	{
 		Sflong_t  ll;
+		int base;
 		if(!up->cp)
 			return "0";
 		if(nv_isattr (np,NV_DOUBLE)==NV_DOUBLE)
@@ -2778,16 +2779,12 @@ char *nv_getval(Namval_t *np)
 		}
         	else
 			ll = *(up->lp);
-		if((numeric=nv_size(np))==10)
-		{
-			if(nv_isattr(np,NV_UNSIGN))
-			{
-				sfprintf(sh.strbuf,"%I*u",sizeof(ll),ll);
-				return sfstruse(sh.strbuf);
-			}
-			numeric = 0;
-		}
-		return fmtbase(ll,numeric, numeric&&numeric!=10);
+		base = nv_size(np);
+		if(base==10)
+			return fmtint(ll, nv_isattr(np,NV_UNSIGN));
+		/* render a possibly signed non-base-10 integer with its base# prefix */
+		sfprintf(sh.strbuf, nv_isattr(np,NV_UNSIGN) ? "%#..*I*u" : "%#..*I*d", base, sizeof ll, ll);
+		return sfstruse(sh.strbuf);
 	}
 done:
 	/*

src/cmd/ksh93/tests/arith.sh

@@ -1049,5 +1049,13 @@ if	[[ $got != '122' ]]   # TODO: should be 123
 then	err_exit "arithmetic assignment to LINENO fails (got $got)"
 fi
 
+# ======
+# non-base-10 numbers may be negative
+# https://github.com/ksh93/ksh/issues/696
+exp=-20#j12
+integer 20 got=$exp
+[[ $got == "$exp" ]] || err_exit "negative base-20 number (expected '$exp', got '$got')"
+unset got
+
 # ======
 exit $((Errors<125?Errors:125))

src/cmd/ksh93/tests/printf.sh

@@ -504,5 +504,14 @@ fi
 
 unset format gd
 
+# ======
+# negative non-base-10 numbers were mangled as they were incorrectly treated as unsigned
+# https://github.com/ksh93/ksh/issues/696
+integer 20 n=20#j12
+printf -v got '%s %s %d %..20d %s' "$n" "$((n *= -1))" n n "$n"
+exp='20#j12 -7622 -7622 -j12 -20#j12'
+[[ $got == "$exp" ]] || err_exit "issue 696 reproducer (expected $(printf %q "$exp"), got $(printf %q "$got"))"
+unset n
+
 # ======
 exit $((Errors<125?Errors:125))

src/lib/libast/Mamfile

@@ -1312,12 +1312,6 @@ make install virtual
 				done misc/fmtrec.c
 				exec - ${CC} ${mam_cc_FLAGS} ${CCFLAGS} -I. -Icomp -Iinclude -Istd -c misc/fmtrec.c
 			done fmtrec.o
-			make fmtbase.o
-				make string/fmtbase.c
-					prev include/ast.h
-				done string/fmtbase.c
-				exec - ${CC} ${mam_cc_FLAGS} ${CCFLAGS} -I. -Icomp -Iinclude -Istd -D__OBSOLETE__=20120101 -c string/fmtbase.c
-			done fmtbase.o
 			make fmtbuf.o
 				make string/fmtbuf.c
 					prev include/ast.h
@@ -4316,7 +4310,7 @@ make install virtual
 				exec - ${CC} ${mam_cc_FLAGS} ${CCFLAGS} -I. -Icomp -Iinclude -Istd -c vmalloc/vmgetmem.c
 			done vmgetmem.o
 			exec - ${AR} rc libast.a state.o opendir.o readdir.o rewinddir.o seekdir.o telldir.o getcwd.o fastfind.o hashalloc.o hashdump.o hashfree.o hashlast.o hashlook.o hashscan.o hashsize.o hashview.o hashwalk.o memhash.o memsum.o strhash.o strkey.o strsum.o stracmp.o strnacmp.o ccmap.o ccmapid.o ccnative.o chresc.o chrtoi.o
-			exec - ${AR} rc libast.a streval.o strexpr.o strmatch.o strcopy.o modei.o modex.o strmode.o strlcat.o strlcpy.o strlook.o strncopy.o strsearch.o strpsearch.o stresc.o stropt.o strtape.o strpcmp.o strnpcmp.o strvcmp.o strnvcmp.o tok.o tokline.o tokscan.o pathaccess.o pathcat.o pathcanon.o pathcheck.o pathpath.o pathexists.o pathfind.o pathicase.o pathkey.o pathprobe.o pathrepl.o pathnative.o pathposix.o pathtemp.o pathtmp.o pathstat.o pathgetlink.o pathsetlink.o pathbin.o pathshell.o pathcd.o pathprog.o ftwalk.o ftwflags.o fts.o astintercept.o conformance.o getenv.o setenviron.o optget.o optjoin.o optesc.o optctx.o strsort.o struniq.o magic.o mime.o mimetype.o signal.o sigflag.o systrace.o error.o errorf.o errormsg.o errorx.o localeconv.o setlocale.o translate.o catopen.o iconv.o lc.o lctab.o mc.o base64.o recfmt.o recstr.o reclen.o fmtrec.o fmtbase.o fmtbuf.o fmtclock.o fmtdev.o fmtelapsed.o fmterror.o fmtesc.o fmtfmt.o fmtfs.o fmtident.o fmtint.o fmtip4.o fmtip6.o fmtls.o fmtmatch.o fmtmode.o fmtnum.o fmtperm.o fmtre.o fmttime.o
+			exec - ${AR} rc libast.a streval.o strexpr.o strmatch.o strcopy.o modei.o modex.o strmode.o strlcat.o strlcpy.o strlook.o strncopy.o strsearch.o strpsearch.o stresc.o stropt.o strtape.o strpcmp.o strnpcmp.o strvcmp.o strnvcmp.o tok.o tokline.o tokscan.o pathaccess.o pathcat.o pathcanon.o pathcheck.o pathpath.o pathexists.o pathfind.o pathicase.o pathkey.o pathprobe.o pathrepl.o pathnative.o pathposix.o pathtemp.o pathtmp.o pathstat.o pathgetlink.o pathsetlink.o pathbin.o pathshell.o pathcd.o pathprog.o ftwalk.o ftwflags.o fts.o astintercept.o conformance.o getenv.o setenviron.o optget.o optjoin.o optesc.o optctx.o strsort.o struniq.o magic.o mime.o mimetype.o signal.o sigflag.o systrace.o error.o errorf.o errormsg.o errorx.o localeconv.o setlocale.o translate.o catopen.o iconv.o lc.o lctab.o mc.o base64.o recfmt.o recstr.o reclen.o fmtrec.o fmtbuf.o fmtclock.o fmtdev.o fmtelapsed.o fmterror.o fmtesc.o fmtfmt.o fmtfs.o fmtident.o fmtint.o fmtip4.o fmtip6.o fmtls.o fmtmatch.o fmtmode.o fmtnum.o fmtperm.o fmtre.o fmttime.o
 			exec - ${AR} rc libast.a fmtuid.o fmtgid.o fmtsignal.o fmtscale.o fmttmx.o fmttv.o fmtversion.o strelapsed.o strperm.o struid.o strgid.o strtoip4.o strtoip6.o stk.o swapget.o swapmem.o swapop.o swapput.o sigdata.o sigcrit.o sigunblock.o procopen.o procclose.o procrun.o procfree.o tmdate.o tmequiv.o tmfix.o tmfmt.o tmform.o tmgoff.o tminit.o tmleap.o tmlex.o tmlocale.o tmmake.o tmpoff.o tmscan.o tmsleep.o tmtime.o tmtype.o tmweek.o tmword.o tmzone.o tmxdate.o tmxduration.o tmxfmt.o tmxgettime.o tmxleap.o tmxmake.o tmxscan.o tmxsettime.o tmxsleep.o tmxtime.o tmxtouch.o tvcmp.o tvgettime.o tvsettime.o tvsleep.o tvtouch.o cmdarg.o vecargs.o vecfile.o vecfree.o vecload.o vecstring.o univdata.o touch.o mnt.o debug.o memccpy.o memchr.o memcmp.o memcpy.o memdup.o memmove.o memset.o mkdir.o mkfifo.o mknod.o rmdir.o remove.o rename.o link.o unlink.o strdup.o strtod.o strtold.o strtol.o strtoll.o strtoul.o strtoull.o strton.o strtonll.o strntod.o strntold.o strnton.o
 			exec - ${AR} rc libast.a strntonll.o strntol.o strntoll.o strntoul.o strntoull.o strcasecmp.o strncasecmp.o strerror.o mktemp.o tmpnam.o fsync.o execlp.o execve.o execvp.o execvpe.o spawnveg.o killpg.o getlogin.o putenv.o setenv.o unsetenv.o lstat.o statvfs.o eaccess.o gross.o omitted.o readlink.o symlink.o getpgrp.o setpgid.o setsid.o fcntl.o open.o arc4random.o getdents.o getwd.o dup2.o errno.o getgroups.o mount.o system.o iblocks.o modedata.o tmdata.o memfatal.o sfkeyprintf.o sfdcdio.o sfdcdos.o sfdcfilter.o sfdcseekable.o sfdcslow.o sfdcsubstr.o sfdctee.o sfdcunion.o sfdcmore.o sfdcprefix.o wc.o wc2utf8.o dirname.o fmtmsglib.o fnmatch.o ftw.o getdate.o getsubopt.o glob.o nftw.o re_comp.o resolvepath.o realpath.o regcmp.o regexp.o strftime.o strptime.o swab.o tempnam.o wordexp.o mktime.o regalloc.o regclass.o regcoll.o regcomp.o regcache.o regdecomp.o regerror.o regexec.o regfatal.o reginit.o
 			exec - ${AR} rc libast.a regnexec.o regsubcomp.o regsubexec.o regsub.o regrecord.o regrexec.o regstat.o dtclose.o dtdisc.o dthash.o dtlist.o dtmethod.o dtopen.o dtstat.o dtstrhash.o dttree.o dtuser.o dtview.o dtwalk.o dtnew.o dtcomp.o sfclose.o sfclrlock.o sfdisc.o sfdlen.o sfexcept.o sfgetl.o sfgetu.o sfcvt.o sfecvt.o sffcvt.o sfextern.o sffilbuf.o sfflsbuf.o sfprints.o sfgetd.o sfgetr.o sfllen.o sfmode.o sfmove.o sfnew.o sfpkrd.o sfnotify.o sfnputc.o sfopen.o sfpeek.o sfpoll.o sfpool.o sfpopen.o sfprintf.o sfputd.o sfputl.o sfputr.o sfputu.o sfrd.o sfread.o sfreserve.o sfscanf.o sfseek.o sfset.o sfsetbuf.o sfsetfd.o sfsize.o sfsk.o sfstack.o sfstrtod.o sfsync.o sfswap.o sftable.o sftell.o sftmp.o sfungetc.o sfvprintf.o sfvscanf.o sfwr.o sfwrite.o sfpurge.o sfraise.o sfwalk.o sfgetm.o sfputm.o sfresize.o _sfclrerr.o _sfeof.o _sferror.o _sffileno.o _sfopen.o _sfstacked.o _sfvalue.o _sfgetc.o _sfgetl.o _sfgetl2.o _sfgetu.o _sfgetu2.o _sfdlen.o _sfllen.o _sfslen.o _sfulen.o _sfputc.o _sfputd.o _sfputl.o _sfputm.o

src/lib/libast/include/ast.h

@@ -2,7 +2,7 @@
 *                                                                      *
 *               This software is part of the ast package               *
 *          Copyright (c) 1985-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                  *
 *                                                                      *
@@ -323,7 +323,6 @@ extern int		chrexp(const char*, char**, int*, int);
 extern int		chrtoi(const char*);
 extern char*		conformance(const char*, size_t);
 extern int		eaccess(const char*, int);
-extern char*		fmtbase(intmax_t, int, int);
 extern char*		fmtbuf(size_t);
 extern char*		fmtclock(Sfulong_t);
 extern char*		fmtelapsed(unsigned long, int);

src/lib/libast/man/fmt.3

@@ -44,7 +44,7 @@ fmt \- string formatting routines
 #include <ast.h>
 #include <ls.h>
 
-char*      fmtbase(long \fInumber\fP, int \fIbase\fP, int \fIprefix\fP);
+char*      fmtint(intmax_t \fInumber\fP, int \fIunsign\fP);
 char*      fmtdev(struct stat* \fIst\fP);
 char*      fmtelapsed(unsigned long \fIcount\fP, int \fIpersec\fP)
 char*      fmterror(int \fIerrno\fP);
@@ -73,19 +73,14 @@ routine that converts in the other direction.
 There is nothing spectacular about this collection other than that
 it provides a single place where the exact format is spelled out.
 .PP
-.L fmtbase
-formats a base
-.I base
-representation for
+.L fmtint
+is a performance-optimized function for formatting a base-10 integer
 .IR number .
 If
-.I "prefix != 0"
-then the base prefix is included in the formatted string.
-If
-.I "number == 0"
-or
-.I "base == 0"
-then the output is signed base 10.
+.I "unsign != 0"
+then the
+.IR number
+is treated as unsigned.
 .PP
 .L fmtdev
 returns the device handle name specified by the