[PATCH 21.5] Division by zero & continuable errors

Jerry James james at xemacs.org
Wed Oct 7 14:24:04 EDT 2009


PATCH 21.5

This patch addresses some possible division by zero errors flagged by
the clang analyzer.  The problem is that Fsignal signals a continuable
error, so a user running in the Lisp debugger may catch the error and
specify a replacement value for the zero, which is then returned to
the caller.  We aren't handling the return value from Fsignal.

My understanding is that instrumenting a function for debugging makes
it interpreted.  If that is incorrect, someone please whack me with a
clue stick.  It seems to me that the right thing to do is to make
division by zero errors noncontinuable when processing bytecode.

In the case of interpreted code, I couldn't leave the division by zero
checks where they were very easily.  If the debugging user specified a
nonzero replacement value of a different numeric type, then we would
be in the wrong function to do the division.  The only way I could see
to solve this problem is to move the division by zero checks up a
level, before we start differentiating by numeric type.

Finally, I changed division by zero errors from arithmetic errors to
domain errors, since the problem is that we've specified a value not
in the domain of the division operator.  Feel free to argue that point
with me.

diff -r a46d7513a364 src/ChangeLog
--- a/src/ChangeLog	Wed Oct 07 19:42:42 2009 +0200
+++ b/src/ChangeLog	Wed Oct 07 12:21:54 2009 -0600
@@ -1,3 +1,42 @@
+2009-10-07  Jerry James  <james at xemacs.org>
+
+	* bytecode.c (bytecode_arithop): Make division-by-zero errors be
+	domain errors instead of arithmetic errors and make them
+	noncontinuable.
+	* floatfns.c (ceiling_two_fixnum): Move division-by-zero check to
+	Fceiling and Ffceiling where we don't have to worry about number
+	types.
+	(ceiling_two_bignum): Ditto.
+	(ceiling_two_ratio): Ditto.
+	(ceiling_two_bigfloat): Ditto.
+	(ceiling_two_float): Ditto.
+	(floor_two_fixnum): Ditto, but to Ffloor and Fffloor.
+	(floor_two_bignum): Ditto.
+	(floor_two_ratio): Ditto.
+	(floor_two_bigfloat): Ditto.
+	(floor_two_float): Ditto.
+	(round_two_fixnum): Ditto, but to Fround and Ffround.
+	(round_two_bignum): Ditto.
+	(round_two_ratio): Ditto.
+	(round_two_bigfloat): Ditto.
+	(round_two_float): Ditto.
+	(truncate_two_fixnum): Ditto, to Ftruncate and Fftruncate.
+	(truncate_two_bignum): Ditto.
+	(truncate_two_ratio): Ditto.
+	(truncate_two_bigfloat): Ditto.
+	(truncate_one_ratio): Ditto, and this was wrong anyway.
+	(truncate_two_float): Ditto.
+	(Fceiling): Make division by zero a domain error, and handle a
+	value returned from a continuable error.
+	(Ffloor): Ditto.
+	(Fround): Ditto.
+	(Ftruncate): Ditto.
+	(Ffceiling): Ditto.
+	(Fffloor): Ditto.
+	(Ffround): Ditto.
+	(Fftruncate): Ditto.
+	* lisp.h (Fzerop): Expose this so it can be called from floatfns.c.
+
 2009-10-05  Jerry James  <james at xemacs.org>

 	* emacs.c (main_1): Check the return value of dup() to quiet gcc.
diff -r a46d7513a364 src/bytecode.c
--- a/src/bytecode.c	Wed Oct 07 19:42:42 2009 +0200
+++ b/src/bytecode.c	Wed Oct 07 12:21:54 2009 -0600
@@ -432,7 +432,8 @@
 	    ival1 *= ival2; break;
 #endif
 	  case Bquo:
-	    if (ival2 == 0) Fsignal (Qarith_error, Qnil);
+	    if (ival2 == 0)
+	      signal_error_2 (Qdomain_error, "division by zero", obj1, obj2);
 	    ival1 /= ival2;
 	    break;
 	  case Bmax:  if (ival1 < ival2) ival1 = ival2; break;
@@ -458,7 +459,7 @@
 	  break;
 	case Bquo:
 	  if (bignum_sign (XBIGNUM_DATA (obj2)) == 0)
-	    Fsignal (Qarith_error, Qnil);
+	    signal_error_2 (Qdomain_error, "division by zero", obj1, obj2);
 	  bignum_div (scratch_bignum, XBIGNUM_DATA (obj1),
 		      XBIGNUM_DATA (obj2));
 	  break;
@@ -486,7 +487,7 @@
 	  break;
 	case Bquo:
 	  if (ratio_sign (XRATIO_DATA (obj2)) == 0)
-	    Fsignal (Qarith_error, Qnil);
+	    signal_error_2 (Qdomain_error, "division by zero", obj1, obj2);
 	  ratio_div (scratch_ratio, XRATIO_DATA (obj1), XRATIO_DATA (obj2));
 	  break;
 	case Bmax:
@@ -518,7 +519,7 @@
 	  break;
 	case Bquo:
 	  if (bigfloat_sign (XBIGFLOAT_DATA (obj2)) == 0)
-	    Fsignal (Qarith_error, Qnil);
+	    signal_error_2 (Qdomain_error, "division by zero", obj1, obj2);
 	  bigfloat_div (scratch_bigfloat, XBIGFLOAT_DATA (obj1),
 			XBIGFLOAT_DATA (obj2));
 	  break;
@@ -540,7 +541,8 @@
 	  case Bdiff: dval1 -= dval2; break;
 	  case Bmult: dval1 *= dval2; break;
 	  case Bquo:
-	    if (dval2 == 0.0) Fsignal (Qarith_error, Qnil);
+	    if (dval2 == 0.0)
+	      signal_error_2 (Qdomain_error, "division by zero", obj1, obj2);
 	    dval1 /= dval2;
 	    break;
 	  case Bmax:  if (dval1 < dval2) dval1 = dval2; break;
@@ -585,7 +587,8 @@
 	case Bdiff: ival1 -= ival2; break;
 	case Bmult: ival1 *= ival2; break;
 	case Bquo:
-	  if (ival2 == 0) Fsignal (Qarith_error, Qnil);
+	  if (ival2 == 0)
+	    signal_error_2 (Qdomain_error, "division by zero", obj1, obj2);
 	  ival1 /= ival2;
 	  break;
 	case Bmax:  if (ival1 < ival2) ival1 = ival2; break;
@@ -603,7 +606,8 @@
 	case Bdiff: dval1 -= dval2; break;
 	case Bmult: dval1 *= dval2; break;
 	case Bquo:
-	  if (dval2 == 0) Fsignal (Qarith_error, Qnil);
+	  if (dval2 == 0)
+	    signal_error_2 (Qdomain_error, "division by zero", obj1, obj2);
 	  dval1 /= dval2;
 	  break;
 	case Bmax:  if (dval1 < dval2) dval1 = dval2; break;
diff -r a46d7513a364 src/floatfns.c
--- a/src/floatfns.c	Wed Oct 07 19:42:42 2009 +0200
+++ b/src/floatfns.c	Wed Oct 07 12:21:54 2009 -0600
@@ -1014,9 +1014,6 @@
   EMACS_INT i2 = XREALINT (divisor);
   EMACS_INT i3 = 0, i4 = 0;

-  if (i2 == 0)
-    Fsignal (Qarith_error, Qnil);
-
   /* With C89's integer /, the result is implementation-defined if either
      operand is negative, so use only nonnegative operands. Here we do
      basically the opposite of what floor_two_fixnum does, we add one in the
@@ -1079,11 +1076,6 @@
 {
   Lisp_Object res0, res1;

-  if (bignum_sign (XBIGNUM_DATA (divisor)) == 0)
-    {
-      Fsignal (Qarith_error, Qnil);
-    }
-
   bignum_ceil (scratch_bignum, XBIGNUM_DATA (number), XBIGNUM_DATA (divisor));

   res0 = return_float ? make_float (bignum_to_double (scratch_bignum)) :
@@ -1111,11 +1103,6 @@
 {
   Lisp_Object res0, res1;

-  if (ratio_sign (XRATIO_DATA (divisor)) == 0)
-    {
-      Fsignal (Qarith_error, Qnil);
-    }
-
   ratio_div (scratch_ratio, XRATIO_DATA (number), XRATIO_DATA (divisor));

   bignum_ceil (scratch_bignum, ratio_numerator (scratch_ratio),
@@ -1148,11 +1135,6 @@
 {
   Lisp_Object res0;

-  if (bigfloat_sign (XBIGFLOAT_DATA (divisor)) == 0)
-    {
-      Fsignal (Qarith_error, Qnil);
-    }
-
   bigfloat_set_prec (scratch_bigfloat, max (XBIGFLOAT_GET_PREC (number),
 					    XBIGFLOAT_GET_PREC (divisor)));
   bigfloat_div (scratch_bigfloat, XBIGFLOAT_DATA (number),
@@ -1248,12 +1230,7 @@
   double f2 = extract_float (divisor);
   double f0, remain;
   Lisp_Object res0;
-	
-  if (f2 == 0.0)
-    {
-      Fsignal (Qarith_error, Qnil);
-    }
-	
+
   IN_FLOAT2 (f0 = ceil (f1 / f2), MAYBE_EFF("ceiling"), number, divisor);
   IN_FLOAT2 (remain = f1 - (f0 * f2), MAYBE_EFF("ceiling"), number, divisor);

@@ -1338,11 +1315,6 @@
   EMACS_INT i3 = 0, i4 = 0;
   Lisp_Object res0;

-  if (i2 == 0)
-    {
-      Fsignal (Qarith_error, Qnil);
-    }
-
   /* With C89's integer /, the result is implementation-defined if either
      operand is negative, so use only nonnegative operands. Notice also that
      we're forcing the quotient of any negative numbers towards minus
@@ -1372,11 +1344,6 @@
 {
   Lisp_Object res0, res1;

-  if (bignum_sign (XBIGNUM_DATA (divisor)) == 0)
-    {
-      Fsignal (Qarith_error, Qnil);
-    }
-
   bignum_floor (scratch_bignum, XBIGNUM_DATA (number),
 		XBIGNUM_DATA (divisor));

@@ -1411,11 +1378,6 @@
 {
   Lisp_Object res0, res1;

-  if (ratio_sign (XRATIO_DATA (divisor)) == 0)
-    {
-      Fsignal (Qarith_error, Qnil);
-    }
-
   ratio_div (scratch_ratio, XRATIO_DATA (number), XRATIO_DATA (divisor));

   bignum_floor (scratch_bignum, ratio_numerator (scratch_ratio),
@@ -1448,11 +1410,6 @@
 {
   Lisp_Object res0;

-  if (bigfloat_sign (XBIGFLOAT_DATA (divisor)) == 0)
-    {
-      Fsignal (Qarith_error, Qnil);
-    }
-
   bigfloat_set_prec (scratch_bigfloat, max (XBIGFLOAT_GET_PREC (number),
 					    XBIGFLOAT_GET_PREC (divisor)));
   bigfloat_div (scratch_bigfloat, XBIGFLOAT_DATA (number),
@@ -1546,12 +1503,7 @@
   double f1 = extract_float (number);
   double f2 = extract_float (divisor);
   double f0, remain;
-	
-  if (f2 == 0.0)
-    {
-      Fsignal (Qarith_error, Qnil);
-    }
-	
+
   IN_FLOAT2 (f0 = floor (f1 / f2), MAYBE_EFF ("floor"), number, divisor);
   IN_FLOAT2 (remain = f1 - (f0 * f2), MAYBE_EFF ("floor"), number, divisor);

@@ -1621,18 +1573,12 @@
 /* Algorithm taken from cl-extra.el, now to be found as cl-round in
    tests/automated/lisp-tests.el.  */
 static Lisp_Object
-round_two_fixnum (Lisp_Object number, Lisp_Object divisor,
-		  int return_float)
+round_two_fixnum (Lisp_Object number, Lisp_Object divisor, int return_float)
 {
   EMACS_INT i1 = XREALINT (number);
   EMACS_INT i2 = XREALINT (divisor);
   EMACS_INT i0, hi2, flooring, floored, flsecond;

-  if (i2 == 0)
-    {
-      Fsignal (Qarith_error, Qnil);
-    }
-
   hi2 = i2 < 0 ? -( -i2 / 2) : i2 / 2;

   flooring = hi2 + i1;
@@ -1716,16 +1662,10 @@
 }

 static Lisp_Object
-round_two_bignum (Lisp_Object number, Lisp_Object divisor,
-		  int return_float)
+round_two_bignum (Lisp_Object number, Lisp_Object divisor, int return_float)
 {
   Lisp_Object res0, res1;

-  if (bignum_sign (XBIGNUM_DATA (divisor)) == 0)
-    {
-      Fsignal (Qarith_error, Qnil);
-    }
-
   round_two_bignum_1 (XBIGNUM_DATA (number), XBIGNUM_DATA (divisor),
 		      &res0, &res1);

@@ -1744,18 +1684,12 @@

 #ifdef HAVE_RATIO
 static Lisp_Object
-round_two_ratio (Lisp_Object number, Lisp_Object divisor,
-		 int return_float)
+round_two_ratio (Lisp_Object number, Lisp_Object divisor, int return_float)
 {
   Lisp_Object res0, res1;

-  if (ratio_sign (XRATIO_DATA (divisor)) == 0)
-    {
-      Fsignal (Qarith_error, Qnil);
-    }
+  ratio_div (scratch_ratio, XRATIO_DATA (number), XRATIO_DATA (divisor));

-  ratio_div (scratch_ratio, XRATIO_DATA (number), XRATIO_DATA (divisor));
-
   round_two_bignum_1 (ratio_numerator (scratch_ratio),
 		      ratio_denominator (scratch_ratio), &res0, &res1);

@@ -1766,7 +1700,7 @@
       ratio_set_bignum (scratch_ratio2, XBIGNUM_DATA (res0));
       ratio_mul (scratch_ratio, scratch_ratio2, XRATIO_DATA (divisor));
       ratio_sub (scratch_ratio, XRATIO_DATA (number), scratch_ratio);
-
+
       res1 = Fcanonicalize_number (make_ratio_rt (scratch_ratio));
     }

@@ -1852,11 +1786,6 @@
   unsigned long prec = max (XBIGFLOAT_GET_PREC (number),
 			    XBIGFLOAT_GET_PREC (divisor));

-  if (bigfloat_sign (XBIGFLOAT_DATA (divisor)) == 0)
-    {
-      Fsignal (Qarith_error, Qnil);
-    }
-
   bigfloat_init (divided);
   bigfloat_set_prec (divided, prec);

@@ -1866,7 +1795,7 @@

   bigfloat_set_prec (scratch_bigfloat, prec);
   bigfloat_set_prec (scratch_bigfloat2, prec);
-
+
   bigfloat_mul (scratch_bigfloat, XBIGFLOAT_DATA (res0),
 		XBIGFLOAT_DATA (divisor));
   bigfloat_sub (scratch_bigfloat2, XBIGFLOAT_DATA (number),
@@ -1948,9 +1877,6 @@
   double f1 = extract_float (number);
   double f2 = extract_float (divisor);
   double f0, remain;
-	
-  if (f2 == 0.0)
-    Fsignal (Qarith_error, Qnil);

   IN_FLOAT2 ((f0 = emacs_rint (f1 / f2)), MAYBE_EFF ("round"), number,
 	     divisor);
@@ -2030,9 +1956,6 @@
   EMACS_INT i2 = XREALINT (divisor);
   EMACS_INT i0;

-  if (i2 == 0)
-    Fsignal (Qarith_error, Qnil);
-
   /* We're truncating towards zero, so apart from avoiding the C89
      implementation-defined behaviour with truncation and negative numbers,
      we don't need to do anything further: */
@@ -2057,11 +1980,6 @@
 {
   Lisp_Object res0;

-  if (bignum_sign (XBIGNUM_DATA (divisor)) == 0)
-    {
-      Fsignal (Qarith_error, Qnil);
-    }
-
   bignum_div (scratch_bignum, XBIGNUM_DATA (number),
 	      XBIGNUM_DATA (divisor));

@@ -2095,11 +2013,6 @@
 {
   Lisp_Object res0;

-  if (ratio_sign (XRATIO_DATA (divisor)) == 0)
-    {
-      Fsignal (Qarith_error, Qnil);
-    }
-
   ratio_div (scratch_ratio, XRATIO_DATA (number), XRATIO_DATA (divisor));

   bignum_div (scratch_bignum, ratio_numerator (scratch_ratio),
@@ -2137,11 +2050,6 @@
   unsigned long prec = max (XBIGFLOAT_GET_PREC (number),
 			    XBIGFLOAT_GET_PREC (divisor));

-  if (bigfloat_sign (XBIGFLOAT_DATA (divisor)) == 0)
-    {
-      Fsignal (Qarith_error, Qnil);
-    }
-
   bigfloat_set_prec (scratch_bigfloat, prec);
   bigfloat_set_prec (scratch_bigfloat2, prec);

@@ -2162,7 +2070,7 @@
       res0 = make_int ((EMACS_INT) bigfloat_to_long (scratch_bigfloat));
 #endif /* HAVE_BIGNUM */
     }
-
+
   bigfloat_mul (scratch_bigfloat2, scratch_bigfloat, XBIGFLOAT_DATA (divisor));
   bigfloat_sub (scratch_bigfloat, XBIGFLOAT_DATA (number), scratch_bigfloat2);

@@ -2177,11 +2085,6 @@
 {
   Lisp_Object res0;

-  if (ratio_sign (XRATIO_DATA (number)) == 0)
-    {
-      Fsignal (Qarith_error, Qnil);
-    }
-
   bignum_div (scratch_bignum, XRATIO_NUMERATOR (number),
 	      XRATIO_DENOMINATOR (number));
   if (return_float)
@@ -2247,11 +2150,6 @@
   double f2 = extract_float (divisor);
   double f0, remain;
   Lisp_Object res0;
-	
-  if (f2 == 0.0)
-    {
-      Fsignal (Qarith_error, Qnil);
-    }

   res0 = float_to_int (f1 / f2, MAYBE_EFF ("truncate"), number, Qunbound);
   f0 = extract_float (res0);
@@ -2325,7 +2223,7 @@
 Return the smallest integer no less than NUMBER.  (Round toward +inf.)

 With optional argument DIVISOR, return the smallest integer no less than
-the quotient of NUMBER and DIVISOR.
+the quotient of NUMBER and DIVISOR.

 This function returns multiple values; see `multiple-value-bind' and
 `multiple-value-call'.  The second returned value is the remainder in the
@@ -2334,6 +2232,8 @@
 */
        (number, divisor))
 {
+  while (!NILP (divisor) && !NILP (Fzerop (divisor)))
+    divisor = domain_error2 ("ceiling", number, divisor);
   ROUNDING_CONVERT(ceiling, 0);
 }

@@ -2349,6 +2249,8 @@
 */
        (number, divisor))
 {
+  while (!NILP (divisor) && !NILP (Fzerop (divisor)))
+    divisor = domain_error2 ("floor", number, divisor);
   ROUNDING_CONVERT(floor, 0);
 }

@@ -2366,6 +2268,8 @@
 */
        (number, divisor))
 {
+  while (!NILP (divisor) && !NILP (Fzerop (divisor)))
+    divisor = domain_error2 ("round", number, divisor);
   ROUNDING_CONVERT(round, 0);
 }

@@ -2380,6 +2284,8 @@
 */
        (number, divisor))
 {
+  while (!NILP (divisor) && !NILP (Fzerop (divisor)))
+    divisor = domain_error2 ("truncate", number, divisor);
   ROUNDING_CONVERT(truncate, 0);
 }
 
@@ -2397,6 +2303,8 @@
 */
        (number, divisor))
 {
+  while (!NILP (divisor) && !NILP (Fzerop (divisor)))
+    divisor = domain_error2 ("ceiling", number, divisor);
   ROUNDING_CONVERT(ceiling, 1);
 }

@@ -2412,6 +2320,8 @@
 */
        (number, divisor))
 {
+  while (!NILP (divisor) && !NILP (Fzerop (divisor)))
+    divisor = domain_error2 ("floor", number, divisor);
   ROUNDING_CONVERT(floor, 1);
 }

@@ -2428,6 +2338,8 @@
 */
        (number, divisor))
 {
+  while (!NILP (divisor) && !NILP (Fzerop (divisor)))
+    divisor = domain_error2 ("round", number, divisor);
   ROUNDING_CONVERT(round, 1);
 }

@@ -2443,6 +2355,8 @@
 */
        (number, divisor))
 {
+  while (!NILP (divisor) && !NILP (Fzerop (divisor)))
+    divisor = domain_error2 ("truncate", number, divisor);
   ROUNDING_CONVERT(truncate, 1);
 }
 
diff -r a46d7513a364 src/lisp.h
--- a/src/lisp.h	Wed Oct 07 19:42:42 2009 +0200
+++ b/src/lisp.h	Wed Oct 07 12:21:54 2009 -0600
@@ -4115,6 +4115,7 @@
 EXFUN (Fsubr_max_args, 1);
 EXFUN (Fsubr_min_args, 1);
 EXFUN (Ftimes, MANY);
+EXFUN (Fzerop, 1);

 DECLARE_DOESNT_RETURN (c_write_error (Lisp_Object));
 DECLARE_DOESNT_RETURN (lisp_write_error (Lisp_Object));

-- 
Jerry James
http://www.jamezone.org/




More information about the XEmacs-Patches mailing list