Skip to content

Commit 57387b2

Browse files
rvermeulenrvermeulen
committed
Merge branch 'main' into rvermeulen/fix-368
2 parents e5881a9 + 5ff2a10 commit 57387b2

23 files changed

+429
-36
lines changed

‎change_notes/2024-01-17-a4-7-1-exclude-pointers.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
* `A4-7-1` - exclude pointer increment and decrement operators from this rule.

‎change_notes/2024-01-24-throwing-functions-exclude-noexcept.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
* Exceptions are no longer propagated from calls to `noexcept` functions, or calls functions with dynamic exception specifications where the exception is not permitted. This is consistent with the default behaviour specified in `[expect.spec]` which indicates that `std::terminate` is called. This has the following impact:
2+
- `A15-4-2`, `ERR55-CPP` - reduce false positives for `noexcept` functions which call other `noexcept` function which may throw.
3+
- `A15-2-2` - reduce false positives for constructors which call `noexcept` functions.
4+
- `A15-4-5` - reduce false positives for checked exceptions that are thrown from `noexcept` functions called by the original function.
5+
- `DCL57-CPP` - do not report exceptions thrown from `noexcept` functions called by deallocation functions or destructors.
6+
- `A15-5-1`, `M15-3-1` - do not report exceptions thrown from `noexcept` functions called by special functions.

‎change_notes/2024-01-30-exclusion-a13-3-1.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
`A13-3-1` - `FunctionThatContainsForwardingReferenceAsItsArgumentOverloaded.ql`:
2+
- Fixes #399. Exclude functions that have different number of parameters.

‎change_notes/2024-01-31-exclusion-a16-0-1.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
`A16-0-1` - `PreProcessorShallOnlyBeUsedForCertainDirectivesPatterns.ql`:
2+
- Exclude all preprocessor elses and also consider elifs separately (ie do not affect valid ifs) but not valid if not meeting the same criteria as an ifdef etc.

‎change_notes/2024-01-31-fix-fp-a7-1-2.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
`A7-1-2` - `VariableMissingConstexpr.ql`:
2+
- Fix FP reported in #466. Addresses incorrect assumption that calls to `constexpr` functions are always compile-time evaluated.

‎cpp/autosar/src/rules/A13-3-1/FunctionThatContainsForwardingReferenceAsItsArgumentOverloaded.ql‎

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,36 @@ class Candidate extends TemplateFunction {
2121
}
2222
}
2323

24-
from Candidate c, Function f
24+
from
25+
Candidate c, Function f, Function overload, Function overloaded, string msg,
26+
string firstMsgSegment
2527
where
2628
not isExcluded(f,
2729
OperatorsPackage::functionThatContainsForwardingReferenceAsItsArgumentOverloadedQuery()) and
2830
not f.isDeleted() and
29-
f = c.getAnOverload()
30-
select f, "Function overloads a $@ with a forwarding reference parameter.", c, "function"
31+
f = c.getAnOverload() and
32+
// allow for overloading with different number of parameters, because there is no
33+
// confusion on what function will be called.
34+
f.getNumberOfParameters() = c.getNumberOfParameters() and
35+
//build a dynamic select statement that guarantees to read that the overloading function is the explicit one
36+
if
37+
(f instanceof CopyConstructor or f instanceof MoveConstructor) and
38+
f.isCompilerGenerated()
39+
then (
40+
(
41+
f instanceof CopyConstructor and
42+
msg = "implicit copy constructor"
43+
or
44+
f instanceof MoveConstructor and
45+
msg = "implicit move constructor"
46+
) and
47+
firstMsgSegment = " with a forwarding reference parameter " and
48+
overloaded = f and
49+
overload = c
50+
) else (
51+
msg = "function with a forwarding reference parameter" and
52+
firstMsgSegment = " " and
53+
overloaded = c and
54+
overload = f
55+
)
56+
select overload, "Function" + firstMsgSegment + "overloads a $@.", overloaded, msg

‎cpp/autosar/src/rules/A16-0-1/PreProcessorShallOnlyBeUsedForCertainDirectivesPatterns.ql‎

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,26 @@ import cpp
2121
import codingstandards.cpp.autosar
2222
import codingstandards.cpp.FunctionLikeMacro
2323

24+
class PermittedInnerDirectiveType extends PreprocessorDirective {
25+
PermittedInnerDirectiveType() {
26+
//permissive listing for directives that can be used in a valid wrapper
27+
this instanceof MacroWrapper or
28+
this instanceof PreprocessorEndif or
29+
this instanceof Include or
30+
this instanceof PermittedMacro or
31+
this instanceof PreprocessorElif or
32+
this instanceof PreprocessorElse
33+
}
34+
}
35+
2436
class PermittedDirectiveType extends PreprocessorDirective {
2537
PermittedDirectiveType() {
2638
//permissive listing in case directive types modelled in ql ever expands (example non valid directives)
2739
this instanceof MacroWrapper or
2840
this instanceof PreprocessorEndif or
2941
this instanceof Include or
30-
this instanceof PermittedMacro
42+
this instanceof PermittedMacro or
43+
this instanceof PreprocessorElse
3144
}
3245
}
3346

@@ -40,9 +53,9 @@ pragma[noinline]
4053
predicate isPreprocConditionalRange(
4154
PreprocessorBranch pb, string filepath, int startLine, int endLine
4255
) {
43-
exists(PreprocessorEndif end | pb.getEndIf() = end |
44-
isPreprocFileAndLine(pb, filepath, startLine) and
45-
isPreprocFileAndLine(end, filepath, endLine)
56+
isPreprocFileAndLine(pb, filepath, startLine) and
57+
exists(PreprocessorDirective end |
58+
pb.getNext() = end and isPreprocFileAndLine(end, filepath, endLine)
4659
)
4760
}
4861

@@ -73,7 +86,7 @@ class MacroWrapper extends PreprocessorIfndef {
7386
class AcceptableWrapper extends PreprocessorBranch {
7487
AcceptableWrapper() {
7588
forall(Element inner | not inner instanceof Comment and this = getAGuard(inner) |
76-
inner instanceof PermittedDirectiveType
89+
inner instanceof PermittedInnerDirectiveType
7790
)
7891
}
7992
}

‎cpp/autosar/src/rules/A7-1-2/VariableMissingConstexpr.ql‎

Lines changed: 74 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import cpp
1717
import codingstandards.cpp.autosar
1818
import codingstandards.cpp.TrivialType
1919
import codingstandards.cpp.SideEffect
20+
import semmle.code.cpp.controlflow.SSA
2021

2122
predicate isZeroInitializable(Variable v) {
2223
not exists(v.getInitializer().getExpr()) and
@@ -33,6 +34,78 @@ predicate isTypeZeroInitializable(Type t) {
3334
t.getUnderlyingType() instanceof ArrayType
3435
}
3536

37+
/**
38+
* An optimized set of expressions used to determine the flow through constexpr variables.
39+
*/
40+
class VariableAccessOrCallOrLiteral extends Expr {
41+
VariableAccessOrCallOrLiteral() {
42+
this instanceof VariableAccess or
43+
this instanceof Call or
44+
this instanceof Literal
45+
}
46+
}
47+
48+
/**
49+
* Holds if the value of source flows through compile time evaluated variables to target.
50+
*/
51+
predicate flowsThroughConstExprVariables(
52+
VariableAccessOrCallOrLiteral source, VariableAccessOrCallOrLiteral target
53+
) {
54+
(
55+
source = target
56+
or
57+
source != target and
58+
exists(SsaDefinition intermediateDef, StackVariable intermediate |
59+
intermediateDef.getAVariable().getFunction() = source.getEnclosingFunction() and
60+
intermediateDef.getAVariable().getFunction() = target.getEnclosingFunction() and
61+
intermediateDef.getAVariable() = intermediate and
62+
intermediate.isConstexpr()
63+
|
64+
DataFlow::localExprFlow(source, intermediateDef.getDefiningValue(intermediate)) and
65+
flowsThroughConstExprVariables(intermediateDef.getAUse(intermediate), target)
66+
)
67+
)
68+
}
69+
70+
/*
71+
* Returns true if the given call may be evaluated at compile time and is compile time evaluated because
72+
* all its arguments are compile time evaluated and its default values are compile time evaluated.
73+
*/
74+
75+
predicate isCompileTimeEvaluated(Call call) {
76+
// 1. The call may be evaluated at compile time, because it is constexpr, and
77+
call.getTarget().isConstexpr() and
78+
// 2. all its arguments are compile time evaluated, and
79+
forall(DataFlow::Node ultimateArgSource, DataFlow::Node argSource |
80+
argSource = DataFlow::exprNode(call.getAnArgument()) and
81+
DataFlow::localFlow(ultimateArgSource, argSource) and
82+
not DataFlow::localFlowStep(_, ultimateArgSource)
83+
|
84+
(
85+
ultimateArgSource.asExpr() instanceof Literal
86+
or
87+
any(Call c | isCompileTimeEvaluated(c)) = ultimateArgSource.asExpr()
88+
) and
89+
// If the ultimate argument source is not the same as the argument source, then it must flow through
90+
// constexpr variables.
91+
(
92+
ultimateArgSource != argSource
93+
implies
94+
flowsThroughConstExprVariables(ultimateArgSource.asExpr(), argSource.asExpr())
95+
)
96+
) and
97+
// 3. all the default values used are compile time evaluated.
98+
forall(Expr defaultValue, Parameter parameterUsingDefaultValue, int idx |
99+
parameterUsingDefaultValue = call.getTarget().getParameter(idx) and
100+
not exists(call.getArgument(idx)) and
101+
parameterUsingDefaultValue.getAnAssignedValue() = defaultValue
102+
|
103+
defaultValue instanceof Literal
104+
or
105+
any(Call c | isCompileTimeEvaluated(c)) = defaultValue
106+
)
107+
}
108+
36109
from Variable v
37110
where
38111
not isExcluded(v, ConstPackage::variableMissingConstexprQuery()) and
@@ -46,7 +119,7 @@ where
46119
(
47120
v.getInitializer().getExpr().isConstant()
48121
or
49-
v.getInitializer().getExpr().(Call).getTarget().isConstexpr()
122+
any(Call call | isCompileTimeEvaluated(call)) = v.getInitializer().getExpr()
50123
or
51124
isZeroInitializable(v)
52125
or

‎cpp/autosar/test/rules/A13-3-1/FunctionThatContainsForwardingReferenceAsItsArgumentOverloaded.expected‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
1-
| test.cpp:24:6:24:7 | F1 | Function overloads a $@ with a forwarding reference parameter. | test.cpp:27:25:27:26 | F1 | function |
2-
| test.cpp:49:3:49:3 | A | Function overloads a $@ with a forwarding reference parameter. | test.cpp:47:3:47:3 | A | function |
3-
| test.cpp:50:3:50:3 | A | Function overloads a $@ with a forwarding reference parameter. | test.cpp:47:3:47:3 | A | function |
4-
| test.cpp:63:8:63:8 | B | Function overloads a $@ with a forwarding reference parameter. | test.cpp:66:3:66:3 | B | function |
5-
| test.cpp:63:8:63:8 | B | Function overloads a $@ with a forwarding reference parameter. | test.cpp:66:3:66:3 | B | function |
1+
| test.cpp:24:6:24:7 | F1 | Function overloads a $@. | test.cpp:27:25:27:26 | F1 | function with a forwarding reference parameter |
2+
| test.cpp:50:3:50:3 | A | Function overloads a $@. | test.cpp:48:3:48:3 | A | function with a forwarding reference parameter |
3+
| test.cpp:51:3:51:3 | A | Function overloads a $@. | test.cpp:48:3:48:3 | A | function with a forwarding reference parameter |
4+
| test.cpp:69:3:69:3 | B | Function with a forwarding reference parameter overloads a $@. | test.cpp:64:8:64:8 | B | implicit copy constructor |
5+
| test.cpp:69:3:69:3 | B | Function with a forwarding reference parameter overloads a $@. | test.cpp:64:8:64:8 | B | implicit move constructor |
6+
| test.cpp:77:25:77:25 | C | Function with a forwarding reference parameter overloads a $@. | test.cpp:74:7:74:7 | C | implicit copy constructor |
7+
| test.cpp:77:25:77:25 | C | Function with a forwarding reference parameter overloads a $@. | test.cpp:74:7:74:7 | C | implicit move constructor |

‎cpp/autosar/test/rules/A13-3-1/test.cpp‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,8 @@ template <class T> void F1(T &&x) {} //
3939

4040
class A {
4141
public:
42-
// COMPLIANT by exception, constrained to not match copy/move ctors
42+
// COMPLIANT[FALSE_POSITIVE] - by exception, constrained to not match
43+
// copy/move ctors
4344
template <
4445
typename T,
4546
std::enable_if_t<!std::is_same<
@@ -61,9 +62,17 @@ A b(a);
6162
void F1(int &) = delete; // COMPLIANT by exception
6263

6364
struct B {
64-
template <typename T,
65-
std::enable_if_t<!std::is_same<T, B>::value> * = nullptr>
66-
B(T &&value) {}
65+
template <
66+
typename T,
67+
std::enable_if_t<!std::is_same<
68+
std::remove_cv_t<std::remove_reference_t<T>>, A>::value> * = nullptr>
69+
B(T &&value) {} // COMPLIANT[FALSE_POSITIVE] - by exception
6770
};
6871

69-
int main() {}
72+
int main() {}
73+
74+
class C {
75+
public:
76+
C() {}
77+
template <typename T> C(T &&) {} // NON_COMPLIANT
78+
};

0 commit comments

Comments
 (0)