Skip to content

Commit

Permalink
A7-1-3: Fix #601.
Browse files Browse the repository at this point in the history
We did not correctly constrain the type mention for the type to
be before the variable declaration itself.
  • Loading branch information
lcartey committed Oct 15, 2024
1 parent 7736c34 commit 3842b4c
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 12 deletions.
2 changes: 2 additions & 0 deletions change_notes/2024-10-15-a7-1-3-multi-refs.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- `A7-1-3` - `CvQualifiersNotPlacedOnTheRightHandSide.ql`:
- Removed false positives where a correctly CV-qualified typedef variable type was also referenced in the initializer.
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,12 @@ import cpp
import codingstandards.cpp.autosar

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

// DeclStmts that have a TypedefType name use (ie TypeMention) in them
Expand All @@ -36,19 +34,19 @@ predicate containsExtraSpecifiers(VariableDeclarationEntry e) {
from VariableDeclarationEntry e, TypedefType t, TypeMention tm
where
not isExcluded(e, ConstPackage::cvQualifiersNotPlacedOnTheRightHandSideQuery()) and
containsExtraSpecifiers(e) and
// Variable type is specified, and has the typedef type as a base type
unwrapIndirection(e.getType()).(SpecifiedType).getBaseType() = t and
exists(string filepath, int startline |
e.getLocation().hasLocationInfo(filepath, startline, _, _, _) and
tm.getLocation().hasLocationInfo(filepath, startline, _, _, _) and
e = t.getATypeNameUse() and
tm.getMentionedType() = t and
// TypeMention occurs before the variable declaration
tm.getLocation().getStartColumn() < e.getLocation().getStartColumn() and
exists(DeclStmt s |
s.getDeclarationEntry(_) = e and
//const could fit in there
// TypeMention occurs after the start of the StmtDecl, with enough space for const/volatile
tm.getLocation().getStartColumn() - s.getLocation().getStartColumn() > 5
//volatile could fit in there
//but the above condition subsumes this one
//l.getStartColumn() - tm.getLocation().getStartColumn() > 8
)
)
select e,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
| 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 |
| 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 |
| 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 |
| 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 |
12 changes: 12 additions & 0 deletions cpp/autosar/test/rules/A7-1-3/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,16 @@ void f() {
constintptr const ptr7 = &l; // COMPLIANT
const constintptr ptr8 = &l; // NON_COMPLIANT
inttypedef ptr9 = l; // COMPLIANT
}

#include <cstdint>

void false_positive() {
std::uint8_t u8{0};

auto const u32 = static_cast<std::uint32_t>(u8); // COMPLIANT - auto ignored
std::uint32_t const u32b = static_cast<std::uint32_t>(u8); // COMPLIANT

const auto u32c = static_cast<std::uint32_t>(u8); // COMPLIANT - auto ignored
const std::uint32_t u32d = static_cast<std::uint32_t>(u8); // NON_COMPLIANT
}

0 comments on commit 3842b4c

Please sign in to comment.