Skip to content

Commit 74d8857

Browse files
rvermeulenrvermeulen
authored
Merge branch 'main' into release-automation/bump-version
2 parents 4c36371 + 795f67c commit 74d8857

File tree

10 files changed

+208
-62
lines changed

10 files changed

+208
-62
lines changed

‎change_notes/2023-02-07-extend-getatypeuse.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
`A0-4-1` - `FloatingPointImplementationShallComplyWithIeeeStandard.ql`:
2+
- May return more results due to improvements to underlying `getATypeUse`.

‎change_notes/2024-03-01-fix-fp-a12-4-1-and-a12-8-6.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
- `A12-4-1` - `DestructorOfABaseClassNotPublicVirtual.ql`:
2+
- Fix FP reported in #392. Improve base class detection for template classes.
3+
- Update the alert message to prevent duplicate alerts for base classes that are both derived and abstract.
4+
- `A12-8-6` - `CopyAndMoveNotDeclaredProtected.ql`:
5+
- Fix FP reported in #392. Improve base class detection for template classes.
6+
- Update the alert message to prevent duplicate alerts for base classes that are both derived and abstract.

‎cpp/autosar/src/rules/A12-4-1/DestructorOfABaseClassNotPublicVirtual.ql

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,9 @@ predicate isProtectedNonVirtual(Destructor d) { d.isProtected() and not d.isVirt
2929
from Destructor d
3030
where
3131
not isExcluded(d, VirtualFunctionsPackage::destructorOfABaseClassNotPublicVirtualQuery()) and
32-
isPossibleBaseClass(d.getDeclaringType(), _) and
32+
d.getDeclaringType() instanceof BaseClass and
3333
(not isPublicOverride(d) and not isProtectedNonVirtual(d) and not isPublicVirtual(d))
3434
// Report the declaration entry in the class body, as that is where the access specifier should be set
3535
select getDeclarationEntryInClassDeclaration(d),
36-
"Destructor of base class " + d.getDeclaringType() +
37-
" is not declared as public virtual, public override, or protected non-virtual."
36+
"Destructor of base class '" + d.getDeclaringType().getQualifiedName() +
37+
"' is not declared as public virtual, public override, or protected non-virtual."

‎cpp/autosar/src/rules/A12-8-6/CopyAndMoveNotDeclaredProtected.ql

Lines changed: 34 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,35 +16,57 @@ import cpp
1616
import codingstandards.cpp.autosar
1717
import codingstandards.cpp.Class
1818

19-
predicate isInvalidConstructor(Constructor f, string constructorType) {
19+
predicate isInvalidConstructor(Constructor f, string constructorType, string missingAction) {
2020
not f.isDeleted() and
2121
not f.isProtected() and
2222
(
23-
f instanceof MoveConstructor and constructorType = "Move constructor"
23+
f instanceof MoveConstructor and
24+
if f.isCompilerGenerated()
25+
then constructorType = "Implicit move constructor" and missingAction = "deleted"
26+
else (
27+
constructorType = "Move constructor" and missingAction = "protected"
28+
)
2429
or
25-
f instanceof CopyConstructor and constructorType = "Copy constructor"
30+
f instanceof CopyConstructor and
31+
if f.isCompilerGenerated()
32+
then constructorType = "Implicit copy constructor" and missingAction = "deleted"
33+
else (
34+
constructorType = "Copy constructor" and missingAction = "protected"
35+
)
2636
)
2737
}
2838

29-
predicate isInvalidAssignment(Operator f, string operatorType) {
39+
predicate isInvalidAssignment(Operator f, string operatorType, string missingAction) {
3040
not f.isDeleted() and
3141
(
32-
f instanceof CopyAssignmentOperator and operatorType = "Copy assignment operator"
42+
f instanceof MoveAssignmentOperator and
43+
if f.isCompilerGenerated()
44+
then operatorType = "Implicit move assignment operator" and missingAction = "deleted"
45+
else (
46+
operatorType = "Move assignment operator" and missingAction = "protected"
47+
)
3348
or
34-
f instanceof MoveAssignmentOperator and operatorType = "Move assignment operator"
49+
f instanceof CopyAssignmentOperator and
50+
if f.isCompilerGenerated()
51+
then operatorType = "Implicit copy assignment operator" and missingAction = "deleted"
52+
else (
53+
operatorType = "Copy assignment operator" and missingAction = "protected"
54+
)
3555
) and
3656
not f.hasSpecifier("protected")
3757
}
3858

39-
from MemberFunction mf, string type, string baseReason
59+
from BaseClass baseClass, MemberFunction mf, string type, string missingAction
4060
where
4161
not isExcluded(mf, OperatorsPackage::copyAndMoveNotDeclaredProtectedQuery()) and
4262
(
43-
isInvalidConstructor(mf, type)
63+
isInvalidConstructor(mf, type, missingAction)
4464
or
45-
isInvalidAssignment(mf, type)
65+
isInvalidAssignment(mf, type, missingAction)
4666
) and
47-
isPossibleBaseClass(mf.getDeclaringType(), baseReason)
67+
baseClass = mf.getDeclaringType()
68+
// To avoid duplicate alerts due to inaccurate location information in the database we don't use the location of the base class.
69+
// This for example happens if multiple copies of the same header file are present in the database.
4870
select getDeclarationEntryInClassDeclaration(mf),
49-
type + " for base class " + mf.getDeclaringType().getQualifiedName() + " (" + baseReason +
50-
") is not declared protected or deleted."
71+
type + " for base class '" + baseClass.getQualifiedName() + "' is not declared " + missingAction +
72+
"."

‎cpp/autosar/test/rules/A12-4-1/DestructorOfABaseClassNotPublicVirtual.expected

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
1-
| test.cpp:4:3:4:4 | definition of ~A | Destructor of base class A is not declared as public virtual, public override, or protected non-virtual. |
2-
| test.cpp:30:3:30:4 | definition of ~E | Destructor of base class E is not declared as public virtual, public override, or protected non-virtual. |
1+
| test.cpp:4:3:4:4 | definition of ~A | Destructor of base class 'A' is not declared as public virtual, public override, or protected non-virtual. |
2+
| test.cpp:30:3:30:4 | definition of ~E | Destructor of base class 'E' is not declared as public virtual, public override, or protected non-virtual. |

‎cpp/autosar/test/rules/A12-8-6/CopyAndMoveNotDeclaredProtected.expected

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,26 @@
1-
| test.cpp:4:3:4:12 | declaration of BaseClass1 | Copy constructor for base class BaseClass1 (a derived class exists) is not declared protected or deleted. |
2-
| test.cpp:5:3:5:12 | declaration of BaseClass1 | Move constructor for base class BaseClass1 (a derived class exists) is not declared protected or deleted. |
3-
| test.cpp:6:15:6:23 | declaration of operator= | Copy assignment operator for base class BaseClass1 (a derived class exists) is not declared protected or deleted. |
4-
| test.cpp:7:15:7:23 | declaration of operator= | Move assignment operator for base class BaseClass1 (a derived class exists) is not declared protected or deleted. |
5-
| test.cpp:15:7:15:7 | declaration of operator= | Copy assignment operator for base class BaseClass2 (a derived class exists) is not declared protected or deleted. |
6-
| test.cpp:15:7:15:7 | declaration of operator= | Move assignment operator for base class BaseClass2 (a derived class exists) is not declared protected or deleted. |
7-
| test.cpp:55:3:55:12 | declaration of BaseClass5 | Copy constructor for base class BaseClass5 (a derived class exists) is not declared protected or deleted. |
8-
| test.cpp:56:3:56:12 | declaration of BaseClass5 | Move constructor for base class BaseClass5 (a derived class exists) is not declared protected or deleted. |
9-
| test.cpp:57:15:57:23 | declaration of operator= | Copy assignment operator for base class BaseClass5 (a derived class exists) is not declared protected or deleted. |
10-
| test.cpp:58:15:58:23 | declaration of operator= | Move assignment operator for base class BaseClass5 (a derived class exists) is not declared protected or deleted. |
11-
| test.cpp:75:3:75:12 | declaration of BaseClass6 | Copy constructor for base class BaseClass6 (the class is abstract) is not declared protected or deleted. |
12-
| test.cpp:76:3:76:12 | declaration of BaseClass6 | Move constructor for base class BaseClass6 (the class is abstract) is not declared protected or deleted. |
13-
| test.cpp:77:15:77:23 | declaration of operator= | Copy assignment operator for base class BaseClass6 (the class is abstract) is not declared protected or deleted. |
14-
| test.cpp:78:15:78:23 | declaration of operator= | Move assignment operator for base class BaseClass6 (the class is abstract) is not declared protected or deleted. |
1+
| test.cpp:4:3:4:12 | declaration of BaseClass1 | Copy constructor for base class 'BaseClass1' is not declared protected. |
2+
| test.cpp:5:3:5:12 | declaration of BaseClass1 | Move constructor for base class 'BaseClass1' is not declared protected. |
3+
| test.cpp:6:15:6:23 | declaration of operator= | Copy assignment operator for base class 'BaseClass1' is not declared protected. |
4+
| test.cpp:7:15:7:23 | declaration of operator= | Move assignment operator for base class 'BaseClass1' is not declared protected. |
5+
| test.cpp:15:7:15:7 | declaration of operator= | Implicit copy assignment operator for base class 'BaseClass2' is not declared deleted. |
6+
| test.cpp:15:7:15:7 | declaration of operator= | Implicit move assignment operator for base class 'BaseClass2' is not declared deleted. |
7+
| test.cpp:55:3:55:12 | declaration of BaseClass5 | Copy constructor for base class 'BaseClass5' is not declared protected. |
8+
| test.cpp:56:3:56:12 | declaration of BaseClass5 | Move constructor for base class 'BaseClass5' is not declared protected. |
9+
| test.cpp:57:15:57:23 | declaration of operator= | Copy assignment operator for base class 'BaseClass5' is not declared protected. |
10+
| test.cpp:58:15:58:23 | declaration of operator= | Move assignment operator for base class 'BaseClass5' is not declared protected. |
11+
| test.cpp:75:3:75:12 | declaration of BaseClass6 | Copy constructor for base class 'BaseClass6' is not declared protected. |
12+
| test.cpp:76:3:76:12 | declaration of BaseClass6 | Move constructor for base class 'BaseClass6' is not declared protected. |
13+
| test.cpp:77:15:77:23 | declaration of operator= | Copy assignment operator for base class 'BaseClass6' is not declared protected. |
14+
| test.cpp:78:15:78:23 | declaration of operator= | Move assignment operator for base class 'BaseClass6' is not declared protected. |
15+
| test.cpp:85:3:85:12 | declaration of BaseClass7 | Copy constructor for base class 'BaseClass7<T1>' is not declared protected. |
16+
| test.cpp:86:3:86:12 | declaration of BaseClass7 | Move constructor for base class 'BaseClass7<T1>' is not declared protected. |
17+
| test.cpp:87:15:87:23 | declaration of operator= | Copy assignment operator for base class 'BaseClass7<T1>' is not declared protected. |
18+
| test.cpp:88:15:88:23 | declaration of operator= | Move assignment operator for base class 'BaseClass7<T1>' is not declared protected. |
19+
| test.cpp:108:3:108:12 | declaration of BaseClass8 | Copy constructor for base class 'BaseClass8' is not declared protected. |
20+
| test.cpp:109:3:109:12 | declaration of BaseClass8 | Move constructor for base class 'BaseClass8' is not declared protected. |
21+
| test.cpp:110:15:110:23 | declaration of operator= | Copy assignment operator for base class 'BaseClass8' is not declared protected. |
22+
| test.cpp:111:15:111:23 | declaration of operator= | Move assignment operator for base class 'BaseClass8' is not declared protected. |
23+
| test.cpp:124:26:124:26 | declaration of BaseClass9 | Implicit copy constructor for base class 'BaseClass9<int>' is not declared deleted. |
24+
| test.cpp:124:26:124:26 | declaration of BaseClass9 | Implicit move constructor for base class 'BaseClass9<int>' is not declared deleted. |
25+
| test.cpp:124:26:124:26 | declaration of operator= | Implicit copy assignment operator for base class 'BaseClass9<int>' is not declared deleted. |
26+
| test.cpp:124:26:124:26 | declaration of operator= | Implicit move assignment operator for base class 'BaseClass9<int>' is not declared deleted. |

‎cpp/autosar/test/rules/A12-8-6/test.cpp

Lines changed: 64 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,4 +77,67 @@ class BaseClass6 {
7777
BaseClass6 &operator=(BaseClass6 const &) = default; // NON_COMPLIANT
7878
BaseClass6 &operator=(BaseClass6 &&) = default; // NON_COMPLIANT
7979
virtual void test() = 0; // pure virtual function, making this abstract
80-
};
80+
};
81+
82+
template <class T1> class BaseClass7 {
83+
public:
84+
BaseClass7() {}
85+
BaseClass7(BaseClass7 const &) = default; // NON_COMPLIANT
86+
BaseClass7(BaseClass7 &&) = default; // NON_COMPLIANT
87+
BaseClass7 &operator=(BaseClass7 const &) = default; // NON_COMPLIANT
88+
BaseClass7 &operator=(BaseClass7 &&) = default; // NON_COMPLIANT
89+
int operator=(int i); // COMPLIANT - not an assignment operator
90+
}; // COMPLIANT
91+
92+
template <class T>
93+
class DerivedClass7 // COMPLIANT - not a base class itself
94+
: public BaseClass7<T> {
95+
public:
96+
DerivedClass7() {}
97+
};
98+
99+
class DerivedClass8 // COMPLIANT - not a base class itself
100+
: public BaseClass7<int> {
101+
public:
102+
DerivedClass8() {}
103+
};
104+
105+
class BaseClass8 {
106+
public:
107+
BaseClass8() {}
108+
BaseClass8(BaseClass8 const &) = default; // NON_COMPLIANT
109+
BaseClass8(BaseClass8 &&) = default; // NON_COMPLIANT
110+
BaseClass8 &operator=(BaseClass8 const &) = default; // NON_COMPLIANT
111+
BaseClass8 &operator=(BaseClass8 &&) = default; // NON_COMPLIANT
112+
};
113+
114+
template <class T>
115+
class DerivedClass9 // COMPLIANT - not a base class itself
116+
: public BaseClass8 {
117+
public:
118+
DerivedClass9() {}
119+
120+
private:
121+
T t;
122+
};
123+
124+
template <class T> class BaseClass9 { // NON_COMPLIANT
125+
126+
public:
127+
BaseClass9() {}
128+
};
129+
130+
template <class T>
131+
class DerivedClass10 // COMPLIANT - not a base class itself
132+
: public BaseClass9<T> {
133+
public:
134+
DerivedClass10() {}
135+
};
136+
137+
void test() {
138+
BaseClass7<int> b;
139+
DerivedClass7<int> d;
140+
DerivedClass9<int> e;
141+
BaseClass9<int> f;
142+
DerivedClass10<int> g;
143+
}

‎cpp/common/src/codingstandards/cpp/Class.qll

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,26 @@
55
import cpp
66
import codingstandards.cpp.Expr
77

8+
private Class getADerivedClass(Class c) {
9+
result = c.getADerivedClass()
10+
or
11+
exists(ClassTemplateInstantiation instantiation |
12+
instantiation.getADerivedClass() = result and c = instantiation.getTemplate()
13+
)
14+
}
15+
816
/**
9-
* Holds if we believe that `c` is used or intended to be used as a base class.
17+
* A class that is used or intended to be used as a base class.
1018
*/
11-
predicate isPossibleBaseClass(Class c, string reason) {
12-
// There exists a derivation in this database
13-
exists(c.getADerivedClass()) and reason = "a derived class exists"
14-
or
15-
// The class must be extended at some point
16-
c.isAbstract() and reason = "the class is abstract"
19+
class BaseClass extends Class {
20+
BaseClass() {
21+
exists(getADerivedClass(this))
22+
or
23+
this.isAbstract()
24+
}
25+
26+
// We don't override `getADerivedClass` because that introduces a non-monotonic recursion.
27+
Class getASubClass() { result = getADerivedClass(this) }
1728
}
1829

1930
/**

‎cpp/common/src/codingstandards/cpp/TypeUses.qll

Lines changed: 42 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ private TypedefType getAnEquivalentTypeDef(TypedefType type) {
3535
* is from within the function signature or field declaration of the type itself.
3636
*/
3737
Locatable getATypeUse(Type type) {
38-
result = getATypeUse_i(type)
38+
result = getATypeUse_i(type, _)
3939
or
4040
// Identify `TypeMention`s of typedef types, where the underlying type is used.
4141
//
@@ -61,11 +61,11 @@ Locatable getATypeUse(Type type) {
6161
tm.getMentionedType() = typedefType
6262
|
6363
exists(tm.getFile().getRelativePath()) and
64-
exists(getATypeUse_i(typedefType.getUnderlyingType()))
64+
exists(getATypeUse_i(typedefType.getUnderlyingType(), _))
6565
)
6666
}
6767

68-
private Locatable getATypeUse_i(Type type) {
68+
private Locatable getATypeUse_i(Type type, string reason) {
6969
(
7070
// Restrict to uses within the source checkout root
7171
exists(result.getFile().getRelativePath())
@@ -82,74 +82,92 @@ private Locatable getATypeUse_i(Type type) {
8282
// Ignore self referential variables and parameters
8383
not v.getDeclaringType().refersTo(type) and
8484
not type = v.(Parameter).getFunction().getDeclaringType()
85-
)
85+
) and
86+
reason = "used as a variable type"
8687
or
8788
// Used a function return type
8889
exists(Function f |
8990
result = f and
9091
not f.isCompilerGenerated() and
9192
not type = f.getDeclaringType()
9293
|
93-
type = f.getType()
94+
type = f.getType() and reason = "used as a function return type"
9495
or
95-
type = f.getATemplateArgument()
96+
type = f.getATemplateArgument() and reason = "used as a function template argument"
9697
)
9798
or
9899
// Used either in a function call as a template argument, or as the declaring type
99100
// of the function
100101
exists(FunctionCall fc | result = fc |
101-
type = fc.getTarget().getDeclaringType()
102+
type = fc.getTarget().getDeclaringType() and reason = "used in call to member function"
102103
or
103-
type = fc.getATemplateArgument()
104+
type = fc.getATemplateArgument() and reason = "used in function call template argument"
104105
)
105106
or
106107
// Aliased in a user typedef
107-
exists(TypedefType t | result = t | type = t.getBaseType())
108+
exists(TypedefType t | result = t | type = t.getBaseType()) and
109+
reason = "aliased in user typedef"
108110
or
109111
// A use in a `FunctionAccess`
110-
exists(FunctionAccess fa | result = fa | type = fa.getTarget().getDeclaringType())
112+
exists(FunctionAccess fa | result = fa | type = fa.getTarget().getDeclaringType()) and
113+
reason = "used in a function accesses"
111114
or
112115
// A use in a `sizeof` expr
113-
exists(SizeofTypeOperator soto | result = soto | type = soto.getTypeOperand())
116+
exists(SizeofTypeOperator soto | result = soto | type = soto.getTypeOperand()) and
117+
reason = "used in a sizeof expr"
114118
or
115119
// A use in a `Cast`
116-
exists(Cast c | c = result | type = c.getType())
120+
exists(Cast c | c = result | type = c.getType()) and
121+
reason = "used in a cast"
117122
or
118123
// Use of the type name in source
119-
exists(TypeName t | t = result | type = t.getType())
124+
exists(TypeName t | t = result | type = t.getType()) and
125+
reason = "used in a typename"
120126
or
121127
// Access of an enum constant
122-
exists(EnumConstantAccess eca | result = eca | type = eca.getTarget().getDeclaringEnum())
128+
exists(EnumConstantAccess eca | result = eca | type = eca.getTarget().getDeclaringEnum()) and
129+
reason = "used in an enum constant access"
123130
or
124131
// Accessing a field on the type
125132
exists(FieldAccess fa |
126133
result = fa and
127134
type = fa.getTarget().getDeclaringType()
128-
)
135+
) and
136+
reason = "used in a field access"
129137
or
130138
// Name qualifiers
131139
exists(NameQualifier nq |
132140
result = nq and
133141
type = nq.getQualifyingElement()
134-
)
142+
) and
143+
reason = "used in name qualifier"
144+
or
145+
// Temporary object creation of type `type`
146+
exists(TemporaryObjectExpr toe | result = toe | type = toe.getType()) and
147+
reason = "used in temporary object expr"
135148
)
136149
or
137150
// Recursive case - used by a used type
138-
exists(Type used | result = getATypeUse_i(used) |
151+
exists(Type used | result = getATypeUse_i(used, _) |
139152
// The `used` class has `type` as a base class
140-
type = used.(DerivedType).getBaseType()
153+
type = used.(DerivedType).getBaseType() and
154+
reason = "used in derived type"
141155
or
142156
// The `used` class has `type` as a template argument
143-
type = used.(Class).getATemplateArgument()
157+
type = used.(Class).getATemplateArgument() and
158+
reason = "used in class template argument"
144159
or
145160
// A used class is derived from the type class
146-
type = used.(Class).getABaseClass()
161+
type = used.(Class).getABaseClass() and
162+
reason = "used in derived class"
147163
or
148164
// This is a TemplateClass where one of the instantiations is used
149-
type.(TemplateClass).getAnInstantiation() = used
165+
type.(TemplateClass).getAnInstantiation() = used and
166+
reason = "used in template class instantiation"
150167
or
151168
// This is a TemplateClass where one of the specializations is used
152-
type = used.(ClassTemplateSpecialization).getPrimaryTemplate()
169+
type = used.(ClassTemplateSpecialization).getPrimaryTemplate() and
170+
reason = "used in template class specialization"
153171
or
154172
// Alias templates - alias templates and instantiations are not properly captured by the
155173
// extractor (last verified in CodeQL CLI 2.7.6). The only distinguishing factor is that
@@ -164,6 +182,7 @@ private Locatable getATypeUse_i(Type type) {
164182
not exists(instantiation.getLocation()) and
165183
// Template and instantiation both have the same qualified name
166184
template.getQualifiedName() = instantiation.getQualifiedName()
167-
)
185+
) and
186+
reason = "used in alias template instantiation"
168187
)
169188
}

0 commit comments

Comments
 (0)