[webkit-reviews] review granted: [Bug 211030] [JSC] Handle BigInt32 INT32_MIN shift amount : [Attachment 397588] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Yusuke Suzuki <ysuzuki at apple.com>'s
request for review:
Bug 211030: [JSC] Handle BigInt32 INT32_MIN shift amount
https://bugs.webkit.org/show_bug.cgi?id=211030

Attachment 397588: Patch

https://bugs.webkit.org/attachment.cgi?id=397588&action=review




--- Comment #2 from Darin Adler <darin at apple.com> ---
Comment on attachment 397588
  --> https://bugs.webkit.org/attachment.cgi?id=397588
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?