mirror of
https://github.com/eclipse-cdt/cdt
synced 2025-09-08 02:53:12 +02:00
Bug 545956 - Added checker for virtual methods in ctor/dtor
Change-Id: I63b8a40447e9a5b6080e046030677a13607c4ea3 Signed-off-by: Marco Stornelli <marco.stornelli@gmail.com>
This commit is contained in:
Marco Stornelli
parent
1065ee7688
commit
1204bf21b1
5 changed files with 477 additions and 0 deletions
|
|
@ -163,3 +163,8 @@ problem.description.MissCaseProblem = This rule will flag switch statements with
|
|||
problem.name.MissDefaultProblem = Missing default in switch
|
||||
problem.messagePattern.MissDefaultProblem = When default case is missing the switch is not complete for all values
|
||||
problem.description.MissDefaultProblem = This rule will flag switch statements without a default case
|
||||
|
||||
checker.name.VirtualMethodCallChecker = Virtual method call checker
|
||||
problem.name.VirtualMethodCallProblem = Virtual method call in constructor/destructor
|
||||
problem.messagePattern.VirtualMethodCallProblem = Calling a virtual method in constructor or destructor can cause crashes and unexpected behavior
|
||||
problem.description.VirtualMethodCallProblem = This rule will flag constructor and destructor with virtual method calls
|
||||
|
|
|
|||
|
|
@ -524,5 +524,20 @@
|
|||
name="%problem.name.MissDefaultProblem">
|
||||
</problem>
|
||||
</checker>
|
||||
<checker
|
||||
class="org.eclipse.cdt.codan.internal.checkers.VirtualMethodCallChecker"
|
||||
id="org.eclipse.cdt.codan.internal.checkers.VirtualMethodCallChecker"
|
||||
name="%checker.name.VirtualMethodCallChecker">
|
||||
<problem
|
||||
category="org.eclipse.cdt.codan.core.categories.ProgrammingProblems"
|
||||
defaultEnabled="false"
|
||||
defaultSeverity="Error"
|
||||
description="%problem.description.VirtualMethodCallProblem"
|
||||
id="org.eclipse.cdt.codan.internal.checkers.VirtualMethodCallProblem"
|
||||
markerType="org.eclipse.cdt.codan.core.codanProblem"
|
||||
messagePattern="%problem.messagePattern.VirtualMethodCallProblem"
|
||||
name="%problem.name.VirtualMethodCallProblem">
|
||||
</problem>
|
||||
</checker>
|
||||
</extension>
|
||||
</plugin>
|
||||
|
|
|
|||
|
|
@ -0,0 +1,207 @@
|
|||
/*******************************************************************************
|
||||
* Copyright (c) 2019 Marco Stornelli
|
||||
*
|
||||
* This program and the accompanying materials
|
||||
* are made available under the terms of the Eclipse Public License 2.0
|
||||
* which accompanies this distribution, and is available at
|
||||
* https://www.eclipse.org/legal/epl-2.0/
|
||||
*
|
||||
* SPDX-License-Identifier: EPL-2.0
|
||||
*
|
||||
*******************************************************************************/
|
||||
package org.eclipse.cdt.codan.internal.checkers;
|
||||
|
||||
import java.util.Stack;
|
||||
|
||||
import org.eclipse.cdt.codan.core.cxx.model.AbstractIndexAstChecker;
|
||||
import org.eclipse.cdt.core.dom.ast.ASTVisitor;
|
||||
import org.eclipse.cdt.core.dom.ast.IASTDeclSpecifier;
|
||||
import org.eclipse.cdt.core.dom.ast.IASTDeclaration;
|
||||
import org.eclipse.cdt.core.dom.ast.IASTExpression;
|
||||
import org.eclipse.cdt.core.dom.ast.IASTFieldReference;
|
||||
import org.eclipse.cdt.core.dom.ast.IASTFunctionCallExpression;
|
||||
import org.eclipse.cdt.core.dom.ast.IASTIdExpression;
|
||||
import org.eclipse.cdt.core.dom.ast.IASTLiteralExpression;
|
||||
import org.eclipse.cdt.core.dom.ast.IASTName;
|
||||
import org.eclipse.cdt.core.dom.ast.IASTNode;
|
||||
import org.eclipse.cdt.core.dom.ast.IASTTranslationUnit;
|
||||
import org.eclipse.cdt.core.dom.ast.IASTUnaryExpression;
|
||||
import org.eclipse.cdt.core.dom.ast.IBinding;
|
||||
import org.eclipse.cdt.core.dom.ast.ICompositeType;
|
||||
import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTCompositeTypeSpecifier;
|
||||
import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTConstructorChainInitializer;
|
||||
import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTFunctionDefinition;
|
||||
import org.eclipse.cdt.core.dom.ast.cpp.ICPPASTUnaryExpression;
|
||||
import org.eclipse.cdt.core.dom.ast.cpp.ICPPClassType;
|
||||
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.SemanticQueries;
|
||||
import org.eclipse.cdt.internal.core.dom.parser.cpp.ClassTypeHelper;
|
||||
import org.eclipse.cdt.internal.core.dom.parser.cpp.ICPPDeferredClassInstance;
|
||||
|
||||
@SuppressWarnings("restriction")
|
||||
public class VirtualMethodCallChecker extends AbstractIndexAstChecker {
|
||||
public static final String VIRTUAL_CALL_ID = "org.eclipse.cdt.codan.internal.checkers.VirtualMethodCallProblem"; //$NON-NLS-1$
|
||||
|
||||
@Override
|
||||
public void processAst(IASTTranslationUnit ast) {
|
||||
ast.accept(new OnEachClass());
|
||||
}
|
||||
|
||||
class OnEachClass extends ASTVisitor {
|
||||
// NOTE: Classes can be nested and even can be declared in constructors of the other classes
|
||||
private final Stack<Boolean> ctorDtorStack = new Stack<>();
|
||||
|
||||
OnEachClass() {
|
||||
shouldVisitDeclarations = true;
|
||||
shouldVisitExpressions = true;
|
||||
shouldVisitDeclSpecifiers = true;
|
||||
}
|
||||
|
||||
@Override
|
||||
public int visit(IASTDeclSpecifier declSpec) {
|
||||
if (declSpec instanceof ICPPASTCompositeTypeSpecifier && !ctorDtorStack.isEmpty()) {
|
||||
ctorDtorStack.push(false);
|
||||
}
|
||||
return PROCESS_CONTINUE;
|
||||
}
|
||||
|
||||
@Override
|
||||
public int leave(IASTDeclSpecifier declSpec) {
|
||||
if (declSpec instanceof ICPPASTCompositeTypeSpecifier && !ctorDtorStack.isEmpty()) {
|
||||
ctorDtorStack.pop();
|
||||
}
|
||||
return PROCESS_CONTINUE;
|
||||
}
|
||||
|
||||
@Override
|
||||
public int visit(IASTDeclaration declaration) {
|
||||
ICPPConstructor constructor = getConstructor(declaration);
|
||||
if (constructor != null) {
|
||||
ctorDtorStack.push(true);
|
||||
} else {
|
||||
ICPPMethod destructor = getDestructor(declaration);
|
||||
if (destructor != null) {
|
||||
ctorDtorStack.push(true);
|
||||
}
|
||||
}
|
||||
return PROCESS_CONTINUE;
|
||||
}
|
||||
|
||||
@Override
|
||||
public int leave(IASTDeclaration declaration) {
|
||||
if (getConstructor(declaration) != null || getDestructor(declaration) != null) {
|
||||
ctorDtorStack.pop();
|
||||
}
|
||||
return PROCESS_CONTINUE;
|
||||
}
|
||||
|
||||
@Override
|
||||
public int visit(IASTExpression expression) {
|
||||
if (!ctorDtorStack.empty() && ctorDtorStack.peek()) {
|
||||
if (expression instanceof IASTFunctionCallExpression) {
|
||||
IASTFunctionCallExpression fCall = (IASTFunctionCallExpression) expression;
|
||||
IASTExpression fNameExp = fCall.getFunctionNameExpression();
|
||||
IBinding fBinding = null;
|
||||
IASTNode problemNode = expression;
|
||||
if (fNameExp instanceof IASTIdExpression) {
|
||||
IASTIdExpression fName = (IASTIdExpression) fNameExp;
|
||||
fBinding = fName.getName().resolveBinding();
|
||||
} else if (fNameExp instanceof IASTFieldReference) {
|
||||
IASTFieldReference fName = (IASTFieldReference) fNameExp;
|
||||
problemNode = fName.getFieldName();
|
||||
if (referencesThis(fName.getFieldOwner()))
|
||||
fBinding = fName.getFieldName().resolveBinding();
|
||||
}
|
||||
if (fBinding instanceof ICPPMethod) {
|
||||
ICPPMethod method = (ICPPMethod) fBinding;
|
||||
if (method.isPureVirtual() || ClassTypeHelper.isVirtual(method)) {
|
||||
reportProblem(VIRTUAL_CALL_ID, problemNode);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
return PROCESS_CONTINUE;
|
||||
}
|
||||
|
||||
/**
|
||||
* Checks that specified declaration is a class constructor
|
||||
* (it is a class member and its name is equal to the class name)
|
||||
*/
|
||||
private ICPPConstructor getConstructor(IASTDeclaration decl) {
|
||||
if (decl instanceof ICPPASTFunctionDefinition) {
|
||||
ICPPASTFunctionDefinition functionDefinition = (ICPPASTFunctionDefinition) decl;
|
||||
if (functionDefinition.isDeleted())
|
||||
return null;
|
||||
IBinding binding = functionDefinition.getDeclarator().getName().resolveBinding();
|
||||
if (binding instanceof ICPPConstructor) {
|
||||
ICPPConstructor constructor = (ICPPConstructor) binding;
|
||||
// Skip defaulted copy and move constructors.
|
||||
if (functionDefinition.isDefaulted() && SemanticQueries.isCopyOrMoveConstructor(constructor))
|
||||
return null;
|
||||
if (constructor.getClassOwner().getKey() == ICompositeType.k_union)
|
||||
return null;
|
||||
// Skip delegating constructors.
|
||||
for (ICPPASTConstructorChainInitializer memberInitializer : functionDefinition
|
||||
.getMemberInitializers()) {
|
||||
IASTName memberName = memberInitializer.getMemberInitializerId();
|
||||
if (memberName != null) {
|
||||
IBinding memberBinding = memberName.resolveBinding();
|
||||
ICPPClassType classType = null;
|
||||
if (memberBinding instanceof ICPPConstructor) {
|
||||
classType = ((ICPPConstructor) memberBinding).getClassOwner();
|
||||
}
|
||||
if (classType instanceof ICPPDeferredClassInstance) {
|
||||
classType = ((ICPPDeferredClassInstance) classType).getClassTemplate();
|
||||
}
|
||||
if (classType != null && classType.isSameType(constructor.getClassOwner()))
|
||||
return null;
|
||||
}
|
||||
}
|
||||
return constructor;
|
||||
}
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Checks whether expression references this (directly, by pointer or by reference)
|
||||
*/
|
||||
private boolean referencesThis(IASTNode expr) {
|
||||
if (expr instanceof IASTLiteralExpression) {
|
||||
IASTLiteralExpression litArg = (IASTLiteralExpression) expr;
|
||||
if (litArg.getKind() == IASTLiteralExpression.lk_this) {
|
||||
return true;
|
||||
}
|
||||
} else if (expr instanceof ICPPASTUnaryExpression) {
|
||||
ICPPASTUnaryExpression unExpr = (ICPPASTUnaryExpression) expr;
|
||||
switch (unExpr.getOperator()) {
|
||||
case IASTUnaryExpression.op_amper:
|
||||
case IASTUnaryExpression.op_star:
|
||||
case IASTUnaryExpression.op_bracketedPrimary:
|
||||
return referencesThis(unExpr.getOperand());
|
||||
}
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Checks that specified declaration is a class destructor
|
||||
*/
|
||||
private ICPPMethod getDestructor(IASTDeclaration decl) {
|
||||
if (decl instanceof ICPPASTFunctionDefinition) {
|
||||
ICPPASTFunctionDefinition functionDefinition = (ICPPASTFunctionDefinition) decl;
|
||||
if (functionDefinition.isDeleted())
|
||||
return null;
|
||||
IBinding binding = functionDefinition.getDeclarator().getName().resolveBinding();
|
||||
if (binding instanceof ICPPMethod) {
|
||||
ICPPMethod method = (ICPPMethod) binding;
|
||||
if (method.isDestructor())
|
||||
return method;
|
||||
}
|
||||
}
|
||||
return null;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,248 @@
|
|||
/*******************************************************************************
|
||||
* Copyright (c) 2019 Marco Stornelli
|
||||
*
|
||||
* This program and the accompanying materials
|
||||
* are made available under the terms of the Eclipse Public License 2.0
|
||||
* which accompanies this distribution, and is available at
|
||||
* https://www.eclipse.org/legal/epl-2.0/
|
||||
*
|
||||
* SPDX-License-Identifier: EPL-2.0
|
||||
*******************************************************************************/
|
||||
package org.eclipse.cdt.codan.core.internal.checkers;
|
||||
|
||||
import org.eclipse.cdt.codan.core.tests.CheckerTestCase;
|
||||
import org.eclipse.cdt.codan.internal.checkers.VirtualMethodCallChecker;
|
||||
|
||||
/**
|
||||
* Test for {@link VirtualMethodCallChecker} class
|
||||
*/
|
||||
public class VirtualMethodCallCheckerTest extends CheckerTestCase {
|
||||
|
||||
private static final String ERR_VIRTUAL_ID = VirtualMethodCallChecker.VIRTUAL_CALL_ID;
|
||||
|
||||
@Override
|
||||
public void setUp() throws Exception {
|
||||
super.setUp();
|
||||
enableProblems(ERR_VIRTUAL_ID);
|
||||
}
|
||||
|
||||
@Override
|
||||
public boolean isCpp() {
|
||||
return true;
|
||||
}
|
||||
|
||||
//class Foo {
|
||||
//public:
|
||||
//Foo();
|
||||
//~Foo();
|
||||
//virtual void bar();
|
||||
//virtual void pure() = 0;
|
||||
//virtual void notpure();
|
||||
//};
|
||||
//Foo::Foo() {
|
||||
// pure();
|
||||
//}
|
||||
//Foo::~Foo() {
|
||||
//}
|
||||
//Foo::bar() {
|
||||
//}
|
||||
public void testWithPureInCtor() throws Exception {
|
||||
loadCodeAndRun(getAboveComment());
|
||||
checkErrorLine(10, ERR_VIRTUAL_ID);
|
||||
}
|
||||
|
||||
//class Foo {
|
||||
//public:
|
||||
//Foo();
|
||||
//~Foo();
|
||||
//virtual void bar();
|
||||
//virtual void pure() = 0;
|
||||
//virtual void notpure();
|
||||
//};
|
||||
//Foo::Foo() {
|
||||
// notpure();
|
||||
//}
|
||||
//Foo::~Foo() {
|
||||
//}
|
||||
//Foo::bar() {
|
||||
//}
|
||||
public void testWithNotPureInCtor() throws Exception {
|
||||
loadCodeAndRun(getAboveComment());
|
||||
checkErrorLine(10, ERR_VIRTUAL_ID);
|
||||
}
|
||||
|
||||
//class Foo {
|
||||
//public:
|
||||
//Foo();
|
||||
//~Foo();
|
||||
//virtual void bar();
|
||||
//virtual void pure() = 0;
|
||||
//virtual void notpure();
|
||||
//};
|
||||
//Foo::Foo() {
|
||||
//}
|
||||
//Foo::~Foo() {
|
||||
// pure();
|
||||
//}
|
||||
//Foo::bar() {
|
||||
//}
|
||||
public void testWithPureInDtor() throws Exception {
|
||||
loadCodeAndRun(getAboveComment());
|
||||
checkErrorLine(12, ERR_VIRTUAL_ID);
|
||||
}
|
||||
|
||||
//class Foo {
|
||||
//public:
|
||||
//Foo();
|
||||
//~Foo();
|
||||
//virtual void bar();
|
||||
//virtual void pure() = 0;
|
||||
//virtual void notpure();
|
||||
//};
|
||||
//Foo::Foo() {
|
||||
//}
|
||||
//Foo::~Foo() {
|
||||
// notpure();
|
||||
//}
|
||||
//Foo::bar() {
|
||||
//}
|
||||
public void testWithNotPureInDtor() throws Exception {
|
||||
loadCodeAndRun(getAboveComment());
|
||||
checkErrorLine(12, ERR_VIRTUAL_ID);
|
||||
}
|
||||
|
||||
//class Foo {
|
||||
//public:
|
||||
//Foo();
|
||||
//~Foo();
|
||||
//virtual void bar();
|
||||
//virtual void pure() = 0;
|
||||
//virtual void notpure();
|
||||
//};
|
||||
//Foo::Foo() {
|
||||
//}
|
||||
//Foo::~Foo() {
|
||||
//}
|
||||
//Foo::bar() {
|
||||
// pure();
|
||||
// notpure();
|
||||
//}
|
||||
public void testWithVirtualInMethod() throws Exception {
|
||||
loadCodeAndRun(getAboveComment());
|
||||
checkNoErrorsOfKind(ERR_VIRTUAL_ID);
|
||||
}
|
||||
|
||||
//class A {
|
||||
//public:
|
||||
// virtual void v() {}
|
||||
//};
|
||||
//class B {
|
||||
//private:
|
||||
//A a;
|
||||
//public:
|
||||
// B() { a.v(); }
|
||||
//};
|
||||
public void testVirtualMethodOtherClass() throws Exception {
|
||||
loadCodeAndRun(getAboveComment());
|
||||
checkNoErrorsOfKind(ERR_VIRTUAL_ID);
|
||||
}
|
||||
|
||||
//class B {
|
||||
//private:
|
||||
//A a;
|
||||
//public:
|
||||
// B() { this->v(); }
|
||||
// virtual void v() {}
|
||||
//};
|
||||
public void testVirtualMethodWithThis() throws Exception {
|
||||
loadCodeAndRun(getAboveComment());
|
||||
checkErrorLine(5, ERR_VIRTUAL_ID);
|
||||
}
|
||||
|
||||
//class B {
|
||||
//private:
|
||||
//A a;
|
||||
//public:
|
||||
// B() { (*this).v(); }
|
||||
// virtual void v() {}
|
||||
//};
|
||||
public void testVirtualMethodWithDerThis() throws Exception {
|
||||
loadCodeAndRun(getAboveComment());
|
||||
checkErrorLine(5, ERR_VIRTUAL_ID);
|
||||
}
|
||||
|
||||
//class A {
|
||||
//public:
|
||||
// virtual void v() {}
|
||||
//};
|
||||
//class B: public A {
|
||||
//public:
|
||||
// B() { A::v(); }
|
||||
//};
|
||||
public void testVirtualMethodChildClass() throws Exception {
|
||||
loadCodeAndRun(getAboveComment());
|
||||
checkErrorLine(7, ERR_VIRTUAL_ID);
|
||||
}
|
||||
|
||||
//class Foo {
|
||||
//public:
|
||||
// Foo() {
|
||||
// class LocalClass {
|
||||
// LocalClass() {}
|
||||
// virtual void foo() {
|
||||
// }
|
||||
// void func() {
|
||||
// foo();
|
||||
// }
|
||||
// virtual ~LocalClass() {}
|
||||
// };
|
||||
// }
|
||||
//};
|
||||
public void testVirtualMethodLocalClass() throws Exception {
|
||||
loadCodeAndRun(getAboveComment());
|
||||
checkNoErrorsOfKind(ERR_VIRTUAL_ID);
|
||||
}
|
||||
|
||||
//class A {
|
||||
//public:
|
||||
// A() : A(5) { foo(); }
|
||||
// A(int a) : a(a) { }
|
||||
// virtual void foo();
|
||||
//private:
|
||||
// int a;
|
||||
//};
|
||||
public void testVirtualMethodDelCtor() throws Exception {
|
||||
loadCodeAndRun(getAboveComment());
|
||||
checkNoErrorsOfKind(ERR_VIRTUAL_ID);
|
||||
}
|
||||
|
||||
//class A {
|
||||
//public:
|
||||
// A() {
|
||||
// A a;
|
||||
// a.foo();
|
||||
// }
|
||||
// virtual void foo();
|
||||
//};
|
||||
public void testVirtualMethodOtherInstance() throws Exception {
|
||||
loadCodeAndRun(getAboveComment());
|
||||
checkNoErrorsOfKind(ERR_VIRTUAL_ID);
|
||||
}
|
||||
|
||||
//class A {
|
||||
//public:
|
||||
// A() {
|
||||
// foo();
|
||||
// }
|
||||
// virtual void foo();
|
||||
// class Nested {
|
||||
// public:
|
||||
// Nested() { bar(); };
|
||||
// virtual void bar();
|
||||
// }
|
||||
//};
|
||||
public void testVirtualMethodNested() throws Exception {
|
||||
loadCodeAndRun(getAboveComment());
|
||||
checkErrorLines(4, 9);
|
||||
}
|
||||
}
|
||||
|
|
@ -36,6 +36,7 @@ import org.eclipse.cdt.codan.core.internal.checkers.SuggestedParenthesisCheckerT
|
|||
import org.eclipse.cdt.codan.core.internal.checkers.SuspiciousSemicolonCheckerTest;
|
||||
import org.eclipse.cdt.codan.core.internal.checkers.SwitchCaseCheckerTest;
|
||||
import org.eclipse.cdt.codan.core.internal.checkers.UnusedSymbolInFileScopeCheckerTest;
|
||||
import org.eclipse.cdt.codan.core.internal.checkers.VirtualMethodCallCheckerTest;
|
||||
import org.eclipse.cdt.codan.internal.checkers.ui.quickfix.AssignmentInConditionQuickFixTest;
|
||||
import org.eclipse.cdt.codan.internal.checkers.ui.quickfix.CaseBreakQuickFixBreakTest;
|
||||
import org.eclipse.cdt.codan.internal.checkers.ui.quickfix.CaseBreakQuickFixCommentTest;
|
||||
|
|
@ -90,6 +91,7 @@ public class AutomatedIntegrationSuite extends TestSuite {
|
|||
suite.addTestSuite(GotoStatementCheckerTest.class);
|
||||
suite.addTestSuite(CopyrightCheckerTest.class);
|
||||
suite.addTestSuite(SwitchCaseCheckerTest.class);
|
||||
suite.addTestSuite(VirtualMethodCallCheckerTest.class);
|
||||
// framework
|
||||
suite.addTest(CodanFastTestSuite.suite());
|
||||
// quick fixes
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue