-
Notifications
You must be signed in to change notification settings - Fork 76
Declarations5 cpp misra 2023 #1101
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
knewbury01
wants to merge
10
commits into
github:main
Choose a base branch
from
knewbury01:knewbury01/cpp-misra2023-declarations5
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 2 commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
b4af299
Add RULE-6-9-1
knewbury01 603244e
Add RULE-6-8-4
knewbury01 9cd7884
Merge branch 'main' of https://github.com/github/codeql-coding-standa…
knewbury01 b7f2378
Fix rules duplicate rows
knewbury01 17f18cc
Fix implementation RULE-6-8-4
knewbury01 34ecd57
Merge branch 'main' into knewbury01/cpp-misra2023-declarations5
knewbury01 3e54854
Address feedback
knewbury01 3365b5a
Address feedback comments
knewbury01 f4a39b4
Remove duplicate predicate result and mark FN
knewbury01 81375fa
Fix missing check for qualifiers on members returning subobject refs
knewbury01 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
44 changes: 44 additions & 0 deletions
44
cpp/common/src/codingstandards/cpp/exclusions/cpp/Declarations5.qll
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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())) | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
110 changes: 110 additions & 0 deletions
110
cpp/misra/src/rules/RULE-6-8-4/MemberFunctionsRefqualified.ql
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,110 @@ | ||
| /** | ||
| * @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 | ||
|
|
||
| abstract class MembersReturningObjectOrSubobject extends MemberFunction { | ||
| string toString() { result = "Members returning object or subobject" } | ||
| } | ||
|
|
||
| class MembersReturningObject extends MembersReturningObjectOrSubobject { | ||
| MembersReturningObject() { | ||
| exists(ReturnStmt r, ThisExpr t | | ||
| r.getEnclosingFunction() = this and | ||
| ( | ||
| r.getAChild*() = t | ||
| or | ||
| exists(PointerDereferenceExpr p | | ||
| p.getAChild*() = t and | ||
| r.getAChild*() = p | ||
| ) | ||
| ) and | ||
| t.getActualType().stripType() = this.getDeclaringType() | ||
| ) | ||
|
knewbury01 marked this conversation as resolved.
|
||
| } | ||
| } | ||
|
|
||
| class MembersReturningSubObject extends MembersReturningObjectOrSubobject { | ||
| MembersReturningSubObject() { | ||
| exists(ReturnStmt r, FieldSubObjectDeclaration field | | ||
| r.getEnclosingFunction() = this and | ||
| ( | ||
| r.getAChild*() = field.(Field).getAnAccess() | ||
| or | ||
| exists(PointerDereferenceExpr p | | ||
| p.getAChild*() = field.(Field).getAnAccess() and | ||
| r.getAChild*() = p | ||
| ) | ||
| ) and | ||
| field.(Field).getDeclaringType() = this.getDeclaringType() | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| predicate relevantTypes(Type a, Type b) { | ||
| exists(MembersReturningObject f, MemberFunction overload | | ||
| f.getAnOverload() = overload and | ||
| exists(int i | | ||
| f.getParameter(i).getType() = a and | ||
| overload.getParameter(i).getType() = b | ||
| ) | ||
| ) | ||
| } | ||
|
|
||
| class AppropriatelyQualified extends MembersReturningObjectOrSubobject { | ||
| AppropriatelyQualified() { | ||
| //non-const-lvalue-ref-qualified | ||
| this.isLValueRefQualified() and | ||
| not this.hasSpecifier("const") | ||
| or | ||
| //const-lvalue-ref-qualified | ||
| this.isLValueRefQualified() and | ||
| this.hasSpecifier("const") and | ||
| //and overload exists that is rvalue-ref-qualified | ||
| exists(MemberFunction overload | | ||
| this.getAnOverload() = overload and | ||
| overload.isRValueRefQualified() and | ||
| //and has same param list | ||
| forall(int i | exists([this, overload].getParameter(i)) | | ||
| TypeEquivalence<TypesCompatibleConfig, relevantTypes/2>::equalTypes(this.getParameter(i) | ||
| .getType(), overload.getParameter(i).getType()) | ||
| ) | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Fields that are not reference type can be subobjects | ||
| */ | ||
| class FieldSubObjectDeclaration extends Declaration { | ||
| FieldSubObjectDeclaration() { | ||
| not this.getADeclarationEntry().getType() instanceof ReferenceType and | ||
|
MichaelRFairhurst marked this conversation as resolved.
Outdated
|
||
| this instanceof Field | ||
| } | ||
| } | ||
|
|
||
| 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." | ||
|
MichaelRFairhurst marked this conversation as resolved.
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 | ||
|
knewbury01 marked this conversation as resolved.
|
||
| 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() | ||
7 changes: 7 additions & 0 deletions
7
cpp/misra/test/rules/RULE-6-8-4/MemberFunctionsRefqualified.expected
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| | test.cpp:10:7:10:11 | Members returning object or subobject | Member function is not properly ref qualified. | | ||
| | 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:28:6:28:18 | 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. | | ||
|
MichaelRFairhurst marked this conversation as resolved.
|
||
1 change: 1 addition & 0 deletions
1
cpp/misra/test/rules/RULE-6-8-4/MemberFunctionsRefqualified.qlref
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| rules/RULE-6-8-4/MemberFunctionsRefqualified.ql |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| 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; } // NON_COMPLIANT | ||
|
|
||
|
knewbury01 marked this conversation as resolved.
|
||
| 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 | ||
| }; | ||
|
|
||
| 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 <typename T> | ||
| 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 | ||
| } | ||
2 changes: 2 additions & 0 deletions
2
cpp/misra/test/rules/RULE-6-9-1/TypeAliasesDeclaration.expected
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 | |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| rules/RULE-6-9-1/TypeAliasesDeclaration.ql |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| typedef int INT; | ||
| using Index = int; | ||
|
|
||
| INT i; | ||
| extern int i; // NON_COMPLIANT | ||
|
MichaelRFairhurst marked this conversation as resolved.
|
||
|
|
||
| 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 | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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" | ||
| } | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.