[webkit-reviews] review granted: [Bug 175642] ChakraCore/test/Function/apply3.js is resulting wrong result in x86_64 : [Attachment 320413] Patch

[bugzilla-daemon at webkit.org]

Saam Barati <sbarati at apple.com> has granted Caio Lima <ticaiolima at gmail.com>'s
request for review:
Bug 175642: ChakraCore/test/Function/apply3.js is resulting wrong result in
x86_64
https://bugs.webkit.org/show_bug.cgi?id=175642

Attachment 320413: Patch

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




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

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

> Source/JavaScriptCore/interpreter/Interpreter.cpp:211
> +	   uint64_t length64 = static_cast<uint64_t>(toLength(callFrame,
jsCast<JSObject*>(cell)));
> +	   if (overflows32Bit)
> +	       *overflows32Bit = length64 != static_cast<unsigned>(length64);

I think we already have code that'll do what you want here. Perhaps some
variant of Checked?

>> Source/JavaScriptCore/interpreter/Interpreter.cpp:243
>> +	unsigned length = sizeOfVarargs(callFrame, arguments,
firstVarArgOffset, &overflows32Bits);
> 
> how is the add below on "length" safe? What if toLength returns UINT_MAX,
length+1 will be zero b/c of overflow.

Oh I see, there is a maxArguments check below.