Summary: | [JSC] Handle BigInt32 INT32_MIN shift amount | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Yusuke Suzuki <ysuzuki> | ||||||||
Component: | JavaScriptCore | Assignee: | Yusuke Suzuki <ysuzuki> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | darin, ews-watchlist, keith_miller, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Yusuke Suzuki
2020-04-25 14:27:28 PDT
Created attachment 397588 [details]
Patch
Comment on attachment 397588 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=397588&action=review > Source/JavaScriptCore/ChangeLog:3 > + [JSC] Handle BitInt32 INT32_MIN shift amount typo: BigInt32 > Source/JavaScriptCore/runtime/Operations.h:776 > + if (rightInt32 == INT32_MIN) { > + // Shift-amount is 0x80000000. For right-shift, shift-amount is reduced to 31. > + if (!isLeft) > + return jsBigInt32(leftInt32 >> 31); > + // Left-shift with 0x80000000 produces too large BigInt, and throws a RangeError. > + // But when leftInt32 is zero, we should return zero. > + if (!leftInt32) > + return jsBigInt32(0); > + throwRangeError(globalObject, scope, "BigInt generated from this operation is too big"_s); > + return { }; > + } > rightInt32 = -rightInt32; Would this simpler implementation still gives us the correct result? if (rightInt32 == INT32_MIN) rightInt32 = INT32_MAX; // Shifts one less than requested, but makes no observable difference. else rightInt32 = -rightInt32; Would you consider it if it does? Comment on attachment 397588 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=397588&action=review Thanks! >> Source/JavaScriptCore/ChangeLog:3 >> + [JSC] Handle BitInt32 INT32_MIN shift amount > > typo: BigInt32 Ooops, fixed. >> Source/JavaScriptCore/runtime/Operations.h:776 >> rightInt32 = -rightInt32; > > Would this simpler implementation still gives us the correct result? > > if (rightInt32 == INT32_MIN) > rightInt32 = INT32_MAX; // Shifts one less than requested, but makes no observable difference. > else > rightInt32 = -rightInt32; > > Would you consider it if it does? Good point! Right, sounds simpler. Fixed. Created attachment 397604 [details]
Patch for landing
Created attachment 397605 [details]
Patch for landing
Committed r260720: <https://trac.webkit.org/changeset/260720> All reviewed patches have been landed. Closing bug and clearing flags on attachment 397605 [details]. |