From 3d0b937994a6de87020c37512624216354b692e3 Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Tue, 14 Mar 2023 15:28:01 -0400 Subject: [PATCH 1/8] Fix FP for issue 214 exclude variable templates and fix case where same scope identifiers are considered --- change_notes/2023-03-14-fp-a12-10-1.md | 1 + cpp/common/src/codingstandards/cpp/Scope.qll | 40 +++++++++++++++++++ .../identifierhidden/IdentifierHidden.qll | 5 ++- .../test/rules/identifierhidden/test.cpp | 6 +++ .../test/rules/identifierhidden/test1.cpp | 2 + 5 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 change_notes/2023-03-14-fp-a12-10-1.md create mode 100644 cpp/common/test/rules/identifierhidden/test1.cpp diff --git a/change_notes/2023-03-14-fp-a12-10-1.md b/change_notes/2023-03-14-fp-a12-10-1.md new file mode 100644 index 0000000000..d92af0e8df --- /dev/null +++ b/change_notes/2023-03-14-fp-a12-10-1.md @@ -0,0 +1 @@ +* `A12-10-1` and `RULE-5-3` - reduce false positives reported for identifiers in same scope, and (relevant for `A12-10-1` only) omitted false positives for template variables \ No newline at end of file diff --git a/cpp/common/src/codingstandards/cpp/Scope.qll b/cpp/common/src/codingstandards/cpp/Scope.qll index d39478d784..1734a1e9e4 100644 --- a/cpp/common/src/codingstandards/cpp/Scope.qll +++ b/cpp/common/src/codingstandards/cpp/Scope.qll @@ -130,6 +130,18 @@ private UserVariable getPotentialScopeOfVariable_candidate(UserVariable v) { ) } +/** Gets a variable that is in the potential scope of variable `v`. */ +private UserVariable getOuterScopesOfVariable_candidate(UserVariable v) { + exists(Scope s | + result = s.getAVariable() and + ( + // Variable in an ancestor scope, but only if there are less than 100 variables in this scope + v = s.getAnAncestor().getAVariable() and + s.getNumberOfVariables() < 100 + ) + ) +} + /** Holds if there exists a translation unit that includes both `f1` and `f2`. */ pragma[noinline] predicate inSameTranslationUnit(File f1, File f2) { @@ -148,6 +160,15 @@ UserVariable getPotentialScopeOfVariable(UserVariable v) { inSameTranslationUnit(v.getFile(), result.getFile()) } +/** + * Gets a user variable which occurs in the "outer scope" of variable `v`. + */ +cached +UserVariable getPotentialScopeOfVariableStrict(UserVariable v) { + result = getOuterScopesOfVariable_candidate(v) and + inSameTranslationUnit(v.getFile(), result.getFile()) +} + /** A file that is a C/C++ source file */ class SourceFile extends File { SourceFile() { @@ -182,6 +203,15 @@ private predicate hides_candidate(UserVariable v1, UserVariable v2) { not (v1.isMember() or v2.isMember()) } +/** Holds if `v2` may hide `v1`. */ +private predicate hides_candidateStrict(UserVariable v1, UserVariable v2) { + not v1 = v2 and + v2 = getPotentialScopeOfVariableStrict(v1) and + v1.getName() = v2.getName() and + // Member variables cannot hide other variables nor be hidden because the can be referenced through their qualified name. + not (v1.isMember() or v2.isMember()) +} + /** Holds if `v2` hides `v1`. */ predicate hides(UserVariable v1, UserVariable v2) { hides_candidate(v1, v2) and @@ -192,6 +222,16 @@ predicate hides(UserVariable v1, UserVariable v2) { ) } +/** Holds if `v2` strictly (`v2` is in an inner scope compared to `v1`) hides `v1`. */ +predicate hidesStrict(UserVariable v1, UserVariable v2) { + hides_candidateStrict(v1, v2) and + // Confirm that there's no closer candidate variable which `v2` hides + not exists(UserVariable mid | + hides_candidateStrict(v1, mid) and + hides_candidateStrict(mid, v2) + ) +} + /** Holds if `decl` has namespace scope. */ predicate hasNamespaceScope(Declaration decl) { // getNamespace always returns a namespace (e.g. the global namespace). diff --git a/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll b/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll index b3cadd6d2a..fc0a01cbd4 100644 --- a/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll +++ b/cpp/common/src/codingstandards/cpp/rules/identifierhidden/IdentifierHidden.qll @@ -14,7 +14,10 @@ Query getQuery() { result instanceof IdentifierHiddenSharedQuery } query predicate problems(UserVariable v2, string message, UserVariable v1, string varName) { not isExcluded(v1, getQuery()) and not isExcluded(v2, getQuery()) and - hides(v1, v2) and + //ignore template variables for this rule + not v1 instanceof TemplateVariable and + not v2 instanceof TemplateVariable and + hidesStrict(v1, v2) and varName = v1.getName() and message = "Variable is hiding variable $@." } diff --git a/cpp/common/test/rules/identifierhidden/test.cpp b/cpp/common/test/rules/identifierhidden/test.cpp index 233ae4c004..aaf4a23129 100644 --- a/cpp/common/test/rules/identifierhidden/test.cpp +++ b/cpp/common/test/rules/identifierhidden/test.cpp @@ -27,4 +27,10 @@ void f3() { for (int id1; id1 < 1; id1++) { } // NON_COMPLIANT } +} + +template constexpr bool foo = false; // COMPLIANT + +namespace { +template bool foo = true; // COMPLIANT } \ No newline at end of file diff --git a/cpp/common/test/rules/identifierhidden/test1.cpp b/cpp/common/test/rules/identifierhidden/test1.cpp new file mode 100644 index 0000000000..ff4adb5381 --- /dev/null +++ b/cpp/common/test/rules/identifierhidden/test1.cpp @@ -0,0 +1,2 @@ +template constexpr bool foo = false; // COMPLIANT +template constexpr bool foo = true; // COMPLIANT \ No newline at end of file From 730b374696a47ad2271860233926a2b1659670a0 Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Tue, 14 Mar 2023 15:33:37 -0400 Subject: [PATCH 2/8] Add missing change notes from fixes for issues 215 and 232 --- change_notes/2023-03-13-fp-a16-0-1.md | 1 + change_notes/2023-03-14-fp-a12-10-1.md | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 change_notes/2023-03-13-fp-a16-0-1.md diff --git a/change_notes/2023-03-13-fp-a16-0-1.md b/change_notes/2023-03-13-fp-a16-0-1.md new file mode 100644 index 0000000000..d1cf580ac6 --- /dev/null +++ b/change_notes/2023-03-13-fp-a16-0-1.md @@ -0,0 +1 @@ + * `A16-0-1` - reduce unneeded results related to `#pragma`, as it's already reported by A16-7-1. \ No newline at end of file diff --git a/change_notes/2023-03-14-fp-a12-10-1.md b/change_notes/2023-03-14-fp-a12-10-1.md index d92af0e8df..f1a2ee65f8 100644 --- a/change_notes/2023-03-14-fp-a12-10-1.md +++ b/change_notes/2023-03-14-fp-a12-10-1.md @@ -1 +1 @@ -* `A12-10-1` and `RULE-5-3` - reduce false positives reported for identifiers in same scope, and (relevant for `A12-10-1` only) omitted false positives for template variables \ No newline at end of file + * `DCL51-CPP` - reduce false positives related to use of `__func__` \ No newline at end of file From 3aaf85353a94b3c9c4bab0b06f002b4a53e4f5b2 Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Tue, 14 Mar 2023 15:40:58 -0400 Subject: [PATCH 3/8] Fix change notes --- .../{2023-03-14-fp-a12-10-1.md => 2023-03-13-fp-dcl51-cpp.md} | 0 change_notes/2023-03-14-fp-a2-10-1.md | 2 ++ 2 files changed, 2 insertions(+) rename change_notes/{2023-03-14-fp-a12-10-1.md => 2023-03-13-fp-dcl51-cpp.md} (100%) create mode 100644 change_notes/2023-03-14-fp-a2-10-1.md diff --git a/change_notes/2023-03-14-fp-a12-10-1.md b/change_notes/2023-03-13-fp-dcl51-cpp.md similarity index 100% rename from change_notes/2023-03-14-fp-a12-10-1.md rename to change_notes/2023-03-13-fp-dcl51-cpp.md diff --git a/change_notes/2023-03-14-fp-a2-10-1.md b/change_notes/2023-03-14-fp-a2-10-1.md new file mode 100644 index 0000000000..f6dcd3d865 --- /dev/null +++ b/change_notes/2023-03-14-fp-a2-10-1.md @@ -0,0 +1,2 @@ + * `A2-10-1` - reduce false positives for identifiers in same scope and relating to template variables + * `RULE-5-3`- reduce false positives for identifiers in same scope \ No newline at end of file From b09235f47fd550b51ee8375f557889d2169b0935 Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Thu, 16 Mar 2023 17:42:19 -0400 Subject: [PATCH 4/8] Fix FP for issue 216 exclude file stream call args and heuristic wrappers --- change_notes/2023-03-16-fp-a5-1-1.md | 1 + .../A5-1-1/LiteralValueUsedOutsideTypeInit.ql | 24 +++++++++++++++++++ .../LiteralValueUsedOutsideTypeInit.expected | 2 ++ cpp/autosar/test/rules/A5-1-1/test.cpp | 24 ++++++++++++++++++- 4 files changed, 50 insertions(+), 1 deletion(-) create mode 100644 change_notes/2023-03-16-fp-a5-1-1.md diff --git a/change_notes/2023-03-16-fp-a5-1-1.md b/change_notes/2023-03-16-fp-a5-1-1.md new file mode 100644 index 0000000000..61e4fed11c --- /dev/null +++ b/change_notes/2023-03-16-fp-a5-1-1.md @@ -0,0 +1 @@ + * `A5-1-1` - reduce false positives by omitting literals written into file streams and wrappers around log and stream calls \ No newline at end of file diff --git a/cpp/autosar/src/rules/A5-1-1/LiteralValueUsedOutsideTypeInit.ql b/cpp/autosar/src/rules/A5-1-1/LiteralValueUsedOutsideTypeInit.ql index c20d0ded55..6758bae54e 100644 --- a/cpp/autosar/src/rules/A5-1-1/LiteralValueUsedOutsideTypeInit.ql +++ b/cpp/autosar/src/rules/A5-1-1/LiteralValueUsedOutsideTypeInit.ql @@ -18,6 +18,25 @@ import cpp import codingstandards.cpp.autosar import codingstandards.cpp.LoggingOperation import codingstandards.cpp.Literals +import codingstandards.cpp.standardlibrary.FileStreams + +/** + * In a wrapper `Function`, all accesses of all `Parameters` + * are in located in logging or stream calls + */ +class LoggerOrStreamWrapperFunction extends Function { + LoggerOrStreamWrapperFunction() { + forall(Parameter p | p.getFunction() = this | + forall(VariableAccess va | va = p.getAnAccess() | + ( + any(FileStreamFunctionCall fc).getAnArgument().getAChild*() = va + or + any(LoggingOperation logOp).getALoggedExpr().getAChild*() = va + ) + ) + ) + } +} from Literal l where @@ -26,6 +45,11 @@ where not exists(ConstructorCall cc | cc.getAnArgument() = l) and not exists(ConstructorFieldInit cf | cf.getExpr() = l) and not l = any(LoggingOperation logOp).getALoggedExpr().getAChild*() and + not l = any(FileStreamFunctionCall fsc).getAnArgument().getAChild*() and + // Exclude arguments to wrapper functions + not exists(FunctionCall fc, LoggerOrStreamWrapperFunction w | + fc.getAnArgument() = l and w.getACallToThisFunction() = fc + ) and // Exclude literal 0 not l.getValue() = "0" and // Exclude character literals diff --git a/cpp/autosar/test/rules/A5-1-1/LiteralValueUsedOutsideTypeInit.expected b/cpp/autosar/test/rules/A5-1-1/LiteralValueUsedOutsideTypeInit.expected index 9e783c3b14..d19df4e43d 100644 --- a/cpp/autosar/test/rules/A5-1-1/LiteralValueUsedOutsideTypeInit.expected +++ b/cpp/autosar/test/rules/A5-1-1/LiteralValueUsedOutsideTypeInit.expected @@ -1,3 +1,5 @@ | test.cpp:5:9:5:25 | constant string | Literal value "constant string" used outside of type initialization StringLiteral | | test.cpp:14:23:14:25 | 100 | Literal value 100 used outside of type initialization Literal | | test.cpp:54:7:54:7 | 1 | Literal value 1 used outside of type initialization Literal | +| test.cpp:75:23:75:28 | test | Literal value "test" used outside of type initialization StringLiteral | +| test.cpp:75:31:75:40 | not okay | Literal value "not okay" used outside of type initialization StringLiteral | diff --git a/cpp/autosar/test/rules/A5-1-1/test.cpp b/cpp/autosar/test/rules/A5-1-1/test.cpp index 691e94d2fa..42abc42f43 100644 --- a/cpp/autosar/test/rules/A5-1-1/test.cpp +++ b/cpp/autosar/test/rules/A5-1-1/test.cpp @@ -52,4 +52,26 @@ void test_class() { void test_assignment() { int x = 0; // COMPLIANT - used in type initialization x = 1; // NON_COMPLIANT - used in assignment -} \ No newline at end of file +} + +void test_stream(std::ostream &os, const char *str) noexcept { + os << str << "logging string"; // COMPLIANT - literal used in stream write +} + +#define WRAPPER_MACRO(X, Y) test_stream(X, Y) + +void test_wrapper_stream(std::ostream &os, const char *str) noexcept { + test_stream(os, "test"); // COMPLIANT - wrapper for stream write + WRAPPER_MACRO(os, "test"); // COMPLIANT - wrapper for stream write +} + +void test_stream_two(std::ostream &os, const char *str, + const char *alt) noexcept { + os << str << "logging string"; // COMPLIANT - literal used in stream write + throw alt; +} + +void test_not_wrapper_stream(std::ostream &os, const char *str) noexcept { + test_stream_two(os, "test", "not okay"); // NON_COMPLIANT - test_stream_two is + // not actually exclusively a wrapper +} From 507d34eb99478736fbef3c1345dcbf5ccd2f82fe Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Fri, 17 Mar 2023 10:35:39 -0400 Subject: [PATCH 5/8] Fix testcase identifiershidden rule --- cpp/common/test/rules/identifierhidden/test.cpp | 9 ++++++++- cpp/common/test/rules/identifierhidden/test1.cpp | 2 -- 2 files changed, 8 insertions(+), 3 deletions(-) delete mode 100644 cpp/common/test/rules/identifierhidden/test1.cpp diff --git a/cpp/common/test/rules/identifierhidden/test.cpp b/cpp/common/test/rules/identifierhidden/test.cpp index aaf4a23129..90f56e7ccf 100644 --- a/cpp/common/test/rules/identifierhidden/test.cpp +++ b/cpp/common/test/rules/identifierhidden/test.cpp @@ -32,5 +32,12 @@ void f3() { template constexpr bool foo = false; // COMPLIANT namespace { -template bool foo = true; // COMPLIANT +template bool foo = true; // COMPLIANT - omit variable templates +} + +template constexpr T foo1 = T(1.1L); + +template T f(T r) { + T v = foo1 * r * r; // COMPLIANT + T v1 = foo1 * r * r; // COMPLIANT } \ No newline at end of file diff --git a/cpp/common/test/rules/identifierhidden/test1.cpp b/cpp/common/test/rules/identifierhidden/test1.cpp deleted file mode 100644 index ff4adb5381..0000000000 --- a/cpp/common/test/rules/identifierhidden/test1.cpp +++ /dev/null @@ -1,2 +0,0 @@ -template constexpr bool foo = false; // COMPLIANT -template constexpr bool foo = true; // COMPLIANT \ No newline at end of file From c6bcdf96cf6551bcb42aba554c8751daf15e83ae Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Thu, 30 Mar 2023 11:49:17 -0400 Subject: [PATCH 6/8] Improve fix for FP for issue 216 --- .../A5-1-1/LiteralValueUsedOutsideTypeInit.ql | 26 ++--------------- .../LiteralValueUsedOutsideTypeInit.expected | 2 -- cpp/autosar/test/rules/A5-1-1/test.cpp | 19 +++++++++++-- .../codingstandards/cpp/LoggingOperation.qll | 28 +++++++++++++++++++ 4 files changed, 47 insertions(+), 28 deletions(-) diff --git a/cpp/autosar/src/rules/A5-1-1/LiteralValueUsedOutsideTypeInit.ql b/cpp/autosar/src/rules/A5-1-1/LiteralValueUsedOutsideTypeInit.ql index 6758bae54e..69649f72b1 100644 --- a/cpp/autosar/src/rules/A5-1-1/LiteralValueUsedOutsideTypeInit.ql +++ b/cpp/autosar/src/rules/A5-1-1/LiteralValueUsedOutsideTypeInit.ql @@ -18,25 +18,6 @@ import cpp import codingstandards.cpp.autosar import codingstandards.cpp.LoggingOperation import codingstandards.cpp.Literals -import codingstandards.cpp.standardlibrary.FileStreams - -/** - * In a wrapper `Function`, all accesses of all `Parameters` - * are in located in logging or stream calls - */ -class LoggerOrStreamWrapperFunction extends Function { - LoggerOrStreamWrapperFunction() { - forall(Parameter p | p.getFunction() = this | - forall(VariableAccess va | va = p.getAnAccess() | - ( - any(FileStreamFunctionCall fc).getAnArgument().getAChild*() = va - or - any(LoggingOperation logOp).getALoggedExpr().getAChild*() = va - ) - ) - ) - } -} from Literal l where @@ -45,11 +26,8 @@ where not exists(ConstructorCall cc | cc.getAnArgument() = l) and not exists(ConstructorFieldInit cf | cf.getExpr() = l) and not l = any(LoggingOperation logOp).getALoggedExpr().getAChild*() and - not l = any(FileStreamFunctionCall fsc).getAnArgument().getAChild*() and - // Exclude arguments to wrapper functions - not exists(FunctionCall fc, LoggerOrStreamWrapperFunction w | - fc.getAnArgument() = l and w.getACallToThisFunction() = fc - ) and + // Exclude Macros with names like *LOG + not exists(MacroInvocation m | m.getMacroName().matches("%LOG") and m.getAnAffectedElement() = l) and // Exclude literal 0 not l.getValue() = "0" and // Exclude character literals diff --git a/cpp/autosar/test/rules/A5-1-1/LiteralValueUsedOutsideTypeInit.expected b/cpp/autosar/test/rules/A5-1-1/LiteralValueUsedOutsideTypeInit.expected index d19df4e43d..9e783c3b14 100644 --- a/cpp/autosar/test/rules/A5-1-1/LiteralValueUsedOutsideTypeInit.expected +++ b/cpp/autosar/test/rules/A5-1-1/LiteralValueUsedOutsideTypeInit.expected @@ -1,5 +1,3 @@ | test.cpp:5:9:5:25 | constant string | Literal value "constant string" used outside of type initialization StringLiteral | | test.cpp:14:23:14:25 | 100 | Literal value 100 used outside of type initialization Literal | | test.cpp:54:7:54:7 | 1 | Literal value 1 used outside of type initialization Literal | -| test.cpp:75:23:75:28 | test | Literal value "test" used outside of type initialization StringLiteral | -| test.cpp:75:31:75:40 | not okay | Literal value "not okay" used outside of type initialization StringLiteral | diff --git a/cpp/autosar/test/rules/A5-1-1/test.cpp b/cpp/autosar/test/rules/A5-1-1/test.cpp index 42abc42f43..afb4b789bc 100644 --- a/cpp/autosar/test/rules/A5-1-1/test.cpp +++ b/cpp/autosar/test/rules/A5-1-1/test.cpp @@ -72,6 +72,21 @@ void test_stream_two(std::ostream &os, const char *str, } void test_not_wrapper_stream(std::ostream &os, const char *str) noexcept { - test_stream_two(os, "test", "not okay"); // NON_COMPLIANT - test_stream_two is - // not actually exclusively a wrapper + test_stream_two( + os, "test", + "not okay"); // NON_COMPLIANT[FALSE_NEGATIVE] - test_stream_two is + // not actually exclusively a wrapper } + +#define MACRO_LOG(test_str) do \ +{\ +struct test_struct {\ + static const char* get_str(){\ + return static_cast(test_str);\ + }\ +};\ +} while (false) + +void f(){ + MACRO_LOG("test"); //COMPLIANT - exclusion +} \ No newline at end of file diff --git a/cpp/common/src/codingstandards/cpp/LoggingOperation.qll b/cpp/common/src/codingstandards/cpp/LoggingOperation.qll index c77f2c450a..4cfcafeaae 100644 --- a/cpp/common/src/codingstandards/cpp/LoggingOperation.qll +++ b/cpp/common/src/codingstandards/cpp/LoggingOperation.qll @@ -1,5 +1,6 @@ import cpp import semmle.code.cpp.security.OutputWrite +import codingstandards.cpp.standardlibrary.FileStreams /** * A operation which may perform logging. @@ -16,9 +17,36 @@ class OutputWriteLogging extends LoggingOperation, OutputWrite { override Expr getALoggedExpr() { result = getASource() } } +/** + * A `FileStreamFunctionCall` operation is considered a log operation for Coding Standards purposes. + */ +class FileStreamLogging extends LoggingOperation, FileStreamFunctionCall { + override Expr getALoggedExpr() { result = getAnArgument() } + + override Expr getFStream() { result = this.getQualifier() } +} + /** A call which looks like `printf`. */ class PrintfLikeCall extends LoggingOperation, Call { PrintfLikeCall() { getTarget().getName().toLowerCase().matches("%printf%") } override Expr getALoggedExpr() { result = getAnArgument() } } + +/** + * In a wrapper `Function`, all accesses of all `Parameters` + * are in located in logging or stream calls + */ +class LoggerOrStreamWrapperFunction extends Function { + LoggerOrStreamWrapperFunction() { + forall(Parameter p | p.getFunction() = this | + forall(VariableAccess va | va = p.getAnAccess() | + ( + any(FileStreamFunctionCall fc).getAnArgument().getAChild*() = va + or + any(LoggingOperation logOp).getALoggedExpr().getAChild*() = va + ) + ) + ) + } +} From 062e0e208dd0fd56efb733db68894fa2e25d387c Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Thu, 30 Mar 2023 11:56:20 -0400 Subject: [PATCH 7/8] Format test A5-1-1 --- cpp/autosar/test/rules/A5-1-1/test.cpp | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/cpp/autosar/test/rules/A5-1-1/test.cpp b/cpp/autosar/test/rules/A5-1-1/test.cpp index afb4b789bc..faaf188f32 100644 --- a/cpp/autosar/test/rules/A5-1-1/test.cpp +++ b/cpp/autosar/test/rules/A5-1-1/test.cpp @@ -78,15 +78,13 @@ void test_not_wrapper_stream(std::ostream &os, const char *str) noexcept { // not actually exclusively a wrapper } -#define MACRO_LOG(test_str) do \ -{\ -struct test_struct {\ - static const char* get_str(){\ - return static_cast(test_str);\ - }\ -};\ -} while (false) - -void f(){ - MACRO_LOG("test"); //COMPLIANT - exclusion +#define MACRO_LOG(test_str) \ + do { \ + struct test_struct { \ + static const char *get_str() { return static_cast(test_str); } \ + }; \ + } while (false) + +void f() { + MACRO_LOG("test"); // COMPLIANT - exclusion } \ No newline at end of file From e1190d89b30c8379f1909056ede25c2bc65fc86f Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Wed, 3 May 2023 13:27:47 -0400 Subject: [PATCH 8/8] Improve fix for FP for issue 216 reintroduce omission wrappers --- .../A5-1-1/LiteralValueUsedOutsideTypeInit.ql | 4 ++++ .../LiteralValueUsedOutsideTypeInit.expected | 2 ++ cpp/autosar/test/rules/A5-1-1/test.cpp | 7 +++---- .../codingstandards/cpp/LoggingOperation.qll | 20 +++++++++---------- 4 files changed, 18 insertions(+), 15 deletions(-) diff --git a/cpp/autosar/src/rules/A5-1-1/LiteralValueUsedOutsideTypeInit.ql b/cpp/autosar/src/rules/A5-1-1/LiteralValueUsedOutsideTypeInit.ql index 69649f72b1..6d551474f3 100644 --- a/cpp/autosar/src/rules/A5-1-1/LiteralValueUsedOutsideTypeInit.ql +++ b/cpp/autosar/src/rules/A5-1-1/LiteralValueUsedOutsideTypeInit.ql @@ -26,6 +26,10 @@ where not exists(ConstructorCall cc | cc.getAnArgument() = l) and not exists(ConstructorFieldInit cf | cf.getExpr() = l) and not l = any(LoggingOperation logOp).getALoggedExpr().getAChild*() and + // Exclude arguments to wrapper functions (depth 1) + not exists(FunctionCall fc, LoggerOrStreamWrapperFunction w | + fc.getAnArgument() = l and w.getACallToThisFunction() = fc + ) and // Exclude Macros with names like *LOG not exists(MacroInvocation m | m.getMacroName().matches("%LOG") and m.getAnAffectedElement() = l) and // Exclude literal 0 diff --git a/cpp/autosar/test/rules/A5-1-1/LiteralValueUsedOutsideTypeInit.expected b/cpp/autosar/test/rules/A5-1-1/LiteralValueUsedOutsideTypeInit.expected index 9e783c3b14..3212f14efb 100644 --- a/cpp/autosar/test/rules/A5-1-1/LiteralValueUsedOutsideTypeInit.expected +++ b/cpp/autosar/test/rules/A5-1-1/LiteralValueUsedOutsideTypeInit.expected @@ -1,3 +1,5 @@ | test.cpp:5:9:5:25 | constant string | Literal value "constant string" used outside of type initialization StringLiteral | | test.cpp:14:23:14:25 | 100 | Literal value 100 used outside of type initialization Literal | | test.cpp:54:7:54:7 | 1 | Literal value 1 used outside of type initialization Literal | +| test.cpp:75:23:75:28 | test | Literal value "test" used outside of type initialization StringLiteral | +| test.cpp:76:19:76:28 | not okay | Literal value "not okay" used outside of type initialization StringLiteral | diff --git a/cpp/autosar/test/rules/A5-1-1/test.cpp b/cpp/autosar/test/rules/A5-1-1/test.cpp index faaf188f32..4c4ad4fb30 100644 --- a/cpp/autosar/test/rules/A5-1-1/test.cpp +++ b/cpp/autosar/test/rules/A5-1-1/test.cpp @@ -72,10 +72,9 @@ void test_stream_two(std::ostream &os, const char *str, } void test_not_wrapper_stream(std::ostream &os, const char *str) noexcept { - test_stream_two( - os, "test", - "not okay"); // NON_COMPLIANT[FALSE_NEGATIVE] - test_stream_two is - // not actually exclusively a wrapper + test_stream_two(os, "test", + "not okay"); // NON_COMPLIANT - test_stream_two is + // not actually exclusively a wrapper } #define MACRO_LOG(test_str) \ diff --git a/cpp/common/src/codingstandards/cpp/LoggingOperation.qll b/cpp/common/src/codingstandards/cpp/LoggingOperation.qll index 4cfcafeaae..de2201a5cd 100644 --- a/cpp/common/src/codingstandards/cpp/LoggingOperation.qll +++ b/cpp/common/src/codingstandards/cpp/LoggingOperation.qll @@ -20,10 +20,12 @@ class OutputWriteLogging extends LoggingOperation, OutputWrite { /** * A `FileStreamFunctionCall` operation is considered a log operation for Coding Standards purposes. */ -class FileStreamLogging extends LoggingOperation, FileStreamFunctionCall { - override Expr getALoggedExpr() { result = getAnArgument() } +class FileStreamLogging extends LoggingOperation { + FileStreamLogging() { this instanceof FileStreamFunctionCall } + + override Expr getALoggedExpr() { result = this.(FileStreamFunctionCall).getAnArgument() } - override Expr getFStream() { result = this.getQualifier() } + Expr getFStream() { result = this.(FileStreamFunctionCall).getQualifier() } } /** A call which looks like `printf`. */ @@ -39,14 +41,10 @@ class PrintfLikeCall extends LoggingOperation, Call { */ class LoggerOrStreamWrapperFunction extends Function { LoggerOrStreamWrapperFunction() { - forall(Parameter p | p.getFunction() = this | - forall(VariableAccess va | va = p.getAnAccess() | - ( - any(FileStreamFunctionCall fc).getAnArgument().getAChild*() = va - or - any(LoggingOperation logOp).getALoggedExpr().getAChild*() = va - ) - ) + forall(VariableAccess va | + exists(Parameter p | p.getFunction() = this and va = p.getAnAccess()) + | + any(LoggingOperation logOp).getALoggedExpr().getAChild*() = va ) } }