SERVER-24054 JS segfault on load of certain nans

(cherry picked from commit 35443b806c8050416e76040e0604da8c9acd3c74)

5 files changed, 56 insertions, 11 deletions

diff --git a/src/mongo/dbtests/jstests.cpp b/src/mongo/dbtests/jstests.cpp
index fee13dde315..53cead63a3a 100644
--- a/src/mongo/dbtests/jstests.cpp
+++ b/src/mongo/dbtests/jstests.cpp
@@ -2238,6 +2238,27 @@ public:
         ASSERT_EQUALS(17, ret.number());
     }
 };
+
+/**
+ * This tests a bug discovered in SERVER-24054, where certain interesting nan patterns crash
+ * spidermonkey by looking like non-double type puns.  This verifies that we put that particular
+ * interesting nan in and that we still get a nan out.
+ */
+class NovelNaN {
+public:
+    void run() {
+        uint8_t bits[] = {
+            16, 0, 0, 0, 0x01, 'a', '\0', 0x61, 0x79, 0xfe, 0xff, 0xff, 0xff, 0xff, 0xff, 0,
+        };
+        unique_ptr<Scope> s(globalScriptEngine->newScope());
+
+        s->setObject("val", BSONObj(reinterpret_cast<char*>(bits)).getOwned());
+
+        s->invoke("val[\"a\"];", 0, 0);
+        ASSERT_TRUE(std::isnan(s->getNumber("__returnValue")));
+    }
+};
+
 class NoReturnSpecified {
 public:
     void run() {
@@ -2395,6 +2416,8 @@ public:
         add<ScopeOut>();
         add<InvalidStoredJS>();
 
+        add<NovelNaN>();
+
         add<NoReturnSpecified>();
 
         add<RecursiveInvoke>();
diff --git a/src/mongo/scripting/mozjs/numberlong.cpp b/src/mongo/scripting/mozjs/numberlong.cpp
index c1bab0979e0..42269a34195 100644
--- a/src/mongo/scripting/mozjs/numberlong.cpp
+++ b/src/mongo/scripting/mozjs/numberlong.cpp
@@ -78,7 +78,7 @@ long long NumberLongInfo::ToNumberLong(JSContext* cx, JS::HandleObject thisv) {
 void NumberLongInfo::Functions::valueOf::call(JSContext* cx, JS::CallArgs args) {
     long long out = NumberLongInfo::ToNumberLong(cx, args.thisv());
 
-    args.rval().setDouble(out);
+    ValueReader(cx, args.rval()).fromDouble(out);
 }
 
 void NumberLongInfo::Functions::toNumber::call(JSContext* cx, JS::CallArgs args) {
@@ -116,7 +116,7 @@ void NumberLongInfo::Functions::compare::call(JSContext* cx, JS::CallArgs args)
         comparison = 1;
     }
 
-    args.rval().setDouble(comparison);
+    ValueReader(cx, args.rval()).fromDouble(comparison);
 }
 
 void NumberLongInfo::construct(JSContext* cx, JS::CallArgs args) {
diff --git a/src/mongo/scripting/mozjs/objectwrapper.cpp b/src/mongo/scripting/mozjs/objectwrapper.cpp
index 2f14816a7f9..1cecbc169e6 100644
--- a/src/mongo/scripting/mozjs/objectwrapper.cpp
+++ b/src/mongo/scripting/mozjs/objectwrapper.cpp
@@ -326,7 +326,7 @@ void ObjectWrapper::getValue(Key key, JS::MutableHandleValue value) {
 
 void ObjectWrapper::setNumber(Key key, double val) {
     JS::RootedValue jsValue(_context);
-    jsValue.setDouble(val);
+    ValueReader(_context, &jsValue).fromDouble(val);
 
     setValue(key, jsValue);
 }
diff --git a/src/mongo/scripting/mozjs/valuereader.cpp b/src/mongo/scripting/mozjs/valuereader.cpp
index bc1b61903ba..9b39457ac6c 100644
--- a/src/mongo/scripting/mozjs/valuereader.cpp
+++ b/src/mongo/scripting/mozjs/valuereader.cpp
@@ -32,6 +32,7 @@
 
 #include "mongo/scripting/mozjs/valuereader.h"
 
+#include <cmath>
 #include <cstdio>
 #include <js/CharacterEncoding.h>
 
@@ -89,7 +90,7 @@ void ValueReader::fromBSONElement(const BSONElement& elem, const BSONObj& parent
             return;
         }
         case mongo::NumberDouble:
-            _value.setDouble(elem.Number());
+            fromDouble(elem.Number());
             return;
         case mongo::NumberInt:
             _value.setInt32(elem.Int());
@@ -162,8 +163,9 @@ void ValueReader::fromBSONElement(const BSONElement& elem, const BSONObj& parent
         case mongo::bsonTimestamp: {
             JS::AutoValueArray<2> args(_context);
 
-            args[0].setDouble(elem.timestampTime().toMillisSinceEpoch() / 1000);
-            args[1].setNumber(elem.timestampInc());
+            ValueReader(_context, args[0])
+                .fromDouble(elem.timestampTime().toMillisSinceEpoch() / 1000);
+            ValueReader(_context, args[1]).fromDouble(elem.timestampInc());
 
             scope->getProto<TimestampInfo>().newInstance(args, _value);
 
@@ -177,15 +179,18 @@ void ValueReader::fromBSONElement(const BSONElement& elem, const BSONObj& parent
                         static_cast<double>(static_cast<long long>(nativeUnsignedLong))) &&
                 nativeUnsignedLong < 9007199254740992ULL) {
                 JS::AutoValueArray<1> args(_context);
-                args[0].setNumber(static_cast<double>(static_cast<long long>(nativeUnsignedLong)));
+                ValueReader(_context, args[0])
+                    .fromDouble(static_cast<double>(static_cast<long long>(nativeUnsignedLong)));
 
                 scope->getProto<NumberLongInfo>().newInstance(args, _value);
             } else {
                 JS::AutoValueArray<3> args(_context);
-                args[0].setNumber(static_cast<double>(static_cast<long long>(nativeUnsignedLong)));
-                args[1].setDouble(nativeUnsignedLong >> 32);
-                args[2].setDouble(
-                    static_cast<unsigned long>(nativeUnsignedLong & 0x00000000ffffffff));
+                ValueReader(_context, args[0])
+                    .fromDouble(static_cast<double>(static_cast<long long>(nativeUnsignedLong)));
+                ValueReader(_context, args[1]).fromDouble(nativeUnsignedLong >> 32);
+                ValueReader(_context, args[2])
+                    .fromDouble(
+                        static_cast<unsigned long>(nativeUnsignedLong & 0x00000000ffffffff));
                 scope->getProto<NumberLongInfo>().newInstance(args, _value);
             }
 
@@ -309,5 +314,21 @@ void ValueReader::fromDecimal128(Decimal128 decimal) {
     NumberDecimalInfo::make(_context, _value, decimal);
 }
 
+/**
+ * SpiderMonkey has a nasty habit of interpreting certain NaN patterns as other boxed types (it
+ * assumes that only one kind of NaN exists in JS, rather than the full ieee754 spectrum).  Thus we
+ * have to flow all double setting through a wrapper which ensures that nan's are coerced to the
+ * canonical javascript NaN.
+ *
+ * See SERVER-24054 for more details.
+ */
+void ValueReader::fromDouble(double d) {
+    if (std::isnan(d)) {
+        _value.set(JS_GetNaNValue(_context));
+    } else {
+        _value.setDouble(d);
+    }
+}
+
 }  // namespace mozjs
 }  // namespace mongo
diff --git a/src/mongo/scripting/mozjs/valuereader.h b/src/mongo/scripting/mozjs/valuereader.h
index f67b313b628..6b03d30086c 100644
--- a/src/mongo/scripting/mozjs/valuereader.h
+++ b/src/mongo/scripting/mozjs/valuereader.h
@@ -49,6 +49,7 @@ public:
 
     void fromBSONElement(const BSONElement& elem, const BSONObj& parent, bool readOnly);
     void fromBSON(const BSONObj& obj, const BSONObj* parent, bool readOnly);
+    void fromDouble(double d);
     void fromStringData(StringData sd);
     void fromDecimal128(Decimal128 decimal);