[9fans] compiler double bug.

[9fans] compiler double bug.

[erik quanstrom]

after 5 hrs of tracking a floating point exception in
awk (present in all versions i could find), i isolated
this sequence of events, which does not require ape:

#include <u.h>
#include <libc.h>

void
main(void)
{
	double g = 4215866817.;

	print("%d\n", (int)g);
	print("%d\n", (int)g);
	exits("");
}

; 8c -FVTw x.c && 8l x.8 && 8.out
-2147483648
8.out 26264: suicide: sys: fp: invalid operation fppc=0x105d status=0xc0a1 pc=0x107a

- erik

Re: [9fans] compiler double bug.

[erik quanstrom]

> seemed to be correct.
> but perhaps i misunderstood

on second thought, i think i was just getting a delayed
fpe.  sorry.

do you know why the fpe is delayed?  so annoying.

- erik

Re: [9fans] compiler double bug.

[Charles Forsyth]

>the implicit conversion on return also appears botched.

both this:

uint
fun(void)
{
	double g;

	g = 4215866817.;
	USED(g);
	return g;
}

void
main(void)
{
	uint x;
	double g;

	x = fun();
	USED(x);
	g = 1.;
	exits("");
}

and this


double
fun(void)
{
	return 4215866817.;
}

void
main(void)
{
	uint x;
	double g;

	x = fun();
	USED(x);
	g = 1.;
	exits("");
}

seemed to be correct.
but perhaps i misunderstood

Re: [9fans] compiler double bug.

[erik quanstrom]

> so getival returning uint or ulong (or unsigned something-big-enough) avoids
> the problem and seems fine to me.

good.  i always get diff backwards.  sorry.
i agree on the standard quite.

> there is, however, a bug in the compiler elsewhere, in that a suitable (uint) cast in a
> direct assignment
> 	int x = (uint)doubleval;
> is (apparently) discarded by the compiler, and it shouldn't be.
> that shouldn't affect your getival cases, though.

the implicit conversion on return also appears botched.

- erik

Re: [9fans] compiler double bug.

[Charles Forsyth]

oh! getival is yours? but it's in the changes on the - lines not the + lines.
the diffs have crossed the equator!
i think i understand now.

so the original had statements such as:
		wc = (int)getfval(x);
and for excessive values in x (that couldn't be represented as an int)
there was a trap. that seems consistent with the behaviour defined below:

6.3.1.4 Real floating and integer

When a finite value of real floating type is converted to integer type other than _Bool,
the fractional part is discarded (i.e., the value is truncated toward zero). If the value of
the integral part cannot be represented by the integer type, the behavior is undefined.[footnote 41]

[41] The remaindering operation performed when a value of integer type is converted to unsigned type
   need not be performed when a value of real floating type is converted to unsigned type. Thus, the
   range of portable real floating values is (−1, Utype_MAX+1).


so getival returning uint or ulong (or unsigned something-big-enough) avoids
the problem and seems fine to me.

there is, however, a bug in the compiler elsewhere, in that a suitable (uint) cast in a
direct assignment
	int x = (uint)doubleval;
is (apparently) discarded by the compiler, and it shouldn't be.
that shouldn't affect your getival cases, though.

Re: [9fans] compiler double bug.

[erik quanstrom]

On Tue Dec  1 16:32:33 EST 2009, forsyth@terzarima.net wrote:
> those fixes really don't seem right to me. the problem is in getival
> or its callers. somewhere along the way, ULONG_MAX is converted to double
> and then back to int (directly or indirectly). that yields a trap. now, in the case
> 	int x;
> 	x = (uint)d;
> the compiler is wrong to eliminate the cast, but then again, if x isn't uint
> that's not going to work properly anyway (since x will go negative for big enough values of d).

i don't claim this is the best way to do this!

but getival does elimate the traps.  try it.  it is okay to pass ULONG_MAX
bac as a uint return value, isn't it?  i don't see the compiler doing the
wrong thing in this case.  and ULONG_MAX is an okay uint
value.  (probablly should have used UINT.)

since there are always numbers (42158668170.) that will trap
even if using uint (or even vlong), i don't see how to avoid doing
the comparison in floating point.

- erik

Re: [9fans] compiler double bug.

[Charles Forsyth]

those fixes really don't seem right to me. the problem is in getival
or its callers. somewhere along the way, ULONG_MAX is converted to double
and then back to int (directly or indirectly). that yields a trap. now, in the case
	int x;
	x = (uint)d;
the compiler is wrong to eliminate the cast, but then again, if x isn't uint
that's not going to work properly anyway (since x will go negative for big enough values of d).

what's getival? there isn't one in the awk copies i can see.

Re: [9fans] compiler double bug.

[erik quanstrom]

i applied the patch to my fresh port of the one true
awk: /n/sources/contrib/quanstro/src/awkupd

- erik

Re: [9fans] compiler double bug.

[erik quanstrom]

On Tue Dec  1 11:58:47 EST 2009, forsyth@terzarima.net wrote:
> i hadn't noticed the changed example.
> it's removing the cast, which probably isn't right, although
> in that particular case it's probably better
> that it should trap (because the result is wrong).
> what is the actual code in awk that's equivalent to that sequence?
>

all these fault:

minooka; awk 'BEGIN {
	s = "01234567890";
	printf "%x\n", 4215866817.
	g = 1.					# delayed fpe
	printf "%08x", 4215866817.
	print substr(s, 4215866817., 1)
	s = substr(s, 1, 4215866817.)
	g= 1.					# delayed fpe
	printf utf(4215866817.);
	g= 1.					# delayed fpe
}'

a (not-so-elegant) fix follows.

- erik

proto.h:92,98 - /n/dump/2009/1129/sys/src/cmd/awk/proto.h:92,97
  extern	void	funnyvar(Cell *, const char *);
  extern	char	*setsval(Cell *, const char *);
  extern	double	getfval(Cell *);
- extern	unsigned long	getival(Cell *, unsigned long, unsigned long);
  extern	char	*getsval(Cell *);
  extern	char	*getpssval(Cell *);     /* for print */
  extern	char	*tostring(const char *);
diff: can't stat /n/dump/2009/1129/sys/src/cmd/awk/trans.c
run.c:50,57 - /n/dump/2009/1129/sys/src/cmd/awk/run.c:50,55
  }
  */

- #define	Imax	ULONG_MAX
-
  /* do we really need these? */
  /* #ifdef _NFILE */
  /* #ifndef FOPEN_MAX */
run.c:355,361 - /n/dump/2009/1129/sys/src/cmd/awk/run.c:353,359
  	case EXIT:
  		if (a[0] != NULL) {
  			y = execute(a[0]);
- 			errorflag = getfval(y) != 0.;
+ 			errorflag = (int) getfval(y);
  			tempfree(y);
  		}
  		longjmp(env, 1);
run.c:752,765 - /n/dump/2009/1129/sys/src/cmd/awk/run.c:750,770
  		setsval(x, "");
  		return(x);
  	}
- 	m = getival(y, 1, k);	/* 1 <= m <= k */
+ 	m = (int) getfval(y);
+ 	if (m <= 0)
+ 		m = 1;
+ 	else if (m > k)
+ 		m = k;
  	tempfree(y);
  	if (a[2] != 0) {
- 		n = getival(z, 0, k-m);
- 				/* n <= 0 <= k-m */
+ 		n = (int) getfval(z);
  		tempfree(z);
  	} else
  		n = k - 1;
+ 	if (n < 0)
+ 		n = 0;
+ 	else if (n > k - m)
+ 		n = k - m;
  	   dprintf( ("substr: m=%d, n=%d, s=%s\n", m, n, s) );
  	y = gettemp();
  	temp = s[n+m-1];	/* with thanks to John Linderman */
run.c:804,810 - /n/dump/2009/1129/sys/src/cmd/awk/run.c:809,815
  	char *p, *t;
  	const char *os;
  	Cell *x;
- 	int flag = 0, n, ch;
+ 	int flag = 0, n;
  	int fmtwd; /* format width */
  	int fmtsz = recsize;
  	char *buf = *pbuf;
run.c:838,844 - /n/dump/2009/1129/sys/src/cmd/awk/run.c:843,849
  			if (*s == '*') {
  				x = execute(a);
  				a = a->nnext;
- 				sprintf(t-1, "%d", fmtwd=getival(x, Imax, Imax));
+ 				sprintf(t-1, "%d", fmtwd=(int) getfval(x));
  				if (fmtwd < 0)
  					fmtwd = -fmtwd;
  				adjbuf(&buf, &bufsize, fmtwd+1+p-buf, recsize, &p, "format");
run.c:894,901 - /n/dump/2009/1129/sys/src/cmd/awk/run.c:900,907
  			sprintf(p, "%s", t);
  			break;
  		case 'f':	sprintf(p, fmt, getfval(x)); break;
- 		case 'd':	sprintf(p, fmt, getival(x, Imax, Imax)); break;
- 		case 'u':	sprintf(p, fmt, getival(x, Imax, Imax)); break;
+ 		case 'd':	sprintf(p, fmt, (long) getfval(x)); break;
+ 		case 'u':	sprintf(p, fmt, (int) getfval(x)); break;
  		case 's':
  			t = getsval(x);
  			n = strlen(t);
run.c:907,915 - /n/dump/2009/1129/sys/src/cmd/awk/run.c:913,920
  			break;
  		case 'c':
  			if (isnum(x)) {
- 				ch = getival(x, Imax, Imax);
- 				if (ch)
- 					sprintf(p, fmt, ch);
+ 				if (getfval(x))
+ 					sprintf(p, fmt, (int) getfval(x));
  				else {
  					*p++ = '\0'; /* explicit null byte */
  					*p = '\0';   /* next output will start here */
  Awkfloat
  strtonum(Cell *x)
  {
run.c:1566,1579 - /n/dump/2009/1129/sys/src/cmd/awk/run.c:1583,1589
  			u = fflush(fp);
  		break;
  	case FUTF:
- 		r = getival(x, Imax, Imax);
- 		if(r < 0)
- 			r = -r;
- 		if(r > Runemax){
- 			WARNING("utf argument out-of-range %g", getfval(x));
- 			r = Runeerror;
- 		}
- 		wc = r;
+ 		wc = (int)getfval(x);
  		mbc[wctomb(mbc, wc)] = 0;
  		tempfree(x);
  		x = gettemp();

Re: [9fans] compiler double bug.

[Charles Forsyth]

i hadn't noticed the changed example.
it's removing the cast, which probably isn't right, although
in that particular case it's probably better
that it should trap (because the result is wrong).
what is the actual code in awk that's equivalent to that sequence?

Re: [9fans] compiler double bug.

[erik quanstrom]

alternatively, a sleep will kill the proc
since restoring the floating point will
trigger the latent trap.

- erik

Re: [9fans] compiler double bug.

[erik quanstrom]

#include <u.h>
#include <libc.h>

void
main(void)
{
	int x;
	double g;

	g = 4215866817.;
	x = (uint)g;
	USED(x);
	g = 1.;
	exits("");
}

Re: [9fans] compiler double bug.

[Charles Forsyth]

#include <u.h>
#include <libc.h>

void
main(void)
{
	double g = 4215866817.;

	print("%d\n", (unsigned int)g);
	print("%d\n", (unsigned int)g);
	exits("");
}
8c r.c
8l r.8
./8.out
-79100479
-79100479

no trap. it generates different code.
(obviously the %d should be %ud in this case, giving

doppio% 8.out
4215866817
4215866817

which looks like the right answer

the cast must be to an unsigned integer.
otherwise the resulting range is signed,
and that's more limited

Re: [9fans] compiler double bug.

[erik quanstrom]

On Tue Dec  1 04:40:09 EST 2009, forsyth@terzarima.net wrote:
> 	g = 4215866817.;
> 	x = (int)g;
>

actually the target variable must be a uint.  a cast still
traps.  is the compiler wrong on this?

- erik

Re: [9fans] compiler double bug.

[Charles Forsyth]

the fppc points to the offending instruction.

Re: [9fans] compiler double bug.

[Charles Forsyth]

	g = 4215866817.;
	x = (int)g;

does g fit in (int)? (no, (int) is 32 bit signed.)
if (int) is (uint) it will fit, and not trap.

Re: [9fans] compiler double bug.

[erik quanstrom]

even more bare-bones version:

#include <u.h>
#include <libc.h>

void
main(void)
{
	int x;
	double g;

	g = 4215866817.;
	x = (int)g;
	g=1.;
	USED(x);
	exits("");
}