Cherry-pick 274910@main (5f1ac64). https://bugs.webkit.org/show_bug.c…

…gi?id=269509

    Preserve sign of NaN operands in fsub{32,64}
    https://bugs.webkit.org/show_bug.cgi?id=269509
    rdar://120780768

    Reviewed by Justin Michaud and Yusuke Suzuki.

    Negating a NaN operand for fsub is incorrect, as IEEE defines the result
    of an arithmetic operation with a NaN parameter as the NaN itself
    (without modification). This is observable when a copysign operation is
    done on the result, as copysign itself is exempt from that restriction
    and can thus detect the improper negation.
    By checking whether the operand is a NaN before negating we avoid this
    issue.

    * JSTests/wasm/stress/fsub-nan-copysign.js: Added.
    * Source/JavaScriptCore/wasm/WasmBBQJIT.cpp:
    (JSC::Wasm::BBQJITImpl::BBQJIT::addF32Sub):
    (JSC::Wasm::BBQJITImpl::BBQJIT::addF64Sub):

    Canonical link: https://commits.webkit.org/274910@main

Canonical link: https://commits.webkit.org/274313.37@webkitglib/2.44

JSTests/wasm/stress/fsub-nan-copysign.js

@@ -0,0 +1,38 @@
+import * as wabt from "../wabt-wrapper.js";
+import * as assert from '../assert.js';
+
+let wat = `
+(module
+    (type (func (param f64) (result i32)))
+    (func (type 0) (local f64) (local f64) (local f64) (local i32)
+        (local.set 1 (local.get 0))
+        (local.set 2 (local.get 0))
+        (local.set 3 (local.get 0))
+        ;; Do the sub-operation with local/const, const/local, and const/const params
+        (local.set 0 (f64.sub (local.get 1) (f64.const nan)))
+        (local.set 1 (f64.copysign (local.get 1) (local.get 0)))
+        (local.set 0 (f64.sub (f64.const nan) (local.get 2)))
+        (local.set 2 (f64.copysign (local.get 2) (local.get 0)))
+        (local.set 0 (f64.sub (f64.const 0.0) (f64.const nan)))
+        (local.set 3 (f64.copysign (local.get 3) (local.get 0)))
+        ;; If any one of the results is negative, then fail: return a positive integer
+        ;; Otherwise return 0 for success
+        (local.set 4 (f64.lt (local.get 1) (f64.const 0)))
+        (local.set 4 (i32.add (local.get 4) (f64.lt (local.get 1) (f64.const 0))))
+        (local.set 4 (i32.add (local.get 4) (f64.lt (local.get 2) (f64.const 0))))
+        (local.set 4 (i32.add (local.get 4) (f64.lt (local.get 3) (f64.const 0))))
+        (local.get 4)
+    )
+  (export "poc" (func 0))
+)
+`
+
+async function test() {
+    const instance = await wabt.instantiate(wat, {}, {});
+
+    for (let i = 0; i < 50; i++) {
+        assert.eq(instance.exports.poc(1), 0);
+    }
+}
+
+assert.asyncTest(test());

Source/JavaScriptCore/wasm/WasmBBQJIT.cpp

@@ -60,6 +60,7 @@
 #include "WasmThunks.h"
 #include "WasmTypeDefinition.h"
 #include <bit>
+#include <cmath>
 #include <wtf/Assertions.h>
 #include <wtf/Compiler.h>
 #include <wtf/HashFunctions.h>
@@ -1717,8 +1718,10 @@ PartialResult WARN_UNUSED_RETURN BBQJIT::addF32Sub(Value lhs, Value rhs, Value&
         ),
         BLOCK(
             if (rhs.isConst()) {
-                // Add a negative if rhs is a constant.
-                emitMoveConst(Value::fromF32(-rhs.asF32()), Location::fromFPR(wasmScratchFPR));
+                // If rhs is a constant, it will be expressed as a positive
+                // value and so needs to be negated unless it is NaN
+                auto floatVal = std::isnan(rhs.asF32()) ? rhs.asF32() : -rhs.asF32();
+                emitMoveConst(Value::fromF32(floatVal), Location::fromFPR(wasmScratchFPR));
                 m_jit.addFloat(lhsLocation.asFPR(), wasmScratchFPR, resultLocation.asFPR());
             } else {
                 emitMoveConst(lhs, Location::fromFPR(wasmScratchFPR));
@@ -1738,8 +1741,10 @@ PartialResult WARN_UNUSED_RETURN BBQJIT::addF64Sub(Value lhs, Value rhs, Value&
         ),
         BLOCK(
             if (rhs.isConst()) {
-                // Add a negative if rhs is a constant.
-                emitMoveConst(Value::fromF64(-rhs.asF64()), Location::fromFPR(wasmScratchFPR));
+                // If rhs is a constant, it will be expressed as a positive
+                // value and so needs to be negated unless it is NaN
+                auto floatVal = std::isnan(rhs.asF64()) ? rhs.asF64() : -rhs.asF64();
+                emitMoveConst(Value::fromF64(floatVal), Location::fromFPR(wasmScratchFPR));
                 m_jit.addDouble(lhsLocation.asFPR(), wasmScratchFPR, resultLocation.asFPR());
             } else {
                 emitMoveConst(lhs, Location::fromFPR(wasmScratchFPR));