Bug 1223512 - Validate that every edge referent is in the heap snapshot; r=shu, a=ritu
authorNick Fitzgerald <fitzgen@gmail.com>
Fri, 13 Nov 2015 11:11:09 -0800
changeset 305539 0141686cbdc0c9eea34959de7016375873412864
parent 305538 36ad43168b8da376ddbf1e7a682feeac95a7d274
child 305540 2a339d6b372566faedde8fdc11d7a47d8266be2a
push id1001
push userraliiev@mozilla.com
push dateMon, 18 Jan 2016 19:06:03 +0000
treeherdermozilla-release@8b89261f3ac4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersshu, ritu
bugs1223512
milestone44.0a2
Bug 1223512 - Validate that every edge referent is in the heap snapshot; r=shu, a=ritu
devtools/shared/heapsnapshot/HeapSnapshot.cpp
devtools/shared/heapsnapshot/HeapSnapshot.h
--- a/devtools/shared/heapsnapshot/HeapSnapshot.cpp
+++ b/devtools/shared/heapsnapshot/HeapSnapshot.cpp
@@ -86,17 +86,17 @@ HeapSnapshot::WrapObject(JSContext* aCx,
 /* static */ already_AddRefed<HeapSnapshot>
 HeapSnapshot::Create(JSContext* cx,
                      GlobalObject& global,
                      const uint8_t* buffer,
                      uint32_t size,
                      ErrorResult& rv)
 {
   RefPtr<HeapSnapshot> snapshot = new HeapSnapshot(cx, global.GetAsSupports());
-  if (!snapshot->init(buffer, size)) {
+  if (!snapshot->init(cx, buffer, size)) {
     rv.Throw(NS_ERROR_UNEXPECTED);
     return nullptr;
   }
   return snapshot.forget();
 }
 
 template<typename MessageType>
 static bool
@@ -198,17 +198,17 @@ HeapSnapshot::getOrInternString(Interned
 #define GET_STRING_OR_REF(msg, property)      \
   (msg.has_##property##ref()                  \
      ? Some(StringOrRef(msg.property##ref())) \
      : msg.has_##property()                   \
        ? Some(StringOrRef(&msg.property()))   \
        : Nothing())
 
 bool
-HeapSnapshot::saveNode(const protobuf::Node& node)
+HeapSnapshot::saveNode(const protobuf::Node& node, NodeIdSet& edgeReferents)
 {
   // NB: de-duplicated string properties must be read back and interned in the
   // same order here as they are written and serialized in
   // `CoreDumpWriter::writeNode` or else indices in references to already
   // serialized strings will be off.
 
   if (NS_WARN_IF(!node.has_id()))
     return false;
@@ -243,16 +243,19 @@ HeapSnapshot::saveNode(const protobuf::N
     return false;
   for (decltype(edgesLength) i = 0; i < edgesLength; i++) {
     auto& protoEdge = node.edges(i);
 
     if (NS_WARN_IF(!protoEdge.has_referent()))
       return false;
     NodeId referent = protoEdge.referent();
 
+    if (NS_WARN_IF(!edgeReferents.put(referent)))
+      return false;
+
     const char16_t* edgeName = nullptr;
     if (protoEdge.EdgeNameOrRef_case() != protobuf::Edge::EDGENAMEORREF_NOT_SET) {
       Maybe<StringOrRef> edgeNameOrRef = GET_STRING_OR_REF(protoEdge, name);
       edgeName = getOrInternString<char16_t>(internedTwoByteStrings, edgeNameOrRef);
       if (NS_WARN_IF(!edgeName))
         return false;
     }
 
@@ -388,17 +391,17 @@ StreamHasData(GzipInputStream& stream)
 
   // There is more data still available in the stream. Return the data we read
   // to the stream and let the parser get at it.
   stream.BackUp(size);
   return true;
 }
 
 bool
-HeapSnapshot::init(const uint8_t* buffer, uint32_t size)
+HeapSnapshot::init(JSContext* cx, const uint8_t* buffer, uint32_t size)
 {
   if (!nodes.init() || !frames.init())
     return false;
 
   ArrayInputStream stream(buffer, size);
   GzipInputStream gzipStream(&stream);
 
   // First is the metadata.
@@ -416,26 +419,39 @@ HeapSnapshot::init(const uint8_t* buffer
     return false;
 
   // Although the id is optional in the protobuf format for future proofing, we
   // can't currently do anything without it.
   if (NS_WARN_IF(!root.has_id()))
     return false;
   rootId = root.id();
 
-  if (NS_WARN_IF(!saveNode(root)))
+  // The set of all node ids we've found edges pointing to.
+  NodeIdSet edgeReferents(cx);
+  if (NS_WARN_IF(!edgeReferents.init()))
+    return false;
+
+  if (NS_WARN_IF(!saveNode(root, edgeReferents)))
     return false;
 
   // Finally, the rest of the nodes in the core dump.
 
   while (StreamHasData(gzipStream)) {
     protobuf::Node node;
     if (!parseMessage(gzipStream, node))
       return false;
-    if (NS_WARN_IF(!saveNode(node)))
+    if (NS_WARN_IF(!saveNode(node, edgeReferents)))
+      return false;
+  }
+
+  // Check the set of node ids referred to by edges we found and ensure that we
+  // have the node corresponding to each id. If we don't have all of them, it is
+  // unsafe to perform analyses of this heap snapshot.
+  for (auto range = edgeReferents.all(); !range.empty(); range.popFront()) {
+    if (NS_WARN_IF(!nodes.has(range.front())))
       return false;
   }
 
   return true;
 }
 
 
 /*** Heap Snapshot Analyses ***********************************************************************/
--- a/devtools/shared/heapsnapshot/HeapSnapshot.h
+++ b/devtools/shared/heapsnapshot/HeapSnapshot.h
@@ -54,21 +54,23 @@ class HeapSnapshot final : public nsISup
     , mParent(aParent)
   {
     MOZ_ASSERT(aParent);
   };
 
   // Initialize this HeapSnapshot from the given buffer that contains a
   // serialized core dump. Do NOT take ownership of the buffer, only borrow it
   // for the duration of the call. Return false on failure.
-  bool init(const uint8_t* buffer, uint32_t size);
+  bool init(JSContext* cx, const uint8_t* buffer, uint32_t size);
+
+  using NodeIdSet = js::HashSet<NodeId>;
 
   // Save the given `protobuf::Node` message in this `HeapSnapshot` as a
   // `DeserializedNode`.
-  bool saveNode(const protobuf::Node& node);
+  bool saveNode(const protobuf::Node& node, NodeIdSet& edgeReferents);
 
   // Save the given `protobuf::StackFrame` message in this `HeapSnapshot` as a
   // `DeserializedStackFrame`. The saved stack frame's id is returned via the
   // out parameter.
   bool saveStackFrame(const protobuf::StackFrame& frame,
                       StackFrameId& outFrameId);
 
 public: