Merge branch 'main' into printfprecision

cpp/ql/lib/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 0.4.2
+
+No user-facing changes.
+
 ## 0.4.1
 
 No user-facing changes.

cpp/ql/lib/change-notes/released/0.4.2.md

@@ -0,0 +1,3 @@
+## 0.4.2
+
+No user-facing changes.

cpp/ql/lib/codeql-pack.release.yml

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 0.4.1
+lastReleaseVersion: 0.4.2

cpp/ql/lib/experimental/semmle/code/cpp/semantic/analysis/ModulusAnalysis.qll

@@ -4,6 +4,12 @@
  * variable), and `v` is an integer in the range `[0 .. m-1]`.
  */
 
+/*
+ * The main recursion has base cases in both `ssaModulus` (for guarded reads) and `semExprModulus`
+ * (for constant values). The most interesting recursive case is `phiModulusRankStep`, which
+ * handles phi inputs.
+ */
+
 private import ModulusAnalysisSpecific::Private
 private import experimental.semmle.code.cpp.semantic.Semantic
 private import ConstantAnalysis
@@ -162,6 +168,11 @@ private predicate phiModulusInit(SemSsaPhiNode phi, SemBound b, int val, int mod
  */
 pragma[nomagic]
 private predicate phiModulusRankStep(SemSsaPhiNode phi, SemBound b, int val, int mod, int rix) {
+  /*
+   * base case. If any phi input is equal to `b + val` modulo `mod`, that's a potential congruence
+   * class for the phi node.
+   */
+
   rix = 0 and
   phiModulusInit(phi, b, val, mod)
   or
@@ -169,6 +180,12 @@ private predicate phiModulusRankStep(SemSsaPhiNode phi, SemBound b, int val, int
     mod != 1 and
     val = remainder(v1, mod)
   |
+    /*
+     * Recursive case. If `inp` = `b + v2` mod `m2`, we combine that with the preceding potential
+     * congruence class `b + v1` mod `m1`. The result will be the congruence class of `v1` modulo
+     * the greatest common denominator of `m1`, `m2`, and `v1 - v2`.
+     */
+
     exists(int v2, int m2 |
       rankedPhiInput(pragma[only_bind_out](phi), inp, edge, rix) and
       phiModulusRankStep(phi, b, v1, m1, rix - 1) and
@@ -176,6 +193,12 @@ private predicate phiModulusRankStep(SemSsaPhiNode phi, SemBound b, int val, int
       mod = m1.gcd(m2).gcd(v1 - v2)
     )
     or
+    /*
+     * Recursive case. If `inp` = `phi` mod `m2`, we combine that with the preceding potential
+     * congruence class `b + v1` mod `m1`. The result will be a congruence class modulo the greatest
+     * common denominator of `m1` and `m2`.
+     */
+
     exists(int m2 |
       rankedPhiInput(phi, inp, edge, rix) and
       phiModulusRankStep(phi, b, v1, m1, rix - 1) and

cpp/ql/lib/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/cpp-all
-version: 0.4.2-dev
+version: 0.4.3-dev
 groups: cpp
 dbscheme: semmlecode.cpp.dbscheme
 extractor: cpp

cpp/ql/src/Best Practices/Likely Errors/CommaBeforeMisleadingIndentation.ql

@@ -16,15 +16,36 @@
 import cpp
 import semmle.code.cpp.commons.Exclusions
 
-/** Gets the sub-expression of 'e' with the earliest-starting Location */
+/**
+ * Gets a child of `e`, including conversions but excluding call arguments.
+ */
+pragma[inline]
+Expr getAChildWithConversions(Expr e) {
+  result.getParentWithConversions() = e and
+  not result = any(Call c).getAnArgument()
+}
+
+/**
+ * Gets the left-most column position of any transitive child of `e` (including
+ * conversions but excluding call arguments).
+ */
+int getCandidateColumn(Expr e) {
+  result = e.getLocation().getStartColumn() or
+  result = getCandidateColumn(getAChildWithConversions(e))
+}
+
+/**
+ * Gets the transitive child of `e` (including conversions but excluding call
+ * arguments) at the left-most column position, preferring less deeply nested
+ * expressions if there is a choice.
+ */
 Expr normalizeExpr(Expr e) {
-  result =
-    min(Expr child |
-      child.getParentWithConversions*() = e.getFullyConverted() and
-      not child.getParentWithConversions*() = any(Call c).getAnArgument()
-    |
-      child order by child.getLocation().getStartColumn(), count(child.getParentWithConversions*())
-    )
+  e.getLocation().getStartColumn() = min(getCandidateColumn(e)) and
+  result = e
+  or
+  not e.getLocation().getStartColumn() = min(getCandidateColumn(e)) and
+  result = normalizeExpr(getAChildWithConversions(e)) and
+  result.getLocation().getStartColumn() = min(getCandidateColumn(e))
 }
 
 predicate isParenthesized(CommaExpr ce) {
@@ -43,8 +64,8 @@ from CommaExpr ce, Expr left, Expr right, Location leftLoc, Location rightLoc
 where
   ce.fromSource() and
   not isFromMacroDefinition(ce) and
-  left = normalizeExpr(ce.getLeftOperand()) and
-  right = normalizeExpr(ce.getRightOperand()) and
+  left = normalizeExpr(ce.getLeftOperand().getFullyConverted()) and
+  right = normalizeExpr(ce.getRightOperand().getFullyConverted()) and
   leftLoc = left.getLocation() and
   rightLoc = right.getLocation() and
   not isParenthesized(ce) and

cpp/ql/src/Best Practices/Unused Entities/UnusedStaticFunctions.ql

@@ -13,16 +13,32 @@
 
 import cpp
 
+pragma[noinline]
+predicate possiblyIncompleteFile(File f) {
+  exists(Diagnostic d | d.getFile() = f and d.getSeverity() >= 3)
+}
+
 predicate immediatelyReachableFunction(Function f) {
-  not f.isStatic() or
-  exists(BlockExpr be | be.getFunction() = f) or
-  f instanceof MemberFunction or
-  f instanceof TemplateFunction or
-  f.getFile() instanceof HeaderFile or
-  f.getAnAttribute().hasName("constructor") or
-  f.getAnAttribute().hasName("destructor") or
-  f.getAnAttribute().hasName("used") or
+  not f.isStatic()
+  or
+  exists(BlockExpr be | be.getFunction() = f)
+  or
+  f instanceof MemberFunction
+  or
+  f instanceof TemplateFunction
+  or
+  f.getFile() instanceof HeaderFile
+  or
+  f.getAnAttribute().hasName("constructor")
+  or
+  f.getAnAttribute().hasName("destructor")
+  or
+  f.getAnAttribute().hasName("used")
+  or
   f.getAnAttribute().hasName("unused")
+  or
+  // a compiler error in the same file suggests we may be missing data
+  possiblyIncompleteFile(f.getFile())
 }
 
 predicate immediatelyReachableVariable(Variable v) {

cpp/ql/src/CHANGELOG.md

@@ -1,3 +1,14 @@
+## 0.4.2
+
+### New Queries
+
+* Added a new medium-precision query, `cpp/comma-before-misleading-indentation`, which detects instances of whitespace that have readability issues.
+
+### Minor Analysis Improvements
+
+* The "Unterminated variadic call" (`cpp/unterminated-variadic-call`) query has been tuned to produce fewer false positive results.
+* Fixed false positives from the "Unused static function" (`cpp/unused-static-function`) query in files that had errors during compilation.
+
 ## 0.4.1
 
 ### Minor Analysis Improvements

cpp/ql/src/change-notes/2022-09-30-comma-before-missing-indentation.md

@@ -1,4 +0,0 @@
----
-category: newQuery
----
-* Added a new medium-precision query, `cpp/comma-before-misleading-indentation`, which detects instances of whitespace that have readability issues.

cpp/ql/src/change-notes/2022-10-06-unterminated-variadic-call.md

@@ -1,4 +0,0 @@
----
-category: minorAnalysis
----
-* The "Unterminated variadic call" (`cpp/unterminated-variadic-call`) query has been tuned to produce fewer false positive results.

cpp/ql/src/change-notes/released/0.4.2.md

@@ -0,0 +1,10 @@
+## 0.4.2
+
+### New Queries
+
+* Added a new medium-precision query, `cpp/comma-before-misleading-indentation`, which detects instances of whitespace that have readability issues.
+
+### Minor Analysis Improvements
+
+* The "Unterminated variadic call" (`cpp/unterminated-variadic-call`) query has been tuned to produce fewer false positive results.
+* Fixed false positives from the "Unused static function" (`cpp/unused-static-function`) query in files that had errors during compilation.

cpp/ql/src/codeql-pack.release.yml

@@ -1,2 +1,2 @@
 ---
-lastReleaseVersion: 0.4.1
+lastReleaseVersion: 0.4.2

cpp/ql/src/qlpack.yml

@@ -1,5 +1,5 @@
 name: codeql/cpp-queries
-version: 0.4.2-dev
+version: 0.4.3-dev
 groups: 
   - cpp
   - queries

cpp/ql/test/library-tests/templates/CPP-223/decls.expected

@@ -13,6 +13,7 @@
 | test.cpp:4:26:4:26 | c<<expression>> |
 | test.cpp:4:26:4:26 | c<<unnamed>> |
 | test.cpp:5:29:5:29 | e |
+| test.cpp:6:24:6:24 | f |
 | test.cpp:6:26:6:26 | (unnamed parameter 0) |
 | test.cpp:6:29:6:31 | (unnamed parameter 1) |
 | test.cpp:7:20:7:20 | f |

cpp/ql/test/query-tests/Best Practices/Unused Entities/UnusedStaticFunctions/extraction_error.c

@@ -0,0 +1,15 @@
+// semmle-extractor-options: --expect_errors
+
+static void my_function1_called() {} // GOOD
+static void my_function2_called_after_error() {} // GOOD
+static void my_function3_not_called() {} // BAD [NOT DETECTED]
+
+int main(void) {
+	my_function1_called();
+
+--- compilation stops here because this line is not valid C code ---
+
+	my_function2_called_after_error();
+
+	return 0;
+}

cpp/ql/test/query-tests/Best Practices/Unused Entities/UnusedStaticFunctions/unused_static_functions.cpp

@@ -33,3 +33,16 @@ static void f6(void);
 static void f5(void) { f6(); }
 static void f6(void) { f5(); }
 
+// f7 and f8 are reachable from `function_caller` 
+static int f7() { return 1; } // GOOD
+static void f8() { } // GOOD
+
+void function_caller()
+{
+	auto my_lambda = []() {
+		return f7();
+	}();
+
+	f8();
+}
+