From c765f93757323ee5218c0670016ec4b76f3387e8 Mon Sep 17 00:00:00 2001
From: Fernando Jose <fernando.jose@woven.toyota>
Date: Thu, 12 Sep 2024 12:22:50 +0900
Subject: [PATCH] Fix(common/cpp): dead code alert on constexpr with array
 sizes.

---
 .../cpp/rules/deadcode/DeadCode.qll           | 24 ++++++++++++++++++-
 .../test/rules/deadcode/DeadCode.expected     |  2 ++
 cpp/common/test/rules/deadcode/test.cpp       |  5 ++++
 3 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/cpp/common/src/codingstandards/cpp/rules/deadcode/DeadCode.qll b/cpp/common/src/codingstandards/cpp/rules/deadcode/DeadCode.qll
index 4a008dc15..5f3b77e66 100644
--- a/cpp/common/src/codingstandards/cpp/rules/deadcode/DeadCode.qll
+++ b/cpp/common/src/codingstandards/cpp/rules/deadcode/DeadCode.qll
@@ -16,11 +16,31 @@ import codingstandards.cpp.Customizations
 import codingstandards.cpp.Exclusions
 import codingstandards.cpp.deadcode.UselessAssignments
 import codingstandards.cpp.deadcode.UnreachableCode
+import codingstandards.cpp.deadcode.UnusedVariables
 
 abstract class DeadCodeSharedQuery extends Query { }
 
 Query getQuery() { result instanceof DeadCodeSharedQuery }
 
+/**
+ * Returns integer value of a constexpr variable
+ */
+int getConstexprValue(Variable v) {
+  result = v.getInitializer().getExpr().getValue().toInt() and v.isConstexpr()
+}
+
+/**
+ * Holds if `Variable` v is used for a local array size with value `n`
+ */
+bindingset[n]
+predicate isUsedInLocalArraySize(Variable v, int n) {
+  // Cf. https://github.com/github/codeql-coding-standards/pull/660/files.
+  count(ArrayType at, LocalVariable arrayVariable |
+        arrayVariable.getType().resolveTypedefs() = at and
+        v.(PotentiallyUnusedLocalVariable).getFunction() = arrayVariable.getFunction() and
+        at.getArraySize() = n) > 0
+}
+
 /**
  * Holds if the `Stmt` `s` is either dead or unreachable.
  */
@@ -39,6 +59,7 @@ predicate isDeadStmt(Stmt s) {
   // - All the declarations are variable declarations
   // - None of those variables are ever accessed in non-dead code
   // - The initializers for each of the variables are pure
+  // - It isn't constexpr and used to declare an array size
   exists(DeclStmt ds |
     ds = s and
     // Use forex so that we don't flag "fake" generated `DeclStmt`s (e.g. those generated by the
@@ -50,7 +71,8 @@ predicate isDeadStmt(Stmt s) {
         not exists(VariableAccess va |
           va.getTarget() = v and
           not isDeadOrUnreachableStmt(va.getEnclosingStmt())
-        )
+        ) and
+        not isUsedInLocalArraySize(v, getConstexprValue(v))
       )
     )
   )
diff --git a/cpp/common/test/rules/deadcode/DeadCode.expected b/cpp/common/test/rules/deadcode/DeadCode.expected
index 6c111d8a9..d3252e940 100644
--- a/cpp/common/test/rules/deadcode/DeadCode.expected
+++ b/cpp/common/test/rules/deadcode/DeadCode.expected
@@ -12,3 +12,5 @@
 | test.cpp:72:3:73:3 | try { ... } | This statement is dead code. |
 | test.cpp:73:17:74:3 | { ... } | This statement is dead code. |
 | test.cpp:79:17:80:3 | { ... } | This statement is dead code. |
+| test.cpp:85:3:85:44 | declaration | This statement is dead code. |
+| test.cpp:87:3:87:30 | declaration | This statement is dead code. |
diff --git a/cpp/common/test/rules/deadcode/test.cpp b/cpp/common/test/rules/deadcode/test.cpp
index ba5c59b07..982c02156 100644
--- a/cpp/common/test/rules/deadcode/test.cpp
+++ b/cpp/common/test/rules/deadcode/test.cpp
@@ -81,5 +81,10 @@ int test_dead_code(int x) {
 
   static_assert(1); // COMPLIANT
 
+  constexpr int constexpr_array_size{6};     // COMPLIANT
+  int unused_array[constexpr_array_size] {}; // NON_COMPLIANT
+
+  constexpr int unused_int{2}; // NON_COMPLIANT
+
   return live5 + live6; // COMPLIANT
 }
\ No newline at end of file