mirrors/cdt
1
0
Fork 0
mirror of https://github.com/eclipse-cdt/cdt synced 2025-08-14 03:35:37 +02:00
Code Issues Wiki Activity

Bug 327375 control flow graph regression - dead connector nodes

Browse source 
basically for the code like
if (a) return 1; else return 2;
a++; // this should be a dead code node

but we don't check connector node of "if" and don't add
it to dead nodes graph resulting it this code not being marked as dead,
which can lead to false positives or false negatives for checkers that
use that. Same apply for other control statements...

Change-Id: Iafb4b48ca960f0ffab2c0265285a31cb08eb0d11
This commit is contained in:
Alena Laskavaia 2015-02-17 22:12:50 -05:00 • committed by Gerrit Code Review @ Eclipse.org
parent 3edc86361c
commit 3cf5ec86ee
3 changed files with 135 additions and 35 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

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

@ -312,16 +312,23 @@ public class ControlFlowGraphBuilder {
addOutgoing(ifNode, elseNode); addOutgoing(ifNode, elseNode);
IBasicBlock els = createSubGraph(elseNode, body.getElseClause()); IBasicBlock els = createSubGraph(elseNode, body.getElseClause());
addJump(els, mergeNode); addJump(els, mergeNode);
fixConnector(mergeNode);
return mergeNode; return mergeNode;
} }
protected void fixConnector(IConnectorNode mergeNode) {
if (mergeNode.getIncomingSize()==0)
dead.add(mergeNode); // dead connector node
}
private IBasicBlock createSwitch(IBasicBlock prev, IASTSwitchStatement body) { private IBasicBlock createSwitch(IBasicBlock prev, IASTSwitchStatement body) {
DecisionNode node = factory.createDecisionNode(body.getControllerExpression()); DecisionNode node = factory.createDecisionNode(body.getControllerExpression());
addOutgoing(prev, node); addOutgoing(prev, node);
IConnectorNode conn = factory.createConnectorNode(); IConnectorNode mergeNode = factory.createConnectorNode();
node.setMergeNode(conn); node.setMergeNode(mergeNode);
createSwitchBody(node, conn, body.getBody()); createSwitchBody(node, mergeNode, body.getBody());
return conn; fixConnector(mergeNode);
return mergeNode;
} }
private void createSwitchBody(IDecisionNode switchNode, IConnectorNode mergeNode, IASTStatement body) { private void createSwitchBody(IDecisionNode switchNode, IConnectorNode mergeNode, IASTStatement body) {
@ -380,9 +387,13 @@ public class ControlFlowGraphBuilder {
IConnectorNode savedBreak = outerBreak; IConnectorNode savedBreak = outerBreak;
outerContinue = nContinue; outerContinue = nContinue;
outerBreak = nBreak; outerBreak = nBreak;
IBasicBlock endBody = createSubGraph(loopStart, forNode.getBody()); IBasicBlock endBody = decision;
outerContinue = savedContinue; try {
outerBreak = savedBreak; endBody = createSubGraph(loopStart, forNode.getBody());
} finally {
outerContinue = savedContinue;
outerBreak = savedBreak;
}
// inc // inc
IPlainNode inc = factory.createPlainNode(forNode.getIterationExpression()); IPlainNode inc = factory.createPlainNode(forNode.getIterationExpression());
addOutgoing(endBody, nContinue); addOutgoing(endBody, nContinue);
@ -393,6 +404,7 @@ public class ControlFlowGraphBuilder {
IBranchNode loopEnd = factory.createBranchNode(IBranchNode.ELSE); IBranchNode loopEnd = factory.createBranchNode(IBranchNode.ELSE);
addOutgoing(decision, loopEnd); addOutgoing(decision, loopEnd);
addJump(loopEnd, nBreak); addJump(loopEnd, nBreak);
fixConnector(nBreak);
return nBreak; return nBreak;
} }
@ -432,6 +444,7 @@ public class ControlFlowGraphBuilder {
IBranchNode loopEnd = factory.createBranchNode(IBranchNode.ELSE); IBranchNode loopEnd = factory.createBranchNode(IBranchNode.ELSE);
addOutgoing(decision, loopEnd); addOutgoing(decision, loopEnd);
addJump(loopEnd, nBreak); addJump(loopEnd, nBreak);
fixConnector(nBreak);
return nBreak; return nBreak;
} }
@ -453,16 +466,21 @@ public class ControlFlowGraphBuilder {
IConnectorNode savedBreak = outerBreak; IConnectorNode savedBreak = outerBreak;
outerContinue = nContinue; outerContinue = nContinue;
outerBreak = nBreak; outerBreak = nBreak;
IBasicBlock endBody = createSubGraph(loopStart, body.getBody()); IBasicBlock endBody = decision;
// Restore try {
outerContinue = savedContinue; endBody = createSubGraph(loopStart, body.getBody());
outerBreak = savedBreak; } finally {
// Restore
outerContinue = savedContinue;
outerBreak = savedBreak;
}
// Backward jump // Backward jump
addJump(endBody, nContinue, true); addJump(endBody, nContinue, true);
// Connect with else branch // Connect with else branch
IBranchNode loopEnd = factory.createBranchNode(IBranchNode.ELSE); IBranchNode loopEnd = factory.createBranchNode(IBranchNode.ELSE);
addOutgoing(decision, loopEnd); addOutgoing(decision, loopEnd);
addJump(loopEnd, nBreak); addJump(loopEnd, nBreak);
fixConnector(nBreak);
return nBreak; return nBreak;
} }
@ -500,6 +518,7 @@ public class ControlFlowGraphBuilder {
// Add break connector // Add break connector
decision.setMergeNode(nBreak); decision.setMergeNode(nBreak);
addJump(loopEnd, nBreak); addJump(loopEnd, nBreak);
fixConnector(nBreak);
return nBreak; return nBreak;
} }

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

@ -10,6 +10,7 @@
*******************************************************************************/ *******************************************************************************/
package org.eclipse.cdt.codan.core.cfg; package org.eclipse.cdt.codan.core.cfg;
import java.util.ArrayList;
import java.util.Collection; import java.util.Collection;
import java.util.Iterator; import java.util.Iterator;
@ -34,7 +35,7 @@ import org.eclipse.cdt.core.dom.ast.c.CASTVisitor;
import org.eclipse.cdt.core.parser.ParserLanguage; import org.eclipse.cdt.core.parser.ParserLanguage;
/** /**
* TODO: add description * Tests for ControlFlowGraph
*/ */
public class ControlFlowGraphTest extends CodanFastCxxAstTestCase { public class ControlFlowGraphTest extends CodanFastCxxAstTestCase {
ControlFlowGraph graph; ControlFlowGraph graph;
@ -111,7 +112,7 @@ public class ControlFlowGraphTest extends CodanFastCxxAstTestCase {
fail("Block " + node + " inconsitent next/prev " + b); fail("Block " + node + " inconsitent next/prev " + b);
} }
if (node instanceof IDecisionNode && decision) { if (node instanceof IDecisionNode && decision) {
assertTrue("decision node outgoing size " + node.getOutgoingSize(), node.getOutgoingSize() > 1); assertTrue("decision node outgoing size " + node.getOutgoingSize(), node.getOutgoingSize() >= 1);
assertNotNull(((IDecisionNode) node).getMergeNode()); assertNotNull(((IDecisionNode) node).getMergeNode());
} }
} }
@ -159,10 +160,12 @@ public class ControlFlowGraphTest extends CodanFastCxxAstTestCase {
parse(code, ParserLanguage.C, true); parse(code, ParserLanguage.C, true);
processAst(tu); processAst(tu);
} }
private void buildCfg_CPP(String code) { private void buildCfg_CPP(String code) {
parse(code, ParserLanguage.CPP, true); parse(code, ParserLanguage.CPP, true);
processAst(tu); processAst(tu);
} }
/** /**
* @param des * @param des
* @return * @return
@ -434,17 +437,16 @@ public class ControlFlowGraphTest extends CodanFastCxxAstTestCase {
IBasicBlock bElse = branchEnd(des, IBranchNode.ELSE); IBasicBlock bElse = branchEnd(des, IBranchNode.ELSE);
IBasicBlock m2 = jumpEnd(branchEnd(des, IBranchNode.THEN)); IBasicBlock m2 = jumpEnd(branchEnd(des, IBranchNode.THEN));
IBasicBlock m1 = jumpEnd(bElse); IBasicBlock m1 = jumpEnd(bElse);
assertNull(m2); assertNull(m2);
assertNotNull(m1); assertNotNull(m1);
} }
// int test1_f() // int test1_f()
// { // {
// while (1) // while (1)
// { // {
// } // }
// } // }
public void test_infiniloop() { public void test_infiniloop() {
buildCfg_C(getAboveComment()); buildCfg_C(getAboveComment());
checkCfg(false); checkCfg(false);
@ -456,7 +458,6 @@ public class ControlFlowGraphTest extends CodanFastCxxAstTestCase {
IBasicBlock bThen = branchEnd(des, IBranchNode.THEN); IBasicBlock bThen = branchEnd(des, IBranchNode.THEN);
IBasicBlock m2 = jumpEnd(bThen); IBasicBlock m2 = jumpEnd(bThen);
IBasicBlock m1 = jumpEnd(bElse); IBasicBlock m1 = jumpEnd(bElse);
assertNotNull(m2); assertNotNull(m2);
assertNull(m1); assertNull(m1);
} }
@ -469,7 +470,6 @@ public class ControlFlowGraphTest extends CodanFastCxxAstTestCase {
public void test_for() { public void test_for() {
buildCfg_CPP(getAboveComment()); buildCfg_CPP(getAboveComment());
checkCfg(false); checkCfg(false);
IStartNode startNode = graph.getStartNode(); IStartNode startNode = graph.getStartNode();
IPlainNode decl = (IPlainNode) startNode.getOutgoing(); IPlainNode decl = (IPlainNode) startNode.getOutgoing();
IConnectorNode conn = (IConnectorNode) decl.getOutgoing(); IConnectorNode conn = (IConnectorNode) decl.getOutgoing();
@ -478,7 +478,6 @@ public class ControlFlowGraphTest extends CodanFastCxxAstTestCase {
IPlainNode bThen = (IPlainNode) branchEnd(des, IBranchNode.THEN); IPlainNode bThen = (IPlainNode) branchEnd(des, IBranchNode.THEN);
assertEquals("bar();", data(bThen)); assertEquals("bar();", data(bThen));
IBasicBlock bElse = branchEnd(des, IBranchNode.ELSE); IBasicBlock bElse = branchEnd(des, IBranchNode.ELSE);
IBasicBlock m1 = jumpEnd(bElse); IBasicBlock m1 = jumpEnd(bElse);
IBasicBlock m2 = bThen.getOutgoing(); IBasicBlock m2 = bThen.getOutgoing();
assertNotNull(m1); assertNotNull(m1);
@ -494,7 +493,6 @@ public class ControlFlowGraphTest extends CodanFastCxxAstTestCase {
public void test_range_loop() { public void test_range_loop() {
buildCfg_CPP(getAboveComment()); buildCfg_CPP(getAboveComment());
checkCfg(false); checkCfg(false);
IStartNode startNode = graph.getStartNode(); IStartNode startNode = graph.getStartNode();
IPlainNode decl = (IPlainNode) startNode.getOutgoing(); IPlainNode decl = (IPlainNode) startNode.getOutgoing();
IConnectorNode conn = (IConnectorNode) decl.getOutgoing(); IConnectorNode conn = (IConnectorNode) decl.getOutgoing();
@ -511,24 +509,23 @@ public class ControlFlowGraphTest extends CodanFastCxxAstTestCase {
assertEquals("", data(((IConnectorNode) m2).getOutgoing())); // increment assertEquals("", data(((IConnectorNode) m2).getOutgoing())); // increment
} }
// int main() { // int main() {
// goto b; // goto b;
// a: // a:
// return 2; // return 2;
// b: // b:
// goto a; // goto a;
// } // }
public void test_bad_labels() { public void test_bad_labels() {
buildAndCheck(getAboveComment()); buildAndCheck(getAboveComment());
assertEquals(0, graph.getUnconnectedNodeSize()); assertEquals(0, graph.getUnconnectedNodeSize());
IStartNode startNode = graph.getStartNode(); IStartNode startNode = graph.getStartNode();
IJumpNode gotoB = (IJumpNode) startNode.getOutgoing(); IJumpNode gotoB = (IJumpNode) startNode.getOutgoing();
IConnectorNode bConn = gotoB.getJumpNode(); IConnectorNode bConn = gotoB.getJumpNode();
IJumpNode gotoA = (IJumpNode) bConn.getOutgoing(); IJumpNode gotoA = (IJumpNode) bConn.getOutgoing();
IConnectorNode aConn = gotoA.getJumpNode(); IConnectorNode aConn = gotoA.getJumpNode();
IExitNode ret = (IExitNode) aConn.getOutgoing(); IExitNode ret = (IExitNode) aConn.getOutgoing();
assertEquals("return 2;", data(ret)); assertEquals("return 2;", data(ret));
} }
// int main(int a) { // int main(int a) {
@ -550,4 +547,74 @@ public class ControlFlowGraphTest extends CodanFastCxxAstTestCase {
buildAndCheck(getAboveComment()); buildAndCheck(getAboveComment());
assertEquals(0, graph.getUnconnectedNodeSize()); assertEquals(0, graph.getUnconnectedNodeSize());
} }
// int main() {
// return 1;
// a:
// return 2;
// b:
// goto a;
// }
public void test_dead_labels() {
buildAndCheck(getAboveComment());
assertEquals(1, graph.getUnconnectedNodeSize());
IStartNode startNode = graph.getStartNode();
IExitNode ret = (IExitNode) startNode.getOutgoing();
assertEquals("return 1;", data(ret));
IBranchNode labelB = (IBranchNode) graph.getUnconnectedNodeIterator().next(); // BranchNode: b:
ArrayList<IBasicBlock> res = new ArrayList<>();
graph.getNodes(labelB, res);
assertEquals(6, res.size());
IJumpNode gotoA = (IJumpNode) ((IConnectorNode) labelB.getOutgoing()).getOutgoing();
IConnectorNode aConn = gotoA.getJumpNode();
IExitNode ret2 = (IExitNode) aConn.getOutgoing();
assertEquals("return 2;", data(ret2));
assertTrue(res.contains(gotoA));// goto a;
assertTrue(res.contains(aConn));
assertTrue(res.contains(ret2)); // return 2;
assertTrue(res.contains(ret2.getIncoming())); // Branch Node: a:
}
// int main(int a) {
// if (a) {
// return 1;
// } else {
// return 2;
// }
// a++;
// }
public void test_dead_connector() {
buildAndCheck(getAboveComment());
assertEquals(1, graph.getUnconnectedNodeSize());
IConnectorNode connIf = (IConnectorNode) graph.getUnconnectedNodeIterator().next();
assertEquals("a++;", data(connIf.getOutgoing()));
}
// int main(int a) {
// for (;1;a++) {
// return 1;
// }
// a--;
// }
public void test_dead_connector_for() {
buildAndCheck(getAboveComment());
assertEquals(2, graph.getUnconnectedNodeSize());
IConnectorNode conn = (IConnectorNode) graph.getUnconnectedNodeIterator().next();
assertEquals("a++", data(conn.getOutgoing()));
}
// int main(int a) {
// while (0) {
// return 1;
// }
//
// a++;
// }
public void test_dead_connector_while() {
buildAndCheck(getAboveComment());
assertEquals(1, graph.getUnconnectedNodeSize());
IBranchNode trueBranch = (IBranchNode) graph.getUnconnectedNodeIterator().next();
assertEquals("return 1;", data(trueBranch.getOutgoing()));
}
} }

14
codan/org.eclipse.cdt.codan.core.test/src/org/eclipse/cdt/codan/core/internal/checkers/ReturnCheckerTest.java
View file

@ -223,6 +223,20 @@ public class ReturnCheckerTest extends CheckerTestCase {
loadCodeAndRunCpp(getAboveComment()); loadCodeAndRunCpp(getAboveComment());
checkNoErrors(); checkNoErrors();
} }
//int bar(int foo)
//{
// if(foo)
// return 0;
// else
// return 0;
// foo++;
//}
public void testBranchesDeadCode_Bug343767() {
loadCodeAndRunCpp(getAboveComment());
checkNoErrors();
}
// int f() // int f()
// { // {
// switch (g()) { // switch (g()) {