From 3842b4cd3e2a8ba9121e9b6b441f105c7c50265f Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Tue, 15 Oct 2024 11:23:05 +0100 Subject: [PATCH 1/2] A7-1-3: Fix #601. We did not correctly constrain the type mention for the type to be before the variable declaration itself. --- change_notes/2024-10-15-a7-1-3-multi-refs.md | 2 ++ ...CvQualifiersNotPlacedOnTheRightHandSide.ql | 22 +++++++++---------- ...ifiersNotPlacedOnTheRightHandSide.expected | 1 + cpp/autosar/test/rules/A7-1-3/test.cpp | 12 ++++++++++ 4 files changed, 25 insertions(+), 12 deletions(-) create mode 100644 change_notes/2024-10-15-a7-1-3-multi-refs.md diff --git a/change_notes/2024-10-15-a7-1-3-multi-refs.md b/change_notes/2024-10-15-a7-1-3-multi-refs.md new file mode 100644 index 0000000000..39e00495cb --- /dev/null +++ b/change_notes/2024-10-15-a7-1-3-multi-refs.md @@ -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. \ No newline at end of file diff --git a/cpp/autosar/src/rules/A7-1-3/CvQualifiersNotPlacedOnTheRightHandSide.ql b/cpp/autosar/src/rules/A7-1-3/CvQualifiersNotPlacedOnTheRightHandSide.ql index 54968dc223..f60fcd51de 100644 --- a/cpp/autosar/src/rules/A7-1-3/CvQualifiersNotPlacedOnTheRightHandSide.ql +++ b/cpp/autosar/src/rules/A7-1-3/CvQualifiersNotPlacedOnTheRightHandSide.ql @@ -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 @@ -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, diff --git a/cpp/autosar/test/rules/A7-1-3/CvQualifiersNotPlacedOnTheRightHandSide.expected b/cpp/autosar/test/rules/A7-1-3/CvQualifiersNotPlacedOnTheRightHandSide.expected index 9d6a710449..7eea341903 100644 --- a/cpp/autosar/test/rules/A7-1-3/CvQualifiersNotPlacedOnTheRightHandSide.expected +++ b/cpp/autosar/test/rules/A7-1-3/CvQualifiersNotPlacedOnTheRightHandSide.expected @@ -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 | diff --git a/cpp/autosar/test/rules/A7-1-3/test.cpp b/cpp/autosar/test/rules/A7-1-3/test.cpp index 621a64115d..39f53b8623 100644 --- a/cpp/autosar/test/rules/A7-1-3/test.cpp +++ b/cpp/autosar/test/rules/A7-1-3/test.cpp @@ -18,4 +18,16 @@ void f() { constintptr const ptr7 = &l; // COMPLIANT const constintptr ptr8 = &l; // NON_COMPLIANT inttypedef ptr9 = l; // COMPLIANT +} + +#include + +void false_positive() { + std::uint8_t u8{0}; + + auto const u32 = static_cast(u8); // COMPLIANT - auto ignored + std::uint32_t const u32b = static_cast(u8); // COMPLIANT + + const auto u32c = static_cast(u8); // COMPLIANT - auto ignored + const std::uint32_t u32d = static_cast(u8); // NON_COMPLIANT } \ No newline at end of file From 917bb45c82c9ee1e5f97698b306bb8d0f9548dfe Mon Sep 17 00:00:00 2001 From: Luke Cartey Date: Tue, 15 Oct 2024 15:57:42 +0100 Subject: [PATCH 2/2] A7-1-3: Avoid producing machine specific locations Modify the alert message to only report a link if the target is within the source root of the database. --- ...CvQualifiersNotPlacedOnTheRightHandSide.ql | 23 +++++++++++++++---- ...ifiersNotPlacedOnTheRightHandSide.expected | 2 +- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/cpp/autosar/src/rules/A7-1-3/CvQualifiersNotPlacedOnTheRightHandSide.ql b/cpp/autosar/src/rules/A7-1-3/CvQualifiersNotPlacedOnTheRightHandSide.ql index f60fcd51de..5d34f89c7d 100644 --- a/cpp/autosar/src/rules/A7-1-3/CvQualifiersNotPlacedOnTheRightHandSide.ql +++ b/cpp/autosar/src/rules/A7-1-3/CvQualifiersNotPlacedOnTheRightHandSide.ql @@ -31,7 +31,9 @@ Type unwrapIndirection(Type type) { // DeclStmts that have a TypedefType name use (ie TypeMention) in them //AND TypeMention.getStartColumn() - DeclStmt.getStartColumn() > len(const) //AND the declared thing contains one of these "extra" specifiers in the DeclarationEntry Location -from VariableDeclarationEntry e, TypedefType t, TypeMention tm +from + VariableDeclarationEntry e, TypedefType t, TypeMention tm, string message, Element explainer, + string explainerMessage where not isExcluded(e, ConstPackage::cvQualifiersNotPlacedOnTheRightHandSideQuery()) and // Variable type is specified, and has the typedef type as a base type @@ -48,7 +50,20 @@ where // TypeMention occurs after the start of the StmtDecl, with enough space for const/volatile tm.getLocation().getStartColumn() - s.getLocation().getStartColumn() > 5 ) + ) and + if exists(t.getFile().getRelativePath()) + then + message = + "There is possibly a const or volatile specifier on the left hand side of typedef name $@." and + explainer = t and + explainerMessage = t.getName() + else ( + // Type occurs outside source root, so don't link + message = + "There is possibly a const or volatile specifier on the left hand side of typedef name " + + t.getName() + "." and + // explainer not used in this case + explainer = e and + explainerMessage = "" ) -select e, - "There is possibly a const or volatile specifier on the left hand side of typedef name $@.", t, - t.getName() +select e, message, explainer, explainerMessage diff --git a/cpp/autosar/test/rules/A7-1-3/CvQualifiersNotPlacedOnTheRightHandSide.expected b/cpp/autosar/test/rules/A7-1-3/CvQualifiersNotPlacedOnTheRightHandSide.expected index 7eea341903..d845df142d 100644 --- a/cpp/autosar/test/rules/A7-1-3/CvQualifiersNotPlacedOnTheRightHandSide.expected +++ b/cpp/autosar/test/rules/A7-1-3/CvQualifiersNotPlacedOnTheRightHandSide.expected @@ -1,4 +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 | +| 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 uint32_t. | test.cpp:32:23:32:26 | definition of u32d | |