Bug 311268 fixed some f.p. in return checker

codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/ReturnChecker.java

@@ -10,7 +10,13 @@
  *******************************************************************************/
 package org.eclipse.cdt.codan.internal.checkers;
 
+import java.util.Iterator;
+
 import org.eclipse.cdt.codan.core.cxx.model.AbstractAstFunctionChecker;
+import org.eclipse.cdt.codan.core.cxx.model.CxxModelsCache;
+import org.eclipse.cdt.codan.core.model.cfg.IControlFlowGraph;
+import org.eclipse.cdt.codan.core.model.cfg.IExitNode;
+import org.eclipse.cdt.codan.internal.core.cfg.AbstractBasicBlock;
 import org.eclipse.cdt.core.dom.ast.ASTVisitor;
 import org.eclipse.cdt.core.dom.ast.IASTDeclSpecifier;
 import org.eclipse.cdt.core.dom.ast.IASTFunctionDeclarator;
@@ -47,7 +53,7 @@ public class ReturnChecker extends AbstractAstFunctionChecker {
 			if (stmt instanceof IASTReturnStatement) {
 				IASTReturnStatement ret = (IASTReturnStatement) stmt;
 				if (!isVoid(func)) {
-					if (getDeclSpecType(func) != ICASTSimpleDeclSpecifier.t_unspecified) {
+					if (isExplicitReturn(func)) {
 						if (ret.getReturnValue() == null)
 							reportProblem(RET_NO_VALUE_ID, ret);
 					}
@@ -74,13 +80,45 @@ public class ReturnChecker extends AbstractAstFunctionChecker {
 		func.accept(visitor);
 		if (!visitor.hasret) {
 			// no return at all
-			if (!isVoid(func)
+			if (!isVoid(func) && isExplicitReturn(func)) {
-					&& getDeclSpecType(func) != ICASTSimpleDeclSpecifier.t_unspecified) {
+				if (endsWithNoExitNode(func))
 					reportProblem(RET_NORET_ID, func.getDeclSpecifier());
 			}
 		}
 	}
 
+	/**
+	 * @param func
+	 * @return
+	 */
+	@SuppressWarnings("restriction")
+	protected boolean endsWithNoExitNode(IASTFunctionDefinition func) {
+		IControlFlowGraph graph = CxxModelsCache.getInstance()
+				.getControlFlowGraph(func);
+		Iterator<IExitNode> exitNodeIterator = graph
+				.getExitNodeIterator();
+		boolean noexitop = false;
+		for (; exitNodeIterator.hasNext();) {
+			IExitNode node = exitNodeIterator.next();
+			if (((AbstractBasicBlock) node).getData() == null) {
+				// if it real exit node such as return, exit or throw data
+				// will be an ast node, it is null it is fake node added by the
+				// graph builder
+				noexitop = true;
+				break;
+			}
+		}
+		return noexitop;
+	}
+
+	/**
+	 * @param func
+	 * @return
+	 */
+	protected boolean isExplicitReturn(IASTFunctionDefinition func) {
+		return getDeclSpecType(func) != ICASTSimpleDeclSpecifier.t_unspecified;
+	}
+
 	/**
 	 * @param func
 	 * @return

codan/org.eclipse.cdt.codan.core.cxx/src/org/eclipse/cdt/codan/core/cxx/internal/model/cfg/ControlFlowGraphBuilder.java

@@ -31,8 +31,10 @@ import org.eclipse.cdt.core.dom.ast.IASTContinueStatement;
 import org.eclipse.cdt.core.dom.ast.IASTDeclarationStatement;
 import org.eclipse.cdt.core.dom.ast.IASTDefaultStatement;
 import org.eclipse.cdt.core.dom.ast.IASTDoStatement;
+import org.eclipse.cdt.core.dom.ast.IASTExpression;
 import org.eclipse.cdt.core.dom.ast.IASTExpressionStatement;
 import org.eclipse.cdt.core.dom.ast.IASTForStatement;
+import org.eclipse.cdt.core.dom.ast.IASTFunctionCallExpression;
 import org.eclipse.cdt.core.dom.ast.IASTFunctionDefinition;
 import org.eclipse.cdt.core.dom.ast.IASTGotoStatement;
 import org.eclipse.cdt.core.dom.ast.IASTIfStatement;
@@ -43,6 +45,7 @@ import org.eclipse.cdt.core.dom.ast.IASTProblemStatement;
 import org.eclipse.cdt.core.dom.ast.IASTReturnStatement;
 import org.eclipse.cdt.core.dom.ast.IASTStatement;
 import org.eclipse.cdt.core.dom.ast.IASTSwitchStatement;
+import org.eclipse.cdt.core.dom.ast.IASTUnaryExpression;
 import org.eclipse.cdt.core.dom.ast.IASTWhileStatement;
 
 /**
@@ -72,6 +75,7 @@ public class ControlFlowGraphBuilder {
 			returnExit = (CxxExitNode) factory.createExitNode(null);
 			returnExit.setStartNode(start);
 			addOutgoing(last, returnExit);
+			exits.add(returnExit);
 		}
 		CxxControlFlowGraph graph = new CxxControlFlowGraph(start, exits);
 		graph.setUnconnectedNodes(dead);
@@ -94,6 +98,10 @@ public class ControlFlowGraphBuilder {
 		} else if (body instanceof IASTExpressionStatement
 				|| body instanceof IASTDeclarationStatement
 				|| body instanceof IASTNullStatement) {
+			if (isThrowStatement(body) || isExitStatement(body)) {
+				CxxExitNode node = createExitNode(prev, body);
+				return node;
+			}
 			CxxPlainNode node = factory.createPlainNode(body);
 			addOutgoing(prev, node);
 			return node;
@@ -106,9 +114,7 @@ public class ControlFlowGraphBuilder {
 		} else if (body instanceof IASTDoStatement) {
 			return createDoWhile(prev, (IASTDoStatement) body);
 		} else if (body instanceof IASTReturnStatement) {
-			CxxExitNode node = factory.createExitNode(body);
+			CxxExitNode node = createExitNode(prev, body);
-			node.setStartNode(start);
-			addOutgoing(prev, node);
 			return node;
 		} else if (body instanceof IASTBreakStatement) {
 			if (outerBreak != null)
@@ -160,6 +166,37 @@ public class ControlFlowGraphBuilder {
 		return prev;
 	}
 
+	/**
+	 * @param body
+	 * @return
+	 */
+	private boolean isThrowStatement(IASTNode body) {
+		if (!(body instanceof IASTExpressionStatement)) return false;
+		IASTExpression expression = ((IASTExpressionStatement) body).getExpression();
+		if (!(expression instanceof IASTUnaryExpression)) return false;
+		return ((IASTUnaryExpression) expression).getOperator() == IASTUnaryExpression.op_throw;
+	}
+	private boolean isExitStatement(IASTNode body) {
+		if (!(body instanceof IASTExpressionStatement)) return false;
+		IASTExpression expression = ((IASTExpressionStatement) body).getExpression();
+		if (!(expression instanceof IASTFunctionCallExpression)) return false;
+		IASTExpression functionNameExpression = ((IASTFunctionCallExpression) expression).getFunctionNameExpression();
+		return functionNameExpression.getRawSignature().equals("exit"); //$NON-NLS-1$
+	}
+
+	/**
+	 * @param prev
+	 * @param body
+	 * @return
+	 */
+	protected CxxExitNode createExitNode(IBasicBlock prev, IASTNode body) {
+		CxxExitNode node = factory.createExitNode(body);
+		node.setStartNode(start);
+		addOutgoing(prev, node);
+		exits.add(node);
+		return node;
+	}
+
 	/**
 	 * @param prev
 	 * @param labelName

codan/org.eclipse.cdt.codan.core.cxx/src/org/eclipse/cdt/codan/core/cxx/model/CxxModelsCache.java

@@ -10,7 +10,12 @@
  *******************************************************************************/
 package org.eclipse.cdt.codan.core.cxx.model;
 
+import java.util.WeakHashMap;
+
+import org.eclipse.cdt.codan.core.cxx.internal.model.cfg.CxxControlFlowGraph;
+import org.eclipse.cdt.codan.core.model.cfg.IControlFlowGraph;
 import org.eclipse.cdt.core.CCorePlugin;
+import org.eclipse.cdt.core.dom.ast.IASTFunctionDefinition;
 import org.eclipse.cdt.core.dom.ast.IASTTranslationUnit;
 import org.eclipse.cdt.core.index.IIndex;
 import org.eclipse.cdt.core.model.CoreModel;
@@ -27,6 +32,7 @@ public class CxxModelsCache {
 	private IASTTranslationUnit ast;
 	private ITranslationUnit tu;
 	private IIndex index;
+	private WeakHashMap<IASTFunctionDefinition, IControlFlowGraph> cfgmap = new WeakHashMap<IASTFunctionDefinition, IControlFlowGraph>(0);
 
 	private static CxxModelsCache instance = new CxxModelsCache();
 
@@ -34,12 +40,20 @@ public class CxxModelsCache {
 		return instance;
 	}
 
+	public synchronized IControlFlowGraph getControlFlowGraph(IASTFunctionDefinition func) {
+		IControlFlowGraph cfg = cfgmap.get(func);
+		if (cfg!=null) return cfg;
+		cfg = CxxControlFlowGraph.build(func);
+		cfgmap.put(func, cfg);
+		return cfg;
+	}
 	public synchronized IASTTranslationUnit getAst(IFile file)
 			throws CoreException, InterruptedException {
 		if (file.equals(this.file)) {
 			return ast;
 		}
 
+		cfgmap.clear();
 		// create translation unit and access index
 		ICElement celement = CoreModel.getDefault().create(file);
 		if (!(celement instanceof ITranslationUnit))

codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/cfg/ControlFlowGraphTest.java

@@ -324,4 +324,36 @@ public class ControlFlowGraphTest extends CodanTestCase {
 		IBasicBlock m3 = jumpEnd(m2);
 		assertSame(m1, m3);
 	}
+	/*-
+	 <code file="test_throw.cc">
+	 int foo() {
+	 	throw 5;
+	 }
+	 </code>
+	 */
+	public void test_throw() {
+		buildAndCheck("test_throw.cc");
+		IStartNode startNode = graph.getStartNode();
+		assertEquals(1, graph.getExitNodeSize());
+		Iterator<IExitNode> exitNodeIterator = graph.getExitNodeIterator();
+		IExitNode exit = exitNodeIterator.next();
+
+		assertEquals("throw 5;", data(exit));
+	}
+	/*-
+	 <code file="test_exit.c">
+	 int foo() {
+	 	exit(0);
+	 }
+	 </code>
+	 */
+	public void test_exit() {
+		buildAndCheck("test_exit.c");
+		IStartNode startNode = graph.getStartNode();
+		assertEquals(1, graph.getExitNodeSize());
+		Iterator<IExitNode> exitNodeIterator = graph.getExitNodeIterator();
+		IExitNode exit = exitNodeIterator.next();
+
+		assertEquals("exit(0);", data(exit));
+	}
 }