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]") }})
Created attachment 437397 [details] Patch
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.
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?
Committed r282081 (241381@main): <https://commits.webkit.org/241381@main>
<rdar://problem/82819521>
Yes, I will submit a test262 PR unless someone beats me to it.
Forgot that I'd already done it: https://github.com/tc39/test262/commit/50f3fca7a0eac6b6e8e5e9aee7af3c2a05831261