diff --git a/codan/org.eclipse.cdt.codan.checkers/OSGI-INF/l10n/bundle.properties b/codan/org.eclipse.cdt.codan.checkers/OSGI-INF/l10n/bundle.properties index 6447ea9b5b3..153d25c41ff 100644 --- a/codan/org.eclipse.cdt.codan.checkers/OSGI-INF/l10n/bundle.properties +++ b/codan/org.eclipse.cdt.codan.checkers/OSGI-INF/l10n/bundle.properties @@ -48,6 +48,9 @@ problem.name.UnusedReturnValue = Unused return value problem.description.NoReturn = No return statement in a function which is declared to return value problem.messagePattern.NoReturn = No return, in function returning non-void problem.name.NoReturn = No return +problem.description.LocalVarReturn = Returning the address of a local variable +problem.messagePattern.LocalVarReturn = Returning the address of local variable ''{0}'' +problem.name.LocalVarReturn = Returning the address of a local variable checker.name.FormatString = Format String problem.description.FormatString = Finds statements lead to format string vulnerability (e.g. 'char[5] str; scanf("%10s", str);'\u0020 problem.messagePattern.FormatString = Format string vulnerability in ''{0}'' diff --git a/codan/org.eclipse.cdt.codan.checkers/plugin.xml b/codan/org.eclipse.cdt.codan.checkers/plugin.xml index 0c2e1b82f68..8a5c6f25084 100644 --- a/codan/org.eclipse.cdt.codan.checkers/plugin.xml +++ b/codan/org.eclipse.cdt.codan.checkers/plugin.xml @@ -119,6 +119,15 @@ messagePattern="%problem.messagePattern.NoReturn" name="%problem.name.NoReturn"> + + Function declared as returning void has non-void return *
  • Function declared as returning non-void has no return (requires control flow graph) */ +@SuppressWarnings("restriction") public class ReturnChecker extends AbstractAstFunctionChecker { public static final String PARAM_IMPLICIT = "implicit"; //$NON-NLS-1$ public static final String RET_NO_VALUE_ID = "org.eclipse.cdt.codan.checkers.noreturn"; //$NON-NLS-1$ public static final String RET_ERR_VALUE_ID = "org.eclipse.cdt.codan.checkers.errreturnvalue"; //$NON-NLS-1$ public static final String RET_NORET_ID = "org.eclipse.cdt.codan.checkers.errnoreturn"; //$NON-NLS-1$ + public static final String RET_LOCAL_ID = "org.eclipse.cdt.codan.checkers.localvarreturn"; //$NON-NLS-1$ private IType cachedReturnType = null; + private enum RetType { + BY_REF, BY_PTR + } + + private class ReturnTypeAnalyzer { + private RetType retType; + private Stack innermostOp; + + public ReturnTypeAnalyzer(RetType t) { + retType = t; + innermostOp = new Stack<>(); + } + + public RetType getType() { + return retType; + } + + public void visit(IASTInitializerClause expr) { + if (expr instanceof IASTCastExpression) { + visit((IASTCastExpression) expr); + } else if (expr instanceof IASTConditionalExpression) { + visit((IASTConditionalExpression) expr); + } else if (expr instanceof IASTIdExpression) { + visit((IASTIdExpression) expr); + } else if (expr instanceof IASTUnaryExpression) { + visit((IASTUnaryExpression) expr); + } else if (expr instanceof IASTFieldReference) { + visit((IASTFieldReference) expr); + } + } + + private void visit(IASTFieldReference expr) { + visit(expr.getFieldOwner()); + } + + private void visit(IASTCastExpression expr) { + IASTTypeId id = expr.getTypeId(); + IASTDeclarator declarator = id.getAbstractDeclarator(); + IASTPointerOperator[] ptr = declarator.getPointerOperators(); + if (ptr.length > 0 && ptr[0] instanceof ICPPASTReferenceOperator) { + innermostOp.push(IASTUnaryExpression.op_amper); + } + visit(expr.getOperand()); + if (ptr.length > 0 && ptr[0] instanceof ICPPASTReferenceOperator) { + innermostOp.pop(); + } + } + + private void visit(IASTConditionalExpression expr) { + visit(expr.getPositiveResultExpression()); + visit(expr.getNegativeResultExpression()); + } + + private void visit(IASTIdExpression expr) { + IBinding binding = expr.getName().resolveBinding(); + if (binding instanceof IVariable && !(binding instanceof IParameter) && !(binding instanceof ICPPField)) { + Integer op = null; + if (!innermostOp.empty()) + op = innermostOp.peek(); + IType t = ((IVariable) binding).getType(); + if (((IVariable) binding).isStatic()) { + return; + } + t = SemanticUtil.getNestedType(t, SemanticUtil.TDEF); + if (retType == RetType.BY_REF && !(t instanceof ICPPReferenceType)) { + if (t instanceof IPointerType && op != null && op == IASTUnaryExpression.op_star) { + return; + } + try { + IScope scope = binding.getScope(); + if (scope.getKind() == EScopeKind.eLocal) { + reportProblem(RET_LOCAL_ID, expr, binding.getName()); + } + } catch (DOMException e) { + CodanCheckersActivator.log(e); + } + } else if (retType == RetType.BY_PTR && op != null && op == IASTUnaryExpression.op_amper) { + try { + IScope scope = binding.getScope(); + if (scope.getKind() == EScopeKind.eLocal) { + reportProblem(RET_LOCAL_ID, expr, binding.getName()); + } + } catch (DOMException e) { + CodanCheckersActivator.log(e); + } + } + } + } + + private void visit(IASTUnaryExpression expr) { + innermostOp.push(expr.getOperator()); + visit(expr.getOperand()); + innermostOp.pop(); + } + } + class ReturnStmpVisitor extends ASTVisitor { private final IASTFunctionDefinition func; + private final ReturnTypeAnalyzer analyzer; boolean hasret; ReturnStmpVisitor(IASTFunctionDefinition func) { @@ -80,6 +200,24 @@ public class ReturnChecker extends AbstractAstFunctionChecker { shouldVisitExpressions = true; this.func = func; this.hasret = false; + IBinding binding = func.getDeclarator().getName().resolveBinding(); + if (binding instanceof IFunction) { + IType retType = SemanticUtil.getNestedType(((IFunction) binding).getType().getReturnType(), + SemanticUtil.TDEF); + if (retType instanceof ICPPReferenceType) { + analyzer = new ReturnTypeAnalyzer(RetType.BY_REF); + } else if (retType instanceof IPointerType) { + analyzer = new ReturnTypeAnalyzer(RetType.BY_PTR); + } else + analyzer = null; + } else + analyzer = null; + } + + RetType getType() { + if (analyzer != null) + return analyzer.getType(); + return null; } @Override @@ -110,6 +248,8 @@ public class ReturnChecker extends AbstractAstFunctionChecker { if (checkImplicitReturn(RET_NO_VALUE_ID) || isExplicitReturn(func)) { if (returnValue == null) reportProblem(RET_NO_VALUE_ID, ret); + else if (analyzer != null) + analyzer.visit(returnValue); } } else if (returnKind == ReturnTypeKind.Void) { if (returnValue instanceof IASTExpression) { @@ -190,7 +330,6 @@ public class ReturnChecker extends AbstractAstFunctionChecker { return false; } - @SuppressWarnings("restriction") // TODO: Any reason not to just expose getDeadNodes() in IControlFlowGraph? public Collection getDeadBlocks(IASTFunctionDefinition func) { IControlFlowGraph graph = getModelCache().getControlFlowGraph(func); @@ -263,7 +402,6 @@ public class ReturnChecker extends AbstractAstFunctionChecker { * @param func the function to check * @return {@code true} if the function has a non void return type */ - @SuppressWarnings("restriction") private ReturnTypeKind getReturnTypeKind(IASTFunctionDefinition func) { if (isConstructorDestructor(func)) return ReturnTypeKind.Void; diff --git a/codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/core/internal/checkers/ReturnCheckerTest.java b/codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/core/internal/checkers/ReturnCheckerTest.java index 9e24f732d79..7487bcce5bb 100644 --- a/codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/core/internal/checkers/ReturnCheckerTest.java +++ b/codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/core/internal/checkers/ReturnCheckerTest.java @@ -26,7 +26,8 @@ public class ReturnCheckerTest extends CheckerTestCase { @Override public void setUp() throws Exception { super.setUp(); - enableProblems(ReturnChecker.RET_NORET_ID, ReturnChecker.RET_ERR_VALUE_ID, ReturnChecker.RET_NO_VALUE_ID); + enableProblems(ReturnChecker.RET_NORET_ID, ReturnChecker.RET_ERR_VALUE_ID, ReturnChecker.RET_NO_VALUE_ID, + ReturnChecker.RET_LOCAL_ID); } @Override @@ -527,4 +528,99 @@ public class ReturnCheckerTest extends CheckerTestCase { public void testNoReturnWithGoto_Bug492878() throws Exception { checkSampleAboveCpp(); } + + // int& bar() { + // int a = 0; + // return a; //error here + // } + public void testReturnByRef_Bug546173() throws Exception { + checkSampleAboveCpp(); + } + + // int* bar() { + // int a = 0; + // return &a; //error here + // } + public void testReturnByPtr_Bug546173() throws Exception { + checkSampleAboveCpp(); + } + + // int& bar() { + // int a = 0; + // return reinterpret_cast(a); //error here + // } + public void testReturnByCastRef_Bug546173() throws Exception { + checkSampleAboveCpp(); + } + + // int* bar() { + // int a = 0; + // return reinterpret_cast(a); + // } + public void testReturnByCastPtr_Bug546173() throws Exception { + checkSampleAboveCpp(); + } + + // int* bar() { + // int a = 0, b = 0; + // bool cond = true; + // return cond ? &a : //error here + // &b; //error here + // } + public void testReturnByTernary_Bug546173() throws Exception { + checkSampleAboveCpp(); + } + + // struct S { int a; } + // int& bar() { + // struct S s; + // return s.a; //error here + // } + public void testReturnLocalStructField_Bug546173() throws Exception { + checkSampleAboveCpp(); + } + + // class Test { + // private: + // int field; + // public: + // int& bar() { + // return field; + // } + // } + public void testReturnClassField_Bug546173() throws Exception { + checkSampleAboveCpp(); + } + + // class Test { + // private: + // int field; + // public: + // void foo(double*); + // void (Test::*bar())(double*) { + // return &Test::foo; + // } + // } + public void testReturnClassMethod_Bug546173() throws Exception { + checkSampleAboveCpp(); + } + + //int& foo() { + // int* a = new int; + // return *a; + //} + public void testReturnRefUsingDerefPtr_Bug546173() throws Exception { + checkSampleAboveCpp(); + } + + //void foo() { + // int local; + // auto s = [&local]() { + // return &local; // ok + // }; + // int* ptr = s(); + //} + public void testReturnLambda_Bug546173() throws Exception { + checkSampleAboveCpp(); + } }