Bug 453349 - stack-only checker skips conditional blocks, r=dmandelin

xpcom/analysis/stack.js

@@ -108,109 +108,91 @@ function isVoidPtr(t)
   return t.isPointer && t.type.name == 'void';
 }
 
-/**
- * Detect a call to operator new. If this is operator new, and not
- * placement-new, return the VAR_DECL of the temporary variable that operator
- * new is always assigned to.
- */
-function operator_new_assign(stmt)
+function xrange(start, end, skip)
 {
-  try {
-    stmt.tree_check(GIMPLE_MODIFY_STMT);
-    let [varDecl, callExpr] = stmt.operands();
-    varDecl.tree_check(VAR_DECL);
-    callExpr.tree_check(CALL_EXPR);
-    
-    let fncall = callable_arg_function_decl(CALL_EXPR_FN(callExpr)).
-                   tree_check(FUNCTION_DECL);
-
-    let nameid = DECL_NAME(fncall);
-    if (IDENTIFIER_OPNAME_P(nameid)) {
-      let name = IDENTIFIER_POINTER(nameid);
-      if (name == "operator new" || name == "operator new []") {
-        // if this is placement-new, ignore it (should we issue a warning?)
-        let fncallobj = dehydra_convert(TREE_TYPE(fncall));
-        if (fncallobj.parameters.length == 2 &&
-            isVoidPtr(fncallobj.parameters[1]))
-          return null;
-
-        return varDecl;
-      }
-    }
-  }
-  catch (e if e.TreeCheckError) { }
-
-  return null;
+  for (;
+       (skip > 0) ? (start < end) : (start > end);
+       start += skip)
+    yield start;
 }
 
-function process_tree(fndecl)
+function process_cp_pre_genericize(fndecl)
 {
   function findconstructors(t, stack)
   {
-    function getLocation(t) {
-      let loc;
-      if (t) {
-        loc = location_of(t);
-        if (loc !== undefined)
-          return loc;
-      }
+    function getLocation() {
+      let loc = location_of(t);
+      if (loc !== undefined)
+        return loc;
 
       for (let i = stack.length - 1; i >= 0; --i) {
-        let loc = location_of(stack[i]);
+        loc = location_of(stack[i]);
         if (loc !== undefined)
           return loc;
       }
       return location_of(DECL_SAVED_TREE(fndecl));
     }
     
-    function check_opnew_assignment(varDecl, stmt)
-    {
-      if (TREE_CODE(stmt) != GIMPLE_MODIFY_STMT) {
-        warning("operator new not followed by a GIMPLE_MODIFY_STMT: " + TREE_CODE(stmt), getLocation(stmt));
+    try {
+      t.tree_check(CALL_EXPR);
+      let fncall =
+        callable_arg_function_decl(CALL_EXPR_FN(t));
+      if (fncall == null)
         return;
+
+      let nameid = DECL_NAME(fncall);
+      if (IDENTIFIER_OPNAME_P(nameid)) {
+        let name = IDENTIFIER_POINTER(nameid);
+        
+        if (name == "operator new" || name == "operator new []") {
+          let fncallobj = dehydra_convert(TREE_TYPE(fncall));
+          if (fncallobj.parameters.length == 2 &&
+              isVoidPtr(fncallobj.parameters[1]))
+            return;
+
+          let i;
+          for (i in xrange(stack.length - 1, -1, -1)) {
+            if (TREE_CODE(stack[i]) != NOP_EXPR)
+              break;
+          }
+          let assign = stack[i];
+          switch (TREE_CODE(assign)) {
+          case VAR_DECL:
+            break;
+            
+          case INIT_EXPR:
+          case MODIFY_EXPR:
+          case TARGET_EXPR:
+            assign = assign.operands()[1];
+            break;
+
+          case CALL_EXPR:
+            assign = stack[i + 1];
+            break;
+            
+          default:
+            error("Unrecognized assignment from operator new: " + TREE_CODE(assign), getLocation());
+            return;
+          }
+          
+          let destType = dehydra_convert(TREE_TYPE(assign));
+          if (!destType.isPointer && !destType.isReference) {
+            error("operator new not assigned to pointer/ref?", getLocation());
+            return;
+          }
+          destType = destType.type;
+
+          let r = isStack(destType);
+          if (r)
+            error("constructed object of type '%s' not on the stack: %s".format(destType.name, r), getLocation());
+        }
       }
-
-      let [destVar, assign] = stmt.operands();
-      if (TREE_CODE(assign) == NOP_EXPR)
-        assign = assign.operands()[0];
-
-      if (assign != varDecl) {
-        warning("operator new not followed by an known assignment pattern", getLocation(stmt));
-        return;
-      }
-
-      let destType = dehydra_convert(TREE_TYPE(destVar));
-      if (!destType.isPointer && !destType.isReference) {
-        error("operator new not assigned to pointer/ref?", getLocation(stmt));
-        return;
-      }
-      destType = destType.type;
-
-      let r = isStack(destType);
-      if (r != null)
-        error('constructed object of type %s on the heap\n%s'.format(destType.name, r), getLocation(stmt));
     }
-
-    if (TREE_CODE(t) != STATEMENT_LIST)
-      return;
-
-    // if we find a tuple of "operator new" / GIMPLE_MODIFY_STMT casting
-    // the result of operator new to a pointer type
-    let opnew = null;
-    for (let stmt in iter_statement_list(t)) {
-      if (opnew != null)
-        check_opnew_assignment(opnew, stmt);
-
-      opnew = operator_new_assign(stmt);
-    }
-
-    if (opnew != null)
-      warning("operator new not followed by an assignment", getLocation());
+    catch (e if e.TreeCheckError) { }
   }
 
   if (hasAttribute(dehydra_convert(fndecl), 'NS_suppress_stackcheck'))
     return;
   
-  let tmap = new Map();
-  walk_tree(DECL_SAVED_TREE(fndecl), findconstructors, tmap);
+  walk_tree(DECL_SAVED_TREE(fndecl), findconstructors);
 }

xpcom/analysis/static-checking.js

@@ -72,11 +72,18 @@ function process_tree(fndecl)
       module.process_tree(fndecl);
 }
 
-function process_var(decl)
+function process_decl(decl)
 {
   for each (let module in modules)
     if (module.hasOwnProperty('process_var'))
-      module.process_var(decl);
+      module.process_decl(decl);
+}
+
+function process_cp_pre_genericize(fndecl)
+{
+  for each (let module in modules)
+    if (module.hasOwnProperty('process_cp_pre_genericize'))
+      module.process_cp_pre_genericize(fndecl);
 }
 
 function input_end()

xpcom/tests/static-checker/Makefile.in

@@ -57,6 +57,7 @@ STACK_FAILURE_TESTCASES = \
   TestStackTemplate.cpp \
   StackNoConstructor.cpp \
   StackVector.cpp \
+  StackConditional.cpp \
   $(NULL)
 
 STACK_PASS_TESTCASES = \

xpcom/tests/static-checker/StackConditional.cpp

@@ -0,0 +1,15 @@
+#define NS_STACK_CLASS __attribute__((user("NS_stack")))
+
+struct NS_STACK_CLASS A
+{
+  int i;
+};
+
+void
+GetIt(int i)
+{
+  A *a;
+
+  if (i)
+    a = new A();
+}