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-13-fp-dcl51-cpp.md b/change_notes/2023-03-13-fp-dcl51-cpp.md new file mode 100644 index 0000000000..f1a2ee65f8 --- /dev/null +++ b/change_notes/2023-03-13-fp-dcl51-cpp.md @@ -0,0 +1 @@ + * `DCL51-CPP` - reduce false positives related to use of `__func__` \ No newline at end of file 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 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..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,12 @@ 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 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..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 691e94d2fa..4c4ad4fb30 100644 --- a/cpp/autosar/test/rules/A5-1-1/test.cpp +++ b/cpp/autosar/test/rules/A5-1-1/test.cpp @@ -52,4 +52,38 @@ void test_class() { void test_assignment() { int x = 0; // COMPLIANT - used in type initialization x = 1; // NON_COMPLIANT - used in assignment +} + +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 +} + +#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..de2201a5cd 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,34 @@ 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 { + FileStreamLogging() { this instanceof FileStreamFunctionCall } + + override Expr getALoggedExpr() { result = this.(FileStreamFunctionCall).getAnArgument() } + + Expr getFStream() { result = this.(FileStreamFunctionCall).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(VariableAccess va | + exists(Parameter p | p.getFunction() = this and va = p.getAnAccess()) + | + any(LoggingOperation logOp).getALoggedExpr().getAChild*() = va + ) + } +} 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..90f56e7ccf 100644 --- a/cpp/common/test/rules/identifierhidden/test.cpp +++ b/cpp/common/test/rules/identifierhidden/test.cpp @@ -27,4 +27,17 @@ void f3() { for (int id1; id1 < 1; id1++) { } // NON_COMPLIANT } +} + +template constexpr bool foo = false; // COMPLIANT + +namespace { +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