-
Notifications
You must be signed in to change notification settings - Fork 76
Add Banned2, Banned3, and Banned4
#1080
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
jeongsoolee09
wants to merge
14
commits into
main
Choose a base branch
from
jeongsoolee09/MISRA-C++-2023-Banned234
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 all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
6dc64be
Add rule package of Banned2
jeongsoolee09 21af40d
Add implementation of Banned2
jeongsoolee09 20f7399
Renumber rules in rules.csv
jeongsoolee09 447d961
Draft out first three cases
jeongsoolee09 1a08d11
Add query files
jeongsoolee09 ba3afac
Add `enumFitsInType` and related test cases
jeongsoolee09 c4179a0
Debug enumFitsInType
jeongsoolee09 c91e5c5
Finish the detection logic
jeongsoolee09 cfc6de7
Minor edits of comments
jeongsoolee09 a1f17d3
Finalize 10.2.3 and update expected results
jeongsoolee09 96fdc0c
Update expected results of 10.2.2
jeongsoolee09 4ae8c1d
Use `NestedEnum` instead of hand-rolled definition
jeongsoolee09 7f4ebd4
Add query files for Banned4
jeongsoolee09 075c51f
Merge branch 'main' into jeongsoolee09/MISRA-C++-2023-Banned234
jeongsoolee09 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
26 changes: 26 additions & 0 deletions
26
cpp/common/src/codingstandards/cpp/exclusions/cpp/Banned2.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,26 @@ | ||
| //** THIS FILE IS AUTOGENERATED, DO NOT MODIFY DIRECTLY. **/ | ||
| import cpp | ||
| import RuleMetadata | ||
| import codingstandards.cpp.exclusions.RuleMetadata | ||
|
|
||
| newtype Banned2Query = TUnscopedEnumerationsShouldNotBeDeclaredQuery() | ||
|
|
||
| predicate isBanned2QueryMetadata(Query query, string queryId, string ruleId, string category) { | ||
| query = | ||
| // `Query` instance for the `unscopedEnumerationsShouldNotBeDeclared` query | ||
| Banned2Package::unscopedEnumerationsShouldNotBeDeclaredQuery() and | ||
| queryId = | ||
| // `@id` for the `unscopedEnumerationsShouldNotBeDeclared` query | ||
| "cpp/misra/unscoped-enumerations-should-not-be-declared" and | ||
| ruleId = "RULE-10-2-2" and | ||
| category = "advisory" | ||
| } | ||
|
|
||
| module Banned2Package { | ||
| Query unscopedEnumerationsShouldNotBeDeclaredQuery() { | ||
| //autogenerate `Query` type | ||
| result = | ||
| // `Query` type for `unscopedEnumerationsShouldNotBeDeclared` query | ||
| TQueryCPP(TBanned2PackageQuery(TUnscopedEnumerationsShouldNotBeDeclaredQuery())) | ||
| } | ||
| } |
26 changes: 26 additions & 0 deletions
26
cpp/common/src/codingstandards/cpp/exclusions/cpp/Banned3.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,26 @@ | ||
| //** THIS FILE IS AUTOGENERATED, DO NOT MODIFY DIRECTLY. **/ | ||
| import cpp | ||
| import RuleMetadata | ||
| import codingstandards.cpp.exclusions.RuleMetadata | ||
|
|
||
| newtype Banned3Query = TUnscopedEnumWithoutFixedUnderlyingTypeUsedQuery() | ||
|
|
||
| predicate isBanned3QueryMetadata(Query query, string queryId, string ruleId, string category) { | ||
| query = | ||
| // `Query` instance for the `unscopedEnumWithoutFixedUnderlyingTypeUsed` query | ||
| Banned3Package::unscopedEnumWithoutFixedUnderlyingTypeUsedQuery() and | ||
| queryId = | ||
| // `@id` for the `unscopedEnumWithoutFixedUnderlyingTypeUsed` query | ||
| "cpp/misra/unscoped-enum-without-fixed-underlying-type-used" and | ||
| ruleId = "RULE-10-2-3" and | ||
| category = "required" | ||
| } | ||
|
|
||
| module Banned3Package { | ||
| Query unscopedEnumWithoutFixedUnderlyingTypeUsedQuery() { | ||
| //autogenerate `Query` type | ||
| result = | ||
| // `Query` type for `unscopedEnumWithoutFixedUnderlyingTypeUsed` query | ||
| TQueryCPP(TBanned3PackageQuery(TUnscopedEnumWithoutFixedUnderlyingTypeUsedQuery())) | ||
| } | ||
| } |
26 changes: 26 additions & 0 deletions
26
cpp/common/src/codingstandards/cpp/exclusions/cpp/Banned4.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,26 @@ | ||
| //** THIS FILE IS AUTOGENERATED, DO NOT MODIFY DIRECTLY. **/ | ||
| import cpp | ||
| import RuleMetadata | ||
| import codingstandards.cpp.exclusions.RuleMetadata | ||
|
|
||
| newtype Banned4Query = TUnnamedNamespacesInHeaderFilesQuery() | ||
|
|
||
| predicate isBanned4QueryMetadata(Query query, string queryId, string ruleId, string category) { | ||
| query = | ||
| // `Query` instance for the `unnamedNamespacesInHeaderFiles` query | ||
| Banned4Package::unnamedNamespacesInHeaderFilesQuery() and | ||
| queryId = | ||
| // `@id` for the `unnamedNamespacesInHeaderFiles` query | ||
| "cpp/misra/unnamed-namespaces-in-header-files" and | ||
| ruleId = "RULE-10-3-1" and | ||
| category = "advisory" | ||
| } | ||
|
|
||
| module Banned4Package { | ||
| Query unnamedNamespacesInHeaderFilesQuery() { | ||
| //autogenerate `Query` type | ||
| result = | ||
| // `Query` type for `unnamedNamespacesInHeaderFiles` query | ||
| TQueryCPP(TBanned4PackageQuery(TUnnamedNamespacesInHeaderFilesQuery())) | ||
| } | ||
| } |
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
28 changes: 28 additions & 0 deletions
28
cpp/misra/src/rules/RULE-10-2-2/UnscopedEnumerationsShouldNotBeDeclared.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,28 @@ | ||
| /** | ||
| * @id cpp/misra/unscoped-enumerations-should-not-be-declared | ||
| * @name RULE-10-2-2: Unscoped enumerations should not be declared | ||
| * @description An unscoped enumeration should not be used outside of a class/struct scope; use | ||
| * 'enum class' instead to prevent name clashes and implicit conversions to integral | ||
| * types. | ||
| * @kind problem | ||
| * @precision very-high | ||
| * @problem.severity error | ||
| * @tags external/misra/id/rule-10-2-2 | ||
| * scope/single-translation-unit | ||
| * correctness | ||
| * external/misra/enforcement/decidable | ||
| * external/misra/obligation/advisory | ||
| */ | ||
|
|
||
| import cpp | ||
| import codingstandards.cpp.misra | ||
|
|
||
| class NestedUnscopedEnum extends Enum, NestedEnum { | ||
| NestedUnscopedEnum() { not this instanceof ScopedEnum } | ||
| } | ||
|
|
||
| from Enum enum | ||
| where | ||
| not isExcluded(enum, Banned2Package::unscopedEnumerationsShouldNotBeDeclaredQuery()) and | ||
| not (enum instanceof ScopedEnum or enum instanceof NestedUnscopedEnum) | ||
| select enum, "This enumeration is an unscoped enum not enclosed in a class or a struct." |
238 changes: 238 additions & 0 deletions
238
cpp/misra/src/rules/RULE-10-2-3/UnscopedEnumWithoutFixedUnderlyingTypeUsed.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,238 @@ | ||
| /** | ||
| * @id cpp/misra/unscoped-enum-without-fixed-underlying-type-used | ||
| * @name RULE-10-2-3: The numeric value of an unscoped enumeration with no fixed underlying type shall not be used | ||
| * @description Treating unscoped enumeration without a fixed underlying type as an integral type is | ||
| * not portable and might cause unintended behaviors. | ||
| * @kind problem | ||
| * @precision very-high | ||
| * @problem.severity error | ||
| * @tags external/misra/id/rule-10-2-3 | ||
| * scope/single-translation-unit | ||
| * correctness | ||
| * external/misra/enforcement/decidable | ||
| * external/misra/obligation/required | ||
| */ | ||
|
|
||
| import cpp | ||
| import codingstandards.cpp.misra | ||
|
|
||
| private predicate isUnscopedEnum(Enum enum) { not enum instanceof ScopedEnum } | ||
|
|
||
| private predicate withoutFixedUnderlyingType(Enum enum) { not enum.hasExplicitUnderlyingType() } | ||
|
|
||
| private predicate isUnscopedEnumWithoutFixedUnderlyingType(Enum enum) { | ||
| isUnscopedEnum(enum) and withoutFixedUnderlyingType(enum) | ||
| } | ||
|
|
||
| class ArithmeticBitwiseLogicalBinaryOperation extends BinaryOperation { | ||
| ArithmeticBitwiseLogicalBinaryOperation() { | ||
| this instanceof BinaryArithmeticOperation or | ||
| this instanceof BinaryBitwiseOperation or | ||
| this instanceof BinaryLogicalOperation | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * ``` C++ | ||
| * static_cast<int>(u) == static_cast<int>(s); // COMPLIANT: comparing ints | ||
| * ``` | ||
| * ^^^ To solve this, we use `getExplicitlyConverted`: | ||
| * `binOp.getLeftOperand().getExplicitlyConverted()` gives `int`. | ||
| */ | ||
| predicate arithmeticBitwiseLogicalOperationUsesUnscopedUnfixedEnum( | ||
| ArithmeticBitwiseLogicalBinaryOperation binOp, Enum enum | ||
| ) { | ||
| /* | ||
| * We want to strip explicit casts and not implicit ones. Without the | ||
| * stripping of explicit casts, our query would raise a false alarm on | ||
| * cases such as below. | ||
| * | ||
| * ``` C++ | ||
| * static_cast<int>(u) + 1 // COMPLIANT | ||
| * ``` | ||
| */ | ||
|
|
||
| isUnscopedEnumWithoutFixedUnderlyingType(enum) and | ||
| ( | ||
| enum = binOp.getLeftOperand().getExplicitlyConverted().getUnderlyingType() or | ||
| enum = binOp.getRightOperand().getExplicitlyConverted().getUnderlyingType() | ||
| ) | ||
| } | ||
|
|
||
| class RelationalEqualityBinaryOperation extends BinaryOperation { | ||
| RelationalEqualityBinaryOperation() { | ||
| this instanceof RelationalOperation or | ||
| this instanceof EqualityOperation | ||
| } | ||
| } | ||
|
|
||
| predicate relationalEqualityOperationUsesUnscopedUnfixedEnum( | ||
| RelationalEqualityBinaryOperation binOp, Enum enum | ||
| ) { | ||
| exists(Type leftOperandType, Type rightOperandType | | ||
| /* | ||
| * We want to strip explicit casts and not implicit ones. Without the | ||
| * stripping of explicit casts, our query would raise a false alarm on | ||
| * cases such as below. | ||
| * | ||
| * ``` C++ | ||
| * static_cast<int>(u) == 1 // COMPLIANT | ||
| * ``` | ||
| */ | ||
|
|
||
| leftOperandType = binOp.getLeftOperand().getExplicitlyConverted().getUnderlyingType() and | ||
| rightOperandType = binOp.getRightOperand().getExplicitlyConverted().getUnderlyingType() | ||
| | | ||
| isUnscopedEnumWithoutFixedUnderlyingType(enum) and | ||
| ( | ||
| enum = leftOperandType or | ||
| enum = rightOperandType | ||
| ) and | ||
| leftOperandType != rightOperandType | ||
| ) | ||
| } | ||
|
|
||
| class ArithmeticBitwiseCompoundAssignment extends AssignOperation { | ||
| ArithmeticBitwiseCompoundAssignment() { | ||
| this instanceof AssignArithmeticOperation or | ||
| this instanceof AssignBitwiseOperation | ||
| } | ||
| } | ||
|
|
||
| predicate compoundAssignmentUsesUnscopedUnfixedEnum( | ||
| ArithmeticBitwiseCompoundAssignment compoundAssignment, Enum enum | ||
| ) { | ||
| isUnscopedEnumWithoutFixedUnderlyingType(enum) and | ||
| enum = compoundAssignment.getAnOperand().getUnderlyingType() | ||
| } | ||
|
|
||
| /** | ||
| * Gets the minimum number of bits required to hold all values of enum `e`. | ||
| */ | ||
| int enumMinBits(Enum e, boolean signed) { | ||
| exists(QlBuiltins::BigInt minVal, QlBuiltins::BigInt maxVal | | ||
| minVal = min(EnumConstant c | c.getDeclaringEnum() = e | c.getValue().toBigInt()) and | ||
| maxVal = max(EnumConstant c | c.getDeclaringEnum() = e | c.getValue().toBigInt()) | ||
| | | ||
| // 8 bits: signed [-128, 127] or unsigned [0, 255] | ||
| if minVal >= "-128".toBigInt() and maxVal <= "127".toBigInt() | ||
| then result = 8 and signed = true | ||
| else | ||
| if minVal >= "0".toBigInt() and maxVal <= "255".toBigInt() | ||
| then ( | ||
| result = 8 and signed = false | ||
| ) else | ||
| // 16 bits: signed [-32768, 32767] or unsigned [0, 65535] | ||
| if minVal >= "-32768".toBigInt() and maxVal <= "32767".toBigInt() | ||
| then ( | ||
| result = 16 and signed = true | ||
| ) else | ||
| if minVal >= "0".toBigInt() and maxVal <= "65535".toBigInt() | ||
| then ( | ||
| result = 16 and signed = false | ||
| ) else | ||
| // 32 bits: signed [-2147483648, 2147483647] or unsigned [0, 4294967295] | ||
| if minVal >= "-2147483648".toBigInt() and maxVal <= "2147483647".toBigInt() | ||
| then ( | ||
| result = 32 and signed = true | ||
| ) else | ||
| if minVal >= "0".toBigInt() and maxVal <= "4294967295".toBigInt() | ||
| then ( | ||
| result = 32 and signed = false | ||
| ) else ( | ||
| // 64 bits: everything else | ||
| result = 64 and signed = true | ||
| ) | ||
| ) | ||
| } | ||
|
|
||
| /** | ||
| * Holds if the enum `e` can fit in an integral type `type`. | ||
| */ | ||
| predicate enumFitsInType(Enum e, IntegralType type) { | ||
| exists(int minBits, boolean signed | minBits = enumMinBits(e, signed) | | ||
| /* If it has exactly the minimum number of bits, then check its signedness. */ | ||
| type.getSize() * 8 = minBits and | ||
| ( | ||
| signed = true and type.isSigned() | ||
| or | ||
| signed = false and type.isUnsigned() | ||
| ) | ||
| or | ||
| /* If it exceeds the minimum number of bits, signedness doesn't matter. */ | ||
| type.getSize() * 8 > minBits | ||
| ) | ||
| } | ||
|
|
||
| predicate assignmentSourceIsUnscopedUnfixedEnum(AssignExpr assign, Enum enum, Type targetType) { | ||
| isUnscopedEnumWithoutFixedUnderlyingType(enum) and | ||
| enum = assign.getRValue().getUnderlyingType() and | ||
| targetType = assign.getLValue().getUnderlyingType() and | ||
| not enumFitsInType(enum, targetType) and | ||
| not enum = targetType | ||
| } | ||
|
|
||
| predicate staticCastSourceIsUnscopedUnfixedEnumVariant(StaticCast cast, Enum enum, Type targetType) { | ||
| isUnscopedEnumWithoutFixedUnderlyingType(enum) and | ||
| enum = cast.getExpr().getUnderlyingType() and | ||
| targetType = cast.getUnderlyingType() and | ||
| not enumFitsInType(enum, targetType) and | ||
| not enum = targetType | ||
| } | ||
|
|
||
| predicate switchConditionIsAnUnfixedEnumVariant(SwitchStmt switch, Enum enum, SwitchCase invalidCase) { | ||
| isUnscopedEnumWithoutFixedUnderlyingType(enum) and | ||
| enum = switch.getExpr().getType() and | ||
| invalidCase = switch.getASwitchCase() and | ||
| not invalidCase.getExpr().getUnderlyingType() = enum | ||
| } | ||
|
|
||
| /** | ||
| * Holds if a `static_cast` expression has an unscoped enum without fixed | ||
| * underlying type as the target type. | ||
| */ | ||
| predicate staticCastTargetIsUnscopedUnfixedEnumVariant(StaticCast cast, Enum enum) { | ||
| isUnscopedEnumWithoutFixedUnderlyingType(enum) and | ||
| enum = cast.getType() and | ||
| not cast.getExpr().getType() = enum | ||
| } | ||
|
|
||
| from Element x, Enum enum, string message | ||
| where | ||
| not isExcluded(x, Banned3Package::unscopedEnumWithoutFixedUnderlyingTypeUsedQuery()) and | ||
| ( | ||
| arithmeticBitwiseLogicalOperationUsesUnscopedUnfixedEnum(x, enum) and | ||
| message = | ||
| "Arithmetic, bitwise, or logical operation uses unscoped enum $@ without fixed underlying type." | ||
| or | ||
| relationalEqualityOperationUsesUnscopedUnfixedEnum(x, enum) and | ||
| message = | ||
| "Relational or equality operation compares unscoped enum $@ without fixed underlying type to a different type." | ||
| or | ||
| compoundAssignmentUsesUnscopedUnfixedEnum(x, enum) and | ||
| message = "Compound assignment uses unscoped enum $@ without fixed underlying type." | ||
| or | ||
| exists(Type targetType | | ||
| assignmentSourceIsUnscopedUnfixedEnum(x, enum, targetType) and | ||
| message = | ||
| "Assignment from unscoped enum $@ without fixed underlying type to '" + targetType.getName() | ||
| + "' which may not be large enough." | ||
| ) | ||
| or | ||
| exists(Type targetType | | ||
| staticCastSourceIsUnscopedUnfixedEnumVariant(x, enum, targetType) and | ||
| message = | ||
| "Static cast from unscoped enum $@ without fixed underlying type to '" + | ||
| targetType.getName() + "' which may not be large enough." | ||
| ) | ||
| or | ||
| exists(SwitchStmt switch | | ||
| switchConditionIsAnUnfixedEnumVariant(switch, enum, x) and | ||
| message = | ||
| "Switch on unscoped enum $@ without fixed underlying type has case not of the same enum type." | ||
| ) | ||
| or | ||
| staticCastTargetIsUnscopedUnfixedEnumVariant(x, enum) and | ||
| message = "Static cast to unscoped enum $@ without fixed underlying type." | ||
| ) | ||
| select x, message, enum, enum.getName() | ||
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
enumMinBitsdefaults the 64-bit case tosigned = true(lines 142-145). This will mis-handle enums whose enumerators require an unsigned 64-bit underlying type (for example, large non-negative values > 2^63-1), potentially causing false positives/negatives in the size/signedness checks. Consider extending the range checks to distinguish signed vs unsigned 64-bit (or derive signedness directly from the enum’s value range / underlying type selection logic).