Revision: 3433
Author: [email protected]
Date: Tue Dec 8 02:18:28 2009
Log: Fix subtle bug in Math.min and Math.max with non-Smi zero.
See http://codereview.chromium.org/470001
From sra.
http://code.google.com/p/v8/source/detail?r=3433
Modified:
/branches/bleeding_edge/src/math.js
/branches/bleeding_edge/test/mjsunit/math-min-max.js
=======================================
--- /branches/bleeding_edge/src/math.js Mon Dec 7 00:38:20 2009
+++ /branches/bleeding_edge/src/math.js Tue Dec 8 02:18:28 2009
@@ -128,8 +128,9 @@
var n = %_Arguments(i);
if (!IS_NUMBER(n)) n = ToNumber(n);
if (NUMBER_IS_NAN(n)) return n;
- // Make sure +0 is considered greater than -0.
- if (n > r || (r === 0 && n === 0 && !%_IsSmi(r))) r = n;
+ // Make sure +0 is considered greater than -0. -0 is never a Smi, +0
can be
+ // a Smi or heap number.
+ if (n > r || (r === 0 && n === 0 && !%_IsSmi(r) && 1 / r < 0)) r = n;
}
return r;
}
@@ -147,8 +148,9 @@
var n = %_Arguments(i);
if (!IS_NUMBER(n)) n = ToNumber(n);
if (NUMBER_IS_NAN(n)) return n;
- // Make sure -0 is considered less than +0.
- if (n < r || (r === 0 && n === 0 && !%_IsSmi(n))) r = n;
+ // Make sure -0 is considered less than +0. -0 is never a Smi, +0 can
b a
+ // Smi or a heap number.
+ if (n < r || (r === 0 && n === 0 && !%_IsSmi(n) && 1 / n < 0)) r = n;
}
return r;
}
=======================================
--- /branches/bleeding_edge/test/mjsunit/math-min-max.js Mon Dec 7
00:38:20 2009
+++ /branches/bleeding_edge/test/mjsunit/math-min-max.js Tue Dec 8
02:18:28 2009
@@ -25,9 +25,11 @@
// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+// Flags: --allow-natives-syntax
+
// Test Math.min().
-assertEquals(Number.POSITIVE_INFINITY, Math.min());
+assertEquals(Infinity, Math.min());
assertEquals(1, Math.min(1));
assertEquals(1, Math.min(1, 2));
assertEquals(1, Math.min(2, 1));
@@ -38,14 +40,26 @@
assertEquals(1.1, Math.min(3.3, 2.2, 1.1));
assertEquals(1.1, Math.min(2.2, 3.3, 1.1));
+// Prepare a non-Smi zero value.
+function returnsNonSmi(){ return 0.25; }
+var ZERO = returnsNonSmi() - returnsNonSmi();
+assertEquals(0, ZERO);
+assertEquals(Infinity, 1/ZERO);
+assertEquals(-Infinity, 1/-ZERO);
+assertFalse(%_IsSmi(ZERO));
+assertFalse(%_IsSmi(-ZERO));
+
var o = {};
o.valueOf = function() { return 1; };
assertEquals(1, Math.min(2, 3, '1'));
assertEquals(1, Math.min(3, o, 2));
assertEquals(1, Math.min(o, 2));
-assertEquals(Number.NEGATIVE_INFINITY, Number.POSITIVE_INFINITY /
Math.min(-0, +0));
-assertEquals(Number.NEGATIVE_INFINITY, Number.POSITIVE_INFINITY /
Math.min(+0, -0));
-assertEquals(Number.NEGATIVE_INFINITY, Number.POSITIVE_INFINITY /
Math.min(+0, -0, 1));
+assertEquals(-Infinity, Infinity / Math.min(-0, +0));
+assertEquals(-Infinity, Infinity / Math.min(+0, -0));
+assertEquals(-Infinity, Infinity / Math.min(+0, -0, 1));
+assertEquals(-Infinity, Infinity / Math.min(-0, ZERO));
+assertEquals(-Infinity, Infinity / Math.min(ZERO, -0));
+assertEquals(-Infinity, Infinity / Math.min(ZERO, -0, 1));
assertEquals(-1, Math.min(+0, -0, -1));
assertEquals(-1, Math.min(-1, +0, -0));
assertEquals(-1, Math.min(+0, -1, -0));
@@ -73,9 +87,12 @@
assertEquals(3, Math.max(2, '3', 1));
assertEquals(3, Math.max(1, o, 2));
assertEquals(3, Math.max(o, 1));
-assertEquals(Number.POSITIVE_INFINITY, Number.POSITIVE_INFINITY /
Math.max(-0, +0));
-assertEquals(Number.POSITIVE_INFINITY, Number.POSITIVE_INFINITY /
Math.max(+0, -0));
-assertEquals(Number.POSITIVE_INFINITY, Number.POSITIVE_INFINITY /
Math.max(+0, -0, -1));
+assertEquals(Infinity, Infinity / Math.max(-0, +0));
+assertEquals(Infinity, Infinity / Math.max(+0, -0));
+assertEquals(Infinity, Infinity / Math.max(+0, -0, -1));
+assertEquals(Infinity, Infinity / Math.max(-0, ZERO));
+assertEquals(Infinity, Infinity / Math.max(ZERO, -0));
+assertEquals(Infinity, Infinity / Math.max(ZERO, -0, -1));
assertEquals(1, Math.max(+0, -0, +1));
assertEquals(1, Math.max(+1, +0, -0));
assertEquals(1, Math.max(+0, +1, -0));
@@ -83,3 +100,6 @@
assertNaN(Math.max('oxen'));
assertNaN(Math.max('oxen', 1));
assertNaN(Math.max(1, 'oxen'));
+
+assertEquals(Infinity, 1/Math.max(ZERO, -0));
+assertEquals(Infinity, 1/Math.max(-0, ZERO));
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
