Bug 1873328 - disallow nested groupings of the same type, to prevent …

…infinite recursion r=jimb

Differential Revision: https://phabricator.services.mozilla.com/D198852

devtools/shared/heapsnapshot/HeapSnapshot.cpp

@@ -36,6 +36,7 @@
 
 #include "jsapi.h"
 #include "jsfriendapi.h"
+#include "js/GCVector.h"
 #include "js/MapAndSet.h"
 #include "js/Object.h"                // JS::GetCompartment
 #include "nsComponentManagerUtils.h"  // do_CreateInstance
@@ -482,7 +483,9 @@ void HeapSnapshot::DescribeNode(JSContext* cx, JS::Handle<JSObject*> breakdown,
                                 ErrorResult& rv) {
   MOZ_ASSERT(breakdown);
   JS::Rooted<JS::Value> breakdownVal(cx, JS::ObjectValue(*breakdown));
-  JS::ubi::CountTypePtr rootType = JS::ubi::ParseBreakdown(cx, breakdownVal);
+  JS::Rooted<JS::GCVector<JSLinearString*>> seen(cx, cx);
+  JS::ubi::CountTypePtr rootType =
+      JS::ubi::ParseBreakdown(cx, breakdownVal, &seen);
   if (NS_WARN_IF(!rootType)) {
     rv.Throw(NS_ERROR_UNEXPECTED);
     return;

js/public/UbiNodeCensus.h

@@ -7,6 +7,7 @@
 #ifndef js_UbiNodeCensus_h
 #define js_UbiNodeCensus_h
 
+#include "js/GCVector.h"
 #include "js/UbiNode.h"
 #include "js/UbiNodeBreadthFirst.h"
 
@@ -222,8 +223,9 @@ using CensusTraversal = BreadthFirst<CensusHandler>;
 // Parse the breakdown language (as described in
 // js/src/doc/Debugger/Debugger.Memory.md) into a CountTypePtr. A null pointer
 // is returned on error and is reported to the cx.
-JS_PUBLIC_API CountTypePtr ParseBreakdown(JSContext* cx,
-                                          HandleValue breakdownValue);
+JS_PUBLIC_API CountTypePtr
+ParseBreakdown(JSContext* cx, HandleValue breakdownValue,
+               MutableHandle<JS::GCVector<JSLinearString*>> seen);
 
 }  // namespace ubi
 }  // namespace JS

js/public/friend/ErrorNumbers.msg

@@ -579,6 +579,7 @@ MSG_DEF(JSMSG_QUERY_LINE_WITHOUT_URL, 0, JSEXN_TYPEERR, "findScripts query objec
 MSG_DEF(JSMSG_DEBUG_CANT_SET_OPT_ENV, 1, JSEXN_REFERENCEERR, "can't set '{0}' in an optimized-out environment")
 MSG_DEF(JSMSG_DEBUG_INVISIBLE_COMPARTMENT, 0, JSEXN_TYPEERR, "object in compartment marked as invisible to Debugger")
 MSG_DEF(JSMSG_DEBUG_CENSUS_BREAKDOWN,  1, JSEXN_TYPEERR, "unrecognized 'by' value in takeCensus breakdown: {0}")
+MSG_DEF(JSMSG_DEBUG_CENSUS_BREAKDOWN_NESTED,  1, JSEXN_TYPEERR, "takeCensus breakdown 'by' value nested within itself: {0}")
 MSG_DEF(JSMSG_DEBUG_PROMISE_NOT_RESOLVED, 0, JSEXN_TYPEERR, "Promise hasn't been resolved")
 MSG_DEF(JSMSG_DEBUG_PROMISE_NOT_FULFILLED, 0, JSEXN_TYPEERR, "Promise hasn't been fulfilled")
 MSG_DEF(JSMSG_DEBUG_PROMISE_NOT_REJECTED, 0, JSEXN_TYPEERR, "Promise hasn't been rejected")

js/src/doc/Debugger/Debugger.Memory.md

@@ -305,7 +305,7 @@ which produces a result like this:
 
 In general, a `breakdown` value has one of the following forms:
 
-* <code>{ by: "count", count:<i>count<i>, bytes:<i>bytes</i> }</code>
+* <code>{ by: "count", count:<i>count</i>, bytes:<i>bytes</i> }</code>
 
   The trivial categorization: none whatsoever. Simply tally up the items
   visited. If <i>count</i> is true, count the number of items visited; if
@@ -409,6 +409,16 @@ In general, a `breakdown` value has one of the following forms:
   breakdown value produces. All breakdown values are optional, and default
   to `{ type: "count" }`.
 
+* `{ by: "filename", then:breakdown, noFilename:noFilenameBreakdown }`
+
+  For scripts only, group by the filename of the script.
+
+  Further categorize all of the scripts from each distinct filename
+  using breakdown.
+
+  Scripts that lack a filename are counted using noFilenameBreakdown.
+  These appear in the result `Map` under the key string `"noFilename"`.
+
 * `{ by: "internalType", then: breakdown }`
 
   Group items by the names given their types internally by SpiderMonkey.
@@ -441,6 +451,9 @@ In general, a `breakdown` value has one of the following forms:
 To simplify breakdown values, all `then` and `other` properties are optional.
 If omitted, they are treated as if they were `{ type: "count" }`.
 
+Breakdown groupings cannot be nested within themselves. This would not be
+useful, and forbidding this prevents infinite recursion.
+
 If the `options` argument has no `breakdown` property, `takeCensus` defaults
 to the following:
 

js/src/jit-test/tests/debug/Memory-takeCensus-06.js

@@ -106,3 +106,29 @@ Pattern({
       other:   { by: 'count', label: 'other' }
     }
   }));
+
+try {
+  const breakdown = { by: "objectClass" };
+  breakdown.then = breakdown;
+  dbg.memory.takeCensus({ breakdown });
+  assertEq(true, false, "should not reach here");
+} catch (e) {
+  assertEq(e.message, "takeCensus breakdown 'by' value nested within itself: \"objectClass\"");
+}
+
+try {
+  const breakdown = { by: "objectClass", then: { by: "objectClass" } };
+  dbg.memory.takeCensus({ breakdown });
+  assertEq(true, false, "should not reach here");
+} catch (e) {
+  assertEq(e.message, "takeCensus breakdown 'by' value nested within itself: \"objectClass\"");
+}
+
+try {
+  const breakdown = { by: "coarseType", scripts: { by: "filename" } };
+  breakdown.scripts.noFilename = breakdown;
+  dbg.memory.takeCensus({ breakdown });
+  assertEq(true, false, "should not reach here");
+} catch (e) {
+  assertEq(e.message, "takeCensus breakdown 'by' value nested within itself: \"coarseType\"");
+}

js/src/vm/UbiNodeCensus.cpp

@@ -6,6 +6,8 @@
 
 #include "js/UbiNodeCensus.h"
 
+#include "mozilla/ScopeExit.h"
+
 #include "builtin/MapObject.h"
 #include "js/friend/ErrorMessages.h"  // js::GetErrorMessage, JSMSG_*
 #include "js/Printer.h"
@@ -1062,17 +1064,19 @@ JS_PUBLIC_API bool CensusHandler::operator()(
 
 /*** Parsing Breakdowns *****************************************************/
 
-static CountTypePtr ParseChildBreakdown(JSContext* cx, HandleObject breakdown,
-                                        PropertyName* prop) {
+static CountTypePtr ParseChildBreakdown(
+    JSContext* cx, HandleObject breakdown, PropertyName* prop,
+    MutableHandle<GCVector<JSLinearString*>> seen) {
   RootedValue v(cx);
   if (!GetProperty(cx, breakdown, breakdown, prop, &v)) {
     return nullptr;
   }
-  return ParseBreakdown(cx, v);
+  return ParseBreakdown(cx, v, seen);
 }
 
-JS_PUBLIC_API CountTypePtr ParseBreakdown(JSContext* cx,
-                                          HandleValue breakdownValue) {
+JS_PUBLIC_API CountTypePtr
+ParseBreakdown(JSContext* cx, HandleValue breakdownValue,
+               MutableHandle<GCVector<JSLinearString*>> seen) {
   if (breakdownValue.isUndefined()) {
     // Construct the default type, { by: 'count' }
     CountTypePtr simple(cx->new_<SimpleCount>());
@@ -1097,6 +1101,24 @@ JS_PUBLIC_API CountTypePtr ParseBreakdown(JSContext* cx,
     return nullptr;
   }
 
+  for (auto candidate : seen.get()) {
+    if (EqualStrings(by, candidate)) {
+      UniqueChars byBytes = QuoteString(cx, by, '"');
+      if (!byBytes) {
+        return nullptr;
+      }
+
+      JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr,
+                                JSMSG_DEBUG_CENSUS_BREAKDOWN_NESTED,
+                                byBytes.get());
+      return nullptr;
+    }
+  }
+  if (!seen.append(by)) {
+    return nullptr;
+  }
+  auto popper = mozilla::MakeScopeExit([&]() { seen.popBack(); });
+
   if (StringEqualsLiteral(by, "count")) {
     RootedValue countValue(cx), bytesValue(cx);
     if (!GetProperty(cx, breakdown, breakdown, cx->names().count,
@@ -1140,13 +1162,14 @@ JS_PUBLIC_API CountTypePtr ParseBreakdown(JSContext* cx,
   }
 
   if (StringEqualsLiteral(by, "objectClass")) {
-    CountTypePtr thenType(ParseChildBreakdown(cx, breakdown, cx->names().then));
+    CountTypePtr thenType(
+        ParseChildBreakdown(cx, breakdown, cx->names().then, seen));
     if (!thenType) {
       return nullptr;
     }
 
     CountTypePtr otherType(
-        ParseChildBreakdown(cx, breakdown, cx->names().other));
+        ParseChildBreakdown(cx, breakdown, cx->names().other, seen));
     if (!otherType) {
       return nullptr;
     }
@@ -1156,27 +1179,27 @@ JS_PUBLIC_API CountTypePtr ParseBreakdown(JSContext* cx,
 
   if (StringEqualsLiteral(by, "coarseType")) {
     CountTypePtr objectsType(
-        ParseChildBreakdown(cx, breakdown, cx->names().objects));
+        ParseChildBreakdown(cx, breakdown, cx->names().objects, seen));
     if (!objectsType) {
       return nullptr;
     }
     CountTypePtr scriptsType(
-        ParseChildBreakdown(cx, breakdown, cx->names().scripts));
+        ParseChildBreakdown(cx, breakdown, cx->names().scripts, seen));
     if (!scriptsType) {
       return nullptr;
     }
     CountTypePtr stringsType(
-        ParseChildBreakdown(cx, breakdown, cx->names().strings));
+        ParseChildBreakdown(cx, breakdown, cx->names().strings, seen));
     if (!stringsType) {
       return nullptr;
     }
     CountTypePtr otherType(
-        ParseChildBreakdown(cx, breakdown, cx->names().other));
+        ParseChildBreakdown(cx, breakdown, cx->names().other, seen));
     if (!otherType) {
       return nullptr;
     }
     CountTypePtr domNodeType(
-        ParseChildBreakdown(cx, breakdown, cx->names().domNode));
+        ParseChildBreakdown(cx, breakdown, cx->names().domNode, seen));
     if (!domNodeType) {
       return nullptr;
     }
@@ -1186,7 +1209,8 @@ JS_PUBLIC_API CountTypePtr ParseBreakdown(JSContext* cx,
   }
 
   if (StringEqualsLiteral(by, "internalType")) {
-    CountTypePtr thenType(ParseChildBreakdown(cx, breakdown, cx->names().then));
+    CountTypePtr thenType(
+        ParseChildBreakdown(cx, breakdown, cx->names().then, seen));
     if (!thenType) {
       return nullptr;
     }
@@ -1195,20 +1219,22 @@ JS_PUBLIC_API CountTypePtr ParseBreakdown(JSContext* cx,
   }
 
   if (StringEqualsLiteral(by, "descriptiveType")) {
-    CountTypePtr thenType(ParseChildBreakdown(cx, breakdown, cx->names().then));
+    CountTypePtr thenType(
+        ParseChildBreakdown(cx, breakdown, cx->names().then, seen));
     if (!thenType) {
       return nullptr;
     }
     return CountTypePtr(cx->new_<ByDomObjectClass>(thenType));
   }
 
   if (StringEqualsLiteral(by, "allocationStack")) {
-    CountTypePtr thenType(ParseChildBreakdown(cx, breakdown, cx->names().then));
+    CountTypePtr thenType(
+        ParseChildBreakdown(cx, breakdown, cx->names().then, seen));
     if (!thenType) {
       return nullptr;
     }
     CountTypePtr noStackType(
-        ParseChildBreakdown(cx, breakdown, cx->names().noStack));
+        ParseChildBreakdown(cx, breakdown, cx->names().noStack, seen));
     if (!noStackType) {
       return nullptr;
     }
@@ -1217,13 +1243,14 @@ JS_PUBLIC_API CountTypePtr ParseBreakdown(JSContext* cx,
   }
 
   if (StringEqualsLiteral(by, "filename")) {
-    CountTypePtr thenType(ParseChildBreakdown(cx, breakdown, cx->names().then));
+    CountTypePtr thenType(
+        ParseChildBreakdown(cx, breakdown, cx->names().then, seen));
     if (!thenType) {
       return nullptr;
     }
 
     CountTypePtr noFilenameType(
-        ParseChildBreakdown(cx, breakdown, cx->names().noFilename));
+        ParseChildBreakdown(cx, breakdown, cx->names().noFilename, seen));
     if (!noFilenameType) {
       return nullptr;
     }
@@ -1307,8 +1334,9 @@ JS_PUBLIC_API bool ParseCensusOptions(JSContext* cx, Census& census,
     return false;
   }
 
+  Rooted<GCVector<JSLinearString*>> seen(cx, cx);
   outResult = breakdown.isUndefined() ? GetDefaultBreakdown(cx)
-                                      : ParseBreakdown(cx, breakdown);
+                                      : ParseBreakdown(cx, breakdown, &seen);
   return !!outResult;
 }