Merge pull request #253 from knewbury01/knewbury01/more-FP-fix

Fixes for #214 and 216 FP reports

Author: knewbury01

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.

change_notes/2023-03-13-fp-dcl51-cpp.md

@@ -0,0 +1 @@
+ * `DCL51-CPP` - reduce false positives related to use of `__func__`

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

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

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

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 |

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<char *>(test_str); }   \
+    };                                                                         \
+  } while (false)
+
+void f() {
+  MACRO_LOG("test"); // COMPLIANT - exclusion
 }

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
+    )
+  }
+}

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).

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 $@."
 }

cpp/common/test/rules/identifierhidden/test.cpp

@@ -27,4 +27,17 @@ void f3() {
     for (int id1; id1 < 1; id1++) {
     } // NON_COMPLIANT
   }
+}
+
+template <typename T> constexpr bool foo = false; // COMPLIANT
+
+namespace {
+template <typename T> bool foo = true; // COMPLIANT - omit variable templates
+}
+
+template <class T> constexpr T foo1 = T(1.1L);
+
+template <class T, class R> T f(T r) {
+  T v = foo1<T> * r * r;  // COMPLIANT
+  T v1 = foo1<R> * r * r; // COMPLIANT
 }