Bug 1038316: Add support for ES6 Symbols to ubi::Node. r=sfink

js/public/UbiNode.h

@@ -23,7 +23,7 @@
 // JS::ubi::Node is a pointer-like type designed for internal use by heap
 // analysis tools. A ubi::Node can refer to:
 //
-// - a JS value, like a string or object;
+// - a JS value, like a string, object, or symbol;
 // - an internal SpiderMonkey structure, like a shape or a scope chain object
 // - an instance of some embedding-provided type: in Firefox, an XPCOM
 //   object, or an internal DOM node class instance
@@ -315,10 +315,10 @@ class Node {
         return is<T>() ? static_cast<T *>(base()->ptr) : nullptr;
     }
 
-    // If this node refers to something that can be represented as a
+    // If this node refers to something that can be represented as a JavaScript
-    // JavaScript value that is safe to expose to JavaScript code, return that
+    // value that is safe to expose to JavaScript code, return that value.
-    // value. Otherwise return UndefinedValue(). JSStrings and some (but not
+    // Otherwise return UndefinedValue(). JSStrings, JS::Symbols, and some (but
-    // all!) JSObjects can be exposed.
+    // not all!) JSObjects can be exposed.
     JS::Value exposeToJS() const;
 
     const jschar *typeName()        const { return base()->typeName(); }
@@ -427,6 +427,7 @@ class TracerConcrete : public Base {
 
 template<> struct Concrete<JSObject> : TracerConcrete<JSObject> { };
 template<> struct Concrete<JSString> : TracerConcrete<JSString> { };
+template<> struct Concrete<JS::Symbol> : TracerConcrete<JS::Symbol> { };
 template<> struct Concrete<JSScript> : TracerConcrete<JSScript> { };
 template<> struct Concrete<js::LazyScript> : TracerConcrete<js::LazyScript> { };
 template<> struct Concrete<js::jit::JitCode> : TracerConcrete<js::jit::JitCode> { };

js/src/builtin/TestingFunctions.cpp

@@ -1790,21 +1790,20 @@ FindPath(JSContext *cx, unsigned argc, jsval *vp)
         return false;
     }
 
-    // We don't ToString non-objects given as 'start' or 'target'. We can't
+    // We don't ToString non-objects given as 'start' or 'target', because this
-    // see edges to non-string primitive values, and it doesn't make much
+    // test is all about object identity, and ToString doesn't preserve that.
-    // sense to ask for paths to or from a freshly allocated string, so
+    // Non-GCThing endpoints don't make much sense.
-    // if a non-string primitive appears here it's probably a mistake.
+    if (!args[0].isObject() && !args[0].isString() && !args[0].isSymbol()) {
-    if (!args[0].isObject() && !args[0].isString()) {
         js_ReportValueErrorFlags(cx, JSREPORT_ERROR, JSMSG_UNEXPECTED_TYPE,
                                  JSDVG_SEARCH_STACK, args[0], JS::NullPtr(),
-                                 "neither an object nor a string", NULL);
+                                 "not an object, string, or symbol", NULL);
         return false;
     }
 
-    if (!args[1].isObject() && !args[1].isString()) {
+    if (!args[1].isObject() && !args[1].isString() && !args[1].isSymbol()) {
         js_ReportValueErrorFlags(cx, JSREPORT_ERROR, JSMSG_UNEXPECTED_TYPE,
                                  JSDVG_SEARCH_STACK, args[0], JS::NullPtr(),
-                                 "neither an object nor a string", NULL);
+                                 "not an object, string, or symbol", NULL);
         return false;
     }
 
@@ -1837,10 +1836,13 @@ FindPath(JSContext *cx, unsigned argc, jsval *vp)
     // Construct a JavaScript array describing the path from the start to the
     // target. Each element has the form:
     //
-    //   { node: <object or string>, edge: <string describing outgoing edge from node> }
+    //   {
+    //     node: <object or string or symbol>,
+    //     edge: <string describing outgoing edge from node>
+    //   }
     //
-    // or, if the node is some internal thing, that isn't a proper
+    // or, if the node is some internal thing that isn't a proper JavaScript
-    // JavaScript value:
+    // value:
     //
     //   { node: undefined, edge: <string> }
     size_t length = nodes.length();
@@ -1868,8 +1870,7 @@ FindPath(JSContext *cx, unsigned argc, jsval *vp)
             return false;
         edgeName.release(); // edgeStr acquired ownership
 
-        if (!JS_DefineProperty(cx, obj, "edge", edgeStr,
+        if (!JS_DefineProperty(cx, obj, "edge", edgeStr, JSPROP_ENUMERATE, nullptr, nullptr))
-                               JSPROP_ENUMERATE, nullptr, nullptr))
             return false;
 
         result->setDenseElement(length - i - 1, ObjectValue(*obj));

js/src/jit-test/tests/heap-analysis/findPath.js

@@ -42,3 +42,8 @@ Match.Pattern([{node: {}, edge: "shape"},
   .assert(findPath(o, o));
 print(findPath(o, o).map((e) => e.edge).toString());
 
+// Check that we can generate ubi::Nodes for Symbols.
+var so = { sym: Symbol() };
+Match.Pattern([{node: {}, edge: "sym" }])
+  .assert(findPath(so, so.sym));
+print(findPath(so, so.sym).map((e) => e.edge).toString());

js/src/vm/UbiNode.cpp

@@ -37,6 +37,7 @@ Node::Node(JSGCTraceKind kind, void *ptr)
     switch (kind) {
       case JSTRACE_OBJECT:      construct(static_cast<JSObject *>(ptr));              break;
       case JSTRACE_STRING:      construct(static_cast<JSString *>(ptr));              break;
+      case JSTRACE_SYMBOL:      construct(static_cast<JS::Symbol *>(ptr));            break;
       case JSTRACE_SCRIPT:      construct(static_cast<JSScript *>(ptr));              break;
       case JSTRACE_LAZY_SCRIPT: construct(static_cast<js::LazyScript *>(ptr));        break;
       case JSTRACE_JITCODE:     construct(static_cast<js::jit::JitCode *>(ptr));      break;
@@ -55,6 +56,8 @@ Node::Node(Value value)
         construct(&value.toObject());
     else if (value.isString())
         construct(value.toString());
+    else if (value.isSymbol())
+        construct(value.toSymbol());
     else
         construct<void>(nullptr);
 }
@@ -75,6 +78,8 @@ Node::exposeToJS() const
         }
     } else if (is<JSString>()) {
         v.setString(as<JSString>());
+    } else if (is<JS::Symbol>()) {
+        v.setSymbol(as<JS::Symbol>());
     } else {
         v.setUndefined();
     }
@@ -210,6 +215,8 @@ template<> const jschar TracerConcrete<JSObject>::concreteTypeName[] =
     MOZ_UTF16("JSObject");
 template<> const jschar TracerConcrete<JSString>::concreteTypeName[] =
     MOZ_UTF16("JSString");
+template<> const jschar TracerConcrete<JS::Symbol>::concreteTypeName[] =
+    MOZ_UTF16("JS::Symbol");
 template<> const jschar TracerConcrete<JSScript>::concreteTypeName[] =
     MOZ_UTF16("JSScript");
 template<> const jschar TracerConcrete<js::LazyScript>::concreteTypeName[] =