Bug 1349417 - Placement new should preserve safety, r?bhackett draft
authorSteve Fink <sfink@mozilla.com>
Wed, 19 Apr 2017 14:47:50 -0700
changeset 565475 689fcecbcf6455b118ee656e72eb9a2f3fd73020
parent 565474 bf8468e259dce509bfe0c4cc51874a0797932e36
child 624990 725ab5f442112d7de4fe9a08a8fe0bd844586d94
push id54867
push userbmo:sphink@gmail.com
push dateWed, 19 Apr 2017 21:49:19 +0000
reviewersbhackett
bugs1349417
milestone55.0a1
Bug 1349417 - Placement new should preserve safety, r?bhackett MozReview-Commit-ID: 7bRj24L7Qqx
js/src/devtools/rootAnalysis/analyzeHeapWrites.js
--- a/js/src/devtools/rootAnalysis/analyzeHeapWrites.js
+++ b/js/src/devtools/rootAnalysis/analyzeHeapWrites.js
@@ -1051,47 +1051,57 @@ function isSafeVariable(entry, variable)
     if (edge) {
         // Treat temporary pointers to DebugOnly contents as thread local.
         if (isDirectCall(edge, /DebugOnly.*?::operator/))
             return true;
 
         // Treat heap allocated pointers as thread local during construction.
         // Hopefully the construction code doesn't leak pointers to the object
         // to places where other threads might access it.
-        if (isDirectCall(edge, /operator new/) ||
-            isDirectCall(edge, /nsCSSValue::Array::Create/))
-        {
+        if (isDirectCall(edge, /nsCSSValue::Array::Create/))
             return true;
-        }
 
-        // References to the contents of an array are threadsafe if the array
-        // itself is threadsafe.
-        if ((isDirectCall(edge, /operator\[\]/) ||
-             isDirectCall(edge, /nsStyleContent::ContentAt/)) &&
-            isEdgeSafeArgument(entry, edge.PEdgeCallInstance.Exp))
-        {
-            return true;
-        }
-
-        // Watch for the coerced result of a getter_AddRefs call.
-        if (isDirectCall(edge, /operator /)) {
-            var otherEdge = expressionValueEdge(edge.PEdgeCallInstance.Exp);
-            if (otherEdge &&
-                isDirectCall(otherEdge, /getter_AddRefs/) &&
-                isEdgeSafeArgument(entry, otherEdge.PEdgeCallArguments.Exp[0]))
+        if ("PEdgeCallInstance" in edge) {
+            // References to the contents of an array are threadsafe if the array
+            // itself is threadsafe.
+            if ((isDirectCall(edge, /operator\[\]/) ||
+                 isDirectCall(edge, /nsStyleContent::ContentAt/)) &&
+                isEdgeSafeArgument(entry, edge.PEdgeCallInstance.Exp))
             {
                 return true;
             }
-        }
+
+            // Watch for the coerced result of a getter_AddRefs call.
+            if (isDirectCall(edge, /operator /)) {
+                var otherEdge = expressionValueEdge(edge.PEdgeCallInstance.Exp);
+                if (otherEdge &&
+                    isDirectCall(otherEdge, /getter_AddRefs/) &&
+                    isEdgeSafeArgument(entry, otherEdge.PEdgeCallArguments.Exp[0]))
+                {
+                    return true;
+                }
+            }
 
-        // Coercion via AsAString preserves safety.
-        if (isDirectCall(edge, /AsAString/) &&
-            isEdgeSafeArgument(entry, edge.PEdgeCallInstance.Exp))
-        {
-            return true;
+            // Placement-new returns a pointer that is as safe as the pointer
+            // passed to it. Exp[0] is the size, Exp[1] is the pointer/address.
+            // Note that the invocation of the constructor is a separate call,
+            // and so need not be considered here.
+            if (isDirectCall(edge, /operator new/) &&
+                edge.PEdgeCallInstance.Exp.length == 2 &&
+                isEdgeSafeArgument(entry, edge.PEdgeCallInstance.Exp[1]))
+            {
+                return true;
+            }
+
+            // Coercion via AsAString preserves safety.
+            if (isDirectCall(edge, /AsAString/) &&
+                isEdgeSafeArgument(entry, edge.PEdgeCallInstance.Exp))
+            {
+                return true;
+            }
         }
 
         // Watch out for variables which were assigned arguments.
         var rhsVariable = variableAssignRhs(edge);
         if (rhsVariable)
             return isSafeVariable(entry, rhsVariable);
     }