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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Feb 28 12:41:09 PST 2019

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.

Attachment 363244: Patch


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

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 *
> +    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.

More information about the webkit-reviews mailing list