Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds the Memory4 package for MISRA C++ and refactors shared query implementations for detecting overlapping object operations across multiple coding standards (MISRA C, MISRA C++, and AUTOSAR C++).
Changes:
- Adds new Memory4 package for MISRA C++ RULE-8-18-1
- Refactors ObjectAssignedToAnOverlappingObject and ObjectCopiedToAnOverlappingObject queries into shared implementations
- Updates existing queries in Representation (AUTOSAR C++) and Contracts7 (MISRA C) packages to use the new shared implementations
Reviewed changes
Copilot reviewed 37 out of 40 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| rules.csv | Updates RULE-8-18-1 package assignment |
| rule_packages/cpp/Memory4.json | New package definition for MISRA C++ Memory4 queries |
| rule_packages/cpp/Representation.json | Updates query naming to include suffix and shared implementation reference |
| rule_packages/c/Contracts7.json | Updates query naming to include suffix and shared implementation reference |
| cpp/common/src/codingstandards/cpp/rules/objectassignedtoanoverlappingobject/ObjectAssignedToAnOverlappingObject.qll | New shared implementation for overlapping assignment detection |
| cpp/common/src/codingstandards/cpp/rules/objectcopiedtoanoverlappingobject/ObjectCopiedToAnOverlappingObject.qll | Refactored shared implementation for overlapping copy detection |
| cpp/common/src/codingstandards/cpp/exclusions/cpp/Memory4.qll | New exclusion metadata for Memory4 package |
| cpp/common/src/codingstandards/cpp/exclusions/cpp/Representation.qll | Updated query references for renamed queries |
| cpp/common/src/codingstandards/cpp/exclusions/c/Contracts7.qll | Updated query references for renamed queries |
| cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll | Added Memory4 package integration |
| cpp/misra/src/rules/RULE-8-18-1/* | New MISRA C++ query implementations |
| cpp/autosar/src/rules/M0-2-1/* | Refactored AUTOSAR C++ query to use shared implementation |
| c/misra/src/rules/RULE-19-1/* | Refactored MISRA C query to use shared implementation |
| /test/ | Updated test infrastructure for all affected queries |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
rule_packages/cpp/Memory4.json
Outdated
| }, | ||
| "queries": [ | ||
| { | ||
| "description": "Copying a member of a union to another causes undefined behavior.", |
There was a problem hiding this comment.
The description says "Copying a member of a union to another" which is grammatically incorrect. It should be "Copying a member of a union to another member" for clarity.
| /** | ||
| * @id cpp/misra/object-assigned-to-an-overlapping-object-misra-cpp | ||
| * @name RULE-8-18-1: A member of a union must not be copied to its another member | ||
| * @description Copying a member of a union to another causes undefined behavior. |
There was a problem hiding this comment.
The description says "Copying a member of a union to another" which is grammatically incorrect. It should be "Copying a member of a union to another member" for clarity.
| exists(Expr lhs, Expr rhs | | ||
| lhs.getType() instanceof Union and | ||
| rhs.getType() instanceof Union and | ||
| lhs = getAQualifier(assignExpr.getLValue()) and | ||
| rhs = getAQualifier(assignExpr.getRValue()) and | ||
| globalValueNumber(lhs) = globalValueNumber(rhs) and | ||
| valuerhs = assignExpr.getRValue() and | ||
| valuelhs = assignExpr.getLValue() and // a.b.c == ((a.b).c) | ||
| overlaps(valuelhs, valuerhs) and | ||
| message = "An object $@ assigned to overlapping object $@." and | ||
| valuelhsTargetName = valuelhs.getTarget().getName() and | ||
| valuerhsTargetName = valuerhs.getTarget().getName() | ||
| ) |
There was a problem hiding this comment.
The query predicate is missing the exclusion check. The commented code shows that the isExcluded check should be included, but it's not present in the actual implementation. This means the query will not respect user-defined exclusions.
rule_packages/cpp/Memory4.json
Outdated
| { | ||
| "description": "Copying a slice of an array to an overlapping region of the same array causes undefined behavior.", | ||
| "kind": "problem", | ||
| "name": "An slice of an array must not be copied to an overlapping region of itself", |
There was a problem hiding this comment.
There is a grammatical error in the query name. It should be "A slice of an array" not "An slice of an array". The article "An" is incorrect before "slice".
| @@ -0,0 +1,24 @@ | |||
| /** | |||
| * @id cpp/misra/object-copied-to-an-overlapping-object-misra-cpp | |||
| * @name RULE-8-18-1: An slice of an array must not be copied to an overlapping region of itself | |||
There was a problem hiding this comment.
There is a grammatical error in the query name. It should be "A slice of an array" not "An slice of an array". The article "An" is incorrect before "slice".
Description
Add
Memory4package.High-level overview
DoNotPassAliasedPointerToParamObjectAssignedToAnOverlappingObject(Factored out)ObjectAssignedToAnOverlappingObject(Factored out)ObjectCopiedToAnOverlappingObject(Factored out)ObjectCopiedToAnOverlappingObject(Imports factored-out shared query)ObjectAssignedToAnOverlappingObject(Imports factored-out shared query)Step-by-step process overview
RULE-19-1andM0-2-1to add"shared_implementation_short_name"(existing)"short_name":ObjectAssignedToAnOverlappingObjectRepresentation"shared_implementation_short_name":ObjectAssignedToAnOverlappingObject"short_name":"ObjectAssignedToAnOverlappingObjectAutosarCpp"(existing)"short_name"1:ObjectAssignedToAnOverlappingObject"short_name"2:ObjectCopiedToAnOverlappingObjectContracts7"shared_implementation_short_name"1:ObjectAssignedToAnOverlappingObject"shared_implementation_short_name"2:ObjectCopiedToAnOverlappingObject"short_name"1:"ObjectAssignedToAnOverlappingObjectMisraC""short_name"2:"ObjectCopiedToAnOverlappingObjectMisraC"(new)Memory4"shared_implementation_short_name"1:ObjectAssignedToAnOverlappingObject"shared_implementation_short_name"2:ObjectCopiedToAnOverlappingObject"short_name"1:"ObjectAssignedToAnOverlappingObjectMisraCpp""short_name"2:"ObjectCopiedToAnOverlappingObjectMisraCpp"cpp/common/src/codingstandards/cpp/rules/objectassignedtoanoverlappingobject/ObjectAssignedToAnOverlappingObject.qllcpp/common/src/codingstandards/cpp/rules/objectcopiedtoanoverlappingobject/ObjectCopiedToAnOverlappingObject.qllfrom-where-selectinto aquery predicate problemin the file right above (step 4)cpp/common/src/codingstandards/cpp/rules/objectassignedtoanoverlappingobject/ObjectAssignedToAnOverlappingObject.qllcpp/common/src/codingstandards/cpp/rules/objectcopiedtoanoverlappingobject/ObjectCopiedToAnOverlappingObject.qllcpp/common/test/rules/objectassignedtoanoverlappingobject/test.cppcpp/common/test/rules/objectcopiedtoanoverlappingobject/test.cppChange request type
.ql,.qll,.qlsor unit tests)Rules with added or modified queries
RULE-8-18-1M0-2-1RULE-19-1Release change checklist
A change note (development_handbook.md#change-notes) is required for any pull request which modifies:
If you are only adding new rule queries, a change note is not required.
Author: Is a change note required?
🚨🚨🚨
Reviewer: Confirm that format of shared queries (not the .qll file, the
.ql file that imports it) is valid by running them within VS Code.
Reviewer: Confirm that either a change note is not required or the change note is required and has been added.
Query development review checklist
For PRs that add new queries or modify existing queries, the following checklist should be completed by both the author and reviewer:
Author
As a rule of thumb, predicates specific to the query should take no more than 1 minute, and for simple queries be under 10 seconds. If this is not the case, this should be highlighted and agreed in the code review process.
Reviewer
As a rule of thumb, predicates specific to the query should take no more than 1 minute, and for simple queries be under 10 seconds. If this is not the case, this should be highlighted and agreed in the code review process.