- Revision
- 260550
- Author
- sbar...@apple.com
- Date
- 2020-04-22 19:18:46 -0700 (Wed, 22 Apr 2020)
Log Message
BigInt32 parsing should be precise
https://bugs.webkit.org/show_bug.cgi?id=210869
Reviewed by Robin Morisset.
JSTests:
* stress/big-int-32-parsing-should-be-precise.js: Added.
Source/_javascript_Core:
Our algorithm before was conservative, and might produce a heap big int even
if the value could be an int32. This patch makes the algorithm precise on
64-bit, always producing a bigint32 if the number is indeed an int32.
* jsc.cpp:
(functionUseBigInt32):
(functionIsBigInt32):
(functionIsHeapBigInt):
* runtime/JSBigInt.cpp:
(JSC::JSBigInt::parseInt):
Modified Paths
Added Paths
Diff
Modified: trunk/JSTests/ChangeLog (260549 => 260550)
--- trunk/JSTests/ChangeLog 2020-04-23 02:14:22 UTC (rev 260549)
+++ trunk/JSTests/ChangeLog 2020-04-23 02:18:46 UTC (rev 260550)
@@ -1,3 +1,12 @@
+2020-04-22 Saam Barati <sbar...@apple.com>
+
+ BigInt32 parsing should be precise
+ https://bugs.webkit.org/show_bug.cgi?id=210869
+
+ Reviewed by Robin Morisset.
+
+ * stress/big-int-32-parsing-should-be-precise.js: Added.
+
2020-04-22 Yusuke Suzuki <ysuz...@apple.com>
[JSC] JSBigInt inc operation does not produce right HeapBigInt zero
Added: trunk/JSTests/stress/big-int-32-parsing-should-be-precise.js (0 => 260550)
--- trunk/JSTests/stress/big-int-32-parsing-should-be-precise.js (rev 0)
+++ trunk/JSTests/stress/big-int-32-parsing-should-be-precise.js 2020-04-23 02:18:46 UTC (rev 260550)
@@ -0,0 +1,30 @@
+function assert(b) {
+ if (!b)
+ throw new Error;
+}
+
+function assertIsBigInt32(arg) {
+ if (useBigInt32())
+ assert(isBigInt32(arg));
+ else
+ assert(isHeapBigInt(arg));
+}
+
+assertIsBigInt32(2147483647n);
+assertIsBigInt32(2147483646n);
+assertIsBigInt32(2127483646n);
+assertIsBigInt32(1127483646n);
+assertIsBigInt32(-2147483648n);
+assertIsBigInt32(-2147483647n);
+assertIsBigInt32(-1147483647n);
+assertIsBigInt32(0n);
+assertIsBigInt32(1n);
+assertIsBigInt32(-1n);
+assertIsBigInt32(42n);
+
+assert(isHeapBigInt(2147483648n));
+assert(isHeapBigInt(-2147483649n));
+assert(isHeapBigInt(3147483648n));
+assert(isHeapBigInt(9147483648n));
+assert(isHeapBigInt(-9147483649n));
+assert(isHeapBigInt(-2147583649n));
Modified: trunk/Source/_javascript_Core/ChangeLog (260549 => 260550)
--- trunk/Source/_javascript_Core/ChangeLog 2020-04-23 02:14:22 UTC (rev 260549)
+++ trunk/Source/_javascript_Core/ChangeLog 2020-04-23 02:18:46 UTC (rev 260550)
@@ -1,5 +1,23 @@
2020-04-22 Saam Barati <sbar...@apple.com>
+ BigInt32 parsing should be precise
+ https://bugs.webkit.org/show_bug.cgi?id=210869
+
+ Reviewed by Robin Morisset.
+
+ Our algorithm before was conservative, and might produce a heap big int even
+ if the value could be an int32. This patch makes the algorithm precise on
+ 64-bit, always producing a bigint32 if the number is indeed an int32.
+
+ * jsc.cpp:
+ (functionUseBigInt32):
+ (functionIsBigInt32):
+ (functionIsHeapBigInt):
+ * runtime/JSBigInt.cpp:
+ (JSC::JSBigInt::parseInt):
+
+2020-04-22 Saam Barati <sbar...@apple.com>
+
Edge use kind asserts are wrong for BigInt32 on ValueBitLShift
https://bugs.webkit.org/show_bug.cgi?id=210872
Modified: trunk/Source/_javascript_Core/jsc.cpp (260549 => 260550)
--- trunk/Source/_javascript_Core/jsc.cpp 2020-04-23 02:14:22 UTC (rev 260549)
+++ trunk/Source/_javascript_Core/jsc.cpp 2020-04-23 02:18:46 UTC (rev 260550)
@@ -275,6 +275,9 @@
#if USE(BIGINT32)
static EncodedJSValue JSC_HOST_CALL functionCreateBigInt32(JSGlobalObject*, CallFrame*);
#endif
+static EncodedJSValue JSC_HOST_CALL functionUseBigInt32(JSGlobalObject*, CallFrame*);
+static EncodedJSValue JSC_HOST_CALL functionIsBigInt32(JSGlobalObject*, CallFrame*);
+static EncodedJSValue JSC_HOST_CALL functionIsHeapBigInt(JSGlobalObject*, CallFrame*);
static EncodedJSValue JSC_HOST_CALL functionPrintStdOut(JSGlobalObject*, CallFrame*);
static EncodedJSValue JSC_HOST_CALL functionPrintStdErr(JSGlobalObject*, CallFrame*);
@@ -550,6 +553,9 @@
#if USE(BIGINT32)
addFunction(vm, "createBigInt32", functionCreateBigInt32, 1);
#endif
+ addFunction(vm, "useBigInt32", functionUseBigInt32, 0);
+ addFunction(vm, "isBigInt32", functionIsBigInt32, 1);
+ addFunction(vm, "isHeapBigInt", functionIsHeapBigInt, 1);
addFunction(vm, "dumpTypesForAllVariables", functionDumpTypesForAllVariables , 0);
@@ -2300,6 +2306,30 @@
}
#endif
+EncodedJSValue JSC_HOST_CALL functionUseBigInt32(JSGlobalObject*, CallFrame*)
+{
+#if USE(BIGINT32)
+ return JSValue::encode(jsBoolean(true));
+#else
+ return JSValue::encode(jsBoolean(false));
+#endif
+}
+
+EncodedJSValue JSC_HOST_CALL functionIsBigInt32(JSGlobalObject*, CallFrame* callFrame)
+{
+#if USE(BIGINT32)
+ return JSValue::encode(jsBoolean(callFrame->argument(0).isBigInt32()));
+#else
+ UNUSED_PARAM(callFrame);
+ return JSValue::encode(jsBoolean(false));
+#endif
+}
+
+EncodedJSValue JSC_HOST_CALL functionIsHeapBigInt(JSGlobalObject*, CallFrame* callFrame)
+{
+ return JSValue::encode(jsBoolean(callFrame->argument(0).isHeapBigInt()));
+}
+
EncodedJSValue JSC_HOST_CALL functionCheckModuleSyntax(JSGlobalObject* globalObject, CallFrame* callFrame)
{
VM& vm = globalObject->vm();
Modified: trunk/Source/_javascript_Core/runtime/JSBigInt.cpp (260549 => 260550)
--- trunk/Source/_javascript_Core/runtime/JSBigInt.cpp 2020-04-23 02:14:22 UTC (rev 260549)
+++ trunk/Source/_javascript_Core/runtime/JSBigInt.cpp 2020-04-23 02:18:46 UTC (rev 260550)
@@ -1866,25 +1866,76 @@
#endif
}
- // The idea is to pick the largest number such that radix ** lengthLimitForBigInt32 <= INT32_MAX
unsigned lengthLimitForBigInt32;
+#if USE(BIGINT32)
+ static_assert(sizeof(Digit) >= sizeof(uint64_t));
+ // The idea is to pick the limit such that:
+ // radix ** lengthLimitForBigInt32 >= INT32_MAX
+ // radix ** (lengthLimitForBigInt32 - 1) <= INT32_MAX
+#if ASSERT_ENABLED
+ auto limitWorks = [&] {
+ double lengthLimit = lengthLimitForBigInt32;
+ double lowerLimit = pow(static_cast<double>(radix), lengthLimit - 1);
+ double upperLimit = pow(static_cast<double>(radix), lengthLimit);
+ double target = std::numeric_limits<int32_t>::max();
+ return lowerLimit <= target && target <= upperLimit && upperLimit <= std::numeric_limits<int64_t>::max();
+ };
+#endif
switch (radix) {
case 2:
+ lengthLimitForBigInt32 = 31;
+ ASSERT(limitWorks());
+ break;
+ case 8:
+ lengthLimitForBigInt32 = 11;
+ ASSERT(limitWorks());
+ break;
+ case 10:
+ lengthLimitForBigInt32 = 10;
+ ASSERT(limitWorks());
+ break;
+ case 16:
+ lengthLimitForBigInt32 = 8;
+ ASSERT(limitWorks());
+ break;
+ default:
+ lengthLimitForBigInt32 = 1;
+ break;
+ }
+#else
+ // The idea is to pick the largest limit such that:
+ // radix ** lengthLimitForBigInt32 <= INT32_MAX
+#if ASSERT_ENABLED
+ auto limitWorks = [&] {
+ double lengthLimit = lengthLimitForBigInt32;
+ double valueLimit = pow(static_cast<double>(radix), lengthLimit);
+ double overValueLimit = pow(static_cast<double>(radix), lengthLimit + 1);
+ double target = std::numeric_limits<int32_t>::max();
+ return valueLimit <= target && target < overValueLimit;
+ };
+#endif
+ switch (radix) {
+ case 2:
lengthLimitForBigInt32 = 30;
+ ASSERT(limitWorks());
break;
case 8:
lengthLimitForBigInt32 = 10;
+ ASSERT(limitWorks());
break;
case 10:
lengthLimitForBigInt32 = 9;
+ ASSERT(limitWorks());
break;
case 16:
lengthLimitForBigInt32 = 7;
+ ASSERT(limitWorks());
break;
default:
lengthLimitForBigInt32 = 1;
break;
}
+#endif // USE(BIGINT32)
JSBigInt* heapResult = nullptr;
@@ -1893,17 +1944,17 @@
unsigned limitA = 'A' + (static_cast<int32_t>(radix) - 10);
unsigned initialLength = length - p;
while (p < length) {
- int32_t digit = 0;
- Checked<int32_t, CrashOnOverflow> multiplier = 1;
- for (unsigned i = 0; i < lengthLimitForBigInt32 && p < length ; ++i, ++p) {
+ Checked<uint64_t, CrashOnOverflow> digit = 0;
+ Checked<uint64_t, CrashOnOverflow> multiplier = 1;
+ for (unsigned i = 0; i < lengthLimitForBigInt32 && p < length; ++i, ++p) {
digit *= radix;
multiplier *= radix;
if (data[p] >= '0' && data[p] < limit0)
- digit += data[p] - '0';
+ digit += static_cast<uint64_t>(data[p] - '0');
else if (data[p] >= 'a' && data[p] < limita)
- digit += data[p] - 'a' + 10;
+ digit += static_cast<uint64_t>(data[p] - 'a' + 10);
else if (data[p] >= 'A' && data[p] < limitA)
- digit += data[p] - 'A' + 10;
+ digit += static_cast<uint64_t>(data[p] - 'A' + 10);
else {
if (errorParseMode == ErrorParseMode::ThrowExceptions) {
ASSERT(globalObject);
@@ -1912,17 +1963,22 @@
return JSValue();
}
}
- ASSERT(digit < multiplier.unsafeGet());
if (!heapResult) {
if (p == length) {
+ ASSERT(digit.unsafeGet() <= std::numeric_limits<int64_t>::max());
+ int64_t maybeResult = digit.unsafeGet();
+ ASSERT(maybeResult >= 0);
if (sign == ParseIntSign::Signed)
- digit *= -1;
+ maybeResult *= -1;
+
+ if (static_cast<int64_t>(static_cast<int32_t>(maybeResult)) == maybeResult) {
#if USE(BIGINT32)
- return JSValue(JSValue::JSBigInt32, digit);
+ return JSValue(JSValue::JSBigInt32, static_cast<int32_t>(maybeResult));
#else
- return createFrom(vm, static_cast<int64_t>(digit));
+ return JSBigInt::createFrom(vm, static_cast<int32_t>(maybeResult));
#endif
+ }
}
heapResult = allocateFor(globalObject, vm, radix, initialLength);
RETURN_IF_EXCEPTION(scope, JSValue());
@@ -1931,7 +1987,10 @@
return JSValue();
heapResult->initialize(InitializationType::WithZero);
}
- heapResult->inplaceMultiplyAdd(static_cast<Digit>(multiplier.unsafeGet()), static_cast<Digit>(digit));
+
+ ASSERT(static_cast<uint64_t>(static_cast<Digit>(multiplier.unsafeGet())) == multiplier.unsafeGet());
+ ASSERT(static_cast<uint64_t>(static_cast<Digit>(digit.unsafeGet())) == digit.unsafeGet());
+ heapResult->inplaceMultiplyAdd(static_cast<Digit>(multiplier.unsafeGet()), static_cast<Digit>(digit.unsafeGet()));
}
heapResult->setSign(sign == ParseIntSign::Signed);
