mirrors/cdt
1
0
Fork 0
mirror of https://github.com/eclipse-cdt/cdt synced 2025-07-03 15:15:25 +02:00
Code Issues Wiki Activity

Bug 428041 - NPE in ReturnChecker

Browse source
This commit is contained in:
Sergey Prigogin 2014-02-12 12:47:45 -08:00
parent 9dabdb0469
commit 8f11a283dd
1 changed files with 33 additions and 78 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

111
codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/ReturnChecker.java
View file

@ -1,5 +1,5 @@
/******************************************************************************* /*******************************************************************************
* Copyright (c) 2009, 2012 Alena Laskavaia * Copyright (c) 2009, 2014 Alena Laskavaia
* All rights reserved. This program and the accompanying materials * All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0 * are made available under the terms of the Eclipse Public License v1.0
* which accompanies this distribution, and is available at * which accompanies this distribution, and is available at
@ -8,6 +8,7 @@
* Contributors: * Contributors:
* Alena Laskavaia - initial API and implementation * Alena Laskavaia - initial API and implementation
* Tomasz Wesolowski - Bug 348387 * Tomasz Wesolowski - Bug 348387
* Sergey Prigogin (Google)
*******************************************************************************/ *******************************************************************************/
package org.eclipse.cdt.codan.internal.checkers; package org.eclipse.cdt.codan.internal.checkers;
@ -32,6 +33,7 @@ import org.eclipse.cdt.core.dom.ast.IASTForStatement;
import org.eclipse.cdt.core.dom.ast.IASTFunctionDeclarator; import org.eclipse.cdt.core.dom.ast.IASTFunctionDeclarator;
import org.eclipse.cdt.core.dom.ast.IASTFunctionDefinition; import org.eclipse.cdt.core.dom.ast.IASTFunctionDefinition;
import org.eclipse.cdt.core.dom.ast.IASTIfStatement; import org.eclipse.cdt.core.dom.ast.IASTIfStatement;
import org.eclipse.cdt.core.dom.ast.IASTInitializerClause;
import org.eclipse.cdt.core.dom.ast.IASTLabelStatement; import org.eclipse.cdt.core.dom.ast.IASTLabelStatement;
import org.eclipse.cdt.core.dom.ast.IASTNamedTypeSpecifier; import org.eclipse.cdt.core.dom.ast.IASTNamedTypeSpecifier;
import org.eclipse.cdt.core.dom.ast.IASTReturnStatement; import org.eclipse.cdt.core.dom.ast.IASTReturnStatement;
@ -51,14 +53,14 @@ import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTTemplateDeclaration;
import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTTryBlockStatement; import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTTryBlockStatement;
import org.eclipse.cdt.core.dom.ast.cpp.ICPPConstructor; import org.eclipse.cdt.core.dom.ast.cpp.ICPPConstructor;
import org.eclipse.cdt.core.dom.ast.cpp.ICPPMethod; import org.eclipse.cdt.core.dom.ast.cpp.ICPPMethod;
import org.eclipse.cdt.core.parser.util.CharArrayUtils;
/** /**
* The checker suppose to find issue related to mismatched return value/function * The checker suppose to find issue related to mismatched return value/function
* declaration<br> * declaration<br>
* <li>Function declared as returning non-void returns void * <li>Function declared as returning non-void returns void
* <li>Function declared as returning void has non-void return * <li>Function declared as returning void has non-void return
* <li>Function declared as returning non-void has no return (requires control * <li>Function declared as returning non-void has no return (requires control flow graph)
* flow graph)
*/ */
public class ReturnChecker extends AbstractAstFunctionChecker { public class ReturnChecker extends AbstractAstFunctionChecker {
public static final String PARAM_IMPLICIT = "implicit"; //$NON-NLS-1$ public static final String PARAM_IMPLICIT = "implicit"; //$NON-NLS-1$
@ -97,21 +99,21 @@ public class ReturnChecker extends AbstractAstFunctionChecker {
public int visit(IASTStatement stmt) { public int visit(IASTStatement stmt) {
if (stmt instanceof IASTReturnStatement) { if (stmt instanceof IASTReturnStatement) {
IASTReturnStatement ret = (IASTReturnStatement) stmt; IASTReturnStatement ret = (IASTReturnStatement) stmt;
boolean hasValue = ret.getReturnArgument() != null; IASTInitializerClause returnValue = ret.getReturnArgument();
if (hasret == false && hasValue) { if (returnValue != null) {
hasret = true; hasret = true;
} }
if (!isVoid(func) && !isConstructorDestructor(func)) { if (!isVoid(func) && !isConstructorDestructor(func)) {
if (checkImplicitReturn(RET_NO_VALUE_ID) || isExplicitReturn(func)) { if (checkImplicitReturn(RET_NO_VALUE_ID) || isExplicitReturn(func)) {
if (!hasValue) if (returnValue == null)
reportProblem(RET_NO_VALUE_ID, ret); reportProblem(RET_NO_VALUE_ID, ret);
} }
} else { } else {
if (hasValue) { if (returnValue instanceof IASTExpression) {
IType type = ret.getReturnValue().getExpressionType(); IType type = ((IASTExpression) returnValue).getExpressionType();
if (isVoid(type)) if (isVoid(type))
return PROCESS_SKIP; return PROCESS_SKIP;
reportProblem(RET_ERR_VALUE_ID, ret.getReturnValue()); reportProblem(RET_ERR_VALUE_ID, returnValue);
} }
} }
return PROCESS_SKIP; return PROCESS_SKIP;
@ -120,11 +122,6 @@ public class ReturnChecker extends AbstractAstFunctionChecker {
} }
} }
/**
* @param func
* @return
*
*/
public boolean isConstructorDestructor(IASTFunctionDefinition func) { public boolean isConstructorDestructor(IASTFunctionDefinition func) {
if (func instanceof ICPPASTFunctionDefinition) { if (func instanceof ICPPASTFunctionDefinition) {
IBinding method = func.getDeclarator().getName().resolveBinding(); IBinding method = func.getDeclarator().getName().resolveBinding();
@ -137,41 +134,35 @@ public class ReturnChecker extends AbstractAstFunctionChecker {
public boolean isMain(IASTFunctionDefinition func) { public boolean isMain(IASTFunctionDefinition func) {
try { try {
String functionName = func.getDeclarator().getName().getRawSignature(); char[] functionName = func.getDeclarator().getName().getSimpleID();
if (functionName.equals("main")) { //$NON-NLS-1$ if (CharArrayUtils.equals(functionName, "main")) { //$NON-NLS-1$
return true; return true;
} }
} catch (Exception e) { } catch (RuntimeException e) {
// well, not main // Well, not main.
} }
return false; return false;
} }
/*
* (non-Javadoc)
*
* @see org.eclipse.cdt.codan.core.cxx.model.AbstractAstFunctionChecker#
* processFunction(org.eclipse.cdt.core.dom.ast.IASTFunctionDefinition)
*/
@Override @Override
protected void processFunction(IASTFunctionDefinition func) { protected void processFunction(IASTFunctionDefinition func) {
if (func.getParent() instanceof ICPPASTTemplateDeclaration) if (func.getParent() instanceof ICPPASTTemplateDeclaration)
return; // if it is template get out of here return; // If it is template get out of here.
ReturnStmpVisitor visitor = new ReturnStmpVisitor(func); ReturnStmpVisitor visitor = new ReturnStmpVisitor(func);
func.accept(visitor); func.accept(visitor);
boolean nonVoid = !isVoid(func); boolean nonVoid = !isVoid(func);
if (nonVoid && !isMain(func)) { if (nonVoid && !isMain(func)) {
// there a return but maybe it is only on one branch // There a return but maybe it is only on one branch.
IASTStatement body = func.getBody(); IASTStatement body = func.getBody();
if (body instanceof IASTCompoundStatement) { if (body instanceof IASTCompoundStatement) {
IASTStatement[] statements = ((IASTCompoundStatement) body).getStatements(); IASTStatement[] statements = ((IASTCompoundStatement) body).getStatements();
if (statements.length > 0) { if (statements.length > 0) {
IASTStatement last = statements[statements.length - 1]; IASTStatement last = statements[statements.length - 1];
// get nested statement if this is a label // Get nested statement if this is a label
while (last instanceof IASTLabelStatement) { while (last instanceof IASTLabelStatement) {
last = ((IASTLabelStatement) last).getNestedStatement(); last = ((IASTLabelStatement) last).getNestedStatement();
} }
// now check if last statement if complex (for optimization reasons, building CFG is expensive) // Now check if last statement if complex (for optimization reasons, building CFG is expensive).
if (isCompoundStatement(last)) { if (isCompoundStatement(last)) {
if (endsWithNoExitNode(func)) if (endsWithNoExitNode(func))
reportNoRet(func, visitor.hasret); reportNoRet(func, visitor.hasret);
@ -185,13 +176,10 @@ public class ReturnChecker extends AbstractAstFunctionChecker {
} }
} }
/**
* @param func
*/
protected void reportNoRet(IASTFunctionDefinition func, boolean hasRet) { protected void reportNoRet(IASTFunctionDefinition func, boolean hasRet) {
if (!hasRet) { if (!hasRet) {
// no return at all // No return at all.
if (checkImplicitReturn(RET_NORET_ID) == false && isExplicitReturn(func) == false) { if (!checkImplicitReturn(RET_NORET_ID) && !isExplicitReturn(func)) {
return; return;
} }
if (isConstructorDestructor(func)) { if (isConstructorDestructor(func)) {
@ -201,10 +189,6 @@ public class ReturnChecker extends AbstractAstFunctionChecker {
reportProblem(RET_NORET_ID, func.getDeclSpecifier()); reportProblem(RET_NORET_ID, func.getDeclSpecifier());
} }
/**
* @param last
* @return
*/
private boolean isCompoundStatement(IASTStatement last) { private boolean isCompoundStatement(IASTStatement last) {
return last instanceof IASTIfStatement || last instanceof IASTWhileStatement || return last instanceof IASTIfStatement || last instanceof IASTWhileStatement ||
last instanceof IASTDoStatement || last instanceof IASTForStatement || last instanceof IASTDoStatement || last instanceof IASTForStatement ||
@ -218,7 +202,7 @@ public class ReturnChecker extends AbstractAstFunctionChecker {
} }
/** /**
* @param if - problem id * @param id the problem id
* @return true if need to check inside functions with implicit return * @return true if need to check inside functions with implicit return
*/ */
protected boolean checkImplicitReturn(String id) { protected boolean checkImplicitReturn(String id) {
@ -226,10 +210,6 @@ public class ReturnChecker extends AbstractAstFunctionChecker {
return (Boolean) getPreference(pt, PARAM_IMPLICIT); return (Boolean) getPreference(pt, PARAM_IMPLICIT);
} }
/**
* @param func
* @return
*/
protected boolean endsWithNoExitNode(IASTFunctionDefinition func) { protected boolean endsWithNoExitNode(IASTFunctionDefinition func) {
IControlFlowGraph graph = getModelCache().getControlFlowGraph(func); IControlFlowGraph graph = getModelCache().getControlFlowGraph(func);
Iterator<IExitNode> exitNodeIterator = graph.getExitNodeIterator(); Iterator<IExitNode> exitNodeIterator = graph.getExitNodeIterator();
@ -237,9 +217,8 @@ public class ReturnChecker extends AbstractAstFunctionChecker {
for (; exitNodeIterator.hasNext();) { for (; exitNodeIterator.hasNext();) {
IExitNode node = exitNodeIterator.next(); IExitNode node = exitNodeIterator.next();
if (((ICfgData) node).getData() == null) { if (((ICfgData) node).getData() == null) {
// if it real exit node such as return, exit or throw data // If it real exit node such as return, exit or throw data will be an AST node,
// will be an ast node, if it is null it is a fake node added by the // if it is null it is a fake node added by the graph builder.
// graph builder
noexitop = true; noexitop = true;
break; break;
} }
@ -247,18 +226,10 @@ public class ReturnChecker extends AbstractAstFunctionChecker {
return noexitop; return noexitop;
} }
/**
* @param func
* @return
*/
protected boolean isExplicitReturn(IASTFunctionDefinition func) { protected boolean isExplicitReturn(IASTFunctionDefinition func) {
return getDeclSpecType(func) != IASTSimpleDeclSpecifier.t_unspecified; return getDeclSpecType(func) != IASTSimpleDeclSpecifier.t_unspecified;
} }
/**
* @param func
* @return
*/
public boolean isVoid(IASTFunctionDefinition func) { public boolean isVoid(IASTFunctionDefinition func) {
int type = getDeclSpecType(func); int type = getDeclSpecType(func);
if (type == IASTSimpleDeclSpecifier.t_void) { if (type == IASTSimpleDeclSpecifier.t_void) {
@ -274,29 +245,15 @@ public class ReturnChecker extends AbstractAstFunctionChecker {
} }
/** /**
* check if type if void * Checks if type if void.
* (uses deprecated API for compatibility with 6.0)
* *
* @param type * @param type
* @throws DOMException * @throws DOMException
*/ */
@SuppressWarnings("deprecation")
public boolean isVoid(IType type) { public boolean isVoid(IType type) {
if (type instanceof IBasicType) { return type instanceof IBasicType && ((IBasicType) type).getKind() == IBasicType.Kind.eVoid;
try {
if (((IBasicType) type).getType() == IBasicType.t_void)
return true;
} catch (DOMException e) {
return false;
}
}
return false;
} }
/**
* @param func
* @return
*/
protected int getDeclSpecType(IASTFunctionDefinition func) { protected int getDeclSpecType(IASTFunctionDefinition func) {
IASTDeclSpecifier declSpecifier = func.getDeclSpecifier(); IASTDeclSpecifier declSpecifier = func.getDeclSpecifier();
int type = -1; int type = -1;
@ -311,7 +268,6 @@ public class ReturnChecker extends AbstractAstFunctionChecker {
return type; return type;
} }
/* checker must implement @link ICheckerWithPreferences */
@Override @Override
public void initPreferences(IProblemWorkingCopy problem) { public void initPreferences(IProblemWorkingCopy problem) {
super.initPreferences(problem); super.initPreferences(problem);
@ -321,15 +277,14 @@ public class ReturnChecker extends AbstractAstFunctionChecker {
} }
/** /**
* Checks if a "void" return type is specified using the C++0x * Checks if a {@code void} return type is specified using the C++11 late-specified return type
* late-specified return type * for a given function definition.
* for a given function definition * <p>
* * For example, <code>auto f() -> void {}</code> would return {@code true}.
* For example, auto f() -> void { } would return true
* *
* @param functionDefinition * @param functionDefinition
* @return True if the function has a void (late-specified) return type, * @return {@code true} if the function has a void (late-specified) return type,
* False otherwise * {@code false} otherwise
*/ */
private boolean isAutoVoid(IASTFunctionDefinition functionDefinition) { private boolean isAutoVoid(IASTFunctionDefinition functionDefinition) {
IASTFunctionDeclarator declarator = functionDefinition.getDeclarator(); IASTFunctionDeclarator declarator = functionDefinition.getDeclarator();
@ -339,7 +294,7 @@ public class ReturnChecker extends AbstractAstFunctionChecker {
if (trailingReturnType != null) { if (trailingReturnType != null) {
IASTDeclarator abstractDeclarator = trailingReturnType.getAbstractDeclarator(); IASTDeclarator abstractDeclarator = trailingReturnType.getAbstractDeclarator();
if (abstractDeclarator != null) { if (abstractDeclarator != null) {
if (abstractDeclarator.getPointerOperators().length > 0) { if (abstractDeclarator.getPointerOperators().length != 0) {
return false; return false;
} }
IASTDeclSpecifier declSpecifier = trailingReturnType.getDeclSpecifier(); IASTDeclSpecifier declSpecifier = trailingReturnType.getDeclSpecifier();