Bug 350168 - Return checker won't report errors in dead code

Also Bug 356908, Bug 348386

Change-Id: I48d2f74e05d2d6d7a7bf0589408ca90bc07a6922
Reviewed-on: https://git.eclipse.org/r/28527
Tested-by: Hudson CI
Reviewed-by: Doug Schaefer <dschaefer@qnx.com>

4 files changed, 78 insertions, 18 deletions

diff --git a/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/ReturnChecker.java b/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/ReturnChecker.java
index 8dc2458d607..09b955faddf 100644
--- a/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/ReturnChecker.java
+++ b/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/ReturnChecker.java
@@ -12,15 +12,19 @@
  *******************************************************************************/
 package org.eclipse.cdt.codan.internal.checkers;
 
+import java.util.Collection;
 import java.util.Iterator;
+import java.util.LinkedHashSet;
 
 import org.eclipse.cdt.codan.core.cxx.CxxAstUtils;
 import org.eclipse.cdt.codan.core.cxx.model.AbstractAstFunctionChecker;
 import org.eclipse.cdt.codan.core.model.IProblem;
 import org.eclipse.cdt.codan.core.model.IProblemWorkingCopy;
+import org.eclipse.cdt.codan.core.model.cfg.IBasicBlock;
 import org.eclipse.cdt.codan.core.model.cfg.ICfgData;
 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.ControlFlowGraph;
 import org.eclipse.cdt.core.dom.ast.ASTVisitor;
 import org.eclipse.cdt.core.dom.ast.DOMException;
 import org.eclipse.cdt.core.dom.ast.IASTCompoundStatement;
@@ -167,7 +171,8 @@ public class ReturnChecker extends AbstractAstFunctionChecker {
 						if (endsWithNoExitNode(func))
 							reportNoRet(func, visitor.hasret);
 					} else if (!isFuncExitStatement(last)) {
-						reportNoRet(func, visitor.hasret);
+						if (!isInDeadCode(func, last))
+							reportNoRet(func, visitor.hasret);
 					}
 				} else {
 					reportNoRet(func, false);
@@ -176,6 +181,27 @@ public class ReturnChecker extends AbstractAstFunctionChecker {
 		}
 	}
 
+	private boolean isInDeadCode(IASTFunctionDefinition func, IASTStatement last) {
+		Collection<IBasicBlock> deadBlocks = getDeadBlocks(func);
+		for (Iterator<IBasicBlock> iterator = deadBlocks.iterator(); iterator.hasNext();) {
+			IBasicBlock bb = iterator.next();
+			if (((ICfgData) bb).getData() == last)
+				return true;
+		}
+		return false;
+	}
+	
+	public Collection<IBasicBlock> getDeadBlocks(IASTFunctionDefinition func) {
+		Collection<IBasicBlock> result = new LinkedHashSet<IBasicBlock>();
+		IControlFlowGraph graph = getModelCache().getControlFlowGraph(func);
+		Iterator<IBasicBlock> unconnectedNodeIterator = graph.getUnconnectedNodeIterator();
+		for (Iterator<IBasicBlock> iterator = unconnectedNodeIterator; iterator.hasNext();) {
+			IBasicBlock block = iterator.next();
+			((ControlFlowGraph) graph).getNodes(block, result);
+		}
+		return result;
+	}
+
 	protected void reportNoRet(IASTFunctionDefinition func, boolean hasRet) {
 		if (!hasRet) {
 			// No return at all.
@@ -186,6 +212,7 @@ public class ReturnChecker extends AbstractAstFunctionChecker {
 				return;
 			}
 		}
+	
 		reportProblem(RET_NORET_ID, func.getDeclSpecifier());
 	}
 
@@ -213,17 +240,18 @@ public class ReturnChecker extends AbstractAstFunctionChecker {
 	protected boolean endsWithNoExitNode(IASTFunctionDefinition func) {
 		IControlFlowGraph graph = getModelCache().getControlFlowGraph(func);
 		Iterator<IExitNode> exitNodeIterator = graph.getExitNodeIterator();
-		boolean noexitop = false;
 		for (; exitNodeIterator.hasNext();) {
 			IExitNode node = exitNodeIterator.next();
-			if (((ICfgData) node).getData() == null) {
+			Object astNode = ((ICfgData) node).getData();
+			if (astNode == null) {
 				// If it real exit node such as return, exit or throw data will be an AST node,
 				// if it is null it is a fake node added by the graph builder.
-				noexitop = true;
-				break;
+				Collection<IBasicBlock> deadBlocks = getDeadBlocks(func);
+				if (!deadBlocks.contains(node)) // exit node is in dead code, not reporting Bug 350168
+					return true;
 			}
 		}
-		return noexitop;
+		return false;
 	}
 
 	protected boolean isExplicitReturn(IASTFunctionDefinition func) {
diff --git a/codan/org.eclipse.cdt.codan.core.cxx/src/org/eclipse/cdt/codan/core/cxx/internal/model/cfg/CxxControlFlowGraph.java b/codan/org.eclipse.cdt.codan.core.cxx/src/org/eclipse/cdt/codan/core/cxx/internal/model/cfg/CxxControlFlowGraph.java
index f7a71043cdd..9ea16e2b44a 100644
--- a/codan/org.eclipse.cdt.codan.core.cxx/src/org/eclipse/cdt/codan/core/cxx/internal/model/cfg/CxxControlFlowGraph.java
+++ b/codan/org.eclipse.cdt.codan.core.cxx/src/org/eclipse/cdt/codan/core/cxx/internal/model/cfg/CxxControlFlowGraph.java
@@ -33,14 +33,8 @@ public class CxxControlFlowGraph extends ControlFlowGraph {
 		return new ControlFlowGraphBuilder().build(def);
 	}
 
-	/*
-	 * (non-Javadoc)
-	 * 
-	 * @see java.lang.Object#toString()
-	 */
 	@Override
 	public String toString() {
-		// TODO Auto-generated method stub
 		return super.toString();
 	}
 }
diff --git a/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/internal/checkers/ReturnCheckerTest.java b/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/internal/checkers/ReturnCheckerTest.java
index a3507569211..d56ff169f05 100644
--- a/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/internal/checkers/ReturnCheckerTest.java
+++ b/codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/internal/checkers/ReturnCheckerTest.java
@@ -346,4 +346,42 @@ public class ReturnCheckerTest extends CheckerTestCase {
 			loadCodeAndRunCpp(getAboveComment());
 			checkErrorLine(1);
 	}
+
+	//	int retindead() {
+	//			return 5;
+	//  ;
+	//	}
+	public void testRetInDeadCode1() throws Exception {
+		// bug 348386
+		loadCodeAndRunCpp(getAboveComment());
+		checkNoErrors();
+	}
+
+	//	int retindead() {
+	//			throw 42;
+	//  ;
+	//	}
+	public void testRetInDeadCodeThrow() throws Exception {
+		// bug 356908
+		loadCodeAndRunCpp(getAboveComment());
+		checkNoErrors();
+	}
+
+//	bool func( int i )
+//	{
+//	    switch( i )
+//	    {
+//	    case 0:
+//	        return true;
+//	    default:
+//	        return false;
+//	        break;
+//	    }
+//	}
+	public void testRetInDeadCodeCase() throws Exception {
+		// Bug 350168
+		loadCodeAndRunCpp(getAboveComment());
+		checkNoErrors();
+	}
+
 }
\ No newline at end of file
diff --git a/codan/org.eclipse.cdt.codan.core/src/org/eclipse/cdt/codan/internal/core/cfg/ControlFlowGraph.java b/codan/org.eclipse.cdt.codan.core/src/org/eclipse/cdt/codan/internal/core/cfg/ControlFlowGraph.java
index eef2262e921..c6d7003038e 100644
--- a/codan/org.eclipse.cdt.codan.core/src/org/eclipse/cdt/codan/internal/core/cfg/ControlFlowGraph.java
+++ b/codan/org.eclipse.cdt.codan.core/src/org/eclipse/cdt/codan/internal/core/cfg/ControlFlowGraph.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2009, 2010 Alena Laskavaia 
+ * Copyright (c) 2009, 2010 Alena Laskavaia
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -61,7 +61,7 @@ public class ControlFlowGraph implements IControlFlowGraph {
 
 	/*
 	 * (non-Javadoc)
-	 * 
+	 *
 	 * @seeorg.eclipse.cdt.codan.provisional.core.model.cfg.IControlFlowGraph#
 	 * getStartNode()
 	 */
@@ -96,7 +96,7 @@ public class ControlFlowGraph implements IControlFlowGraph {
 
 	/*
 	 * (non-Javadoc)
-	 * 
+	 *
 	 * @seeorg.eclipse.cdt.codan.provisional.core.model.cfg.IControlFlowGraph#
 	 * getUnconnectedNodeIterator()
 	 */
@@ -107,7 +107,7 @@ public class ControlFlowGraph implements IControlFlowGraph {
 
 	/*
 	 * (non-Javadoc)
-	 * 
+	 *
 	 * @seeorg.eclipse.cdt.codan.provisional.core.model.cfg.IControlFlowGraph#
 	 * getUnconnectedNodeSize()
 	 */
@@ -118,7 +118,7 @@ public class ControlFlowGraph implements IControlFlowGraph {
 
 	/*
 	 * (non-Javadoc)
-	 * 
+	 *
 	 * @see org.eclipse.cdt.codan.core.model.cfg.IControlFlowGraph#getNodes ()
 	 */
 	@Override
@@ -136,7 +136,7 @@ public class ControlFlowGraph implements IControlFlowGraph {
 	 * @param d
 	 * @param result
 	 */
-	private void getNodes(IBasicBlock start, Collection<IBasicBlock> result) {
+	public void getNodes(IBasicBlock start, Collection<IBasicBlock> result) {
 		if (start == null)
 			return; // huh
 		if (result.contains(start))