-
Notifications
You must be signed in to change notification settings - Fork 75
Implement RULE-6-7-2 banning most kinds of global variables #1052
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
Merged
Merged
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
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
3 changes: 3 additions & 0 deletions
3
change_notes/2026-02-22-shared-default-initialization-logic.md
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,3 @@ | ||
| - `A3-3-2` - `StaticOrThreadLocalObjectsNonConstantInit`: | ||
| - The checks for non-constant initialization have been moved to be usable in other queries, such as MISRA C++23 Rule 6.7.2. | ||
| - No visible changes in query results 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
26 changes: 26 additions & 0 deletions
26
cpp/common/src/codingstandards/cpp/exclusions/cpp/Banned1.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 Banned1Query = TGlobalVariableUsedQuery() | ||
|
|
||
| predicate isBanned1QueryMetadata(Query query, string queryId, string ruleId, string category) { | ||
| query = | ||
| // `Query` instance for the `globalVariableUsed` query | ||
| Banned1Package::globalVariableUsedQuery() and | ||
| queryId = | ||
| // `@id` for the `globalVariableUsed` query | ||
| "cpp/misra/global-variable-used" and | ||
| ruleId = "RULE-6-7-2" and | ||
| category = "required" | ||
| } | ||
|
|
||
| module Banned1Package { | ||
| Query globalVariableUsedQuery() { | ||
| //autogenerate `Query` type | ||
| result = | ||
| // `Query` type for `globalVariableUsed` query | ||
| TQueryCPP(TBanned1PackageQuery(TGlobalVariableUsedQuery())) | ||
| } | ||
| } |
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
95 changes: 95 additions & 0 deletions
95
cpp/common/src/codingstandards/cpp/orderofevaluation/Initialization.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,95 @@ | ||
| import cpp | ||
|
|
||
| /** | ||
| * Holds if `e` is a full expression or `AggregateLiteral` in the initializer of a | ||
| * `StaticStorageDurationVariable`. | ||
| * | ||
| * Although `AggregateLiteral`s are expressions according to our model, they are not considered | ||
| * expressions from the perspective of the standard. Therefore, we should consider components of | ||
| * aggregate literals within static initializers to also be full expressions. | ||
| */ | ||
| private predicate isFullExprOrAggregateInStaticInitializer(Expr e) { | ||
| exists(StaticStorageDurationVariable var | e = var.getInitializer().getExpr()) | ||
| or | ||
| isFullExprOrAggregateInStaticInitializer(e.getParent().(AggregateLiteral)) | ||
| } | ||
|
|
||
| /** | ||
| * Holds if `e` is a non-constant full expression in a static initializer, for the given `reason` | ||
| * and `reasonElement`. | ||
| */ | ||
| private predicate nonConstantFullExprInStaticInitializer( | ||
| Expr e, Element reasonElement, string reason | ||
| ) { | ||
| isFullExprOrAggregateInStaticInitializer(e) and | ||
| if e instanceof AggregateLiteral | ||
| then | ||
| // If this is an aggregate literal, we apply this recursively | ||
| nonConstantFullExprInStaticInitializer(e.getAChild(), reasonElement, reason) | ||
| else ( | ||
| // Otherwise we check this component to determine if it is constant | ||
| not ( | ||
| e.getFullyConverted().isConstant() or | ||
| e.(Call).getTarget().isConstexpr() or | ||
| e.(VariableAccess).getTarget().isConstexpr() | ||
| ) and | ||
| reason = "uses a non-constant element in the initialization" and | ||
| reasonElement = e | ||
| ) | ||
| } | ||
|
|
||
| /** | ||
| * A `ConstructorCall` that does not represent a constant initializer for an object according to | ||
| * `[basic.start.init]`. | ||
| * | ||
| * In addition to identifying `ConstructorCall`s which are not constant initializers, this also | ||
| * provides an explanatory "reason" for why this constructor is not considered to be a constant | ||
| * initializer. | ||
| */ | ||
| predicate isNotConstantInitializer(ConstructorCall cc, Element reasonElement, string reason) { | ||
| // Must call a constexpr constructor | ||
| not cc.getTarget().isConstexpr() and | ||
| reason = | ||
| "calls the " + cc.getTarget().getName() + "(..) constructor which is not marked as constexpr" and | ||
| reasonElement = cc | ||
| or | ||
| // And all arguments must either be constant, or themselves call constexpr constructors | ||
| cc.getTarget().isConstexpr() and | ||
| exists(Expr arg | arg = cc.getAnArgument() | | ||
| isNotConstantInitializer(arg, reasonElement, reason) | ||
| or | ||
| not arg instanceof ConstructorCall and | ||
| not arg.getFullyConverted().isConstant() and | ||
| not arg.(Call).getTarget().isConstexpr() and | ||
| not arg.(VariableAccess).getTarget().isConstexpr() and | ||
| reason = "includes a non constant " + arg.getType() + " argument to a constexpr constructor" and | ||
| reasonElement = arg | ||
| ) | ||
| } | ||
|
|
||
| /** | ||
| * Identifies if a `StaticStorageDurationVariable` is not constant initialized according to | ||
| * `[basic.start.init]`. | ||
| */ | ||
| predicate isNotConstantInitialized( | ||
| StaticStorageDurationVariable v, string reason, Element reasonElement | ||
| ) { | ||
| if v.getInitializer().getExpr() instanceof ConstructorCall | ||
| then | ||
| // (2.2) if initialized by a constructor call, then that constructor call must be a constant | ||
| // initializer for the variable to be constant initialized | ||
| isNotConstantInitializer(v.getInitializer().getExpr(), reasonElement, reason) | ||
| else | ||
| // (2.3) If it is not initialized by a constructor call, then it must be the case that every full | ||
| // expr in the initializer is a constant expression or that the object was "value initialized" | ||
| // but without a constructor call. For value initialization, there are two non-constructor call | ||
| // cases to consider: | ||
| // | ||
| // 1. The object was zero initialized - in which case, the extractor does not include a | ||
| // constructor call - instead, it has a blank aggregate literal, or no initializer. | ||
| // 2. The object is an array, which will be initialized by an aggregate literal. | ||
| // | ||
| // In both cases it is sufficient for us to find a non-constant full expression in the static | ||
| // initializer | ||
| nonConstantFullExprInStaticInitializer(v.getInitializer().getExpr(), reasonElement, reason) | ||
| } |
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,43 @@ | ||
| /** | ||
| * @id cpp/misra/global-variable-used | ||
| * @name RULE-6-7-2: Global variables shall not be used | ||
| * @description Global variables can be accessed and modified from distant and unclear points in the | ||
| * code, creating a risk of data races and unexpected behavior. | ||
| * @kind problem | ||
| * @precision very-high | ||
| * @problem.severity error | ||
| * @tags external/misra/id/rule-6-7-2 | ||
| * scope/single-translation-unit | ||
| * maintainability | ||
| * external/misra/enforcement/decidable | ||
| * external/misra/obligation/required | ||
| */ | ||
|
|
||
| import cpp | ||
| import codingstandards.cpp.misra | ||
| import codingstandards.cpp.Linkage | ||
| import codingstandards.cpp.orderofevaluation.Initialization | ||
|
|
||
| class GlobalVariable extends Variable { | ||
| GlobalVariable() { | ||
| hasNamespaceScope(this) or | ||
| this.(MemberVariable).isStatic() | ||
| } | ||
| } | ||
|
|
||
| from GlobalVariable var, string suffix, Element reasonElement, string reasonString | ||
| where | ||
| not isExcluded(var, Banned1Package::globalVariableUsedQuery()) and | ||
| not var.isConstexpr() and | ||
| ( | ||
| not var.isConst() and | ||
| suffix = "as non-const" and | ||
| // Placeholder is not used, but they must be bound. | ||
| reasonString = "" and | ||
| reasonElement = var | ||
| or | ||
| var.isConst() and | ||
| isNotConstantInitialized(var, reasonString, reasonElement) and | ||
| suffix = "as const, but is not constant initialized because it $@" | ||
| ) | ||
| select var, "Global variable " + var.getName() + " declared " + suffix, reasonElement, reasonString |
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,8 @@ | ||
| | test.cpp:7:14:7:15 | g1 | Global variable g1 declared as non-const | test.cpp:7:14:7:15 | g1 | | | ||
| | test.cpp:8:20:8:21 | g2 | Global variable g2 declared as const, but is not constant initialized because it $@ | test.cpp:9:5:9:6 | g1 | uses a non-constant element in the initialization | | ||
| | test.cpp:14:14:14:15 | g5 | Global variable g5 declared as non-const | test.cpp:14:14:14:15 | g5 | | | ||
| | test.cpp:26:19:26:20 | g9 | Global variable g9 declared as const, but is not constant initialized because it $@ | test.cpp:26:22:26:22 | call to ComplexInit | calls the ComplexInit(..) constructor which is not marked as constexpr | | ||
| | test.cpp:27:14:27:16 | g10 | Global variable g10 declared as non-const | test.cpp:27:14:27:16 | g10 | | | ||
| | test.cpp:28:20:28:22 | g11 | Global variable g11 declared as const, but is not constant initialized because it $@ | test.cpp:28:24:28:25 | call to f1 | uses a non-constant element in the initialization | | ||
| | test.cpp:32:23:32:24 | m2 | Global variable m2 declared as non-const | test.cpp:32:23:32:24 | m2 | | | ||
| | test.cpp:38:14:38:29 | m3 | Global variable m3 declared as non-const | test.cpp:38:14:38:29 | m3 | | |
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-7-2/GlobalVariableUsed.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,44 @@ | ||
| #include <cstdint> | ||
|
|
||
| // Non-compliant global variables (namespace scope, dynamically initialized or | ||
| // mutable) | ||
| std::int32_t f1(); | ||
|
|
||
| std::int32_t g1{f1()}; // NON_COMPLIANT - dynamic initialization | ||
| const std::int32_t g2{ | ||
| g1}; // NON_COMPLIANT - dynamic initialization (depends on g1) | ||
| const std::int32_t g3{42}; // COMPLIANT - const with static initialization | ||
| constexpr std::int32_t g4{0}; // COMPLIANT - constexpr | ||
|
|
||
| namespace { | ||
| std::int32_t g5{0}; // NON_COMPLIANT - mutable namespace scope variable | ||
| constexpr std::int32_t g6{100}; // COMPLIANT - constexpr | ||
| const std::int32_t g7{100}; // COMPLIANT - const with static initialization | ||
|
|
||
| constexpr std::int32_t f2() { return 42; } | ||
| constexpr std::int32_t g8{f2()}; // COMPLIANT - constexpr | ||
| } // namespace | ||
|
|
||
| struct ComplexInit { | ||
| ComplexInit() {} | ||
| }; | ||
|
|
||
| const ComplexInit g9{}; // NON_COMPLIANT - dynamic initialization | ||
| std::int32_t g10{0}; // NON_COMPLIANT - mutable namespace scope variable | ||
| const std::int32_t g11{f1()}; // NON_COMPLIANT - dynamic initialization | ||
|
|
||
| class StaticMember { | ||
| std::int32_t m1; | ||
| static std::int32_t m2; // NON_COMPLIANT - class static data member | ||
| static std::int32_t m3; // marked non_compliant at definition below | ||
| static constexpr std::int32_t m4{0}; // COMPLIANT - constexpr static member | ||
| static const std::int32_t m5; | ||
| }; | ||
|
|
||
| std::int32_t StaticMember::m3 = | ||
| 0; // NON_COMPLIANT - class static data member definition | ||
| const std::int32_t StaticMember::m5 = | ||
| 42; // COMPLIANT - const with static initialization | ||
|
|
||
| constexpr auto g12 = // COMPLIANT - constexpr lambda | ||
| [](auto x, auto y) { return x + y; }; |
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,25 @@ | ||
| { | ||
| "MISRA-C++-2023": { | ||
| "RULE-6-7-2": { | ||
| "properties": { | ||
| "enforcement": "decidable", | ||
| "obligation": "required" | ||
| }, | ||
| "queries": [ | ||
| { | ||
| "description": "Global variables can be accessed and modified from distant and unclear points in the code, creating a risk of data races and unexpected behavior.", | ||
| "kind": "problem", | ||
| "name": "Global variables shall not be used", | ||
| "precision": "very-high", | ||
| "severity": "error", | ||
| "short_name": "GlobalVariableUsed", | ||
| "tags": [ | ||
| "scope/single-translation-unit", | ||
| "maintainability" | ||
| ] | ||
| } | ||
| ], | ||
| "title": "Global variables shall not be used" | ||
| } | ||
| } | ||
| } |
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
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.
Uh oh!
There was an error while loading. Please reload this page.