[webkit-reviews] review denied: [Bug 194888] BigInt should store its data in the primitive gigacage. : [Attachment 363244] Patch

[bugzilla-daemon at webkit.org]

Saam Barati <sbarati at apple.com> has denied Keith Miller
<keith_miller at apple.com>'s request for review:
Bug 194888: BigInt should store its data in the primitive gigacage.
https://bugs.webkit.org/show_bug.cgi?id=194888

Attachment 363244: Patch

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




--- Comment #8 from Saam Barati <sbarati at apple.com> ---
Comment on attachment 363244
  --> https://bugs.webkit.org/attachment.cgi?id=363244
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=363244&action=review

> Source/JavaScriptCore/runtime/JSBigInt.cpp:119
>  JSBigInt* JSBigInt::createWithLengthUnchecked(VM& vm, unsigned length)
>  {
>      ASSERT(length <= maxLength);
> -    JSBigInt* bigInt = new (NotNull, allocateCell<JSBigInt>(vm.heap,
allocationSize(length))) JSBigInt(vm, vm.bigIntStructure.get(), length);
> +    void* data = Gigacage::tryMalloc(Gigacage::Primitive, length *
sizeof(Digit));
> +    if (!data)
> +	   return nullptr;
> +
> +    JSBigInt* bigInt = new (NotNull, allocateCell<JSBigInt>(vm.heap))
JSBigInt(vm, vm.bigIntStructure.get(), reinterpret_cast<Digit*>(data), length);
>      bigInt->finishCreation(vm);
>      return bigInt;
>  }

The point of this function is that we don't return nullptr. So this shouldn't
be using tryMalloc. Perhaps somewhere earlier we should be tryMallocing and
throwing an OOM? If you grep for callers of this, you'll see that we just
assume it's non-null.