Commit 1685818a authored by Nick Fitzgerald's avatar Nick Fitzgerald
Browse files

Bug 1254092 - TraceIncomingCCWs should work at the JSCompartment level of granularity. r=jimb 

There can be multiple compartments within the same zone, only one of which is a
debuggee. In this scenario, CCWs from other compartments into the debuggee
compartment should be traced and treated as roots. Therefore, dealing with CCWs
at the JS::Zone level is incorrect, and this patch changes the granularity level
to JSCompartments. If you look at the callers and uses of the function, it makes
much more sense now.

Additionally, it renames `JS_TraceIncomingCCWs` to `JS::TraceIncomingCCWs`.

--HG--
rename : devtools/shared/heapsnapshot/tests/gtest/DoesCrossZoneBoundaries.cpp => devtools/shared/heapsnapshot/tests/gtest/DoesCrossCompartmentBoundaries.cpp
rename : devtools/shared/heapsnapshot/tests/gtest/DoesntCrossZoneBoundaries.cpp => devtools/shared/heapsnapshot/tests/gtest/DoesntCrossCompartmentBoundaries.cpp
parent dc62409f

devtools/shared/heapsnapshot/HeapSnapshot.cpp

+33 −31
Original line number Diff line number Diff line
@@ -31,6 +31,7 @@ 
#include "mozilla/Telemetry.h"



#include "jsapi.h"

#include "jsfriendapi.h"

#include "nsCycleCollectionParticipant.h"

#include "nsCRTGlue.h"

#include "nsDirectoryServiceDefs.h"
@@ -722,17 +723,17 @@ HeapSnapshot::ComputeShortestPaths(JSContext*cx, uint64_t start, 
/*** Saving Heap Snapshots ************************************************************************/



// If we are only taking a snapshot of the heap affected by the given set of

// globals, find the set of zones the globals are allocated within. Returns

// false on OOM failure.

// globals, find the set of compartments the globals are allocated

// within. Returns false on OOM failure.

static bool

PopulateZonesWithGlobals(ZoneSet& zones, AutoObjectVector& globals)

PopulateCompartmentsWithGlobals(CompartmentSet& compartments, AutoObjectVector& globals)

{

  if (!zones.init())

  if (!compartments.init())

    return false;



  unsigned length = globals.length();

  for (unsigned i = 0; i < length; i++) {

    if (!zones.put(GetObjectZone(globals[i])))

    if (!compartments.put(GetObjectCompartment(globals[i])))

      return false;

  }
@@ -757,9 +758,10 @@ AddGlobalsAsRoots(AutoObjectVector& globals, ubi::RootList& roots) 


// Choose roots and limits for a traversal, given `boundaries`. Set `roots` to

// the set of nodes within the boundaries that are referred to by nodes

// outside. If `boundaries` does not include all JS zones, initialize `zones` to

// the set of included zones; otherwise, leave `zones` uninitialized. (You can

// use zones.initialized() to check.)

// outside. If `boundaries` does not include all JS compartments, initialize

// `compartments` to the set of included compartments; otherwise, leave

// `compartments` uninitialized. (You can use compartments.initialized() to

// check.)

//

// If `boundaries` is incoherent, or we encounter an error while trying to

// handle it, or we run out of memory, set `rv` appropriately and return
@@ -769,10 +771,10 @@ EstablishBoundaries(JSContext* cx, 
                    ErrorResult& rv,

                    const HeapSnapshotBoundaries& boundaries,

                    ubi::RootList& roots,

                    ZoneSet& zones)

                    CompartmentSet& compartments)

{

  MOZ_ASSERT(!roots.initialized());

  MOZ_ASSERT(!zones.initialized());

  MOZ_ASSERT(!compartments.initialized());



  bool foundBoundaryProperty = false;
@@ -805,8 +807,8 @@ EstablishBoundaries(JSContext* cx, 


    AutoObjectVector globals(cx);

    if (!dbg::GetDebuggeeGlobals(cx, *dbgObj, globals) ||

        !PopulateZonesWithGlobals(zones, globals) ||

        !roots.init(zones) ||

        !PopulateCompartmentsWithGlobals(compartments, globals) ||

        !roots.init(compartments) ||

        !AddGlobalsAsRoots(globals, roots))

    {

      rv.Throw(NS_ERROR_OUT_OF_MEMORY);
@@ -840,8 +842,8 @@ EstablishBoundaries(JSContext* cx, 
      }

    }



    if (!PopulateZonesWithGlobals(zones, globals) ||

        !roots.init(zones) ||

    if (!PopulateCompartmentsWithGlobals(compartments, globals) ||

        !roots.init(compartments) ||

        !AddGlobalsAsRoots(globals, roots))

    {

      rv.Throw(NS_ERROR_OUT_OF_MEMORY);
@@ -855,8 +857,8 @@ EstablishBoundaries(JSContext* cx, 
  }



  MOZ_ASSERT(roots.initialized());

  MOZ_ASSERT_IF(boundaries.mDebugger.WasPassed(), zones.initialized());

  MOZ_ASSERT_IF(boundaries.mGlobals.WasPassed(), zones.initialized());

  MOZ_ASSERT_IF(boundaries.mDebugger.WasPassed(), compartments.initialized());

  MOZ_ASSERT_IF(boundaries.mGlobals.WasPassed(), compartments.initialized());

  return true;

}
@@ -1322,7 +1324,7 @@ public: 
class MOZ_STACK_CLASS HeapSnapshotHandler

{

  CoreDumpWriter&     writer;

  JS::ZoneSet*    zones;

  JS::CompartmentSet* compartments;



public:

  // For telemetry.
@@ -1330,9 +1332,9 @@ public: 
  uint32_t edgeCount;



  HeapSnapshotHandler(CoreDumpWriter& writer,

                      JS::ZoneSet* zones)

                      JS::CompartmentSet* compartments)

    : writer(writer),

      zones(zones)

      compartments(compartments)

  { }



  // JS::ubi::BreadthFirst handler interface.
@@ -1360,21 +1362,21 @@ public: 


    const JS::ubi::Node& referent = edge.referent;



    if (!zones)

      // We aren't targeting a particular set of zones, so serialize all the

    if (!compartments)

      // We aren't targeting a particular set of compartments, so serialize all the

      // things!

      return writer.writeNode(referent, CoreDumpWriter::INCLUDE_EDGES);



    // We are targeting a particular set of zones. If this node is in our target

    // We are targeting a particular set of compartments. If this node is in our target

    // set, serialize it and all of its edges. If this node is _not_ in our

    // target set, we also serialize under the assumption that it is a shared

    // resource being used by something in our target zones since we reached it

    // resource being used by something in our target compartments since we reached it

    // by traversing the heap graph. However, we do not serialize its outgoing

    // edges and we abandon further traversal from this node.



    JS::Zone* zone = referent.zone();

    JSCompartment* compartment = referent.compartment();



    if (zones->has(zone))

    if (compartments->has(compartment))

      return writer.writeNode(referent, CoreDumpWriter::INCLUDE_EDGES);



    traversal.abandonReferent();
@@ -1388,7 +1390,7 @@ WriteHeapGraph(JSContext* cx, 
               const JS::ubi::Node& node,

               CoreDumpWriter& writer,

               bool wantNames,

               JS::ZoneSet* zones,

               JS::CompartmentSet* compartments,

               JS::AutoCheckCannotGC& noGC,

               uint32_t& outNodeCount,

               uint32_t& outEdgeCount)
@@ -1402,7 +1404,7 @@ WriteHeapGraph(JSContext* cx, 
  // Walk the heap graph starting from the given node and serialize it into the

  // core dump.



  HeapSnapshotHandler handler(writer, zones);

  HeapSnapshotHandler handler(writer, compartments);

  HeapSnapshotHandler::Traversal traversal(JS_GetRuntime(cx), handler, noGC);

  if (!traversal.init())

    return false;
@@ -1548,7 +1550,7 @@ ThreadSafeChromeUtils::SaveHeapSnapshot(GlobalObject& global, 
  auto start = TimeStamp::Now();



  bool wantNames = true;

  ZoneSet zones;

  CompartmentSet compartments;

  uint32_t nodeCount = 0;

  uint32_t edgeCount = 0;
@@ -1569,7 +1571,7 @@ ThreadSafeChromeUtils::SaveHeapSnapshot(GlobalObject& global, 
  {

    Maybe<AutoCheckCannotGC> maybeNoGC;

    ubi::RootList rootList(JS_GetRuntime(cx), maybeNoGC, wantNames);

    if (!EstablishBoundaries(cx, rv, boundaries, rootList, zones))

    if (!EstablishBoundaries(cx, rv, boundaries, rootList, compartments))

      return;



    MOZ_ASSERT(maybeNoGC.isSome());
@@ -1583,7 +1585,7 @@ ThreadSafeChromeUtils::SaveHeapSnapshot(GlobalObject& global, 
                        roots,

                        writer,

                        wantNames,

                        zones.initialized() ? &zones : nullptr,

                        compartments.initialized() ? &compartments : nullptr,

                        maybeNoGC.ref(),

                        nodeCount,

                        edgeCount))

devtools/shared/heapsnapshot/HeapSnapshot.h

+3 −3
Original line number Diff line number Diff line
@@ -212,7 +212,7 @@ WriteHeapGraph(JSContext* cx, 
               const JS::ubi::Node& node,

               CoreDumpWriter& writer,

               bool wantNames,

               JS::ZoneSet* zones,

               JS::CompartmentSet* compartments,

               JS::AutoCheckCannotGC& noGC,

               uint32_t& outNodeCount,

               uint32_t& outEdgeCount);
@@ -221,12 +221,12 @@ WriteHeapGraph(JSContext* cx, 
               const JS::ubi::Node& node,

               CoreDumpWriter& writer,

               bool wantNames,

               JS::ZoneSet* zones,

               JS::CompartmentSet* compartments,

               JS::AutoCheckCannotGC& noGC)

{

  uint32_t ignoreNodeCount;

  uint32_t ignoreEdgeCount;

  return WriteHeapGraph(cx, node, writer, wantNames, zones, noGC,

  return WriteHeapGraph(cx, node, writer, wantNames, compartments, noGC,

                        ignoreNodeCount, ignoreEdgeCount);

}

devtools/shared/heapsnapshot/tests/gtest/DoesCrossZoneBoundaries.cpp→devtools/shared/heapsnapshot/tests/gtest/DoesCrossCompartmentBoundaries.cpp

+22 −22
Original line number Diff line number Diff line
@@ -3,12 +3,12 @@ 
 * License, v. 2.0. If a copy of the MPL was not distributed with this

 * file, You can obtain one at http://mozilla.org/MPL/2.0/. */



// Test that heap snapshots cross zone boundaries when expected.

// Test that heap snapshots cross compartment boundaries when expected.



#include "DevTools.h"



DEF_TEST(DoesCrossZoneBoundaries, {

    // Create a new global to get a new zone.

DEF_TEST(DoesCrossCompartmentBoundaries, {

    // Create a new global to get a new compartment.

    JS::CompartmentOptions options;

    JS::RootedObject newGlobal(cx, JS_NewGlobalObject(cx,

                                                      getGlobalClass(),
@@ -16,30 +16,30 @@ DEF_TEST(DoesCrossZoneBoundaries, { 
                                                      JS::FireOnNewGlobalHook,

                                                      options));

    ASSERT_TRUE(newGlobal);

    JS::Zone* newZone = nullptr;

    JSCompartment* newCompartment = nullptr;

    {

      JSAutoCompartment ac(cx, newGlobal);

      ASSERT_TRUE(JS_InitStandardClasses(cx, newGlobal));

      newZone = js::GetContextZone(cx);

      newCompartment = js::GetContextCompartment(cx);

    }

    ASSERT_TRUE(newZone);

    ASSERT_NE(newZone, zone);

    ASSERT_TRUE(newCompartment);

    ASSERT_NE(newCompartment, compartment);



    // Our set of target zones is both the old and new zones.

    JS::ZoneSet targetZones;

    ASSERT_TRUE(targetZones.init());

    ASSERT_TRUE(targetZones.put(zone));

    ASSERT_TRUE(targetZones.put(newZone));

    // Our set of target compartments is both the old and new compartments.

    JS::CompartmentSet targetCompartments;

    ASSERT_TRUE(targetCompartments.init());

    ASSERT_TRUE(targetCompartments.put(compartment));

    ASSERT_TRUE(targetCompartments.put(newCompartment));



    FakeNode nodeA;

    FakeNode nodeB;

    FakeNode nodeC;

    FakeNode nodeD;



    nodeA.zone = zone;

    nodeB.zone = nullptr;

    nodeC.zone = newZone;

    nodeD.zone = nullptr;

    nodeA.compartment = compartment;

    nodeB.compartment = nullptr;

    nodeC.compartment = newCompartment;

    nodeD.compartment = nullptr;



    AddEdge(nodeA, nodeB);

    AddEdge(nodeA, nodeC);
@@ -47,19 +47,19 @@ DEF_TEST(DoesCrossZoneBoundaries, { 


    ::testing::NiceMock<MockWriter> writer;



    // Should serialize nodeA, because it is in one of our target zones.

    // Should serialize nodeA, because it is in one of our target compartments.

    ExpectWriteNode(writer, nodeA);



    // Should serialize nodeB, because it doesn't belong to a zone and is

    // Should serialize nodeB, because it doesn't belong to a compartment and is

    // therefore assumed to be shared.

    ExpectWriteNode(writer, nodeB);



    // Should also serialize nodeC, which is in our target zones, but a

    // different zone than A.

    // Should also serialize nodeC, which is in our target compartments, but a

    // different compartment than A.

    ExpectWriteNode(writer, nodeC);



    // However, should not serialize nodeD because nodeB doesn't belong to one

    // of our target zones and so its edges are excluded from serialization.

    // of our target compartments and so its edges are excluded from serialization.



    JS::AutoCheckCannotGC noGC(rt);
@@ -67,6 +67,6 @@ DEF_TEST(DoesCrossZoneBoundaries, { 
                               JS::ubi::Node(&nodeA),

                               writer,

                               /* wantNames = */ false,

                               &targetZones,

                               &targetCompartments,

                               noGC));

  });

devtools/shared/heapsnapshot/tests/gtest/DoesntCrossZoneBoundaries.cpp→devtools/shared/heapsnapshot/tests/gtest/DoesntCrossCompartmentBoundaries.cpp

+18 −18
Original line number Diff line number Diff line
@@ -3,12 +3,12 @@ 
 * License, v. 2.0. If a copy of the MPL was not distributed with this

 * file, You can obtain one at http://mozilla.org/MPL/2.0/. */



// Test that heap snapshots walk the zone boundaries correctly.

// Test that heap snapshots walk the compartment boundaries correctly.



#include "DevTools.h"



DEF_TEST(DoesntCrossZoneBoundaries, {

    // Create a new global to get a new zone.

DEF_TEST(DoesntCrossCompartmentBoundaries, {

    // Create a new global to get a new compartment.

    JS::CompartmentOptions options;

    JS::RootedObject newGlobal(cx, JS_NewGlobalObject(cx,

                                                      getGlobalClass(),
@@ -16,38 +16,38 @@ DEF_TEST(DoesntCrossZoneBoundaries, { 
                                                      JS::FireOnNewGlobalHook,

                                                      options));

    ASSERT_TRUE(newGlobal);

    JS::Zone* newZone = nullptr;

    JSCompartment* newCompartment = nullptr;

    {

      JSAutoCompartment ac(cx, newGlobal);

      ASSERT_TRUE(JS_InitStandardClasses(cx, newGlobal));

      newZone = js::GetContextZone(cx);

      newCompartment = js::GetContextCompartment(cx);

    }

    ASSERT_TRUE(newZone);

    ASSERT_NE(newZone, zone);

    ASSERT_TRUE(newCompartment);

    ASSERT_NE(newCompartment, compartment);



    // Our set of target zones is only the pre-existing zone and does not

    // include the new zone.

    JS::ZoneSet targetZones;

    ASSERT_TRUE(targetZones.init());

    ASSERT_TRUE(targetZones.put(zone));

    // Our set of target compartments is only the pre-existing compartment and

    // does not include the new compartment.

    JS::CompartmentSet targetCompartments;

    ASSERT_TRUE(targetCompartments.init());

    ASSERT_TRUE(targetCompartments.put(compartment));



    FakeNode nodeA;

    FakeNode nodeB;

    FakeNode nodeC;



    nodeA.zone = zone;

    nodeB.zone = nullptr;

    nodeC.zone = newZone;

    nodeA.compartment = compartment;

    nodeB.compartment = nullptr;

    nodeC.compartment = newCompartment;



    AddEdge(nodeA, nodeB);

    AddEdge(nodeB, nodeC);



    ::testing::NiceMock<MockWriter> writer;



    // Should serialize nodeA, because it is in our target zones.

    // Should serialize nodeA, because it is in our target compartments.

    ExpectWriteNode(writer, nodeA);



    // Should serialize nodeB, because it doesn't belong to a zone and is

    // Should serialize nodeB, because it doesn't belong to a compartment and is

    // therefore assumed to be shared.

    ExpectWriteNode(writer, nodeB);
@@ -59,6 +59,6 @@ DEF_TEST(DoesntCrossZoneBoundaries, { 
                               JS::ubi::Node(&nodeA),

                               writer,

                               /* wantNames = */ false,

                               &targetZones,

                               &targetCompartments,

                               noGC));

  });

devtools/shared/heapsnapshot/tests/gtest/moz.build

+2 −2
Original line number Diff line number Diff line
@@ -13,8 +13,8 @@ LOCAL_INCLUDES += [ 
UNIFIED_SOURCES = [

    'DeserializedNodeUbiNodes.cpp',

    'DeserializedStackFrameUbiStackFrames.cpp',

    'DoesCrossZoneBoundaries.cpp',

    'DoesntCrossZoneBoundaries.cpp',

    'DoesCrossCompartmentBoundaries.cpp',

    'DoesntCrossCompartmentBoundaries.cpp',

    'SerializesEdgeNames.cpp',

    'SerializesEverythingInHeapGraphOnce.cpp',

    'SerializesTypeNames.cpp',