PATCH: prevent SIGFPE on systems where LONG_MIN < -LONG_MAX

PATCH: prevent SIGFPE on systems where LONG_MIN < -LONG_MAX

[Jérémie Roquet]

[-- Attachment #1: Type: text/plain, Size: 217 bytes --]

Hello,

$ echo $[ - 2**63 / -1 ]
zsh: floating point exception  ./Src/zsh

$ echo $[ - 2**63 % -1 ]
zsh: floating point exception  ./Src/zsh

The attached patch fixes this.

Best regards,

-- 
Jérémie

[-- Attachment #2: zsh_fix_sigfpe.patch --]
[-- Type: application/octet-stream, Size: 1072 bytes --]

diff --git a/Src/math.c b/Src/math.c
index cca5210..5e0a509 100644
--- a/Src/math.c
+++ b/Src/math.c
@@ -32,6 +32,7 @@ struct mathvalue;
 #include "zsh.mdh"
 #include "math.pro"
 
+#include <limits.h>
 #include <math.h>
 
 /* nonzero means we are not evaluating, just parsing */
@@ -962,6 +963,19 @@ notzero(mnumber a)
     return 1;
 }
 
+/**/
+static int
+representable(mnumber a, mnumber b)
+{
+#if LONG_MIN < -LONG_MAX
+    if (a.u.l - 1 > a.u.l && b.u.l == -1) {
+	zerr("non representable result");
+	return 0;
+    }
+#endif
+    return 1;
+}
+
 /* macro to pop three values off the value stack */
 
 /**/
@@ -1053,13 +1067,18 @@ op(int what)
 		    return;
 		if (c.type == MN_FLOAT)
 		    c.u.d = a.u.d / b.u.d;
-		else
+		else {
+                    if (!representable(a, b))
+                        return;
 		    c.u.l = a.u.l / b.u.l;
+                }
 		break;
 	    case MOD:
 	    case MODEQ:
 		if (!notzero(b))
 		    return;
+                if (!representable(a, b))
+                    return;
 		c.u.l = a.u.l % b.u.l;
 		break;
 	    case PLUS:

Re: PATCH: prevent SIGFPE on systems where LONG_MIN < -LONG_MAX

[Mikael Magnusson]

See also this thread,
http://www.zsh.org/mla/workers/2011/msg00613.html

On 03/09/2012, Jérémie Roquet <arkanosis@gmail.com> wrote:
> Hello,
>
> $ echo $[ - 2**63 / -1 ]
> zsh: floating point exception  ./Src/zsh
>
> $ echo $[ - 2**63 % -1 ]
> zsh: floating point exception  ./Src/zsh
>
> The attached patch fixes this.

Re: PATCH: prevent SIGFPE on systems where LONG_MIN < -LONG_MAX

[Peter Stephenson]

On Mon, 3 Sep 2012 16:40:37 +0200
Mikael Magnusson <mikachu@gmail.com> wrote:
> See also this thread,
> http://www.zsh.org/mla/workers/2011/msg00613.html
> 
> On 03/09/2012, Jérémie Roquet <arkanosis@gmail.com> wrote:
> > Hello,
> >
> > $ echo $[ - 2**63 / -1 ]
> > zsh: floating point exception  ./Src/zsh
> >
> > $ echo $[ - 2**63 % -1 ]
> > zsh: floating point exception  ./Src/zsh
> >
> > The attached patch fixes this.
> 
> 
>  To report this email as spam click https://www.mailcontrol.com/sr/wQw0zmjPoHdJTZGyOCrrhg== .

So what's the answer, then?  Jérémie's patch makes only limited
assumptions; I think the #if is there simply to see if we can assume
two's complement arithmetic (it seems the preprocessor isn't keen on
"#if LONG_MIN - 1 == LONG_MAX", I suppose it's not required to use
"long" precision) in which case the test should work.  It might not
cover all real cases, but that's a lesser evil.

-- 
Peter Stephenson <pws@csr.com>            Software Engineer
Tel: +44 (0)1223 692070                   Cambridge Silicon Radio Limited
Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, UK


Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Follow CSR on Twitter at http://twitter.com/CSR_PLC and read our blog at www.csr.com/blog

Re: PATCH: prevent SIGFPE on systems where LONG_MIN < -LONG_MAX

[Jérémie Roquet]

2012/9/3 Mikael Magnusson <mikachu@gmail.com>:
> See also this thread,
> http://www.zsh.org/mla/workers/2011/msg00613.html

Thanks.

I think we want a warning, as we have for division by zero. The shell
should not crash, neither should it return incorrect results.

The patch should work whatever the underlying type behind zlong is.

2012/9/3 Peter Stephenson <Peter.Stephenson@csr.com>:
> So what's the answer, then?  Jérémie's patch makes only limited
> assumptions; I think the #if is there simply to see if we can assume
> two's complement arithmetic (it seems the preprocessor isn't keen on
> "#if LONG_MIN - 1 == LONG_MAX", I suppose it's not required to use
> "long" precision) in which case the test should work.  It might not
> cover all real cases, but that's a lesser evil.

Testing LONG_MIN < -LONG_MAX tells if the opposite of the minimal long
value is a representable value. It makes the assumption that the
answer is valid for ZSH_64_BIT_TYPE too.
On systems where LONG_MIN == -LONG_MAX, the shell should not raise a warning.

Best regards,

-- 
Jérémie

Re: PATCH: prevent SIGFPE on systems where LONG_MIN < -LONG_MAX

[Vincent Lefevre]

On 2012-09-03 17:08:18 +0200, Jérémie Roquet wrote:
> I think we want a warning, as we have for division by zero.

I don't think so. The division by zero is mathematically undefined,
so that a warning is fine. But a division by -1 could be replaced by
a negate operation; then you have the usual modular arithmetic rules.
They already occur for

$ echo $[ 2**63 ]
-9223372036854775808

anyway.

> The shell should not crash, neither should it return incorrect
> results.

Agreed.

-- 
Vincent Lefèvre <vincent@vinc17.net> - Web: <http://www.vinc17.net/>
100% accessible validated (X)HTML - Blog: <http://www.vinc17.net/blog/>
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)

Re: PATCH: prevent SIGFPE on systems where LONG_MIN < -LONG_MAX

[Peter Stephenson]

On Mon, 3 Sep 2012 18:26:02 +0200
Vincent Lefevre <vincent@vinc17.net> wrote:
> On 2012-09-03 17:08:18 +0200, Jérémie Roquet wrote:
> > I think we want a warning, as we have for division by zero.
> 
> I don't think so. The division by zero is mathematically undefined,
> so that a warning is fine. But a division by -1 could be replaced by
> a negate operation; then you have the usual modular arithmetic rules.
> They already occur for
> 
> $ echo $[ 2**63 ]
> -9223372036854775808
> 
> anyway.

Right, so we could just do this as a special case as below that gives
answers that are plausible as far as fixed precision integer arithmetic
goes even if they're not mathematically accurate, which they can't be
for the division (they can for the mod).  The result should now be
sane everywhere.
 
> > The shell should not crash, neither should it return incorrect
> > results.
> 
> Agreed.

Well, that depends what you mean by an incorrect result.  Mathematically
incorrect results are inevitable if you don't account of the precision,
but we don't warn for any other rounding error so I agree there's no
reason to here.

Index: Src/math.c
===================================================================
RCS file: /cvsroot/zsh/zsh/Src/math.c,v
retrieving revision 1.41
diff -p -u -r1.41 math.c
--- Src/math.c	19 Jun 2011 16:26:11 -0000	1.41
+++ Src/math.c	4 Sep 2012 18:23:36 -0000
@@ -1053,14 +1053,34 @@ op(int what)
 		    return;
 		if (c.type == MN_FLOAT)
 		    c.u.d = a.u.d / b.u.d;
-		else
-		    c.u.l = a.u.l / b.u.l;
+		else {
+		    /*
+		     * Avoid exception when dividing the smallest
+		     * negative integer by -1.  Always treat it the
+		     * same as multiplication.  This still doesn't give
+		     * numerically the right answer in two's complement,
+		     * but treating both these in the same way seems
+		     * reasonable.
+		     */
+		    if (b.u.l == -1)
+			c.u.l = - a.u.l;
+		    else
+			c.u.l = a.u.l / b.u.l;
+		}
 		break;
 	    case MOD:
 	    case MODEQ:
 		if (!notzero(b))
 		    return;
-		c.u.l = a.u.l % b.u.l;
+		/*
+		 * Avoid exception as above.
+		 * Any integer mod -1 is the same as any integer mod 1
+		 * i.e. zero.
+		 */
+		if (b.u.l == -1)
+		    c.u.l = 0;
+		else
+		    c.u.l = a.u.l % b.u.l;
 		break;
 	    case PLUS:
 	    case PLUSEQ:

-- 
Peter Stephenson <p.w.stephenson@ntlworld.com>
Web page now at http://homepage.ntlworld.com/p.w.stephenson/

Re: PATCH: prevent SIGFPE on systems where LONG_MIN < -LONG_MAX

[Vincent Lefevre]

On 2012-09-04 19:26:44 +0100, Peter Stephenson wrote:
> > > The shell should not crash, neither should it return incorrect
> > > results.
> > 
> > Agreed.
> 
> Well, that depends what you mean by an incorrect result.  Mathematically
> incorrect results are inevitable if you don't account of the precision,
> but we don't warn for any other rounding error so I agree there's no
> reason to here.

Here these are integer operations, and it seems that zsh chose
modular arithmetic, so that a "correct" representable result
is always possible under these conditions ("correct" but not
necessarily meaningful, but no-one can prevent the user from
writing meaningless expressions).

-- 
Vincent Lefèvre <vincent@vinc17.net> - Web: <http://www.vinc17.net/>
100% accessible validated (X)HTML - Blog: <http://www.vinc17.net/blog/>
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)