Bug 229843 – Math.hypot checks for infinite values prematurely

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Richard Gibson]

The algorithm for Math.hypot at https://tc39.es/ecma262/#sec-math.hypot requires coercing each argument to a Number before doing anything else. However, this coercion is incorrectly aborted after encountering an infinite value.

The following expression should throw an exception, but does not:
  Math.hypot({valueOf(){ return Infinity }}, {valueOf(){ throw new Error("arguments[1]") }})

[Yusuke Suzuki]

Created attachment 437397 [details]
Patch

[Ross Kirsling]

Comment on attachment 437397 [details]
Patch

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

r=me with comments.

> Source/JavaScriptCore/ChangeLog:8
> +        We should throw an error about non finite argument after coercing all arguments to doubles.

Don't forget the spec link. :)
https://tc39.es/ecma262/#sec-math.hypot

> JSTests/stress/math-hypot-evaluation-ordering.js:18
> +shouldThrow(() => {
> +    Math.hypot({valueOf(){ return Infinity }}, {valueOf(){ throw new Error("arguments[1]") }})
> +}, `Error: arguments[1]`);

I think this test should be upstreamed to test262.

[Yusuke Suzuki]

Comment on attachment 437397 [details]
Patch

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

>> Source/JavaScriptCore/ChangeLog:8
>> +        We should throw an error about non finite argument after coercing all arguments to doubles.
> 
> Don't forget the spec link. :)
> https://tc39.es/ecma262/#sec-math.hypot

Added.

>> JSTests/stress/math-hypot-evaluation-ordering.js:18
>> +}, `Error: arguments[1]`);
> 
> I think this test should be upstreamed to test262.

@Gibson,

do you have a plan adding that?

[Yusuke Suzuki]

Committed r282081 (241381@main): <https://commits.webkit.org/241381@main>

[Radar WebKit Bug Importer]

<rdar://problem/82819521>

[Richard Gibson]

Yes, I will submit a test262 PR unless someone beats me to it.

[Richard Gibson]

Forgot that I'd already done it: https://github.com/tc39/test262/commit/50f3fca7a0eac6b6e8e5e9aee7af3c2a05831261