Skip to content

Commit 3842b4c

Browse files
committed
A7-1-3: Fix #601.
We did not correctly constrain the type mention for the type to be before the variable declaration itself.
1 parent 7736c34 commit 3842b4c

File tree

4 files changed

+25
-12
lines changed

4 files changed

+25
-12
lines changed
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
- `A7-1-3` - `CvQualifiersNotPlacedOnTheRightHandSide.ql`:
2+
- Removed false positives where a correctly CV-qualified typedef variable type was also referenced in the initializer.

cpp/autosar/src/rules/A7-1-3/CvQualifiersNotPlacedOnTheRightHandSide.ql

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,12 @@ import cpp
2020
import codingstandards.cpp.autosar
2121

2222
/**
23-
* Holds if declaration `e` using a `TypedefType` is CV-qualified
24-
*
25-
* For example, given `using intconstptr = int * const`:
26-
* the predicate holds for `const/volatile intconstptr ptr1`, but not for `intconstptr ptr2`
23+
* Unwrap layers of indirection that occur on the right side of the type.
2724
*/
28-
predicate containsExtraSpecifiers(VariableDeclarationEntry e) {
29-
e.getType().toString().matches("const %") or
30-
e.getType().toString().matches("volatile %")
25+
Type unwrapIndirection(Type type) {
26+
if type instanceof DerivedType and not type instanceof SpecifiedType
27+
then result = unwrapIndirection(type.(DerivedType).getBaseType())
28+
else result = type
3129
}
3230

3331
// DeclStmts that have a TypedefType name use (ie TypeMention) in them
@@ -36,19 +34,19 @@ predicate containsExtraSpecifiers(VariableDeclarationEntry e) {
3634
from VariableDeclarationEntry e, TypedefType t, TypeMention tm
3735
where
3836
not isExcluded(e, ConstPackage::cvQualifiersNotPlacedOnTheRightHandSideQuery()) and
39-
containsExtraSpecifiers(e) and
37+
// Variable type is specified, and has the typedef type as a base type
38+
unwrapIndirection(e.getType()).(SpecifiedType).getBaseType() = t and
4039
exists(string filepath, int startline |
4140
e.getLocation().hasLocationInfo(filepath, startline, _, _, _) and
4241
tm.getLocation().hasLocationInfo(filepath, startline, _, _, _) and
4342
e = t.getATypeNameUse() and
4443
tm.getMentionedType() = t and
44+
// TypeMention occurs before the variable declaration
45+
tm.getLocation().getStartColumn() < e.getLocation().getStartColumn() and
4546
exists(DeclStmt s |
4647
s.getDeclarationEntry(_) = e and
47-
//const could fit in there
48+
// TypeMention occurs after the start of the StmtDecl, with enough space for const/volatile
4849
tm.getLocation().getStartColumn() - s.getLocation().getStartColumn() > 5
49-
//volatile could fit in there
50-
//but the above condition subsumes this one
51-
//l.getStartColumn() - tm.getLocation().getStartColumn() > 8
5250
)
5351
)
5452
select e,
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
11
| test.cpp:9:16:9:19 | definition of ptr1 | There is possibly a const or volatile specifier on the left hand side of typedef name $@. | test.cpp:1:7:1:12 | intptr | intptr |
22
| test.cpp:10:19:10:22 | definition of ptr2 | There is possibly a const or volatile specifier on the left hand side of typedef name $@. | test.cpp:1:7:1:12 | intptr | intptr |
33
| test.cpp:19:21:19:24 | definition of ptr8 | There is possibly a const or volatile specifier on the left hand side of typedef name $@. | test.cpp:3:7:3:17 | constintptr | constintptr |
4+
| test.cpp:32:23:32:26 | definition of u32d | There is possibly a const or volatile specifier on the left hand side of typedef name $@. | file:///Users/luke/git/codeql-coding-standards/cpp/common/test/includes/standard-library/cstdint.h:9:22:9:29 | uint32_t | uint32_t |

cpp/autosar/test/rules/A7-1-3/test.cpp

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,16 @@ void f() {
1818
constintptr const ptr7 = &l; // COMPLIANT
1919
const constintptr ptr8 = &l; // NON_COMPLIANT
2020
inttypedef ptr9 = l; // COMPLIANT
21+
}
22+
23+
#include <cstdint>
24+
25+
void false_positive() {
26+
std::uint8_t u8{0};
27+
28+
auto const u32 = static_cast<std::uint32_t>(u8); // COMPLIANT - auto ignored
29+
std::uint32_t const u32b = static_cast<std::uint32_t>(u8); // COMPLIANT
30+
31+
const auto u32c = static_cast<std::uint32_t>(u8); // COMPLIANT - auto ignored
32+
const std::uint32_t u32d = static_cast<std::uint32_t>(u8); // NON_COMPLIANT
2133
}

0 commit comments

Comments
 (0)