From 52a02f38ffa642ba4b7c0d03d69880ffc94cf326 Mon Sep 17 00:00:00 2001 From: "rakesh.pothengil" Date: Wed, 2 Oct 2024 16:13:21 +0900 Subject: [PATCH 01/10] Fix #711 --- change_notes/2024-10-02-fix-fp-711-M0-1-10.md | 2 + .../src/rules/M0-1-10/UnusedFunction.ql | 3 +- .../rules/M0-1-10/UnusedSplMemberFunction.ql | 32 ++++++++++++ .../rules/M0-1-10/UnusedFunction.expected | 3 +- .../M0-1-10/UnusedSplMemberFunction.expected | 2 + .../M0-1-10/UnusedSplMemberFunction.qlref | 1 + cpp/autosar/test/rules/M0-1-10/test.cpp | 51 ++++++++++++++++++- cpp/autosar/test/rules/M0-1-10/test.hpp | 4 ++ .../cpp/EncapsulatingFunctions.qll | 33 ++++++++++++ .../cpp/deadcode/UnusedFunctions.qll | 40 ++++++++++++++- 10 files changed, 166 insertions(+), 5 deletions(-) create mode 100644 change_notes/2024-10-02-fix-fp-711-M0-1-10.md create mode 100644 cpp/autosar/src/rules/M0-1-10/UnusedSplMemberFunction.ql create mode 100644 cpp/autosar/test/rules/M0-1-10/UnusedSplMemberFunction.expected create mode 100644 cpp/autosar/test/rules/M0-1-10/UnusedSplMemberFunction.qlref create mode 100644 cpp/autosar/test/rules/M0-1-10/test.hpp diff --git a/change_notes/2024-10-02-fix-fp-711-M0-1-10.md b/change_notes/2024-10-02-fix-fp-711-M0-1-10.md new file mode 100644 index 0000000000..cff5d5ab43 --- /dev/null +++ b/change_notes/2024-10-02-fix-fp-711-M0-1-10.md @@ -0,0 +1,2 @@ +- `M0-1-10` - `UnusedFunction.ql`: + - Fixes #711. Excludes constexpr functions, considers functions from GoogleTest as an EntryPoint and does not consider special member functions. Another query called UnusedSplMemberFunction.ql is created that reports unused special member functions. This is done so as to enable deviations to be applied to this case. diff --git a/cpp/autosar/src/rules/M0-1-10/UnusedFunction.ql b/cpp/autosar/src/rules/M0-1-10/UnusedFunction.ql index b8593e75c0..27306a9fc1 100644 --- a/cpp/autosar/src/rules/M0-1-10/UnusedFunction.ql +++ b/cpp/autosar/src/rules/M0-1-10/UnusedFunction.ql @@ -26,5 +26,6 @@ where then name = unusedFunction.getQualifiedName() else name = unusedFunction.getName() ) and - not unusedFunction.isDeleted() + not unusedFunction.isDeleted() and + not UnusedFunctions::isASpecialMemberFunction(unusedFunction) select unusedFunction, "Function " + name + " is " + unusedFunction.getDeadCodeType() diff --git a/cpp/autosar/src/rules/M0-1-10/UnusedSplMemberFunction.ql b/cpp/autosar/src/rules/M0-1-10/UnusedSplMemberFunction.ql new file mode 100644 index 0000000000..bf073dcced --- /dev/null +++ b/cpp/autosar/src/rules/M0-1-10/UnusedSplMemberFunction.ql @@ -0,0 +1,32 @@ +/** + * @id cpp/autosar/unused-spl-member-function + * @name M0-1-10: Every defined function should be called at least once + * @description Uncalled functions complicate the program and can indicate a possible mistake on the + * part of the programmer. This query specifically looks for unused Special Member + * Functions. + * @kind problem + * @precision medium + * @problem.severity warning + * @tags external/autosar/id/m0-1-10 + * readability + * maintainability + * external/autosar/allocated-target/implementation + * external/autosar/enforcement/automated + * external/autosar/obligation/advisory + */ + +import cpp +import codingstandards.cpp.autosar +import codingstandards.cpp.deadcode.UnusedFunctions + +from UnusedFunctions::UnusedSplMemberFunction unusedSplMemFunction, string name +where + not isExcluded(unusedSplMemFunction, DeadCodePackage::unusedFunctionQuery()) and + ( + if exists(unusedSplMemFunction.getQualifiedName()) + then name = unusedSplMemFunction.getQualifiedName() + else name = unusedSplMemFunction.getName() + ) and + not unusedSplMemFunction.isDeleted() +select unusedSplMemFunction, + "Special member function " + name + " is " + unusedSplMemFunction.getDeadCodeType() diff --git a/cpp/autosar/test/rules/M0-1-10/UnusedFunction.expected b/cpp/autosar/test/rules/M0-1-10/UnusedFunction.expected index d9ab0d38ac..912e2104e8 100644 --- a/cpp/autosar/test/rules/M0-1-10/UnusedFunction.expected +++ b/cpp/autosar/test/rules/M0-1-10/UnusedFunction.expected @@ -10,4 +10,5 @@ | test.cpp:50:5:50:6 | i3 | Function C::i3 is never called. | | test.cpp:51:8:51:9 | i4 | Function C::i4 is never called. | | test.cpp:52:15:52:16 | i5 | Function C::i5 is never called. | -| test.cpp:69:17:69:18 | g4 | Function g4 is never called. | +| test.cpp:79:6:79:21 | anUnusedFunction | Function anUnusedFunction is never called. | +| test.cpp:113:17:113:18 | g4 | Function g4 is never called. | diff --git a/cpp/autosar/test/rules/M0-1-10/UnusedSplMemberFunction.expected b/cpp/autosar/test/rules/M0-1-10/UnusedSplMemberFunction.expected new file mode 100644 index 0000000000..e2bf0acc79 --- /dev/null +++ b/cpp/autosar/test/rules/M0-1-10/UnusedSplMemberFunction.expected @@ -0,0 +1,2 @@ +| test.cpp:71:5:71:16 | ANestedClass | Special member function ANestedClass is never called. | +| test.cpp:82:5:82:22 | AnotherNestedClass | Special member function AnotherNestedClass is never called from a main function or entry point. | diff --git a/cpp/autosar/test/rules/M0-1-10/UnusedSplMemberFunction.qlref b/cpp/autosar/test/rules/M0-1-10/UnusedSplMemberFunction.qlref new file mode 100644 index 0000000000..b04687a48b --- /dev/null +++ b/cpp/autosar/test/rules/M0-1-10/UnusedSplMemberFunction.qlref @@ -0,0 +1 @@ +rules/M0-1-10/UnusedSplMemberFunction.ql diff --git a/cpp/autosar/test/rules/M0-1-10/test.cpp b/cpp/autosar/test/rules/M0-1-10/test.cpp index 748d2196ef..6e1220be5d 100644 --- a/cpp/autosar/test/rules/M0-1-10/test.cpp +++ b/cpp/autosar/test/rules/M0-1-10/test.cpp @@ -52,6 +52,50 @@ template class C { inline void i5() {} // NON_COMPLIANT - never used in any instantiation }; +#include "test.hpp" +#include + +template +constexpr bool aConstExprFunc() noexcept { // COMPLIANT + static_assert(std::is_trivially_copy_constructible() && + std::is_trivially_copy_constructible(), + "assert"); + return true; +} + +template class AClass { T anArr[val]; }; + +void aCalledFunc1() // COMPLIANT +{ + struct ANestedClass { + ANestedClass() noexcept(false) { // COMPLIANT: False Positive! + static_cast(0); + } + }; + static_assert(std::is_trivially_copy_constructible>(), + "Must be trivially copy constructible"); +} + +void anUnusedFunction() // NON_COMPLIANT +{ + struct AnotherNestedClass { + AnotherNestedClass() noexcept(false) { // NON_COMPLAINT + static_cast(0); + } + }; + AnotherNestedClass d; +} + +void aCalledFunc2() // COMPLIANT +{ + struct YetAnotherNestedClass { + YetAnotherNestedClass() noexcept(false) { + static_cast(0); + } // COMPLIANT + }; + YetAnotherNestedClass d; +}; + int main() { // COMPLIANT - this is a main like function which acts as an entry // point f3(); @@ -88,8 +132,13 @@ int main() { // COMPLIANT - this is a main like function which acts as an entry c1.getAT(); S s; c2.i1(s); + + int aVar; + aConstExprFunc(); + aCalledFunc1(); + aCalledFunc2(); } class M { public: M(const M &) = delete; // COMPLIANT - ignore if deleted -}; \ No newline at end of file +}; diff --git a/cpp/autosar/test/rules/M0-1-10/test.hpp b/cpp/autosar/test/rules/M0-1-10/test.hpp new file mode 100644 index 0000000000..a2da990951 --- /dev/null +++ b/cpp/autosar/test/rules/M0-1-10/test.hpp @@ -0,0 +1,4 @@ +template +constexpr T aCalledFuncInHeader(T value) noexcept { // COMPLIANT + return static_cast(value); +} diff --git a/cpp/common/src/codingstandards/cpp/EncapsulatingFunctions.qll b/cpp/common/src/codingstandards/cpp/EncapsulatingFunctions.qll index d8d9739033..f619429d0d 100644 --- a/cpp/common/src/codingstandards/cpp/EncapsulatingFunctions.qll +++ b/cpp/common/src/codingstandards/cpp/EncapsulatingFunctions.qll @@ -18,6 +18,39 @@ class MainFunction extends MainLikeFunction { } } +/** + * A test function from the GoogleTest infrastructure. + * + * Such functions can be treated as valid EntryPoint functions during analysis + * of "called" or "unused" functions. It is not straightforward to identify + * such functions, however, they have certain features that can be used for + * identification. This can be refined based on experiments/real-world use. + */ +class GTestFunction extends MainLikeFunction { + GTestFunction() { + // A GoogleTest function is named "TestBody" and + this.hasName("TestBody") and + // is enclosed by a class that inherits from a base class + this.getEnclosingAccessHolder() instanceof Class and + exists(Class base | + base = this.getEnclosingAccessHolder().(Class).getABaseClass() and + ( + // called "Test" or + exists(Class c | base.getABaseClass() = c and c.hasName("Test")) + or + // defined under a namespace called "testing" or + exists(Namespace n | n = base.getNamespace() | n.hasName("testing")) + or + // is templatized by a parameter called "gtest_TypeParam_" + exists(TemplateParameter tp | + tp = base.getATemplateArgument() and + tp.hasName("gtest_TypeParam_") + ) + ) + ) + } +} + /** * A "task main" function. */ diff --git a/cpp/common/src/codingstandards/cpp/deadcode/UnusedFunctions.qll b/cpp/common/src/codingstandards/cpp/deadcode/UnusedFunctions.qll index b01b80208e..2dc24025ce 100644 --- a/cpp/common/src/codingstandards/cpp/deadcode/UnusedFunctions.qll +++ b/cpp/common/src/codingstandards/cpp/deadcode/UnusedFunctions.qll @@ -75,7 +75,9 @@ module UnusedFunctions { */ private class MainLikeFunctionEntryPoint extends EntryPoint, MainLikeFunction { - MainLikeFunctionEntryPoint() { this instanceof MainLikeFunction } + MainLikeFunctionEntryPoint() { + this instanceof MainLikeFunction or this instanceof GTestFunction + } override Function getAReachableFunction() { reachable*(this, result) } } @@ -111,6 +113,26 @@ module UnusedFunctions { } } + /** + * A `MemberFunction` which is either a Default constructor, Destructor + * CopyConstructor, CopyAssingmentOperator, MoveConstructor or a + * MoveAssignmentOperator + */ + predicate isASpecialMemberFunction(MemberFunction f) { + // Default constructor + f instanceof NoArgConstructor + or + f instanceof Destructor + or + f instanceof CopyConstructor + or + f instanceof CopyAssignmentOperator + or + f instanceof MoveConstructor + or + f instanceof MoveAssignmentOperator + } + /** * A `Function` which is not used from an `EntryPoint`. * @@ -119,7 +141,12 @@ module UnusedFunctions { class UnusedFunction extends UsableFunction { UnusedFunction() { // This function, or an equivalent function, is not reachable from any entry point - not exists(EntryPoint ep | getAnEquivalentFunction(this) = ep.getAReachableFunction()) + not exists(EntryPoint ep | getAnEquivalentFunction(this) = ep.getAReachableFunction()) and + // and it is not a constexpr. Refer issue #646. + // The usages of constexpr is not well tracked and hence + // to avoid false positives, this is added. In case there is an improvement in + // handling constexpr in CodeQL, we can consider removing it. + not this.isConstexpr() } string getDeadCodeType() { @@ -128,4 +155,13 @@ module UnusedFunctions { else result = "never called." } } + + /** + * A Special `MemberFunction` which is an `UnusedFunction`. + * + * Refer isASpecialMemberFunction predicate. + */ + class UnusedSplMemberFunction extends UnusedFunction { + UnusedSplMemberFunction() { isASpecialMemberFunction(this) } + } } From 7bf12c4902c6acd803b90856b8af6b3c2fff317c Mon Sep 17 00:00:00 2001 From: "rakesh.pothengil" Date: Thu, 3 Oct 2024 10:07:05 +0900 Subject: [PATCH 02/10] Fix new query addition --- .../rules/M0-1-10/UnusedSplMemberFunction.ql | 5 ++--- .../rules/M0-1-10/UnusedSplMemberFunction.qlref | 2 +- .../cpp/exclusions/cpp/DeadCode.qll | 17 +++++++++++++++++ rule_packages/cpp/DeadCode.json | 15 +++++++++++++++ 4 files changed, 35 insertions(+), 4 deletions(-) diff --git a/cpp/autosar/src/rules/M0-1-10/UnusedSplMemberFunction.ql b/cpp/autosar/src/rules/M0-1-10/UnusedSplMemberFunction.ql index bf073dcced..9efa4bdfd1 100644 --- a/cpp/autosar/src/rules/M0-1-10/UnusedSplMemberFunction.ql +++ b/cpp/autosar/src/rules/M0-1-10/UnusedSplMemberFunction.ql @@ -2,8 +2,7 @@ * @id cpp/autosar/unused-spl-member-function * @name M0-1-10: Every defined function should be called at least once * @description Uncalled functions complicate the program and can indicate a possible mistake on the - * part of the programmer. This query specifically looks for unused Special Member - * Functions. + * part of the programmer. * @kind problem * @precision medium * @problem.severity warning @@ -21,7 +20,7 @@ import codingstandards.cpp.deadcode.UnusedFunctions from UnusedFunctions::UnusedSplMemberFunction unusedSplMemFunction, string name where - not isExcluded(unusedSplMemFunction, DeadCodePackage::unusedFunctionQuery()) and + not isExcluded(unusedSplMemberFunctionQuery, DeadCodePackage::unusedFunctionQuery()) and ( if exists(unusedSplMemFunction.getQualifiedName()) then name = unusedSplMemFunction.getQualifiedName() diff --git a/cpp/autosar/test/rules/M0-1-10/UnusedSplMemberFunction.qlref b/cpp/autosar/test/rules/M0-1-10/UnusedSplMemberFunction.qlref index b04687a48b..899f00fda1 100644 --- a/cpp/autosar/test/rules/M0-1-10/UnusedSplMemberFunction.qlref +++ b/cpp/autosar/test/rules/M0-1-10/UnusedSplMemberFunction.qlref @@ -1 +1 @@ -rules/M0-1-10/UnusedSplMemberFunction.ql +rules/M0-1-10/UnusedSplMemberFunction.ql \ No newline at end of file diff --git a/cpp/common/src/codingstandards/cpp/exclusions/cpp/DeadCode.qll b/cpp/common/src/codingstandards/cpp/exclusions/cpp/DeadCode.qll index 40b8795e5e..f11741fde5 100644 --- a/cpp/common/src/codingstandards/cpp/exclusions/cpp/DeadCode.qll +++ b/cpp/common/src/codingstandards/cpp/exclusions/cpp/DeadCode.qll @@ -12,6 +12,7 @@ newtype DeadCodeQuery = TUnusedTypeDeclarationsQuery() or TUnreachableCodeQuery() or TUnusedFunctionQuery() or + TUnusedSplMemberFunctionQuery() or TInfeasiblePathQuery() or TUnusedLocalVariableQuery() or TUnusedGlobalOrNamespaceVariableQuery() or @@ -94,6 +95,15 @@ predicate isDeadCodeQueryMetadata(Query query, string queryId, string ruleId, st ruleId = "M0-1-10" and category = "advisory" or + query = + // `Query` instance for the `unusedSplMemberFunction` query + DeadCodePackage::unusedSplMemberFunctionQuery() and + queryId = + // `@id` for the `unusedSplMemberFunction` query + "cpp/autosar/unused-spl-member-function" and + ruleId = "M0-1-10" and + category = "advisory" + or query = // `Query` instance for the `infeasiblePath` query DeadCodePackage::infeasiblePathQuery() and @@ -224,6 +234,13 @@ module DeadCodePackage { TQueryCPP(TDeadCodePackageQuery(TUnusedFunctionQuery())) } + Query unusedSplMemberFunctionQuery() { + //autogenerate `Query` type + result = + // `Query` type for `unusedSplMemberFunction` query + TQueryCPP(TDeadCodePackageQuery(TUnusedSplMemberFunctionQuery())) + } + Query infeasiblePathQuery() { //autogenerate `Query` type result = diff --git a/rule_packages/cpp/DeadCode.json b/rule_packages/cpp/DeadCode.json index 7eb5c9f6f9..4746f86dee 100644 --- a/rule_packages/cpp/DeadCode.json +++ b/rule_packages/cpp/DeadCode.json @@ -194,6 +194,21 @@ "readability", "maintainability" ] + }, + { + "description": "Uncalled functions complicate the program and can indicate a possible mistake on the part of the programmer.", + "kind": "problem", + "name": "Every defined function should be called at least once", + "precision": "medium", + "severity": "warning", + "short_name": "UnusedSplMemberFunction", + "tags": [ + "readability", + "maintainability" + ], + "implementation_scope": { + "description": "In limited cases, this query can raise false-positives for special member function calls invoked from the C++ Metaprogramming library." + } } ], "title": "Every defined function should be called at least once." From a48c7cc6af11ed445cee5a43d1e823db6b2c3833 Mon Sep 17 00:00:00 2001 From: "rakesh.pothengil" Date: Thu, 3 Oct 2024 17:12:41 +0900 Subject: [PATCH 03/10] Simplify GoogleTestFunction --- .../cpp/EncapsulatingFunctions.qll | 26 +++++++------------ 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/cpp/common/src/codingstandards/cpp/EncapsulatingFunctions.qll b/cpp/common/src/codingstandards/cpp/EncapsulatingFunctions.qll index 4f7e423254..ad11bea21c 100644 --- a/cpp/common/src/codingstandards/cpp/EncapsulatingFunctions.qll +++ b/cpp/common/src/codingstandards/cpp/EncapsulatingFunctions.qll @@ -26,27 +26,21 @@ class MainFunction extends MainLikeFunction { * such functions, however, they have certain features that can be used for * identification. This can be refined based on experiments/real-world use. */ -class GTestFunction extends MainLikeFunction { - GTestFunction() { +class GoogleTestFunction extends MainLikeFunction { + GoogleTestFunction() { // A GoogleTest function is named "TestBody" and this.hasName("TestBody") and - // is enclosed by a class that inherits from a base class - this.getEnclosingAccessHolder() instanceof Class and + // it's parent class inherits a base class exists(Class base | - base = this.getEnclosingAccessHolder().(Class).getABaseClass() and + base = this.getEnclosingAccessHolder().(Class).getABaseClass+() and + // with a name "Test" inside a namespace called "testing" ( - // called "Test" or - exists(Class c | base.getABaseClass() = c and c.hasName("Test")) - or - // defined under a namespace called "testing" or - exists(Namespace n | n = base.getNamespace() | n.hasName("testing")) - or - // is templatized by a parameter called "gtest_TypeParam_" - exists(TemplateParameter tp | - tp = base.getATemplateArgument() and - tp.hasName("gtest_TypeParam_") - ) + base.hasName("Test") and + base.getNamespace().hasName("testing") ) + or + // or at a location in a file called "gtest.h". + base.getDefinitionLocation().getFile().getBaseName() = "gtest.h" ) } } From bd83048f7d9d9e76a9c7e8a5d6ee0d252947ff83 Mon Sep 17 00:00:00 2001 From: "rakesh.pothengil" Date: Thu, 3 Oct 2024 17:39:05 +0900 Subject: [PATCH 04/10] Reuse SpecialMemberFunction class --- .../cpp/deadcode/UnusedFunctions.qll | 35 +++---------------- 1 file changed, 4 insertions(+), 31 deletions(-) diff --git a/cpp/common/src/codingstandards/cpp/deadcode/UnusedFunctions.qll b/cpp/common/src/codingstandards/cpp/deadcode/UnusedFunctions.qll index 2dc24025ce..fdd713b436 100644 --- a/cpp/common/src/codingstandards/cpp/deadcode/UnusedFunctions.qll +++ b/cpp/common/src/codingstandards/cpp/deadcode/UnusedFunctions.qll @@ -13,6 +13,7 @@ import cpp import codingstandards.cpp.DynamicCallGraph import codingstandards.cpp.EncapsulatingFunctions import codingstandards.cpp.FunctionEquivalence +import codingstandards.cpp.Class module UnusedFunctions { /** @@ -75,9 +76,7 @@ module UnusedFunctions { */ private class MainLikeFunctionEntryPoint extends EntryPoint, MainLikeFunction { - MainLikeFunctionEntryPoint() { - this instanceof MainLikeFunction or this instanceof GTestFunction - } + MainLikeFunctionEntryPoint() { this instanceof MainLikeFunction } override Function getAReachableFunction() { reachable*(this, result) } } @@ -113,26 +112,6 @@ module UnusedFunctions { } } - /** - * A `MemberFunction` which is either a Default constructor, Destructor - * CopyConstructor, CopyAssingmentOperator, MoveConstructor or a - * MoveAssignmentOperator - */ - predicate isASpecialMemberFunction(MemberFunction f) { - // Default constructor - f instanceof NoArgConstructor - or - f instanceof Destructor - or - f instanceof CopyConstructor - or - f instanceof CopyAssignmentOperator - or - f instanceof MoveConstructor - or - f instanceof MoveAssignmentOperator - } - /** * A `Function` which is not used from an `EntryPoint`. * @@ -156,12 +135,6 @@ module UnusedFunctions { } } - /** - * A Special `MemberFunction` which is an `UnusedFunction`. - * - * Refer isASpecialMemberFunction predicate. - */ - class UnusedSplMemberFunction extends UnusedFunction { - UnusedSplMemberFunction() { isASpecialMemberFunction(this) } - } + /** A `SpecialMemberFunction` which is an `UnusedFunction`. */ + class UnusedSplMemberFunction extends UnusedFunction, SpecialMemberFunction { } } From 97bf53af4294acb6afe3a0a794f4840e75c93bb1 Mon Sep 17 00:00:00 2001 From: "rakesh.pothengil" Date: Mon, 7 Oct 2024 10:21:03 +0900 Subject: [PATCH 05/10] Correct exclusion --- cpp/autosar/src/rules/M0-1-10/UnusedSplMemberFunction.ql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/autosar/src/rules/M0-1-10/UnusedSplMemberFunction.ql b/cpp/autosar/src/rules/M0-1-10/UnusedSplMemberFunction.ql index 9efa4bdfd1..03ddfbbd43 100644 --- a/cpp/autosar/src/rules/M0-1-10/UnusedSplMemberFunction.ql +++ b/cpp/autosar/src/rules/M0-1-10/UnusedSplMemberFunction.ql @@ -20,7 +20,7 @@ import codingstandards.cpp.deadcode.UnusedFunctions from UnusedFunctions::UnusedSplMemberFunction unusedSplMemFunction, string name where - not isExcluded(unusedSplMemberFunctionQuery, DeadCodePackage::unusedFunctionQuery()) and + not isExcluded(DeadCodePackage::unusedSplMemberFunctionQuery(), DeadCodePackage::unusedFunctionQuery()) and ( if exists(unusedSplMemFunction.getQualifiedName()) then name = unusedSplMemFunction.getQualifiedName() From c5cdf0e0b7e40239966e5bfb0127264f0b1e5627 Mon Sep 17 00:00:00 2001 From: "rakesh.pothengil" Date: Mon, 7 Oct 2024 14:21:19 +0900 Subject: [PATCH 06/10] Add GoogleTest stub and test cases --- .../M0-1-10/UnusedSplMemberFunction.expected | 1 + cpp/autosar/test/rules/M0-1-10/test.cpp | 19 ++++++++++++ .../cpp/EncapsulatingFunctions.qll | 5 ++-- .../custom-library/gtest/gtest-internal.h | 29 +++++++++++++++++++ .../includes/custom-library/gtest/gtest.h | 28 ++++++++++++++++++ cpp/options | 2 +- 6 files changed, 81 insertions(+), 3 deletions(-) create mode 100644 cpp/common/test/includes/custom-library/gtest/gtest-internal.h create mode 100644 cpp/common/test/includes/custom-library/gtest/gtest.h diff --git a/cpp/autosar/test/rules/M0-1-10/UnusedSplMemberFunction.expected b/cpp/autosar/test/rules/M0-1-10/UnusedSplMemberFunction.expected index e2bf0acc79..f26e8dfe33 100644 --- a/cpp/autosar/test/rules/M0-1-10/UnusedSplMemberFunction.expected +++ b/cpp/autosar/test/rules/M0-1-10/UnusedSplMemberFunction.expected @@ -1,2 +1,3 @@ | test.cpp:71:5:71:16 | ANestedClass | Special member function ANestedClass is never called. | | test.cpp:82:5:82:22 | AnotherNestedClass | Special member function AnotherNestedClass is never called from a main function or entry point. | +| test.cpp:155:1:157:37 | ~sample_test_called_from_google_test_function_Test | Special member function sample_test_called_from_google_test_function_Test::~sample_test_called_from_google_test_function_Test is never called. | diff --git a/cpp/autosar/test/rules/M0-1-10/test.cpp b/cpp/autosar/test/rules/M0-1-10/test.cpp index 6e1220be5d..27534590fd 100644 --- a/cpp/autosar/test/rules/M0-1-10/test.cpp +++ b/cpp/autosar/test/rules/M0-1-10/test.cpp @@ -142,3 +142,22 @@ class M { public: M(const M &) = delete; // COMPLIANT - ignore if deleted }; + +#include +int called_from_google_test_function( + int a_param) // COMPLIANT - called from TEST +{ + int something = a_param; + something++; + return something; +} + +TEST( + sample_test, + called_from_google_test_function) // COMPLIANT - False positive! + // ~sample_test_called_from_google_test_function_Test +{ + bool pass = false; + if (called_from_google_test_function(0) >= 10) + pass = true; +} diff --git a/cpp/common/src/codingstandards/cpp/EncapsulatingFunctions.qll b/cpp/common/src/codingstandards/cpp/EncapsulatingFunctions.qll index ad11bea21c..7b2d715d01 100644 --- a/cpp/common/src/codingstandards/cpp/EncapsulatingFunctions.qll +++ b/cpp/common/src/codingstandards/cpp/EncapsulatingFunctions.qll @@ -39,8 +39,9 @@ class GoogleTestFunction extends MainLikeFunction { base.getNamespace().hasName("testing") ) or - // or at a location in a file called "gtest.h". - base.getDefinitionLocation().getFile().getBaseName() = "gtest.h" + // or at a location in a file called gtest.h (or gtest-internal.h, + // gtest-typed-test.h etc). + base.getDefinitionLocation().getFile().getBaseName().regexpMatch("gtest*.h") ) } } diff --git a/cpp/common/test/includes/custom-library/gtest/gtest-internal.h b/cpp/common/test/includes/custom-library/gtest/gtest-internal.h new file mode 100644 index 0000000000..31d47b714f --- /dev/null +++ b/cpp/common/test/includes/custom-library/gtest/gtest-internal.h @@ -0,0 +1,29 @@ +#ifndef GOOGLETEST_INCLUDE_GTEST_INTERNAL_GTEST_INTERNAL_H_ +#define GOOGLETEST_INCLUDE_GTEST_INTERNAL_GTEST_INTERNAL_H_ + +#define GTEST_TEST_CLASS_NAME_(test_suite_name, test_name) \ + test_suite_name##_##test_name##_Test + +#define GTEST_TEST_(test_suite_name, test_name, parent_class) \ + class GTEST_TEST_CLASS_NAME_(test_suite_name, test_name) \ + : public parent_class { \ + public: \ + GTEST_TEST_CLASS_NAME_(test_suite_name, test_name)() = default; \ + ~GTEST_TEST_CLASS_NAME_(test_suite_name, test_name)() override = default; \ + GTEST_TEST_CLASS_NAME_(test_suite_name, test_name) \ + (const GTEST_TEST_CLASS_NAME_(test_suite_name, test_name) &) = delete; \ + GTEST_TEST_CLASS_NAME_(test_suite_name, test_name) & operator=( \ + const GTEST_TEST_CLASS_NAME_(test_suite_name, \ + test_name) &) = delete; /* NOLINT */ \ + GTEST_TEST_CLASS_NAME_(test_suite_name, test_name) \ + (GTEST_TEST_CLASS_NAME_(test_suite_name, test_name) &&) noexcept = delete; \ + GTEST_TEST_CLASS_NAME_(test_suite_name, test_name) & operator=( \ + GTEST_TEST_CLASS_NAME_(test_suite_name, \ + test_name) &&) noexcept = delete; /* NOLINT */ \ + \ + private: \ + void TestBody() override; \ + }; \ + void GTEST_TEST_CLASS_NAME_(test_suite_name, test_name)::TestBody() \ + +#endif // GOOGLETEST_INCLUDE_GTEST_INTERNAL_GTEST_INTERNAL_H_ diff --git a/cpp/common/test/includes/custom-library/gtest/gtest.h b/cpp/common/test/includes/custom-library/gtest/gtest.h new file mode 100644 index 0000000000..65fce9fc5a --- /dev/null +++ b/cpp/common/test/includes/custom-library/gtest/gtest.h @@ -0,0 +1,28 @@ +#ifndef GOOGLETEST_INCLUDE_GTEST_GTEST_H_ +#define GOOGLETEST_INCLUDE_GTEST_GTEST_H_ + +#include "gtest/gtest-internal.h" + +namespace testing { + +class Test +{ + public: + virtual ~Test(); + protected: + // Creates a Test object. + Test(); + private: + virtual void TestBody() = 0; + Test(const Test&) = delete; + Test& operator=(const Test&) = delete; +}; + +#define GTEST_TEST(test_suite_name, test_name) \ + GTEST_TEST_(test_suite_name, test_name, ::testing::Test) + +#define TEST(test_suite_name, test_name) GTEST_TEST(test_suite_name, test_name) + +} // namespace testing + +#endif // GOOGLETEST_INCLUDE_GTEST_GTEST_H_ diff --git a/cpp/options b/cpp/options index 1f8961ecda..44267e9323 100644 --- a/cpp/options +++ b/cpp/options @@ -1 +1 @@ -semmle-extractor-options:--clang -std=c++14 -nostdinc++ -I../../../../common/test/includes/standard-library -I../../../../common/test/includes/custom-library \ No newline at end of file +semmle-extractor-options:--clang -std=c++14 -nostdinc++ -I../../../../common/test/includes/standard-library -I../../../../common/test/includes/custom-library From 23574944cb6a31795a28a7a343971f0e76b62294 Mon Sep 17 00:00:00 2001 From: "rakesh.pothengil" Date: Tue, 8 Oct 2024 10:44:43 +0900 Subject: [PATCH 07/10] Dont report SpecialMemberFunction --- cpp/autosar/src/rules/M0-1-10/UnusedFunction.ql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/autosar/src/rules/M0-1-10/UnusedFunction.ql b/cpp/autosar/src/rules/M0-1-10/UnusedFunction.ql index 27306a9fc1..f175cb8992 100644 --- a/cpp/autosar/src/rules/M0-1-10/UnusedFunction.ql +++ b/cpp/autosar/src/rules/M0-1-10/UnusedFunction.ql @@ -27,5 +27,5 @@ where else name = unusedFunction.getName() ) and not unusedFunction.isDeleted() and - not UnusedFunctions::isASpecialMemberFunction(unusedFunction) + not unusedFunction instanceof SpecialMemberFunction select unusedFunction, "Function " + name + " is " + unusedFunction.getDeadCodeType() From 2ffccb0003c382882f6009034fe430e36ad13d77 Mon Sep 17 00:00:00 2001 From: Luke Cartey <5377966+lcartey@users.noreply.github.com> Date: Wed, 9 Oct 2024 09:18:34 +0100 Subject: [PATCH 08/10] Fix isExcluded function parameter in query --- cpp/autosar/src/rules/M0-1-10/UnusedSplMemberFunction.ql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/autosar/src/rules/M0-1-10/UnusedSplMemberFunction.ql b/cpp/autosar/src/rules/M0-1-10/UnusedSplMemberFunction.ql index 03ddfbbd43..bcbf6f4e1b 100644 --- a/cpp/autosar/src/rules/M0-1-10/UnusedSplMemberFunction.ql +++ b/cpp/autosar/src/rules/M0-1-10/UnusedSplMemberFunction.ql @@ -20,7 +20,7 @@ import codingstandards.cpp.deadcode.UnusedFunctions from UnusedFunctions::UnusedSplMemberFunction unusedSplMemFunction, string name where - not isExcluded(DeadCodePackage::unusedSplMemberFunctionQuery(), DeadCodePackage::unusedFunctionQuery()) and + not isExcluded(unusedSplMemFunction, DeadCodePackage::unusedFunctionQuery()) and ( if exists(unusedSplMemFunction.getQualifiedName()) then name = unusedSplMemFunction.getQualifiedName() From 4b1429b6bf94bce541c32395eab0529a6d5ce9cb Mon Sep 17 00:00:00 2001 From: "rakesh.pothengil" Date: Wed, 9 Oct 2024 19:07:52 +0900 Subject: [PATCH 09/10] SpecialMemberFunction from GoogleTest as entry pts --- .../M0-1-10/UnusedSplMemberFunction.expected | 1 - cpp/autosar/test/rules/M0-1-10/test.cpp | 6 ++---- .../cpp/EncapsulatingFunctions.qll | 16 ++++++++++------ 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/cpp/autosar/test/rules/M0-1-10/UnusedSplMemberFunction.expected b/cpp/autosar/test/rules/M0-1-10/UnusedSplMemberFunction.expected index f26e8dfe33..e2bf0acc79 100644 --- a/cpp/autosar/test/rules/M0-1-10/UnusedSplMemberFunction.expected +++ b/cpp/autosar/test/rules/M0-1-10/UnusedSplMemberFunction.expected @@ -1,3 +1,2 @@ | test.cpp:71:5:71:16 | ANestedClass | Special member function ANestedClass is never called. | | test.cpp:82:5:82:22 | AnotherNestedClass | Special member function AnotherNestedClass is never called from a main function or entry point. | -| test.cpp:155:1:157:37 | ~sample_test_called_from_google_test_function_Test | Special member function sample_test_called_from_google_test_function_Test::~sample_test_called_from_google_test_function_Test is never called. | diff --git a/cpp/autosar/test/rules/M0-1-10/test.cpp b/cpp/autosar/test/rules/M0-1-10/test.cpp index 27534590fd..84b17c4c21 100644 --- a/cpp/autosar/test/rules/M0-1-10/test.cpp +++ b/cpp/autosar/test/rules/M0-1-10/test.cpp @@ -152,10 +152,8 @@ int called_from_google_test_function( return something; } -TEST( - sample_test, - called_from_google_test_function) // COMPLIANT - False positive! - // ~sample_test_called_from_google_test_function_Test +TEST(sample_test, + called_from_google_test_function) // COMPLIANT - Google Test function { bool pass = false; if (called_from_google_test_function(0) >= 10) diff --git a/cpp/common/src/codingstandards/cpp/EncapsulatingFunctions.qll b/cpp/common/src/codingstandards/cpp/EncapsulatingFunctions.qll index 7b2d715d01..8a0d4ffab9 100644 --- a/cpp/common/src/codingstandards/cpp/EncapsulatingFunctions.qll +++ b/cpp/common/src/codingstandards/cpp/EncapsulatingFunctions.qll @@ -3,6 +3,7 @@ */ import cpp +import codingstandards.cpp.Class /** A function which represents the entry point into a specific thread of execution in the program. */ abstract class MainLikeFunction extends Function { } @@ -29,19 +30,22 @@ class MainFunction extends MainLikeFunction { class GoogleTestFunction extends MainLikeFunction { GoogleTestFunction() { // A GoogleTest function is named "TestBody" and - this.hasName("TestBody") and + ( + this.hasName("TestBody") or + this instanceof SpecialMemberFunction + ) and // it's parent class inherits a base class exists(Class base | base = this.getEnclosingAccessHolder().(Class).getABaseClass+() and - // with a name "Test" inside a namespace called "testing" ( + // with a name "Test" inside a namespace called "testing" base.hasName("Test") and base.getNamespace().hasName("testing") + or + // or at a location in a file called gtest.h (or gtest-internal.h, + // gtest-typed-test.h etc). + base.getDefinitionLocation().getFile().getBaseName().regexpMatch("gtest*.h") ) - or - // or at a location in a file called gtest.h (or gtest-internal.h, - // gtest-typed-test.h etc). - base.getDefinitionLocation().getFile().getBaseName().regexpMatch("gtest*.h") ) } } From 3bedebb4df2dc52cf1e1a587e0e007adef78b1e1 Mon Sep 17 00:00:00 2001 From: "rakesh.pothengil" Date: Thu, 10 Oct 2024 11:06:15 +0900 Subject: [PATCH 10/10] Minor improvement --- cpp/autosar/test/rules/M0-1-10/test.cpp | 7 +++++++ .../src/codingstandards/cpp/EncapsulatingFunctions.qll | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/cpp/autosar/test/rules/M0-1-10/test.cpp b/cpp/autosar/test/rules/M0-1-10/test.cpp index 84b17c4c21..e1a19abf24 100644 --- a/cpp/autosar/test/rules/M0-1-10/test.cpp +++ b/cpp/autosar/test/rules/M0-1-10/test.cpp @@ -158,4 +158,11 @@ TEST(sample_test, bool pass = false; if (called_from_google_test_function(0) >= 10) pass = true; + struct a_nested_class_in_gtest { + a_nested_class_in_gtest() noexcept(false) { + static_cast(0); + } // COMPLIANT + }; + static_assert(std::is_trivially_copy_constructible(), + "assert"); } diff --git a/cpp/common/src/codingstandards/cpp/EncapsulatingFunctions.qll b/cpp/common/src/codingstandards/cpp/EncapsulatingFunctions.qll index 8a0d4ffab9..559c04ce98 100644 --- a/cpp/common/src/codingstandards/cpp/EncapsulatingFunctions.qll +++ b/cpp/common/src/codingstandards/cpp/EncapsulatingFunctions.qll @@ -36,7 +36,7 @@ class GoogleTestFunction extends MainLikeFunction { ) and // it's parent class inherits a base class exists(Class base | - base = this.getEnclosingAccessHolder().(Class).getABaseClass+() and + base = this.getEnclosingAccessHolder+().(Class).getABaseClass+() and ( // with a name "Test" inside a namespace called "testing" base.hasName("Test") and