Skip to content

Commit d20c643

Browse files
mbaludambaluda
authored
Merge pull request #1052 from github/michaelrfairhurst/banned1-global-variables
Implement RULE-6-7-2 banning most kinds of global variables
2 parents d677a0a + 7cbb3b2 commit d20c643

File tree

11 files changed

+250
-95
lines changed

11 files changed

+250
-95
lines changed

‎change_notes/2026-02-22-shared-default-initialization-logic.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
- `A3-3-2` - `StaticOrThreadLocalObjectsNonConstantInit`:
2+
- The checks for non-constant initialization have been moved to be usable in other queries, such as MISRA C++23 Rule 6.7.2.
3+
- No visible changes in query results expected.

‎cpp/autosar/src/rules/A3-3-2/StaticOrThreadLocalObjectsNonConstantInit.ql‎

Lines changed: 1 addition & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -15,100 +15,7 @@
1515

1616
import cpp
1717
import codingstandards.cpp.autosar
18-
19-
/**
20-
* Holds if `e` is a full expression or `AggregateLiteral` in the initializer of a
21-
* `StaticStorageDurationVariable`.
22-
*
23-
* Although `AggregateLiteral`s are expressions according to our model, they are not considered
24-
* expressions from the perspective of the standard. Therefore, we should consider components of
25-
* aggregate literals within static initializers to also be full expressions.
26-
*/
27-
private predicate isFullExprOrAggregateInStaticInitializer(Expr e) {
28-
exists(StaticStorageDurationVariable var | e = var.getInitializer().getExpr())
29-
or
30-
isFullExprOrAggregateInStaticInitializer(e.getParent().(AggregateLiteral))
31-
}
32-
33-
/**
34-
* Holds if `e` is a non-constant full expression in a static initializer, for the given `reason`
35-
* and `reasonElement`.
36-
*/
37-
private predicate nonConstantFullExprInStaticInitializer(
38-
Expr e, Element reasonElement, string reason
39-
) {
40-
isFullExprOrAggregateInStaticInitializer(e) and
41-
if e instanceof AggregateLiteral
42-
then
43-
// If this is an aggregate literal, we apply this recursively
44-
nonConstantFullExprInStaticInitializer(e.getAChild(), reasonElement, reason)
45-
else (
46-
// Otherwise we check this component to determine if it is constant
47-
not (
48-
e.getFullyConverted().isConstant() or
49-
e.(Call).getTarget().isConstexpr() or
50-
e.(VariableAccess).getTarget().isConstexpr()
51-
) and
52-
reason = "uses a non-constant element in the initialization" and
53-
reasonElement = e
54-
)
55-
}
56-
57-
/**
58-
* A `ConstructorCall` that does not represent a constant initializer for an object according to
59-
* `[basic.start.init]`.
60-
*
61-
* In addition to identifying `ConstructorCall`s which are not constant initializers, this also
62-
* provides an explanatory "reason" for why this constructor is not considered to be a constant
63-
* initializer.
64-
*/
65-
predicate isNotConstantInitializer(ConstructorCall cc, Element reasonElement, string reason) {
66-
// Must call a constexpr constructor
67-
not cc.getTarget().isConstexpr() and
68-
reason =
69-
"calls the " + cc.getTarget().getName() + "(..) constructor which is not marked as constexpr" and
70-
reasonElement = cc
71-
or
72-
// And all arguments must either be constant, or themselves call constexpr constructors
73-
cc.getTarget().isConstexpr() and
74-
exists(Expr arg | arg = cc.getAnArgument() |
75-
isNotConstantInitializer(arg, reasonElement, reason)
76-
or
77-
not arg instanceof ConstructorCall and
78-
not arg.getFullyConverted().isConstant() and
79-
not arg.(Call).getTarget().isConstexpr() and
80-
not arg.(VariableAccess).getTarget().isConstexpr() and
81-
reason = "includes a non constant " + arg.getType() + " argument to a constexpr constructor" and
82-
reasonElement = arg
83-
)
84-
}
85-
86-
/**
87-
* Identifies if a `StaticStorageDurationVariable` is not constant initialized according to
88-
* `[basic.start.init]`.
89-
*/
90-
predicate isNotConstantInitialized(
91-
StaticStorageDurationVariable v, string reason, Element reasonElement
92-
) {
93-
if v.getInitializer().getExpr() instanceof ConstructorCall
94-
then
95-
// (2.2) if initialized by a constructor call, then that constructor call must be a constant
96-
// initializer for the variable to be constant initialized
97-
isNotConstantInitializer(v.getInitializer().getExpr(), reasonElement, reason)
98-
else
99-
// (2.3) If it is not initialized by a constructor call, then it must be the case that every full
100-
// expr in the initializer is a constant expression or that the object was "value initialized"
101-
// but without a constructor call. For value initialization, there are two non-constructor call
102-
// cases to consider:
103-
//
104-
// 1. The object was zero initialized - in which case, the extractor does not include a
105-
// constructor call - instead, it has a blank aggregate literal, or no initializer.
106-
// 2. The object is an array, which will be initialized by an aggregate literal.
107-
//
108-
// In both cases it is sufficient for us to find a non-constant full expression in the static
109-
// initializer
110-
nonConstantFullExprInStaticInitializer(v.getInitializer().getExpr(), reasonElement, reason)
111-
}
18+
import codingstandards.cpp.orderofevaluation.Initialization
11219

11320
from StaticStorageDurationVariable staticOrThreadLocalVar, string reason, Element reasonElement
11421
where

‎cpp/common/src/codingstandards/cpp/exclusions/cpp/Banned1.qll‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
//** THIS FILE IS AUTOGENERATED, DO NOT MODIFY DIRECTLY. **/
2+
import cpp
3+
import RuleMetadata
4+
import codingstandards.cpp.exclusions.RuleMetadata
5+
6+
newtype Banned1Query = TGlobalVariableUsedQuery()
7+
8+
predicate isBanned1QueryMetadata(Query query, string queryId, string ruleId, string category) {
9+
query =
10+
// `Query` instance for the `globalVariableUsed` query
11+
Banned1Package::globalVariableUsedQuery() and
12+
queryId =
13+
// `@id` for the `globalVariableUsed` query
14+
"cpp/misra/global-variable-used" and
15+
ruleId = "RULE-6-7-2" and
16+
category = "required"
17+
}
18+
19+
module Banned1Package {
20+
Query globalVariableUsedQuery() {
21+
//autogenerate `Query` type
22+
result =
23+
// `Query` type for `globalVariableUsed` query
24+
TQueryCPP(TBanned1PackageQuery(TGlobalVariableUsedQuery()))
25+
}
26+
}

‎cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import cpp
33
import codingstandards.cpp.exclusions.RuleMetadata
44
//** Import packages for this language **/
55
import Allocations
6+
import Banned1
67
import BannedAPIs
78
import BannedFunctions
89
import BannedLibraries
@@ -77,6 +78,7 @@ import VirtualFunctions
7778
/** The TQuery type representing this language * */
7879
newtype TCPPQuery =
7980
TAllocationsPackageQuery(AllocationsQuery q) or
81+
TBanned1PackageQuery(Banned1Query q) or
8082
TBannedAPIsPackageQuery(BannedAPIsQuery q) or
8183
TBannedFunctionsPackageQuery(BannedFunctionsQuery q) or
8284
TBannedLibrariesPackageQuery(BannedLibrariesQuery q) or
@@ -151,6 +153,7 @@ newtype TCPPQuery =
151153
/** The metadata predicate * */
152154
predicate isQueryMetadata(Query query, string queryId, string ruleId, string category) {
153155
isAllocationsQueryMetadata(query, queryId, ruleId, category) or
156+
isBanned1QueryMetadata(query, queryId, ruleId, category) or
154157
isBannedAPIsQueryMetadata(query, queryId, ruleId, category) or
155158
isBannedFunctionsQueryMetadata(query, queryId, ruleId, category) or
156159
isBannedLibrariesQueryMetadata(query, queryId, ruleId, category) or

‎cpp/common/src/codingstandards/cpp/orderofevaluation/Initialization.qll‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
import cpp
2+
3+
/**
4+
* Holds if `e` is a full expression or `AggregateLiteral` in the initializer of a
5+
* `StaticStorageDurationVariable`.
6+
*
7+
* Although `AggregateLiteral`s are expressions according to our model, they are not considered
8+
* expressions from the perspective of the standard. Therefore, we should consider components of
9+
* aggregate literals within static initializers to also be full expressions.
10+
*/
11+
private predicate isFullExprOrAggregateInStaticInitializer(Expr e) {
12+
exists(StaticStorageDurationVariable var | e = var.getInitializer().getExpr())
13+
or
14+
isFullExprOrAggregateInStaticInitializer(e.getParent().(AggregateLiteral))
15+
}
16+
17+
/**
18+
* Holds if `e` is a non-constant full expression in a static initializer, for the given `reason`
19+
* and `reasonElement`.
20+
*/
21+
private predicate nonConstantFullExprInStaticInitializer(
22+
Expr e, Element reasonElement, string reason
23+
) {
24+
isFullExprOrAggregateInStaticInitializer(e) and
25+
if e instanceof AggregateLiteral
26+
then
27+
// If this is an aggregate literal, we apply this recursively
28+
nonConstantFullExprInStaticInitializer(e.getAChild(), reasonElement, reason)
29+
else (
30+
// Otherwise we check this component to determine if it is constant
31+
not (
32+
e.getFullyConverted().isConstant() or
33+
e.(Call).getTarget().isConstexpr() or
34+
e.(VariableAccess).getTarget().isConstexpr()
35+
) and
36+
reason = "uses a non-constant element in the initialization" and
37+
reasonElement = e
38+
)
39+
}
40+
41+
/**
42+
* A `ConstructorCall` that does not represent a constant initializer for an object according to
43+
* `[basic.start.init]`.
44+
*
45+
* In addition to identifying `ConstructorCall`s which are not constant initializers, this also
46+
* provides an explanatory "reason" for why this constructor is not considered to be a constant
47+
* initializer.
48+
*/
49+
predicate isNotConstantInitializer(ConstructorCall cc, Element reasonElement, string reason) {
50+
// Must call a constexpr constructor
51+
not cc.getTarget().isConstexpr() and
52+
reason =
53+
"calls the " + cc.getTarget().getName() + "(..) constructor which is not marked as constexpr" and
54+
reasonElement = cc
55+
or
56+
// And all arguments must either be constant, or themselves call constexpr constructors
57+
cc.getTarget().isConstexpr() and
58+
exists(Expr arg | arg = cc.getAnArgument() |
59+
isNotConstantInitializer(arg, reasonElement, reason)
60+
or
61+
not arg instanceof ConstructorCall and
62+
not arg.getFullyConverted().isConstant() and
63+
not arg.(Call).getTarget().isConstexpr() and
64+
not arg.(VariableAccess).getTarget().isConstexpr() and
65+
reason = "includes a non constant " + arg.getType() + " argument to a constexpr constructor" and
66+
reasonElement = arg
67+
)
68+
}
69+
70+
/**
71+
* Identifies if a `StaticStorageDurationVariable` is not constant initialized according to
72+
* `[basic.start.init]`.
73+
*/
74+
predicate isNotConstantInitialized(
75+
StaticStorageDurationVariable v, string reason, Element reasonElement
76+
) {
77+
if v.getInitializer().getExpr() instanceof ConstructorCall
78+
then
79+
// (2.2) if initialized by a constructor call, then that constructor call must be a constant
80+
// initializer for the variable to be constant initialized
81+
isNotConstantInitializer(v.getInitializer().getExpr(), reasonElement, reason)
82+
else
83+
// (2.3) If it is not initialized by a constructor call, then it must be the case that every full
84+
// expr in the initializer is a constant expression or that the object was "value initialized"
85+
// but without a constructor call. For value initialization, there are two non-constructor call
86+
// cases to consider:
87+
//
88+
// 1. The object was zero initialized - in which case, the extractor does not include a
89+
// constructor call - instead, it has a blank aggregate literal, or no initializer.
90+
// 2. The object is an array, which will be initialized by an aggregate literal.
91+
//
92+
// In both cases it is sufficient for us to find a non-constant full expression in the static
93+
// initializer
94+
nonConstantFullExprInStaticInitializer(v.getInitializer().getExpr(), reasonElement, reason)
95+
}

‎cpp/misra/src/rules/RULE-6-7-2/GlobalVariableUsed.ql‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
/**
2+
* @id cpp/misra/global-variable-used
3+
* @name RULE-6-7-2: Global variables shall not be used
4+
* @description Global variables can be accessed and modified from distant and unclear points in the
5+
* code, creating a risk of data races and unexpected behavior.
6+
* @kind problem
7+
* @precision very-high
8+
* @problem.severity error
9+
* @tags external/misra/id/rule-6-7-2
10+
* scope/single-translation-unit
11+
* maintainability
12+
* external/misra/enforcement/decidable
13+
* external/misra/obligation/required
14+
*/
15+
16+
import cpp
17+
import codingstandards.cpp.misra
18+
import codingstandards.cpp.Linkage
19+
import codingstandards.cpp.orderofevaluation.Initialization
20+
21+
class GlobalVariable extends Variable {
22+
GlobalVariable() {
23+
hasNamespaceScope(this) or
24+
this.(MemberVariable).isStatic()
25+
}
26+
}
27+
28+
from GlobalVariable var, string suffix, Element reasonElement, string reasonString
29+
where
30+
not isExcluded(var, Banned1Package::globalVariableUsedQuery()) and
31+
not var.isConstexpr() and
32+
(
33+
not var.isConst() and
34+
suffix = "as non-const" and
35+
// Placeholder is not used, but they must be bound.
36+
reasonString = "" and
37+
reasonElement = var
38+
or
39+
var.isConst() and
40+
isNotConstantInitialized(var, reasonString, reasonElement) and
41+
suffix = "as const, but is not constant initialized because it $@"
42+
)
43+
select var, "Global variable " + var.getName() + " declared " + suffix, reasonElement, reasonString

‎cpp/misra/test/rules/RULE-6-7-2/GlobalVariableUsed.expected‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
| test.cpp:7:14:7:15 | g1 | Global variable g1 declared as non-const | test.cpp:7:14:7:15 | g1 | |
2+
| 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 |
3+
| test.cpp:14:14:14:15 | g5 | Global variable g5 declared as non-const | test.cpp:14:14:14:15 | g5 | |
4+
| 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 |
5+
| test.cpp:27:14:27:16 | g10 | Global variable g10 declared as non-const | test.cpp:27:14:27:16 | g10 | |
6+
| 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 |
7+
| test.cpp:32:23:32:24 | m2 | Global variable m2 declared as non-const | test.cpp:32:23:32:24 | m2 | |
8+
| test.cpp:38:14:38:29 | m3 | Global variable m3 declared as non-const | test.cpp:38:14:38:29 | m3 | |

‎cpp/misra/test/rules/RULE-6-7-2/GlobalVariableUsed.qlref‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
rules/RULE-6-7-2/GlobalVariableUsed.ql

‎cpp/misra/test/rules/RULE-6-7-2/test.cpp‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
#include <cstdint>
2+
3+
// Non-compliant global variables (namespace scope, dynamically initialized or
4+
// mutable)
5+
std::int32_t f1();
6+
7+
std::int32_t g1{f1()}; // NON_COMPLIANT - dynamic initialization
8+
const std::int32_t g2{
9+
g1}; // NON_COMPLIANT - dynamic initialization (depends on g1)
10+
const std::int32_t g3{42}; // COMPLIANT - const with static initialization
11+
constexpr std::int32_t g4{0}; // COMPLIANT - constexpr
12+
13+
namespace {
14+
std::int32_t g5{0}; // NON_COMPLIANT - mutable namespace scope variable
15+
constexpr std::int32_t g6{100}; // COMPLIANT - constexpr
16+
const std::int32_t g7{100}; // COMPLIANT - const with static initialization
17+
18+
constexpr std::int32_t f2() { return 42; }
19+
constexpr std::int32_t g8{f2()}; // COMPLIANT - constexpr
20+
} // namespace
21+
22+
struct ComplexInit {
23+
ComplexInit() {}
24+
};
25+
26+
const ComplexInit g9{}; // NON_COMPLIANT - dynamic initialization
27+
std::int32_t g10{0}; // NON_COMPLIANT - mutable namespace scope variable
28+
const std::int32_t g11{f1()}; // NON_COMPLIANT - dynamic initialization
29+
30+
class StaticMember {
31+
std::int32_t m1;
32+
static std::int32_t m2; // NON_COMPLIANT - class static data member
33+
static std::int32_t m3; // marked non_compliant at definition below
34+
static constexpr std::int32_t m4{0}; // COMPLIANT - constexpr static member
35+
static const std::int32_t m5;
36+
};
37+
38+
std::int32_t StaticMember::m3 =
39+
0; // NON_COMPLIANT - class static data member definition
40+
const std::int32_t StaticMember::m5 =
41+
42; // COMPLIANT - const with static initialization
42+
43+
constexpr auto g12 = // COMPLIANT - constexpr lambda
44+
[](auto x, auto y) { return x + y; };

‎rule_packages/cpp/Banned1.json‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
{
2+
"MISRA-C++-2023": {
3+
"RULE-6-7-2": {
4+
"properties": {
5+
"enforcement": "decidable",
6+
"obligation": "required"
7+
},
8+
"queries": [
9+
{
10+
"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.",
11+
"kind": "problem",
12+
"name": "Global variables shall not be used",
13+
"precision": "very-high",
14+
"severity": "error",
15+
"short_name": "GlobalVariableUsed",
16+
"tags": [
17+
"scope/single-translation-unit",
18+
"maintainability"
19+
]
20+
}
21+
],
22+
"title": "Global variables shall not be used"
23+
}
24+
}
25+
}

0 commit comments

Comments
 (0)