diff --git a/codan/org.eclipse.cdt.codan.checkers/OSGI-INF/l10n/bundle.properties b/codan/org.eclipse.cdt.codan.checkers/OSGI-INF/l10n/bundle.properties
index 310446405e2..735b445d5e6 100644
--- a/codan/org.eclipse.cdt.codan.checkers/OSGI-INF/l10n/bundle.properties
+++ b/codan/org.eclipse.cdt.codan.checkers/OSGI-INF/l10n/bundle.properties
@@ -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
diff --git a/codan/org.eclipse.cdt.codan.checkers/plugin.xml b/codan/org.eclipse.cdt.codan.checkers/plugin.xml
index 45fbd7329c5..470dbc9bd9a 100644
--- a/codan/org.eclipse.cdt.codan.checkers/plugin.xml
+++ b/codan/org.eclipse.cdt.codan.checkers/plugin.xml
@@ -524,5 +524,20 @@
name="%problem.name.MissDefaultProblem">
+
+
+
+
diff --git a/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/VirtualMethodCallChecker.java b/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/VirtualMethodCallChecker.java
new file mode 100644
index 00000000000..dd971602833
--- /dev/null
+++ b/codan/org.eclipse.cdt.codan.checkers/src/org/eclipse/cdt/codan/internal/checkers/VirtualMethodCallChecker.java
@@ -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 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;
+ }
+ }
+}
diff --git a/codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/core/internal/checkers/VirtualMethodCallCheckerTest.java b/codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/core/internal/checkers/VirtualMethodCallCheckerTest.java
new file mode 100644
index 00000000000..fa054060294
--- /dev/null
+++ b/codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/core/internal/checkers/VirtualMethodCallCheckerTest.java
@@ -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);
+ }
+}
diff --git a/codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/core/tests/AutomatedIntegrationSuite.java b/codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/core/tests/AutomatedIntegrationSuite.java
index 51ec39307eb..9a0cb3e2399 100644
--- a/codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/core/tests/AutomatedIntegrationSuite.java
+++ b/codan/org.eclipse.cdt.codan.core.tests/src/org/eclipse/cdt/codan/core/tests/AutomatedIntegrationSuite.java
@@ -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