diff --git a/cpp/common/src/codingstandards/cpp/exclusions/cpp/Declarations5.qll b/cpp/common/src/codingstandards/cpp/exclusions/cpp/Declarations5.qll new file mode 100644 index 000000000..ff4806a5b --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/exclusions/cpp/Declarations5.qll @@ -0,0 +1,44 @@ +//** THIS FILE IS AUTOGENERATED, DO NOT MODIFY DIRECTLY. **/ +import cpp +import RuleMetadata +import codingstandards.cpp.exclusions.RuleMetadata + +newtype Declarations5Query = + TMemberFunctionsRefqualifiedQuery() or + TTypeAliasesDeclarationQuery() + +predicate isDeclarations5QueryMetadata(Query query, string queryId, string ruleId, string category) { + query = + // `Query` instance for the `memberFunctionsRefqualified` query + Declarations5Package::memberFunctionsRefqualifiedQuery() and + queryId = + // `@id` for the `memberFunctionsRefqualified` query + "cpp/misra/member-functions-refqualified" and + ruleId = "RULE-6-8-4" and + category = "advisory" + or + query = + // `Query` instance for the `typeAliasesDeclaration` query + Declarations5Package::typeAliasesDeclarationQuery() and + queryId = + // `@id` for the `typeAliasesDeclaration` query + "cpp/misra/type-aliases-declaration" and + ruleId = "RULE-6-9-1" and + category = "required" +} + +module Declarations5Package { + Query memberFunctionsRefqualifiedQuery() { + //autogenerate `Query` type + result = + // `Query` type for `memberFunctionsRefqualified` query + TQueryCPP(TDeclarations5PackageQuery(TMemberFunctionsRefqualifiedQuery())) + } + + Query typeAliasesDeclarationQuery() { + //autogenerate `Query` type + result = + // `Query` type for `typeAliasesDeclaration` query + TQueryCPP(TDeclarations5PackageQuery(TTypeAliasesDeclarationQuery())) + } +} diff --git a/cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll b/cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll index 5618f4d85..7b55e22bc 100644 --- a/cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll +++ b/cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll @@ -39,6 +39,7 @@ import Declarations1 import Declarations2 import Declarations3 import Declarations4 +import Declarations5 import Declarations6 import Declarations7 import ExceptionSafety @@ -144,6 +145,7 @@ newtype TCPPQuery = TDeclarations2PackageQuery(Declarations2Query q) or TDeclarations3PackageQuery(Declarations3Query q) or TDeclarations4PackageQuery(Declarations4Query q) or + TDeclarations5PackageQuery(Declarations5Query q) or TDeclarations6PackageQuery(Declarations6Query q) or TDeclarations7PackageQuery(Declarations7Query q) or TExceptionSafetyPackageQuery(ExceptionSafetyQuery q) or @@ -249,6 +251,7 @@ predicate isQueryMetadata(Query query, string queryId, string ruleId, string cat isDeclarations2QueryMetadata(query, queryId, ruleId, category) or isDeclarations3QueryMetadata(query, queryId, ruleId, category) or isDeclarations4QueryMetadata(query, queryId, ruleId, category) or + isDeclarations5QueryMetadata(query, queryId, ruleId, category) or isDeclarations6QueryMetadata(query, queryId, ruleId, category) or isDeclarations7QueryMetadata(query, queryId, ruleId, category) or isExceptionSafetyQueryMetadata(query, queryId, ruleId, category) or diff --git a/cpp/common/src/codingstandards/cpp/lifetimes/CppObjects.qll b/cpp/common/src/codingstandards/cpp/lifetimes/CppObjects.qll index 1c38859cb..9501f7e06 100644 --- a/cpp/common/src/codingstandards/cpp/lifetimes/CppObjects.qll +++ b/cpp/common/src/codingstandards/cpp/lifetimes/CppObjects.qll @@ -2,6 +2,7 @@ import cpp import codingstandards.cpp.lifetimes.StorageDuration import semmle.code.cpp.valuenumbering.HashCons import codingstandards.cpp.Clvalues +import codingstandards.cpp.types.Pointers /** * A library for handling "Objects" in C++. @@ -136,19 +137,41 @@ abstract class ObjectIdentityBase extends Element { } /** - * Finds expressions `e.x` or `e[x]` for expression `e`, recursively. Does not resolve pointers. + * Finds expressions `e.x` or `e[x]` for expression `e`, recursively. + * + * Omits accesses to reference fields, as they are not subobjects. * * Note that this does not hold for `e->x` or `e[x]` where `e` is a pointer. */ -private Expr getASubobjectAccessOf(Expr e) { +Expr getASubobjectAccessOf(Expr e) { result = e or - result.(DotFieldAccess).getQualifier() = getASubobjectAccessOf(e) + result.(DotFieldAccess).getQualifier() = getASubobjectAccessOf(e) and + not result.(DotFieldAccess).getTarget().getUnderlyingType() instanceof ReferenceType or result.(ArrayExpr).getArrayBase() = getASubobjectAccessOf(e) and not result.(ArrayExpr).getArrayBase().getUnspecifiedType() instanceof PointerType } +/** + * Finds subobjects of the pointee for expression `e`, + * where `e` has the type pointer type. + * + * For `e` this will be subobject accesses of `*e`. + * Or for `e->x` the subobject access is `x`. + * + * This predicate is not used/tested extensively so + * verify it before use. + */ +Expr getASubobjectAccessOfPointee(Expr e) { + e.getParent() instanceof PointerDereferenceExpr and + result = getASubobjectAccessOf(e.getParent()) + or + // for e1->e2 : getASubobjectAccessOfPointee(e1) = e2 and or subobjects of e2 + result = getASubobjectAccessOf(e.getParent().(PointerFieldAccess)) and + not result.(PointerFieldAccess).getTarget().getUnderlyingType() instanceof ReferenceType +} + /** * Find the object types that are embedded within the current type. * diff --git a/cpp/common/src/codingstandards/cpp/types/Compatible.qll b/cpp/common/src/codingstandards/cpp/types/Compatible.qll index e2f8654b8..091742a29 100644 --- a/cpp/common/src/codingstandards/cpp/types/Compatible.qll +++ b/cpp/common/src/codingstandards/cpp/types/Compatible.qll @@ -486,11 +486,11 @@ signature predicate interestedInFunctionDeclarations( ); module FunctionDeclarationTypeEquivalence< - TypeEquivalenceSig Config, interestedInFunctionDeclarations/2 interestedInFunctions> + TypeEquivalenceSig Config, interestedInFunctionDeclarations/2 interestedInFunDecls> { private predicate interestedInReturnTypes(Type a, Type b) { exists(FunctionDeclarationEntry aFun, FunctionDeclarationEntry bFun | - interestedInFunctions(pragma[only_bind_into](aFun), pragma[only_bind_into](bFun)) and + interestedInFunDecls(pragma[only_bind_into](aFun), pragma[only_bind_into](bFun)) and a = aFun.getType() and b = bFun.getType() ) @@ -498,19 +498,19 @@ module FunctionDeclarationTypeEquivalence< private predicate interestedInParameterTypes(Type a, Type b) { exists(FunctionDeclarationEntry aFun, FunctionDeclarationEntry bFun, int i | - interestedInFunctions(pragma[only_bind_into](aFun), pragma[only_bind_into](bFun)) and + interestedInFunDecls(pragma[only_bind_into](aFun), pragma[only_bind_into](bFun)) and a = aFun.getParameterDeclarationEntry(i).getType() and b = bFun.getParameterDeclarationEntry(i).getType() ) } predicate equalReturnTypes(FunctionDeclarationEntry f1, FunctionDeclarationEntry f2) { - interestedInFunctions(f1, f2) and + interestedInFunDecls(f1, f2) and TypeEquivalence::equalTypes(f1.getType(), f2.getType()) } predicate equalParameterTypes(FunctionDeclarationEntry f1, FunctionDeclarationEntry f2) { - interestedInFunctions(f1, f2) and + interestedInFunDecls(f1, f2) and f1.getDeclaration() = f2.getDeclaration() and forall(int i | exists([f1, f2].getParameterDeclarationEntry(i)) | equalParameterTypesAt(f1, f2, pragma[only_bind_into](i)) @@ -518,13 +518,40 @@ module FunctionDeclarationTypeEquivalence< } predicate equalParameterTypesAt(FunctionDeclarationEntry f1, FunctionDeclarationEntry f2, int i) { - interestedInFunctions(f1, f2) and + interestedInFunDecls(f1, f2) and f1.getDeclaration() = f2.getDeclaration() and TypeEquivalence::equalTypes(f1.getParameterDeclarationEntry(pragma[only_bind_into](i)) .getType(), f2.getParameterDeclarationEntry(pragma[only_bind_into](i)).getType()) } } +signature predicate interestedInFunctionsByFunction(Function f1, Function f2); + +module FunctionEquivalence< + TypeEquivalenceSig Config, interestedInFunctionsByFunction/2 interestedInFuncs> +{ + private predicate interestedInParameterTypes(Type a, Type b) { + exists(Function aFun, Function bFun, int i | + interestedInFuncs(pragma[only_bind_into](aFun), pragma[only_bind_into](bFun)) and + a = aFun.getParameter(i).getType() and + b = bFun.getParameter(i).getType() + ) + } + + predicate equalParameterTypes(Function f1, Function f2) { + interestedInFuncs(f1, f2) and + forall(int i | exists([f1, f2].getParameter(i)) | + equalParameterTypesAt(f1, f2, pragma[only_bind_into](i)) + ) + } + + predicate equalParameterTypesAt(Function f1, Function f2, int i) { + interestedInFuncs(f1, f2) and + TypeEquivalence::equalTypes(f1.getParameter(pragma[only_bind_into](i)) + .getType(), f2.getParameter(pragma[only_bind_into](i)).getType()) + } +} + private class LeafType extends Type { LeafType() { not this instanceof DerivedType and diff --git a/cpp/misra/src/rules/RULE-6-8-4/MemberFunctionsRefqualified.ql b/cpp/misra/src/rules/RULE-6-8-4/MemberFunctionsRefqualified.ql new file mode 100644 index 000000000..800e84818 --- /dev/null +++ b/cpp/misra/src/rules/RULE-6-8-4/MemberFunctionsRefqualified.ql @@ -0,0 +1,105 @@ +/** + * @id cpp/misra/member-functions-refqualified + * @name RULE-6-8-4: Member functions returning references to their object should be refqualified appropriately + * @description Member functions that return references to temporary objects (or subobjects) can + * lead to dangling pointers. + * @kind problem + * @precision very-high + * @problem.severity error + * @tags external/misra/id/rule-6-8-4 + * correctness + * scope/single-translation-unit + * external/misra/enforcement/decidable + * external/misra/obligation/advisory + */ + +import cpp +import codingstandards.cpp.misra +import codingstandards.cpp.types.Compatible +import codingstandards.cpp.Operator +import codingstandards.cpp.types.Pointers +import codingstandards.cpp.lifetimes.CppObjects + +class MembersReturningPointerOrRef extends MemberFunction { + MembersReturningPointerOrRef() { this.getType() instanceof PointerLikeType } +} + +abstract class MembersReturningObjectOrSubobject extends MembersReturningPointerOrRef { + string toString() { result = "Members returning object or subobject" } +} + +/** + * Members that have an explicit `this` access within their return statement. + */ +class MembersReturningObject extends MembersReturningObjectOrSubobject { + MembersReturningObject() { + exists(ReturnStmt r, ThisExpr t | + r.getEnclosingFunction() = this and + //return `this` + r.getAChild() = t and + t.getActualType().stripType() = this.getDeclaringType() + ) + } +} + +/** + * Members that have an explicit subobject access within their return statement. + * + * Specifically, this captures when the return is a reference or pointer + * to a subobject. + */ +class MembersReturningSubObject extends MembersReturningObjectOrSubobject { + MembersReturningSubObject() { + exists(ReturnStmt r, FieldAccess access, Expr e | + r.getEnclosingFunction() = this and + (not exists(access.getQualifier()) or access.getQualifier() instanceof ThisExpr) and + ( + //subobject returned by address + r.getAChild() = access.getParent() and + e = getASubobjectAccessOf(access) and + access.getParent() instanceof AddressOfExpr + or + //reference to subobject returned + r.getAChild() = e and + e = getASubobjectAccessOf(access) and + this.getType() instanceof ReferenceType + ) + ) + } +} + +predicate relevantFunctions(Function a, Function b) { + a instanceof MembersReturningObjectOrSubobject and + a.getAnOverload() = b +} + +class AppropriatelyQualified extends MemberFunction { + AppropriatelyQualified() { + //non-const-lvalue-ref-qualified + this.isLValueRefQualified() and + this.getType().(PointerLikeType).pointsToNonConst() + or + //const-lvalue-ref-qualified + this.isLValueRefQualified() and + this.getType().(PointerLikeType).pointsToConst() and + //and overload exists that is rvalue-ref-qualified + exists(MemberFunction overload | + this.getAnOverload() = overload and + overload.isRValueRefQualified() and + //and has same param list + FunctionEquivalence::equalParameterTypes(this, + overload) + ) + } +} + +class DefaultedAssignmentOperator extends AssignmentOperator { + DefaultedAssignmentOperator() { this.isDefaulted() } +} + +from MembersReturningObjectOrSubobject f +where + not isExcluded(f, Declarations5Package::memberFunctionsRefqualifiedQuery()) and + not f instanceof AppropriatelyQualified and + not f instanceof DefaultedAssignmentOperator +select f, "Member function is not properly ref qualified." diff --git a/cpp/misra/src/rules/RULE-6-9-1/TypeAliasesDeclaration.ql b/cpp/misra/src/rules/RULE-6-9-1/TypeAliasesDeclaration.ql new file mode 100644 index 000000000..d8898377d --- /dev/null +++ b/cpp/misra/src/rules/RULE-6-9-1/TypeAliasesDeclaration.ql @@ -0,0 +1,33 @@ +/** + * @id cpp/misra/type-aliases-declaration + * @name RULE-6-9-1: The same type aliases shall be used in all declarations of the same entity + * @description Using different type aliases on redeclarations can make code hard to understand and + * maintain. + * @kind problem + * @precision very-high + * @problem.severity warning + * @tags external/misra/id/rule-6-9-1 + * maintainability + * readability + * scope/single-translation-unit + * external/misra/enforcement/decidable + * external/misra/obligation/required + */ + +import cpp +import codingstandards.cpp.misra + +from DeclarationEntry decl1, DeclarationEntry decl2, TypedefType t +where + not isExcluded(decl1, Declarations5Package::typeAliasesDeclarationQuery()) and + not isExcluded(decl2, Declarations5Package::typeAliasesDeclarationQuery()) and + not decl1 = decl2 and + decl1.getDeclaration() = decl2.getDeclaration() and + t.getATypeNameUse() = decl1 and + not t.getATypeNameUse() = decl2 and + //exception cases - we dont want to disallow struct typedef name use + not t.getBaseType() instanceof Struct and + not t.getBaseType() instanceof Enum +select decl1, + "Declaration entry has a different type alias than $@ where the type alias used is '$@'.", decl2, + decl2.getName(), t, t.getName() diff --git a/cpp/misra/test/rules/RULE-6-8-4/MemberFunctionsRefqualified.expected b/cpp/misra/test/rules/RULE-6-8-4/MemberFunctionsRefqualified.expected new file mode 100644 index 000000000..f01cedf9e --- /dev/null +++ b/cpp/misra/test/rules/RULE-6-8-4/MemberFunctionsRefqualified.expected @@ -0,0 +1,13 @@ +| test.cpp:12:14:12:20 | Members returning object or subobject | Member function is not properly ref qualified. | +| test.cpp:24:12:24:23 | Members returning object or subobject | Member function is not properly ref qualified. | +| test.cpp:42:16:42:16 | Members returning object or subobject | Member function is not properly ref qualified. | +| test.cpp:42:16:42:16 | Members returning object or subobject | Member function is not properly ref qualified. | +| test.cpp:42:16:42:16 | Members returning object or subobject | Member function is not properly ref qualified. | +| test.cpp:61:8:61:8 | Members returning object or subobject | Member function is not properly ref qualified. | +| test.cpp:71:9:71:10 | Members returning object or subobject | Member function is not properly ref qualified. | +| test.cpp:79:8:79:9 | Members returning object or subobject | Member function is not properly ref qualified. | +| test.cpp:89:9:89:10 | Members returning object or subobject | Member function is not properly ref qualified. | +| test.cpp:103:8:103:8 | Members returning object or subobject | Member function is not properly ref qualified. | +| test.cpp:113:9:113:10 | Members returning object or subobject | Member function is not properly ref qualified. | +| test.cpp:121:8:121:9 | Members returning object or subobject | Member function is not properly ref qualified. | +| test.cpp:131:9:131:10 | Members returning object or subobject | Member function is not properly ref qualified. | diff --git a/cpp/misra/test/rules/RULE-6-8-4/MemberFunctionsRefqualified.qlref b/cpp/misra/test/rules/RULE-6-8-4/MemberFunctionsRefqualified.qlref new file mode 100644 index 000000000..c214d5ca1 --- /dev/null +++ b/cpp/misra/test/rules/RULE-6-8-4/MemberFunctionsRefqualified.qlref @@ -0,0 +1 @@ +rules/RULE-6-8-4/MemberFunctionsRefqualified.ql \ No newline at end of file diff --git a/cpp/misra/test/rules/RULE-6-8-4/test.cpp b/cpp/misra/test/rules/RULE-6-8-4/test.cpp new file mode 100644 index 000000000..2fcc80088 --- /dev/null +++ b/cpp/misra/test/rules/RULE-6-8-4/test.cpp @@ -0,0 +1,138 @@ +struct A { + int a; + int &b; + + int &geta() & { return a; } // COMPLIANT + + int const &geta2() const & { // COMPLIANT -- due to overload below + return a; + } + int geta2() && { return a; } + + int const &getabad() const & { // NON_COMPLIANT -- no overload provided + return a; + } + + int getb() && { return b; } // COMPLIANT -- b is not a subobject + + A const *getstruct() const & { // COMPLIANT -- due to overload below + return this; + } + + A getstruct() const && = delete; + + A const *getstructbad() const & { // NON_COMPLIANT -- no overload provided + return this; + } + + A &getstructbad2() { return *this; } // NON_COMPLIANT[FALSE_NEGATIVE] +}; + +class C { + C *f() { // COMPLIANT -- this is not explicitly designated therefore this is + // not + // relevant for this rule + C *thisclass = this; + return thisclass; + } +}; + +struct Templ { + template + Templ const *f(T) const & { // NON_COMPLIANT -- for an instantiation below + return this; + } + + void f(int) const && = delete; +}; + +void f(int p, float p1) { + Templ t; + t.f(p); + t.f(p1); // instantiation that causes issue due to parameter list type meaning + // there is no overload +} + +class B { + int i; + int *ip; + int **ip2; + + int *f() { + return &i; // NON_COMPLIANT + } + int *f1() { + return ip; // COMPLIANT -- reads pointer + } + int *f2() { + return *ip2; // COMPLIANT -- reads pointer + } + + int **f3() { + return &ip; // NON_COMPLIANT + } + + int **f4() { + return ip2; // COMPLIANT -- copies pointer + } + + int &f5() { + return i; // NON_COMPLIANT + } + int &f6() { + return *ip; // COMPLIANT -- reads pointer + } + int &f7() { + return **ip2; // COMPLIANT -- reads pointer + } + + int *&f8() { + // return &p; // won't compile + return ip; // NON_COMPLIANT + } + int *&f9() { + return *ip2; // COMPLIANT -- reads pointer + } +}; + +class D { + int i; + int *ip; + int **ip2; + + int *f() { + return &(this->i); // NON_COMPLIANT + } + int *f1() { + return this->ip; // COMPLIANT -- reads pointer + } + int *f2() { + return *this->ip2; // COMPLIANT -- reads pointer + } + + int **f3() { + return &this->ip; // NON_COMPLIANT + } + + int **f4() { + return this->ip2; // COMPLIANT -- copies pointer + } + + int &f5() { + return this->i; // NON_COMPLIANT + } + int &f6() { + return *this->ip; // COMPLIANT -- reads pointer + } + int &f7() { + return **this->ip2; // COMPLIANT -- reads pointer + } + + int *&f8() { + // return &p; // won't compile + return this->ip; // NON_COMPLIANT + } + int *&f9() { + return *this->ip2; // COMPLIANT -- reads pointer + } +}; \ No newline at end of file diff --git a/cpp/misra/test/rules/RULE-6-9-1/TypeAliasesDeclaration.expected b/cpp/misra/test/rules/RULE-6-9-1/TypeAliasesDeclaration.expected new file mode 100644 index 000000000..db0ef7b72 --- /dev/null +++ b/cpp/misra/test/rules/RULE-6-9-1/TypeAliasesDeclaration.expected @@ -0,0 +1,2 @@ +| test.cpp:4:5:4:5 | definition of i | Declaration entry has a different type alias than $@ where the type alias used is '$@'. | test.cpp:5:12:5:12 | declaration of i | i | test.cpp:1:13:1:15 | INT | INT | +| test.cpp:11:20:11:20 | declaration of i | Declaration entry has a different type alias than $@ where the type alias used is '$@'. | test.cpp:10:12:10:12 | declaration of i | i | test.cpp:2:7:2:11 | Index | Index | diff --git a/cpp/misra/test/rules/RULE-6-9-1/TypeAliasesDeclaration.qlref b/cpp/misra/test/rules/RULE-6-9-1/TypeAliasesDeclaration.qlref new file mode 100644 index 000000000..a3cf4823c --- /dev/null +++ b/cpp/misra/test/rules/RULE-6-9-1/TypeAliasesDeclaration.qlref @@ -0,0 +1 @@ +rules/RULE-6-9-1/TypeAliasesDeclaration.ql \ No newline at end of file diff --git a/cpp/misra/test/rules/RULE-6-9-1/test.cpp b/cpp/misra/test/rules/RULE-6-9-1/test.cpp new file mode 100644 index 000000000..dfc18becf --- /dev/null +++ b/cpp/misra/test/rules/RULE-6-9-1/test.cpp @@ -0,0 +1,15 @@ +typedef int INT; +using Index = int; + +INT i; +extern int i; // NON_COMPLIANT + +INT j; +extern INT j; // COMPLIANT + +void g(int i); +void g(Index const i); // NON_COMPLIANT + +void h(Index i); +void h(Index const i); // COMPLIANT +void h(int *i); // COMPLIANT \ No newline at end of file diff --git a/rule_packages/cpp/Declarations5.json b/rule_packages/cpp/Declarations5.json new file mode 100644 index 000000000..4f4c08e7d --- /dev/null +++ b/rule_packages/cpp/Declarations5.json @@ -0,0 +1,47 @@ +{ + "MISRA-C++-2023": { + "RULE-6-8-4": { + "properties": { + "enforcement": "decidable", + "obligation": "advisory" + }, + "queries": [ + { + "description": "Member functions that return references to temporary objects (or subobjects) can lead to dangling pointers.", + "kind": "problem", + "name": "Member functions returning references to their object should be refqualified appropriately", + "precision": "very-high", + "severity": "error", + "short_name": "MemberFunctionsRefqualified", + "tags": [ + "correctness", + "scope/single-translation-unit" + ] + } + ], + "title": "Member functions returning references to their object should be refqualified appropriately" + }, + "RULE-6-9-1": { + "properties": { + "enforcement": "decidable", + "obligation": "required" + }, + "queries": [ + { + "description": "Using different type aliases on redeclarations can make code hard to understand and maintain.", + "kind": "problem", + "name": "The same type aliases shall be used in all declarations of the same entity", + "precision": "very-high", + "severity": "warning", + "short_name": "TypeAliasesDeclaration", + "tags": [ + "maintainability", + "readability", + "scope/single-translation-unit" + ] + } + ], + "title": "The same type aliases shall be used in all declarations of the same entity" + } + } +} \ No newline at end of file