From 9123cda70b829920ae9730bd3b3cf6316434c553 Mon Sep 17 00:00:00 2001 From: Benjamin Smedberg Date: Wed, 3 Sep 2008 13:00:13 -0400 Subject: [PATCH] Bug 453349 - stack-only checker skips conditional blocks, r=dmandelin --- xpcom/analysis/stack.js | 148 ++++++++---------- xpcom/analysis/static-checking.js | 11 +- xpcom/tests/static-checker/Makefile.in | 1 + .../tests/static-checker/StackConditional.cpp | 15 ++ 4 files changed, 90 insertions(+), 85 deletions(-) create mode 100644 xpcom/tests/static-checker/StackConditional.cpp diff --git a/xpcom/analysis/stack.js b/xpcom/analysis/stack.js index 43362363122b..6922a33c7f7f 100644 --- a/xpcom/analysis/stack.js +++ b/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); } diff --git a/xpcom/analysis/static-checking.js b/xpcom/analysis/static-checking.js index c8637f6ee0b8..d8256710c30f 100644 --- a/xpcom/analysis/static-checking.js +++ b/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() diff --git a/xpcom/tests/static-checker/Makefile.in b/xpcom/tests/static-checker/Makefile.in index c23985efcb44..7dfb48b3c19f 100644 --- a/xpcom/tests/static-checker/Makefile.in +++ b/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 = \ diff --git a/xpcom/tests/static-checker/StackConditional.cpp b/xpcom/tests/static-checker/StackConditional.cpp new file mode 100644 index 000000000000..34cf7aba8300 --- /dev/null +++ b/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(); +}