From aaf3ce259082446301438a3907dc53ad4ea94e9a Mon Sep 17 00:00:00 2001
From: "REDMOND\\brodes" <brodes@microsoft.com>
Date: Mon, 4 Nov 2024 14:20:48 -0500
Subject: [PATCH 01/10] Adding test cases for boolean flow.

---
 .../controlflow/guards-ir/test.c              | 11 ++++
 .../controlflow/guards-ir/tests.expected      | 63 +++++++++++++++++++
 .../controlflow/guards/Guards.expected        |  5 ++
 .../controlflow/guards/GuardsCompare.expected | 17 +++++
 .../controlflow/guards/GuardsControl.expected |  6 ++
 .../controlflow/guards/GuardsEnsure.expected  | 11 ++++
 .../library-tests/controlflow/guards/test.c   | 11 ++++
 7 files changed, 124 insertions(+)

diff --git a/cpp/ql/test/library-tests/controlflow/guards-ir/test.c b/cpp/ql/test/library-tests/controlflow/guards-ir/test.c
index 140507237cf4..ee7f7243e183 100644
--- a/cpp/ql/test/library-tests/controlflow/guards-ir/test.c
+++ b/cpp/ql/test/library-tests/controlflow/guards-ir/test.c
@@ -184,3 +184,14 @@ int abort_test(int x) {
         abort();
     }
 }
+
+void boolean_flow(int x){
+    int b = x > 5;
+
+    if(!b){
+    }
+
+    if(b&&x<100){
+        
+    }
+}
\ No newline at end of file
diff --git a/cpp/ql/test/library-tests/controlflow/guards-ir/tests.expected b/cpp/ql/test/library-tests/controlflow/guards-ir/tests.expected
index d097fa7dfa67..c344eef808a4 100644
--- a/cpp/ql/test/library-tests/controlflow/guards-ir/tests.expected
+++ b/cpp/ql/test/library-tests/controlflow/guards-ir/tests.expected
@@ -36,6 +36,11 @@ astGuards
 | test.c:165:9:165:18 | ... < ... |
 | test.c:175:13:175:32 | ... == ... |
 | test.c:181:9:181:9 | x |
+| test.c:191:8:191:9 | ! ... |
+| test.c:191:9:191:9 | b |
+| test.c:194:8:194:8 | b |
+| test.c:194:8:194:15 | ... && ... |
+| test.c:194:11:194:15 | ... < ... |
 | test.cpp:18:8:18:10 | call to get |
 | test.cpp:31:7:31:13 | ... == ... |
 | test.cpp:42:13:42:20 | call to getABool |
@@ -211,6 +216,20 @@ astGuardsCompare
 | 175 | call to foo == 0+0 when ... == ... is true |
 | 181 | x != 0 when x is true |
 | 181 | x == 0 when x is false |
+| 191 | ! ... != 0 when ! ... is true |
+| 191 | ! ... == 0 when ! ... is false |
+| 191 | b != 0 when ! ... is false |
+| 191 | b != 0 when b is true |
+| 191 | b == 0 when b is false |
+| 194 | 100 < x+1 when ... < ... is false |
+| 194 | 100 >= x+1 when ... && ... is true |
+| 194 | 100 >= x+1 when ... < ... is true |
+| 194 | b != 0 when ... && ... is true |
+| 194 | b != 0 when b is true |
+| 194 | b == 0 when b is false |
+| 194 | x < 100+0 when ... && ... is true |
+| 194 | x < 100+0 when ... < ... is true |
+| 194 | x >= 100+0 when ... < ... is false |
 astGuardsControl
 | test.c:7:9:7:13 | ... > ... | false | 10 | 11 |
 | test.c:7:9:7:13 | ... > ... | true | 7 | 9 |
@@ -306,6 +325,12 @@ astGuardsControl
 | test.c:181:9:181:9 | x | false | 183 | 184 |
 | test.c:181:9:181:9 | x | true | 181 | 182 |
 | test.c:181:9:181:9 | x | true | 186 | 180 |
+| test.c:191:8:191:9 | ! ... | true | 191 | 192 |
+| test.c:191:9:191:9 | b | false | 191 | 192 |
+| test.c:194:8:194:8 | b | true | 194 | 194 |
+| test.c:194:8:194:8 | b | true | 194 | 196 |
+| test.c:194:8:194:15 | ... && ... | true | 194 | 196 |
+| test.c:194:11:194:15 | ... < ... | true | 194 | 196 |
 | test.cpp:18:8:18:10 | call to get | true | 19 | 19 |
 | test.cpp:31:7:31:13 | ... == ... | false | 30 | 30 |
 | test.cpp:31:7:31:13 | ... == ... | false | 34 | 34 |
@@ -482,6 +507,10 @@ astGuardsEnsure
 | test.c:175:13:175:32 | ... == ... | test.c:175:13:175:15 | call to foo | == | test.c:175:32:175:32 | 0 | 0 | 175 | 175 |
 | test.c:175:13:175:32 | ... == ... | test.c:175:32:175:32 | 0 | != | test.c:175:13:175:15 | call to foo | 0 | 175 | 175 |
 | test.c:175:13:175:32 | ... == ... | test.c:175:32:175:32 | 0 | == | test.c:175:13:175:15 | call to foo | 0 | 175 | 175 |
+| test.c:194:8:194:15 | ... && ... | test.c:194:11:194:11 | x | < | test.c:194:13:194:15 | 100 | 0 | 194 | 196 |
+| test.c:194:8:194:15 | ... && ... | test.c:194:13:194:15 | 100 | >= | test.c:194:11:194:11 | x | 1 | 194 | 196 |
+| test.c:194:11:194:15 | ... < ... | test.c:194:11:194:11 | x | < | test.c:194:13:194:15 | 100 | 0 | 194 | 196 |
+| test.c:194:11:194:15 | ... < ... | test.c:194:13:194:15 | 100 | >= | test.c:194:11:194:11 | x | 1 | 194 | 196 |
 | test.cpp:31:7:31:13 | ... == ... | test.cpp:31:7:31:7 | x | != | test.cpp:31:12:31:13 | - ... | 0 | 30 | 30 |
 | test.cpp:31:7:31:13 | ... == ... | test.cpp:31:7:31:7 | x | != | test.cpp:31:12:31:13 | - ... | 0 | 34 | 34 |
 | test.cpp:31:7:31:13 | ... == ... | test.cpp:31:7:31:7 | x | == | test.cpp:31:12:31:13 | - ... | 0 | 30 | 30 |
@@ -534,6 +563,11 @@ astGuardsEnsure_const
 | test.c:181:9:181:9 | x | test.c:181:9:181:9 | x | != | 0 | 181 | 182 |
 | test.c:181:9:181:9 | x | test.c:181:9:181:9 | x | != | 0 | 186 | 180 |
 | test.c:181:9:181:9 | x | test.c:181:9:181:9 | x | == | 0 | 183 | 184 |
+| test.c:191:8:191:9 | ! ... | test.c:191:8:191:9 | ! ... | != | 0 | 191 | 192 |
+| test.c:191:9:191:9 | b | test.c:191:9:191:9 | b | == | 0 | 191 | 192 |
+| test.c:194:8:194:8 | b | test.c:194:8:194:8 | b | != | 0 | 194 | 194 |
+| test.c:194:8:194:8 | b | test.c:194:8:194:8 | b | != | 0 | 194 | 196 |
+| test.c:194:8:194:15 | ... && ... | test.c:194:8:194:8 | b | != | 0 | 194 | 196 |
 | test.cpp:18:8:18:10 | call to get | test.cpp:18:8:18:10 | call to get | != | 0 | 19 | 19 |
 | test.cpp:31:7:31:13 | ... == ... | test.cpp:31:7:31:7 | x | != | -1 | 30 | 30 |
 | test.cpp:31:7:31:13 | ... == ... | test.cpp:31:7:31:7 | x | != | -1 | 34 | 34 |
@@ -573,6 +607,10 @@ irGuards
 | test.c:165:9:165:18 | CompareLT: ... < ... |
 | test.c:175:13:175:32 | CompareEQ: ... == ... |
 | test.c:181:9:181:9 | Load: x |
+| test.c:191:8:191:9 | LogicalNot: ! ... |
+| test.c:191:9:191:9 | Load: b |
+| test.c:194:8:194:8 | Load: b |
+| test.c:194:11:194:15 | CompareLT: ... < ... |
 | test.cpp:18:8:18:12 | CompareNE: (bool)... |
 | test.cpp:31:7:31:13 | CompareEQ: ... == ... |
 | test.cpp:42:13:42:20 | Call: call to getABool |
@@ -746,6 +784,19 @@ irGuardsCompare
 | 175 | call to foo == 0+0 when CompareEQ: ... == ... is true |
 | 181 | x != 0 when Load: x is true |
 | 181 | x == 0 when Load: x is false |
+| 191 | ! ... != 0 when LogicalNot: ! ... is true |
+| 191 | ! ... == 0 when LogicalNot: ! ... is false |
+| 191 | b != 0 when Load: b is true |
+| 191 | b != 0 when LogicalNot: ! ... is false |
+| 191 | b == 0 when Load: b is false |
+| 194 | 100 < x+1 when CompareLT: ... < ... is false |
+| 194 | 100 >= x+1 when CompareLT: ... < ... is true |
+| 194 | b != 0 when Load: b is true |
+| 194 | b == 0 when Load: b is false |
+| 194 | x < 100 when CompareLT: ... < ... is true |
+| 194 | x < 100+0 when CompareLT: ... < ... is true |
+| 194 | x >= 100 when CompareLT: ... < ... is false |
+| 194 | x >= 100+0 when CompareLT: ... < ... is false |
 irGuardsControl
 | test.c:7:9:7:13 | CompareGT: ... > ... | false | 11 | 11 |
 | test.c:7:9:7:13 | CompareGT: ... > ... | true | 8 | 8 |
@@ -833,6 +884,11 @@ irGuardsControl
 | test.c:175:13:175:32 | CompareEQ: ... == ... | true | 175 | 175 |
 | test.c:181:9:181:9 | Load: x | false | 184 | 184 |
 | test.c:181:9:181:9 | Load: x | true | 182 | 182 |
+| test.c:191:8:191:9 | LogicalNot: ! ... | true | 191 | 192 |
+| test.c:191:9:191:9 | Load: b | false | 191 | 192 |
+| test.c:194:8:194:8 | Load: b | true | 194 | 194 |
+| test.c:194:8:194:8 | Load: b | true | 194 | 196 |
+| test.c:194:11:194:15 | CompareLT: ... < ... | true | 194 | 196 |
 | test.cpp:18:8:18:12 | CompareNE: (bool)... | true | 19 | 19 |
 | test.cpp:31:7:31:13 | CompareEQ: ... == ... | false | 34 | 34 |
 | test.cpp:31:7:31:13 | CompareEQ: ... == ... | true | 30 | 30 |
@@ -992,6 +1048,8 @@ irGuardsEnsure
 | test.c:175:13:175:32 | CompareEQ: ... == ... | test.c:175:13:175:15 | Call: call to foo | == | test.c:175:32:175:32 | Constant: 0 | 0 | 175 | 175 |
 | test.c:175:13:175:32 | CompareEQ: ... == ... | test.c:175:32:175:32 | Constant: 0 | != | test.c:175:13:175:15 | Call: call to foo | 0 | 175 | 175 |
 | test.c:175:13:175:32 | CompareEQ: ... == ... | test.c:175:32:175:32 | Constant: 0 | == | test.c:175:13:175:15 | Call: call to foo | 0 | 175 | 175 |
+| test.c:194:11:194:15 | CompareLT: ... < ... | test.c:194:11:194:11 | Load: x | < | test.c:194:13:194:15 | Constant: 100 | 0 | 194 | 196 |
+| test.c:194:11:194:15 | CompareLT: ... < ... | test.c:194:13:194:15 | Constant: 100 | >= | test.c:194:11:194:11 | Load: x | 1 | 194 | 196 |
 | test.cpp:18:8:18:12 | CompareNE: (bool)... | test.cpp:18:8:18:10 | Call: call to get | != | test.cpp:18:8:18:12 | Constant: (bool)... | 0 | 19 | 19 |
 | test.cpp:18:8:18:12 | CompareNE: (bool)... | test.cpp:18:8:18:12 | Constant: (bool)... | != | test.cpp:18:8:18:10 | Call: call to get | 0 | 19 | 19 |
 | test.cpp:31:7:31:13 | CompareEQ: ... == ... | test.cpp:31:7:31:7 | Load: x | != | test.cpp:31:12:31:13 | Constant: - ... | 0 | 34 | 34 |
@@ -1083,6 +1141,11 @@ irGuardsEnsure_const
 | test.c:175:13:175:32 | CompareEQ: ... == ... | test.c:175:13:175:15 | Call: call to foo | == | 0 | 175 | 175 |
 | test.c:181:9:181:9 | Load: x | test.c:181:9:181:9 | Load: x | != | 0 | 182 | 182 |
 | test.c:181:9:181:9 | Load: x | test.c:181:9:181:9 | Load: x | == | 0 | 184 | 184 |
+| test.c:191:8:191:9 | LogicalNot: ! ... | test.c:191:8:191:9 | LogicalNot: ! ... | != | 0 | 191 | 192 |
+| test.c:191:9:191:9 | Load: b | test.c:191:9:191:9 | Load: b | == | 0 | 191 | 192 |
+| test.c:194:8:194:8 | Load: b | test.c:194:8:194:8 | Load: b | != | 0 | 194 | 194 |
+| test.c:194:8:194:8 | Load: b | test.c:194:8:194:8 | Load: b | != | 0 | 194 | 196 |
+| test.c:194:11:194:15 | CompareLT: ... < ... | test.c:194:11:194:11 | Load: x | < | 100 | 194 | 196 |
 | test.cpp:18:8:18:12 | CompareNE: (bool)... | test.cpp:18:8:18:10 | Call: call to get | != | 0 | 19 | 19 |
 | test.cpp:31:7:31:13 | CompareEQ: ... == ... | test.cpp:31:7:31:7 | Load: x | != | -1 | 34 | 34 |
 | test.cpp:31:7:31:13 | CompareEQ: ... == ... | test.cpp:31:7:31:7 | Load: x | == | -1 | 30 | 30 |
diff --git a/cpp/ql/test/library-tests/controlflow/guards/Guards.expected b/cpp/ql/test/library-tests/controlflow/guards/Guards.expected
index 757356c247cd..4f86180ec7f0 100644
--- a/cpp/ql/test/library-tests/controlflow/guards/Guards.expected
+++ b/cpp/ql/test/library-tests/controlflow/guards/Guards.expected
@@ -32,6 +32,11 @@
 | test.c:164:8:164:8 | s |
 | test.c:170:8:170:9 | ! ... |
 | test.c:170:9:170:9 | s |
+| test.c:178:8:178:9 | ! ... |
+| test.c:178:9:178:9 | b |
+| test.c:181:8:181:8 | b |
+| test.c:181:8:181:15 | ... && ... |
+| test.c:181:11:181:15 | ... < ... |
 | test.cpp:18:8:18:10 | call to get |
 | test.cpp:31:7:31:13 | ... == ... |
 | test.cpp:42:13:42:20 | call to getABool |
diff --git a/cpp/ql/test/library-tests/controlflow/guards/GuardsCompare.expected b/cpp/ql/test/library-tests/controlflow/guards/GuardsCompare.expected
index 8480a1f86138..107f86b902f5 100644
--- a/cpp/ql/test/library-tests/controlflow/guards/GuardsCompare.expected
+++ b/cpp/ql/test/library-tests/controlflow/guards/GuardsCompare.expected
@@ -223,3 +223,20 @@
 | 170 | s != 0 when ! ... is false |
 | 170 | s != 0 when s is true |
 | 170 | s == 0 when s is false |
+| 178 | ! ... != 0 when ! ... is true |
+| 178 | ! ... == 0 when ! ... is false |
+| 178 | b != 0 when ! ... is false |
+| 178 | b != 0 when b is true |
+| 178 | b == 0 when b is false |
+| 181 | 100 < x+1 when ... < ... is false |
+| 181 | 100 >= x+1 when ... && ... is true |
+| 181 | 100 >= x+1 when ... < ... is true |
+| 181 | b != 0 when ... && ... is true |
+| 181 | b != 0 when b is true |
+| 181 | b == 0 when b is false |
+| 181 | x < 100 when ... && ... is true |
+| 181 | x < 100 when ... < ... is true |
+| 181 | x < 100+0 when ... && ... is true |
+| 181 | x < 100+0 when ... < ... is true |
+| 181 | x >= 100 when ... < ... is false |
+| 181 | x >= 100+0 when ... < ... is false |
diff --git a/cpp/ql/test/library-tests/controlflow/guards/GuardsControl.expected b/cpp/ql/test/library-tests/controlflow/guards/GuardsControl.expected
index 83275c8011f9..865df598da21 100644
--- a/cpp/ql/test/library-tests/controlflow/guards/GuardsControl.expected
+++ b/cpp/ql/test/library-tests/controlflow/guards/GuardsControl.expected
@@ -85,6 +85,12 @@
 | test.c:164:8:164:8 | s | true | 164 | 166 |
 | test.c:170:8:170:9 | ! ... | true | 170 | 172 |
 | test.c:170:9:170:9 | s | false | 170 | 172 |
+| test.c:178:8:178:9 | ! ... | true | 178 | 179 |
+| test.c:178:9:178:9 | b | false | 178 | 179 |
+| test.c:181:8:181:8 | b | true | 181 | 181 |
+| test.c:181:8:181:8 | b | true | 181 | 183 |
+| test.c:181:8:181:15 | ... && ... | true | 181 | 183 |
+| test.c:181:11:181:15 | ... < ... | true | 181 | 183 |
 | test.cpp:18:8:18:10 | call to get | true | 19 | 19 |
 | test.cpp:31:7:31:13 | ... == ... | false | 30 | 30 |
 | test.cpp:31:7:31:13 | ... == ... | false | 34 | 34 |
diff --git a/cpp/ql/test/library-tests/controlflow/guards/GuardsEnsure.expected b/cpp/ql/test/library-tests/controlflow/guards/GuardsEnsure.expected
index c520b48f94e4..a9e877ca815a 100644
--- a/cpp/ql/test/library-tests/controlflow/guards/GuardsEnsure.expected
+++ b/cpp/ql/test/library-tests/controlflow/guards/GuardsEnsure.expected
@@ -147,6 +147,10 @@ binary
 | test.c:109:9:109:23 | ... \|\| ... | test.c:109:23:109:23 | 0 | < | test.c:109:19:109:19 | y | 1 | 113 | 113 |
 | test.c:109:19:109:23 | ... < ... | test.c:109:19:109:19 | y | >= | test.c:109:23:109:23 | 0 | 0 | 113 | 113 |
 | test.c:109:19:109:23 | ... < ... | test.c:109:23:109:23 | 0 | < | test.c:109:19:109:19 | y | 1 | 113 | 113 |
+| test.c:181:8:181:15 | ... && ... | test.c:181:11:181:11 | x | < | test.c:181:13:181:15 | 100 | 0 | 181 | 183 |
+| test.c:181:8:181:15 | ... && ... | test.c:181:13:181:15 | 100 | >= | test.c:181:11:181:11 | x | 1 | 181 | 183 |
+| test.c:181:11:181:15 | ... < ... | test.c:181:11:181:11 | x | < | test.c:181:13:181:15 | 100 | 0 | 181 | 183 |
+| test.c:181:11:181:15 | ... < ... | test.c:181:13:181:15 | 100 | >= | test.c:181:11:181:11 | x | 1 | 181 | 183 |
 | test.cpp:31:7:31:13 | ... == ... | test.cpp:31:7:31:7 | x | != | test.cpp:31:12:31:13 | - ... | 0 | 30 | 30 |
 | test.cpp:31:7:31:13 | ... == ... | test.cpp:31:7:31:7 | x | != | test.cpp:31:12:31:13 | - ... | 0 | 34 | 34 |
 | test.cpp:31:7:31:13 | ... == ... | test.cpp:31:7:31:7 | x | == | test.cpp:31:12:31:13 | - ... | 0 | 30 | 30 |
@@ -264,6 +268,13 @@ unary
 | test.c:164:8:164:8 | s | test.c:164:8:164:8 | s | != | 0 | 164 | 166 |
 | test.c:170:8:170:9 | ! ... | test.c:170:8:170:9 | ! ... | != | 0 | 170 | 172 |
 | test.c:170:9:170:9 | s | test.c:170:9:170:9 | s | == | 0 | 170 | 172 |
+| test.c:178:8:178:9 | ! ... | test.c:178:8:178:9 | ! ... | != | 0 | 178 | 179 |
+| test.c:178:9:178:9 | b | test.c:178:9:178:9 | b | == | 0 | 178 | 179 |
+| test.c:181:8:181:8 | b | test.c:181:8:181:8 | b | != | 0 | 181 | 181 |
+| test.c:181:8:181:8 | b | test.c:181:8:181:8 | b | != | 0 | 181 | 183 |
+| test.c:181:8:181:15 | ... && ... | test.c:181:8:181:8 | b | != | 0 | 181 | 183 |
+| test.c:181:8:181:15 | ... && ... | test.c:181:11:181:11 | x | < | 100 | 181 | 183 |
+| test.c:181:11:181:15 | ... < ... | test.c:181:11:181:11 | x | < | 100 | 181 | 183 |
 | test.cpp:18:8:18:10 | call to get | test.cpp:18:8:18:10 | call to get | != | 0 | 19 | 19 |
 | test.cpp:31:7:31:13 | ... == ... | test.cpp:31:7:31:7 | x | != | -1 | 30 | 30 |
 | test.cpp:31:7:31:13 | ... == ... | test.cpp:31:7:31:7 | x | != | -1 | 34 | 34 |
diff --git a/cpp/ql/test/library-tests/controlflow/guards/test.c b/cpp/ql/test/library-tests/controlflow/guards/test.c
index 207e23baa0e3..9fb226bbf59f 100644
--- a/cpp/ql/test/library-tests/controlflow/guards/test.c
+++ b/cpp/ql/test/library-tests/controlflow/guards/test.c
@@ -169,5 +169,16 @@ void test8(short s) {
 void test9(short s) {
     if(!s) {
         
+    }
+}
+
+void boolean_flow(int x){
+    int b = x > 5;
+
+    if(!b){
+    }
+
+    if(b&&x<100){
+        
     }
 }
\ No newline at end of file

From 40a678628451446b6c717aaa6a917ef2e68a7dd5 Mon Sep 17 00:00:00 2001
From: "REDMOND\\brodes" <brodes@microsoft.com>
Date: Mon, 4 Nov 2024 14:23:09 -0500
Subject: [PATCH 02/10] Adding expected false positive conditions for
 MissingCheckScanf once boolean flow logic is completed.

---
 .../Critical/MissingCheckScanf/test.cpp       | 25 +++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/cpp/ql/test/query-tests/Critical/MissingCheckScanf/test.cpp b/cpp/ql/test/query-tests/Critical/MissingCheckScanf/test.cpp
index efc37060a554..9338bb84a5e5 100644
--- a/cpp/ql/test/query-tests/Critical/MissingCheckScanf/test.cpp
+++ b/cpp/ql/test/query-tests/Critical/MissingCheckScanf/test.cpp
@@ -553,3 +553,28 @@ void switch_cases(const char *data) {
 			break;
 	}
 }
+
+
+void test_scanf_compared_right_away() {
+  int i;
+  bool success = scanf("%d", &i) == 1;
+  if(success) {
+    use(i); // GOOD
+  }
+}
+
+void test_scanf_compared_in_conjunct_right(bool b) {
+  int i;
+  bool success = b && scanf("%d", &i) == 1;
+  if(success) {
+    use(i); // GOOD [FALSE POSITIVE]
+  }
+}
+
+void test_scanf_compared_in_conjunct_left(bool b) {
+  int i;
+  bool success = scanf("%d", &i) == 1 && b;
+  if(success) {
+    use(i); // GOOD [FALSE POSITIVE]
+  }
+}
\ No newline at end of file

From bb989ba9b70a95f5f3ac10ab769f2cc9a1325d5f Mon Sep 17 00:00:00 2001
From: "REDMOND\\brodes" <brodes@microsoft.com>
Date: Mon, 4 Nov 2024 14:26:40 -0500
Subject: [PATCH 03/10] Addingg expected file for test changes for
 MissingCheckScanf before changes made to guards (baseline expected file).

---
 .../MissingCheckScanf/MissingCheckScanf.expected     | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/cpp/ql/test/query-tests/Critical/MissingCheckScanf/MissingCheckScanf.expected b/cpp/ql/test/query-tests/Critical/MissingCheckScanf/MissingCheckScanf.expected
index dac8afd3fd31..2b1dcccfb888 100644
--- a/cpp/ql/test/query-tests/Critical/MissingCheckScanf/MissingCheckScanf.expected
+++ b/cpp/ql/test/query-tests/Critical/MissingCheckScanf/MissingCheckScanf.expected
@@ -52,6 +52,9 @@ edges
 | test.cpp:541:39:541:40 | sscanf output argument | test.cpp:549:8:549:8 | e | provenance |  |
 | test.cpp:541:43:541:44 | sscanf output argument | test.cpp:545:8:545:8 | f | provenance |  |
 | test.cpp:541:43:541:44 | sscanf output argument | test.cpp:550:8:550:8 | f | provenance |  |
+| test.cpp:560:30:560:31 | scanf output argument | test.cpp:562:9:562:9 | i | provenance |  |
+| test.cpp:568:35:568:36 | scanf output argument | test.cpp:570:9:570:9 | i | provenance |  |
+| test.cpp:576:30:576:31 | scanf output argument | test.cpp:578:9:578:9 | i | provenance |  |
 nodes
 | test.cpp:34:15:34:16 | scanf output argument | semmle.label | scanf output argument |
 | test.cpp:35:7:35:7 | i | semmle.label | i |
@@ -154,6 +157,12 @@ nodes
 | test.cpp:548:8:548:8 | d | semmle.label | d |
 | test.cpp:549:8:549:8 | e | semmle.label | e |
 | test.cpp:550:8:550:8 | f | semmle.label | f |
+| test.cpp:560:30:560:31 | scanf output argument | semmle.label | scanf output argument |
+| test.cpp:562:9:562:9 | i | semmle.label | i |
+| test.cpp:568:35:568:36 | scanf output argument | semmle.label | scanf output argument |
+| test.cpp:570:9:570:9 | i | semmle.label | i |
+| test.cpp:576:30:576:31 | scanf output argument | semmle.label | scanf output argument |
+| test.cpp:578:9:578:9 | i | semmle.label | i |
 subpaths
 #select
 | test.cpp:35:7:35:7 | i | test.cpp:34:15:34:16 | scanf output argument | test.cpp:35:7:35:7 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:34:3:34:7 | call to scanf | call to scanf |
@@ -177,3 +186,6 @@ subpaths
 | test.cpp:484:9:484:9 | i | test.cpp:480:25:480:26 | scanf output argument | test.cpp:484:9:484:9 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:480:13:480:17 | call to scanf | call to scanf |
 | test.cpp:495:8:495:8 | i | test.cpp:491:25:491:26 | scanf output argument | test.cpp:495:8:495:8 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:491:13:491:17 | call to scanf | call to scanf |
 | test.cpp:545:8:545:8 | f | test.cpp:541:43:541:44 | sscanf output argument | test.cpp:545:8:545:8 | f | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 3. | test.cpp:541:10:541:15 | call to sscanf | call to sscanf |
+| test.cpp:562:9:562:9 | i | test.cpp:560:30:560:31 | scanf output argument | test.cpp:562:9:562:9 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:560:18:560:22 | call to scanf | call to scanf |
+| test.cpp:570:9:570:9 | i | test.cpp:568:35:568:36 | scanf output argument | test.cpp:570:9:570:9 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:568:23:568:27 | call to scanf | call to scanf |
+| test.cpp:578:9:578:9 | i | test.cpp:576:30:576:31 | scanf output argument | test.cpp:578:9:578:9 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:576:18:576:22 | call to scanf | call to scanf |

From ddfbb0813023302a4ddf0346749525694881aaa6 Mon Sep 17 00:00:00 2001
From: "REDMOND\\brodes" <brodes@microsoft.com>
Date: Mon, 4 Nov 2024 14:28:07 -0500
Subject: [PATCH 04/10] IRGuard overhaul to parse conditions using GVN.

---
 .../semmle/code/cpp/controlflow/IRGuards.qll  | 59 +++++++++++++------
 1 file changed, 40 insertions(+), 19 deletions(-)

diff --git a/cpp/ql/lib/semmle/code/cpp/controlflow/IRGuards.qll b/cpp/ql/lib/semmle/code/cpp/controlflow/IRGuards.qll
index 9b4d28430ff3..410ce4159da2 100644
--- a/cpp/ql/lib/semmle/code/cpp/controlflow/IRGuards.qll
+++ b/cpp/ql/lib/semmle/code/cpp/controlflow/IRGuards.qll
@@ -5,9 +5,21 @@
 
 import cpp
 import semmle.code.cpp.ir.IR
+private import semmle.code.cpp.ir.ValueNumbering
 private import semmle.code.cpp.ir.implementation.raw.internal.TranslatedExpr
 private import semmle.code.cpp.ir.implementation.raw.internal.InstructionTag
 
+/**
+ * Returns `instr` or any instruction used to define `instr`.
+ */
+private Instruction getDerivedInstruction(Instruction instr) {
+  result = valueNumber(instr).getAnInstruction() and
+  result.toString() != instr.toString() and
+  result instanceof CompareInstruction
+  or
+  result = instr
+}
+
 /**
  * Holds if `block` consists of an `UnreachedInstruction`.
  *
@@ -517,7 +529,7 @@ class IRGuardCondition extends Instruction {
   cached
   predicate comparesLt(Operand left, Operand right, int k, boolean isLessThan, boolean testIsTrue) {
     exists(BooleanValue value |
-      compares_lt(this, left, right, k, isLessThan, value) and
+      compares_lt(getDerivedInstruction(this), left, right, k, isLessThan, value) and
       value.getValue() = testIsTrue
     )
   }
@@ -528,7 +540,7 @@ class IRGuardCondition extends Instruction {
    */
   cached
   predicate comparesLt(Operand op, int k, boolean isLessThan, AbstractValue value) {
-    compares_lt(this, op, k, isLessThan, value)
+    compares_lt(getDerivedInstruction(this), op, k, isLessThan, value)
   }
 
   /**
@@ -538,7 +550,8 @@ class IRGuardCondition extends Instruction {
   cached
   predicate ensuresLt(Operand left, Operand right, int k, IRBlock block, boolean isLessThan) {
     exists(AbstractValue value |
-      compares_lt(this, left, right, k, isLessThan, value) and this.valueControls(block, value)
+      compares_lt(getDerivedInstruction(this), left, right, k, isLessThan, value) and
+      this.valueControls(block, value)
     )
   }
 
@@ -549,7 +562,8 @@ class IRGuardCondition extends Instruction {
   cached
   predicate ensuresLt(Operand op, int k, IRBlock block, boolean isLessThan) {
     exists(AbstractValue value |
-      compares_lt(this, op, k, isLessThan, value) and this.valueControls(block, value)
+      compares_lt(getDerivedInstruction(this), op, k, isLessThan, value) and
+      this.valueControls(block, value)
     )
   }
 
@@ -562,7 +576,7 @@ class IRGuardCondition extends Instruction {
     Operand left, Operand right, int k, IRBlock pred, IRBlock succ, boolean isLessThan
   ) {
     exists(AbstractValue value |
-      compares_lt(this, left, right, k, isLessThan, value) and
+      compares_lt(getDerivedInstruction(this), left, right, k, isLessThan, value) and
       this.valueControlsEdge(pred, succ, value)
     )
   }
@@ -574,7 +588,7 @@ class IRGuardCondition extends Instruction {
   cached
   predicate ensuresLtEdge(Operand left, int k, IRBlock pred, IRBlock succ, boolean isLessThan) {
     exists(AbstractValue value |
-      compares_lt(this, left, k, isLessThan, value) and
+      compares_lt(getDerivedInstruction(this), left, k, isLessThan, value) and
       this.valueControlsEdge(pred, succ, value)
     )
   }
@@ -583,7 +597,7 @@ class IRGuardCondition extends Instruction {
   cached
   predicate comparesEq(Operand left, Operand right, int k, boolean areEqual, boolean testIsTrue) {
     exists(BooleanValue value |
-      compares_eq(this, left, right, k, areEqual, value) and
+      compares_eq(getDerivedInstruction(this), left, right, k, areEqual, value) and
       value.getValue() = testIsTrue
     )
   }
@@ -591,7 +605,7 @@ class IRGuardCondition extends Instruction {
   /** Holds if (determined by this guard) `op == k` evaluates to `areEqual` if this expression evaluates to `value`. */
   cached
   predicate comparesEq(Operand op, int k, boolean areEqual, AbstractValue value) {
-    unary_compares_eq(this, op, k, areEqual, false, value)
+    unary_compares_eq(getDerivedInstruction(this), op, k, areEqual, false, value)
   }
 
   /**
@@ -601,7 +615,8 @@ class IRGuardCondition extends Instruction {
   cached
   predicate ensuresEq(Operand left, Operand right, int k, IRBlock block, boolean areEqual) {
     exists(AbstractValue value |
-      compares_eq(this, left, right, k, areEqual, value) and this.valueControls(block, value)
+      compares_eq(getDerivedInstruction(this), left, right, k, areEqual, value) and
+      this.valueControls(block, value)
     )
   }
 
@@ -612,7 +627,8 @@ class IRGuardCondition extends Instruction {
   cached
   predicate ensuresEq(Operand op, int k, IRBlock block, boolean areEqual) {
     exists(AbstractValue value |
-      unary_compares_eq(this, op, k, areEqual, false, value) and this.valueControls(block, value)
+      unary_compares_eq(getDerivedInstruction(this), op, k, areEqual, false, value) and
+      this.valueControls(block, value)
     )
   }
 
@@ -625,7 +641,7 @@ class IRGuardCondition extends Instruction {
     Operand left, Operand right, int k, IRBlock pred, IRBlock succ, boolean areEqual
   ) {
     exists(AbstractValue value |
-      compares_eq(this, left, right, k, areEqual, value) and
+      compares_eq(getDerivedInstruction(this), left, right, k, areEqual, value) and
       this.valueControlsEdge(pred, succ, value)
     )
   }
@@ -637,7 +653,7 @@ class IRGuardCondition extends Instruction {
   cached
   predicate ensuresEqEdge(Operand op, int k, IRBlock pred, IRBlock succ, boolean areEqual) {
     exists(AbstractValue value |
-      unary_compares_eq(this, op, k, areEqual, false, value) and
+      unary_compares_eq(getDerivedInstruction(this), op, k, areEqual, false, value) and
       this.valueControlsEdge(pred, succ, value)
     )
   }
@@ -759,10 +775,12 @@ private predicate compares_eq(
   or
   /* (x is true => (left == right + k)) => (!x is false => (left == right + k)) */
   exists(AbstractValue dual | value = dual.getDualValue() |
-    compares_eq(test.(LogicalNotInstruction).getUnary(), left, right, k, areEqual, dual)
+    compares_eq(getDerivedInstruction(test.(LogicalNotInstruction).getUnary()), left, right, k,
+      areEqual, dual)
   )
   or
-  compares_eq(test.(BuiltinExpectCallInstruction).getCondition(), left, right, k, areEqual, value)
+  compares_eq(getDerivedInstruction(test.(BuiltinExpectCallInstruction).getCondition()), left,
+    right, k, areEqual, value)
 }
 
 /**
@@ -817,7 +835,8 @@ private predicate unary_compares_eq(
   /* (x is true => (op == k)) => (!x is false => (op == k)) */
   exists(AbstractValue dual, boolean inNonZeroCase0 |
     value = dual.getDualValue() and
-    unary_compares_eq(test.(LogicalNotInstruction).getUnary(), op, k, inNonZeroCase0, areEqual, dual)
+    unary_compares_eq(getDerivedInstruction(test.(LogicalNotInstruction).getUnary()), op, k,
+      inNonZeroCase0, areEqual, dual)
   |
     k = 0 and inNonZeroCase = inNonZeroCase0
     or
@@ -937,7 +956,7 @@ private predicate builtin_expect_eq(
   exists(BuiltinExpectCallInstruction call, Instruction const, AbstractValue innerValue |
     int_value(const) = 0 and
     cmp.hasOperands(call.getAUse(), const.getAUse()) and
-    compares_eq(call.getCondition(), left, right, k, areEqual, innerValue)
+    compares_eq(getDerivedInstruction(call.getCondition()), left, right, k, areEqual, innerValue)
   |
     cmp instanceof CompareNEInstruction and
     value = innerValue
@@ -968,7 +987,8 @@ private predicate unary_builtin_expect_eq(
   exists(BuiltinExpectCallInstruction call, Instruction const, AbstractValue innerValue |
     int_value(const) = 0 and
     cmp.hasOperands(call.getAUse(), const.getAUse()) and
-    unary_compares_eq(call.getCondition(), op, k, areEqual, inNonZeroCase, innerValue)
+    unary_compares_eq(getDerivedInstruction(call.getCondition()), op, k, areEqual, inNonZeroCase,
+      innerValue)
   |
     cmp instanceof CompareNEInstruction and
     value = innerValue
@@ -1008,7 +1028,8 @@ private predicate compares_lt(
   or
   /* (x is true => (left < right + k)) => (!x is false => (left < right + k)) */
   exists(AbstractValue dual | value = dual.getDualValue() |
-    compares_lt(test.(LogicalNotInstruction).getUnary(), left, right, k, isLt, dual)
+    compares_lt(getDerivedInstruction(test.(LogicalNotInstruction).getUnary()), left, right, k,
+      isLt, dual)
   )
 }
 
@@ -1021,7 +1042,7 @@ private predicate compares_lt(Instruction test, Operand op, int k, boolean isLt,
   or
   /* (x is true => (op < k)) => (!x is false => (op < k)) */
   exists(AbstractValue dual | value = dual.getDualValue() |
-    compares_lt(test.(LogicalNotInstruction).getUnary(), op, k, isLt, dual)
+    compares_lt(getDerivedInstruction(test.(LogicalNotInstruction).getUnary()), op, k, isLt, dual)
   )
   or
   exists(int k1, int k2, ConstantInstruction const |

From 0795bccead947ff644680a415a8733f234b81b51 Mon Sep 17 00:00:00 2001
From: "REDMOND\\brodes" <brodes@microsoft.com>
Date: Mon, 4 Nov 2024 14:32:52 -0500
Subject: [PATCH 05/10] Updating expected files for new IRGuard changes.

---
 .../controlflow/guards-ir/tests.expected      | 54 +++++++++++++++++++
 .../controlflow/guards/GuardsCompare.expected | 21 ++++++++
 .../controlflow/guards/GuardsEnsure.expected  | 15 ++++++
 .../MissingCheckScanf.expected                |  1 -
 4 files changed, 90 insertions(+), 1 deletion(-)

diff --git a/cpp/ql/test/library-tests/controlflow/guards-ir/tests.expected b/cpp/ql/test/library-tests/controlflow/guards-ir/tests.expected
index c344eef808a4..51ceef81009c 100644
--- a/cpp/ql/test/library-tests/controlflow/guards-ir/tests.expected
+++ b/cpp/ql/test/library-tests/controlflow/guards-ir/tests.expected
@@ -216,19 +216,33 @@ astGuardsCompare
 | 175 | call to foo == 0+0 when ... == ... is true |
 | 181 | x != 0 when x is true |
 | 181 | x == 0 when x is false |
+| 191 | 5 < x+0 when ! ... is false |
+| 191 | 5 < x+0 when b is true |
+| 191 | 5 >= x+0 when ! ... is true |
+| 191 | 5 >= x+0 when b is false |
 | 191 | ! ... != 0 when ! ... is true |
 | 191 | ! ... == 0 when ! ... is false |
 | 191 | b != 0 when ! ... is false |
 | 191 | b != 0 when b is true |
 | 191 | b == 0 when b is false |
+| 191 | x < 5+1 when ! ... is true |
+| 191 | x < 5+1 when b is false |
+| 191 | x >= 5+1 when ! ... is false |
+| 191 | x >= 5+1 when b is true |
+| 194 | 5 < x+0 when ... && ... is true |
+| 194 | 5 < x+0 when b is true |
+| 194 | 5 >= x+0 when b is false |
 | 194 | 100 < x+1 when ... < ... is false |
 | 194 | 100 >= x+1 when ... && ... is true |
 | 194 | 100 >= x+1 when ... < ... is true |
 | 194 | b != 0 when ... && ... is true |
 | 194 | b != 0 when b is true |
 | 194 | b == 0 when b is false |
+| 194 | x < 5+1 when b is false |
 | 194 | x < 100+0 when ... && ... is true |
 | 194 | x < 100+0 when ... < ... is true |
+| 194 | x >= 5+1 when ... && ... is true |
+| 194 | x >= 5+1 when b is true |
 | 194 | x >= 100+0 when ... < ... is false |
 astGuardsControl
 | test.c:7:9:7:13 | ... > ... | false | 10 | 11 |
@@ -507,6 +521,16 @@ astGuardsEnsure
 | test.c:175:13:175:32 | ... == ... | test.c:175:13:175:15 | call to foo | == | test.c:175:32:175:32 | 0 | 0 | 175 | 175 |
 | test.c:175:13:175:32 | ... == ... | test.c:175:32:175:32 | 0 | != | test.c:175:13:175:15 | call to foo | 0 | 175 | 175 |
 | test.c:175:13:175:32 | ... == ... | test.c:175:32:175:32 | 0 | == | test.c:175:13:175:15 | call to foo | 0 | 175 | 175 |
+| test.c:191:8:191:9 | ! ... | test.c:189:13:189:13 | x | < | test.c:189:17:189:17 | 5 | 1 | 191 | 192 |
+| test.c:191:8:191:9 | ! ... | test.c:189:17:189:17 | 5 | >= | test.c:189:13:189:13 | x | 0 | 191 | 192 |
+| test.c:191:9:191:9 | b | test.c:189:13:189:13 | x | < | test.c:189:17:189:17 | 5 | 1 | 191 | 192 |
+| test.c:191:9:191:9 | b | test.c:189:17:189:17 | 5 | >= | test.c:189:13:189:13 | x | 0 | 191 | 192 |
+| test.c:194:8:194:8 | b | test.c:189:13:189:13 | x | >= | test.c:189:17:189:17 | 5 | 1 | 194 | 194 |
+| test.c:194:8:194:8 | b | test.c:189:13:189:13 | x | >= | test.c:189:17:189:17 | 5 | 1 | 194 | 196 |
+| test.c:194:8:194:8 | b | test.c:189:17:189:17 | 5 | < | test.c:189:13:189:13 | x | 0 | 194 | 194 |
+| test.c:194:8:194:8 | b | test.c:189:17:189:17 | 5 | < | test.c:189:13:189:13 | x | 0 | 194 | 196 |
+| test.c:194:8:194:15 | ... && ... | test.c:189:13:189:13 | x | >= | test.c:189:17:189:17 | 5 | 1 | 194 | 196 |
+| test.c:194:8:194:15 | ... && ... | test.c:189:17:189:17 | 5 | < | test.c:189:13:189:13 | x | 0 | 194 | 196 |
 | test.c:194:8:194:15 | ... && ... | test.c:194:11:194:11 | x | < | test.c:194:13:194:15 | 100 | 0 | 194 | 196 |
 | test.c:194:8:194:15 | ... && ... | test.c:194:13:194:15 | 100 | >= | test.c:194:11:194:11 | x | 1 | 194 | 196 |
 | test.c:194:11:194:15 | ... < ... | test.c:194:11:194:11 | x | < | test.c:194:13:194:15 | 100 | 0 | 194 | 196 |
@@ -784,17 +808,35 @@ irGuardsCompare
 | 175 | call to foo == 0+0 when CompareEQ: ... == ... is true |
 | 181 | x != 0 when Load: x is true |
 | 181 | x == 0 when Load: x is false |
+| 191 | 5 < x+0 when Load: b is true |
+| 191 | 5 < x+0 when LogicalNot: ! ... is false |
+| 191 | 5 >= x+0 when Load: b is false |
+| 191 | 5 >= x+0 when LogicalNot: ! ... is true |
 | 191 | ! ... != 0 when LogicalNot: ! ... is true |
 | 191 | ! ... == 0 when LogicalNot: ! ... is false |
 | 191 | b != 0 when Load: b is true |
 | 191 | b != 0 when LogicalNot: ! ... is false |
 | 191 | b == 0 when Load: b is false |
+| 191 | x < 5+1 when Load: b is false |
+| 191 | x < 5+1 when LogicalNot: ! ... is true |
+| 191 | x < 6 when Load: b is false |
+| 191 | x < 6 when LogicalNot: ! ... is true |
+| 191 | x >= 5+1 when Load: b is true |
+| 191 | x >= 5+1 when LogicalNot: ! ... is false |
+| 191 | x >= 6 when Load: b is true |
+| 191 | x >= 6 when LogicalNot: ! ... is false |
+| 194 | 5 < x+0 when Load: b is true |
+| 194 | 5 >= x+0 when Load: b is false |
 | 194 | 100 < x+1 when CompareLT: ... < ... is false |
 | 194 | 100 >= x+1 when CompareLT: ... < ... is true |
 | 194 | b != 0 when Load: b is true |
 | 194 | b == 0 when Load: b is false |
+| 194 | x < 5+1 when Load: b is false |
+| 194 | x < 6 when Load: b is false |
 | 194 | x < 100 when CompareLT: ... < ... is true |
 | 194 | x < 100+0 when CompareLT: ... < ... is true |
+| 194 | x >= 5+1 when Load: b is true |
+| 194 | x >= 6 when Load: b is true |
 | 194 | x >= 100 when CompareLT: ... < ... is false |
 | 194 | x >= 100+0 when CompareLT: ... < ... is false |
 irGuardsControl
@@ -1048,6 +1090,14 @@ irGuardsEnsure
 | test.c:175:13:175:32 | CompareEQ: ... == ... | test.c:175:13:175:15 | Call: call to foo | == | test.c:175:32:175:32 | Constant: 0 | 0 | 175 | 175 |
 | test.c:175:13:175:32 | CompareEQ: ... == ... | test.c:175:32:175:32 | Constant: 0 | != | test.c:175:13:175:15 | Call: call to foo | 0 | 175 | 175 |
 | test.c:175:13:175:32 | CompareEQ: ... == ... | test.c:175:32:175:32 | Constant: 0 | == | test.c:175:13:175:15 | Call: call to foo | 0 | 175 | 175 |
+| test.c:191:8:191:9 | LogicalNot: ! ... | test.c:189:13:189:13 | Load: x | < | test.c:189:17:189:17 | Constant: 5 | 1 | 191 | 192 |
+| test.c:191:8:191:9 | LogicalNot: ! ... | test.c:189:17:189:17 | Constant: 5 | >= | test.c:189:13:189:13 | Load: x | 0 | 191 | 192 |
+| test.c:191:9:191:9 | Load: b | test.c:189:13:189:13 | Load: x | < | test.c:189:17:189:17 | Constant: 5 | 1 | 191 | 192 |
+| test.c:191:9:191:9 | Load: b | test.c:189:17:189:17 | Constant: 5 | >= | test.c:189:13:189:13 | Load: x | 0 | 191 | 192 |
+| test.c:194:8:194:8 | Load: b | test.c:189:13:189:13 | Load: x | >= | test.c:189:17:189:17 | Constant: 5 | 1 | 194 | 194 |
+| test.c:194:8:194:8 | Load: b | test.c:189:13:189:13 | Load: x | >= | test.c:189:17:189:17 | Constant: 5 | 1 | 194 | 196 |
+| test.c:194:8:194:8 | Load: b | test.c:189:17:189:17 | Constant: 5 | < | test.c:189:13:189:13 | Load: x | 0 | 194 | 194 |
+| test.c:194:8:194:8 | Load: b | test.c:189:17:189:17 | Constant: 5 | < | test.c:189:13:189:13 | Load: x | 0 | 194 | 196 |
 | test.c:194:11:194:15 | CompareLT: ... < ... | test.c:194:11:194:11 | Load: x | < | test.c:194:13:194:15 | Constant: 100 | 0 | 194 | 196 |
 | test.c:194:11:194:15 | CompareLT: ... < ... | test.c:194:13:194:15 | Constant: 100 | >= | test.c:194:11:194:11 | Load: x | 1 | 194 | 196 |
 | test.cpp:18:8:18:12 | CompareNE: (bool)... | test.cpp:18:8:18:10 | Call: call to get | != | test.cpp:18:8:18:12 | Constant: (bool)... | 0 | 19 | 19 |
@@ -1141,8 +1191,12 @@ irGuardsEnsure_const
 | test.c:175:13:175:32 | CompareEQ: ... == ... | test.c:175:13:175:15 | Call: call to foo | == | 0 | 175 | 175 |
 | test.c:181:9:181:9 | Load: x | test.c:181:9:181:9 | Load: x | != | 0 | 182 | 182 |
 | test.c:181:9:181:9 | Load: x | test.c:181:9:181:9 | Load: x | == | 0 | 184 | 184 |
+| test.c:191:8:191:9 | LogicalNot: ! ... | test.c:189:13:189:13 | Load: x | < | 6 | 191 | 192 |
 | test.c:191:8:191:9 | LogicalNot: ! ... | test.c:191:8:191:9 | LogicalNot: ! ... | != | 0 | 191 | 192 |
+| test.c:191:9:191:9 | Load: b | test.c:189:13:189:13 | Load: x | < | 6 | 191 | 192 |
 | test.c:191:9:191:9 | Load: b | test.c:191:9:191:9 | Load: b | == | 0 | 191 | 192 |
+| test.c:194:8:194:8 | Load: b | test.c:189:13:189:13 | Load: x | >= | 6 | 194 | 194 |
+| test.c:194:8:194:8 | Load: b | test.c:189:13:189:13 | Load: x | >= | 6 | 194 | 196 |
 | test.c:194:8:194:8 | Load: b | test.c:194:8:194:8 | Load: b | != | 0 | 194 | 194 |
 | test.c:194:8:194:8 | Load: b | test.c:194:8:194:8 | Load: b | != | 0 | 194 | 196 |
 | test.c:194:11:194:15 | CompareLT: ... < ... | test.c:194:11:194:11 | Load: x | < | 100 | 194 | 196 |
diff --git a/cpp/ql/test/library-tests/controlflow/guards/GuardsCompare.expected b/cpp/ql/test/library-tests/controlflow/guards/GuardsCompare.expected
index 107f86b902f5..ae2b4a10e680 100644
--- a/cpp/ql/test/library-tests/controlflow/guards/GuardsCompare.expected
+++ b/cpp/ql/test/library-tests/controlflow/guards/GuardsCompare.expected
@@ -223,20 +223,41 @@
 | 170 | s != 0 when ! ... is false |
 | 170 | s != 0 when s is true |
 | 170 | s == 0 when s is false |
+| 178 | 5 < x+0 when ! ... is false |
+| 178 | 5 < x+0 when b is true |
+| 178 | 5 >= x+0 when ! ... is true |
+| 178 | 5 >= x+0 when b is false |
 | 178 | ! ... != 0 when ! ... is true |
 | 178 | ! ... == 0 when ! ... is false |
 | 178 | b != 0 when ! ... is false |
 | 178 | b != 0 when b is true |
 | 178 | b == 0 when b is false |
+| 178 | x < 5+1 when ! ... is true |
+| 178 | x < 5+1 when b is false |
+| 178 | x < 6 when ! ... is true |
+| 178 | x < 6 when b is false |
+| 178 | x >= 5+1 when ! ... is false |
+| 178 | x >= 5+1 when b is true |
+| 178 | x >= 6 when ! ... is false |
+| 178 | x >= 6 when b is true |
+| 181 | 5 < x+0 when ... && ... is true |
+| 181 | 5 < x+0 when b is true |
+| 181 | 5 >= x+0 when b is false |
 | 181 | 100 < x+1 when ... < ... is false |
 | 181 | 100 >= x+1 when ... && ... is true |
 | 181 | 100 >= x+1 when ... < ... is true |
 | 181 | b != 0 when ... && ... is true |
 | 181 | b != 0 when b is true |
 | 181 | b == 0 when b is false |
+| 181 | x < 5+1 when b is false |
+| 181 | x < 6 when b is false |
 | 181 | x < 100 when ... && ... is true |
 | 181 | x < 100 when ... < ... is true |
 | 181 | x < 100+0 when ... && ... is true |
 | 181 | x < 100+0 when ... < ... is true |
+| 181 | x >= 5+1 when ... && ... is true |
+| 181 | x >= 5+1 when b is true |
+| 181 | x >= 6 when ... && ... is true |
+| 181 | x >= 6 when b is true |
 | 181 | x >= 100 when ... < ... is false |
 | 181 | x >= 100+0 when ... < ... is false |
diff --git a/cpp/ql/test/library-tests/controlflow/guards/GuardsEnsure.expected b/cpp/ql/test/library-tests/controlflow/guards/GuardsEnsure.expected
index a9e877ca815a..3c2af3b88bff 100644
--- a/cpp/ql/test/library-tests/controlflow/guards/GuardsEnsure.expected
+++ b/cpp/ql/test/library-tests/controlflow/guards/GuardsEnsure.expected
@@ -147,6 +147,16 @@ binary
 | test.c:109:9:109:23 | ... \|\| ... | test.c:109:23:109:23 | 0 | < | test.c:109:19:109:19 | y | 1 | 113 | 113 |
 | test.c:109:19:109:23 | ... < ... | test.c:109:19:109:19 | y | >= | test.c:109:23:109:23 | 0 | 0 | 113 | 113 |
 | test.c:109:19:109:23 | ... < ... | test.c:109:23:109:23 | 0 | < | test.c:109:19:109:19 | y | 1 | 113 | 113 |
+| test.c:178:8:178:9 | ! ... | test.c:176:13:176:13 | x | < | test.c:176:17:176:17 | 5 | 1 | 178 | 179 |
+| test.c:178:8:178:9 | ! ... | test.c:176:17:176:17 | 5 | >= | test.c:176:13:176:13 | x | 0 | 178 | 179 |
+| test.c:178:9:178:9 | b | test.c:176:13:176:13 | x | < | test.c:176:17:176:17 | 5 | 1 | 178 | 179 |
+| test.c:178:9:178:9 | b | test.c:176:17:176:17 | 5 | >= | test.c:176:13:176:13 | x | 0 | 178 | 179 |
+| test.c:181:8:181:8 | b | test.c:176:13:176:13 | x | >= | test.c:176:17:176:17 | 5 | 1 | 181 | 181 |
+| test.c:181:8:181:8 | b | test.c:176:13:176:13 | x | >= | test.c:176:17:176:17 | 5 | 1 | 181 | 183 |
+| test.c:181:8:181:8 | b | test.c:176:17:176:17 | 5 | < | test.c:176:13:176:13 | x | 0 | 181 | 181 |
+| test.c:181:8:181:8 | b | test.c:176:17:176:17 | 5 | < | test.c:176:13:176:13 | x | 0 | 181 | 183 |
+| test.c:181:8:181:15 | ... && ... | test.c:176:13:176:13 | x | >= | test.c:176:17:176:17 | 5 | 1 | 181 | 183 |
+| test.c:181:8:181:15 | ... && ... | test.c:176:17:176:17 | 5 | < | test.c:176:13:176:13 | x | 0 | 181 | 183 |
 | test.c:181:8:181:15 | ... && ... | test.c:181:11:181:11 | x | < | test.c:181:13:181:15 | 100 | 0 | 181 | 183 |
 | test.c:181:8:181:15 | ... && ... | test.c:181:13:181:15 | 100 | >= | test.c:181:11:181:11 | x | 1 | 181 | 183 |
 | test.c:181:11:181:15 | ... < ... | test.c:181:11:181:11 | x | < | test.c:181:13:181:15 | 100 | 0 | 181 | 183 |
@@ -268,10 +278,15 @@ unary
 | test.c:164:8:164:8 | s | test.c:164:8:164:8 | s | != | 0 | 164 | 166 |
 | test.c:170:8:170:9 | ! ... | test.c:170:8:170:9 | ! ... | != | 0 | 170 | 172 |
 | test.c:170:9:170:9 | s | test.c:170:9:170:9 | s | == | 0 | 170 | 172 |
+| test.c:178:8:178:9 | ! ... | test.c:176:13:176:13 | x | < | 6 | 178 | 179 |
 | test.c:178:8:178:9 | ! ... | test.c:178:8:178:9 | ! ... | != | 0 | 178 | 179 |
+| test.c:178:9:178:9 | b | test.c:176:13:176:13 | x | < | 6 | 178 | 179 |
 | test.c:178:9:178:9 | b | test.c:178:9:178:9 | b | == | 0 | 178 | 179 |
+| test.c:181:8:181:8 | b | test.c:176:13:176:13 | x | >= | 6 | 181 | 181 |
+| test.c:181:8:181:8 | b | test.c:176:13:176:13 | x | >= | 6 | 181 | 183 |
 | test.c:181:8:181:8 | b | test.c:181:8:181:8 | b | != | 0 | 181 | 181 |
 | test.c:181:8:181:8 | b | test.c:181:8:181:8 | b | != | 0 | 181 | 183 |
+| test.c:181:8:181:15 | ... && ... | test.c:176:13:176:13 | x | >= | 6 | 181 | 183 |
 | test.c:181:8:181:15 | ... && ... | test.c:181:8:181:8 | b | != | 0 | 181 | 183 |
 | test.c:181:8:181:15 | ... && ... | test.c:181:11:181:11 | x | < | 100 | 181 | 183 |
 | test.c:181:11:181:15 | ... < ... | test.c:181:11:181:11 | x | < | 100 | 181 | 183 |
diff --git a/cpp/ql/test/query-tests/Critical/MissingCheckScanf/MissingCheckScanf.expected b/cpp/ql/test/query-tests/Critical/MissingCheckScanf/MissingCheckScanf.expected
index 2b1dcccfb888..09a24003d729 100644
--- a/cpp/ql/test/query-tests/Critical/MissingCheckScanf/MissingCheckScanf.expected
+++ b/cpp/ql/test/query-tests/Critical/MissingCheckScanf/MissingCheckScanf.expected
@@ -186,6 +186,5 @@ subpaths
 | test.cpp:484:9:484:9 | i | test.cpp:480:25:480:26 | scanf output argument | test.cpp:484:9:484:9 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:480:13:480:17 | call to scanf | call to scanf |
 | test.cpp:495:8:495:8 | i | test.cpp:491:25:491:26 | scanf output argument | test.cpp:495:8:495:8 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:491:13:491:17 | call to scanf | call to scanf |
 | test.cpp:545:8:545:8 | f | test.cpp:541:43:541:44 | sscanf output argument | test.cpp:545:8:545:8 | f | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 3. | test.cpp:541:10:541:15 | call to sscanf | call to sscanf |
-| test.cpp:562:9:562:9 | i | test.cpp:560:30:560:31 | scanf output argument | test.cpp:562:9:562:9 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:560:18:560:22 | call to scanf | call to scanf |
 | test.cpp:570:9:570:9 | i | test.cpp:568:35:568:36 | scanf output argument | test.cpp:570:9:570:9 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:568:23:568:27 | call to scanf | call to scanf |
 | test.cpp:578:9:578:9 | i | test.cpp:576:30:576:31 | scanf output argument | test.cpp:578:9:578:9 | i | This variable is read, but may not have been written. It should be guarded by a check that the $@ returns at least 1. | test.cpp:576:18:576:22 | call to scanf | call to scanf |

From d17dee53b5d65c0c5b9a16d8ff3b69161905b4f8 Mon Sep 17 00:00:00 2001
From: "REDMOND\\brodes" <brodes@microsoft.com>
Date: Mon, 4 Nov 2024 15:28:02 -0500
Subject: [PATCH 06/10] Updating IRGuards.qll getDerivedInstruction. Always get
 the deriving instruction if one exists.

---
 .../semmle/code/cpp/controlflow/IRGuards.qll   | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/cpp/ql/lib/semmle/code/cpp/controlflow/IRGuards.qll b/cpp/ql/lib/semmle/code/cpp/controlflow/IRGuards.qll
index 410ce4159da2..665c61a71ddb 100644
--- a/cpp/ql/lib/semmle/code/cpp/controlflow/IRGuards.qll
+++ b/cpp/ql/lib/semmle/code/cpp/controlflow/IRGuards.qll
@@ -13,11 +13,19 @@ private import semmle.code.cpp.ir.implementation.raw.internal.InstructionTag
  * Returns `instr` or any instruction used to define `instr`.
  */
 private Instruction getDerivedInstruction(Instruction instr) {
-  result = valueNumber(instr).getAnInstruction() and
-  result.toString() != instr.toString() and
-  result instanceof CompareInstruction
-  or
-  result = instr
+  // If there is a defining value, use it, else derived instruction is `instr`
+  // This forces backwards examination of guards if the current instruction is 
+  // derived from a prior instruction. 
+  if
+    exists(Instruction derived |
+      derived = valueNumber(instr).getAnInstruction() and
+      derived.toString() != instr.toString()
+    )
+  then
+    result = valueNumber(instr).getAnInstruction() and
+    result.toString() != instr.toString() and
+    result instanceof CompareInstruction
+  else result = instr
 }
 
 /**

From 6f17460e53583a95373587d01dcbdf776b2f0a59 Mon Sep 17 00:00:00 2001
From: "REDMOND\\brodes" <brodes@microsoft.com>
Date: Mon, 4 Nov 2024 15:30:25 -0500
Subject: [PATCH 07/10] Revert "Updating IRGuards.qll getDerivedInstruction.
 Always get the deriving instruction if one exists."

This reverts commit d17dee53b5d65c0c5b9a16d8ff3b69161905b4f8.
---
 .../semmle/code/cpp/controlflow/IRGuards.qll   | 18 +++++-------------
 1 file changed, 5 insertions(+), 13 deletions(-)

diff --git a/cpp/ql/lib/semmle/code/cpp/controlflow/IRGuards.qll b/cpp/ql/lib/semmle/code/cpp/controlflow/IRGuards.qll
index 665c61a71ddb..410ce4159da2 100644
--- a/cpp/ql/lib/semmle/code/cpp/controlflow/IRGuards.qll
+++ b/cpp/ql/lib/semmle/code/cpp/controlflow/IRGuards.qll
@@ -13,19 +13,11 @@ private import semmle.code.cpp.ir.implementation.raw.internal.InstructionTag
  * Returns `instr` or any instruction used to define `instr`.
  */
 private Instruction getDerivedInstruction(Instruction instr) {
-  // If there is a defining value, use it, else derived instruction is `instr`
-  // This forces backwards examination of guards if the current instruction is 
-  // derived from a prior instruction. 
-  if
-    exists(Instruction derived |
-      derived = valueNumber(instr).getAnInstruction() and
-      derived.toString() != instr.toString()
-    )
-  then
-    result = valueNumber(instr).getAnInstruction() and
-    result.toString() != instr.toString() and
-    result instanceof CompareInstruction
-  else result = instr
+  result = valueNumber(instr).getAnInstruction() and
+  result.toString() != instr.toString() and
+  result instanceof CompareInstruction
+  or
+  result = instr
 }
 
 /**

From 41e7dae54fde45d3534c26f8eee8bafc552aea8f Mon Sep 17 00:00:00 2001
From: "REDMOND\\brodes" <brodes@microsoft.com>
Date: Thu, 7 Nov 2024 08:59:43 -0500
Subject: [PATCH 08/10] getDerivedInstruction uses getSourceValue()

---
 cpp/ql/lib/semmle/code/cpp/controlflow/IRGuards.qll | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/cpp/ql/lib/semmle/code/cpp/controlflow/IRGuards.qll b/cpp/ql/lib/semmle/code/cpp/controlflow/IRGuards.qll
index 410ce4159da2..3c34909e3e8d 100644
--- a/cpp/ql/lib/semmle/code/cpp/controlflow/IRGuards.qll
+++ b/cpp/ql/lib/semmle/code/cpp/controlflow/IRGuards.qll
@@ -13,9 +13,7 @@ private import semmle.code.cpp.ir.implementation.raw.internal.InstructionTag
  * Returns `instr` or any instruction used to define `instr`.
  */
 private Instruction getDerivedInstruction(Instruction instr) {
-  result = valueNumber(instr).getAnInstruction() and
-  result.toString() != instr.toString() and
-  result instanceof CompareInstruction
+  result = valueNumber(instr).getAnInstruction().(StoreInstruction).getSourceValue()
   or
   result = instr
 }

From 418b1135ed2c6ca8a94ff183cd4a0da71b3cbb22 Mon Sep 17 00:00:00 2001
From: "REDMOND\\brodes" <brodes@microsoft.com>
Date: Mon, 11 Nov 2024 15:14:22 -0500
Subject: [PATCH 09/10] Adding conversions to the getDerivedInstruction
 predicate. Changed all uses of getDerivedInstruction to occur in 'base
 cases", specially only the compares_lt compares_eq and unary_compares_eq
 predicates. The premise is that users can modify/add any other logic without
 having to know about getDerivedInstruction; however, any updates to the
 compares_lt/eq predicates are the only place where the derived instruction
 must be used.

---
 .../semmle/code/cpp/controlflow/IRGuards.qll  | 205 ++++++++++--------
 1 file changed, 111 insertions(+), 94 deletions(-)

diff --git a/cpp/ql/lib/semmle/code/cpp/controlflow/IRGuards.qll b/cpp/ql/lib/semmle/code/cpp/controlflow/IRGuards.qll
index 3c34909e3e8d..045ee292ce37 100644
--- a/cpp/ql/lib/semmle/code/cpp/controlflow/IRGuards.qll
+++ b/cpp/ql/lib/semmle/code/cpp/controlflow/IRGuards.qll
@@ -13,9 +13,18 @@ private import semmle.code.cpp.ir.implementation.raw.internal.InstructionTag
  * Returns `instr` or any instruction used to define `instr`.
  */
 private Instruction getDerivedInstruction(Instruction instr) {
+  // The instruction itself
+  result = instr
+  or
+  // or the GVN definition for the current instruction
   result = valueNumber(instr).getAnInstruction().(StoreInstruction).getSourceValue()
   or
-  result = instr
+  // Special handling for conversions
+  result =
+    getDerivedInstruction(valueNumber(instr).getAnInstruction().(CompareNEInstruction).getLeft())
+  or
+  result =
+    getDerivedInstruction(valueNumber(instr).getAnInstruction().(ConvertInstruction).getUnary())
 }
 
 /**
@@ -527,7 +536,7 @@ class IRGuardCondition extends Instruction {
   cached
   predicate comparesLt(Operand left, Operand right, int k, boolean isLessThan, boolean testIsTrue) {
     exists(BooleanValue value |
-      compares_lt(getDerivedInstruction(this), left, right, k, isLessThan, value) and
+      compares_lt(this, left, right, k, isLessThan, value) and
       value.getValue() = testIsTrue
     )
   }
@@ -538,7 +547,7 @@ class IRGuardCondition extends Instruction {
    */
   cached
   predicate comparesLt(Operand op, int k, boolean isLessThan, AbstractValue value) {
-    compares_lt(getDerivedInstruction(this), op, k, isLessThan, value)
+    compares_lt(this, op, k, isLessThan, value)
   }
 
   /**
@@ -548,7 +557,7 @@ class IRGuardCondition extends Instruction {
   cached
   predicate ensuresLt(Operand left, Operand right, int k, IRBlock block, boolean isLessThan) {
     exists(AbstractValue value |
-      compares_lt(getDerivedInstruction(this), left, right, k, isLessThan, value) and
+      compares_lt(this, left, right, k, isLessThan, value) and
       this.valueControls(block, value)
     )
   }
@@ -560,7 +569,7 @@ class IRGuardCondition extends Instruction {
   cached
   predicate ensuresLt(Operand op, int k, IRBlock block, boolean isLessThan) {
     exists(AbstractValue value |
-      compares_lt(getDerivedInstruction(this), op, k, isLessThan, value) and
+      compares_lt(this, op, k, isLessThan, value) and
       this.valueControls(block, value)
     )
   }
@@ -574,7 +583,7 @@ class IRGuardCondition extends Instruction {
     Operand left, Operand right, int k, IRBlock pred, IRBlock succ, boolean isLessThan
   ) {
     exists(AbstractValue value |
-      compares_lt(getDerivedInstruction(this), left, right, k, isLessThan, value) and
+      compares_lt(this, left, right, k, isLessThan, value) and
       this.valueControlsEdge(pred, succ, value)
     )
   }
@@ -586,7 +595,7 @@ class IRGuardCondition extends Instruction {
   cached
   predicate ensuresLtEdge(Operand left, int k, IRBlock pred, IRBlock succ, boolean isLessThan) {
     exists(AbstractValue value |
-      compares_lt(getDerivedInstruction(this), left, k, isLessThan, value) and
+      compares_lt(this, left, k, isLessThan, value) and
       this.valueControlsEdge(pred, succ, value)
     )
   }
@@ -595,7 +604,7 @@ class IRGuardCondition extends Instruction {
   cached
   predicate comparesEq(Operand left, Operand right, int k, boolean areEqual, boolean testIsTrue) {
     exists(BooleanValue value |
-      compares_eq(getDerivedInstruction(this), left, right, k, areEqual, value) and
+      compares_eq(this, left, right, k, areEqual, value) and
       value.getValue() = testIsTrue
     )
   }
@@ -603,7 +612,7 @@ class IRGuardCondition extends Instruction {
   /** Holds if (determined by this guard) `op == k` evaluates to `areEqual` if this expression evaluates to `value`. */
   cached
   predicate comparesEq(Operand op, int k, boolean areEqual, AbstractValue value) {
-    unary_compares_eq(getDerivedInstruction(this), op, k, areEqual, false, value)
+    unary_compares_eq(this, op, k, areEqual, false, value)
   }
 
   /**
@@ -613,7 +622,7 @@ class IRGuardCondition extends Instruction {
   cached
   predicate ensuresEq(Operand left, Operand right, int k, IRBlock block, boolean areEqual) {
     exists(AbstractValue value |
-      compares_eq(getDerivedInstruction(this), left, right, k, areEqual, value) and
+      compares_eq(this, left, right, k, areEqual, value) and
       this.valueControls(block, value)
     )
   }
@@ -625,7 +634,7 @@ class IRGuardCondition extends Instruction {
   cached
   predicate ensuresEq(Operand op, int k, IRBlock block, boolean areEqual) {
     exists(AbstractValue value |
-      unary_compares_eq(getDerivedInstruction(this), op, k, areEqual, false, value) and
+      unary_compares_eq(this, op, k, areEqual, false, value) and
       this.valueControls(block, value)
     )
   }
@@ -639,7 +648,7 @@ class IRGuardCondition extends Instruction {
     Operand left, Operand right, int k, IRBlock pred, IRBlock succ, boolean areEqual
   ) {
     exists(AbstractValue value |
-      compares_eq(getDerivedInstruction(this), left, right, k, areEqual, value) and
+      compares_eq(this, left, right, k, areEqual, value) and
       this.valueControlsEdge(pred, succ, value)
     )
   }
@@ -651,7 +660,7 @@ class IRGuardCondition extends Instruction {
   cached
   predicate ensuresEqEdge(Operand op, int k, IRBlock pred, IRBlock succ, boolean areEqual) {
     exists(AbstractValue value |
-      unary_compares_eq(getDerivedInstruction(this), op, k, areEqual, false, value) and
+      unary_compares_eq(this, op, k, areEqual, false, value) and
       this.valueControlsEdge(pred, succ, value)
     )
   }
@@ -756,29 +765,30 @@ private Instruction getBranchForCondition(Instruction guard) {
 private predicate compares_eq(
   Instruction test, Operand left, Operand right, int k, boolean areEqual, AbstractValue value
 ) {
-  /* The simple case where the test *is* the comparison so areEqual = testIsTrue xor eq. */
-  exists(AbstractValue v | simple_comparison_eq(test, left, right, k, v) |
-    areEqual = true and value = v
+  exists(Instruction derived | derived = getDerivedInstruction(test) |
+    /* The simple case where the test *is* the comparison so areEqual = testIsTrue xor eq. */
+    exists(AbstractValue v | simple_comparison_eq(derived, left, right, k, v) |
+      areEqual = true and value = v
+      or
+      areEqual = false and value = v.getDualValue()
+    )
     or
-    areEqual = false and value = v.getDualValue()
-  )
-  or
-  // I think this is handled by forwarding in controlsBlock.
-  //or
-  //logical_comparison_eq(test, left, right, k, areEqual, testIsTrue)
-  /* a == b + k => b == a - k */
-  exists(int mk | k = -mk | compares_eq(test, right, left, mk, areEqual, value))
-  or
-  complex_eq(test, left, right, k, areEqual, value)
-  or
-  /* (x is true => (left == right + k)) => (!x is false => (left == right + k)) */
-  exists(AbstractValue dual | value = dual.getDualValue() |
-    compares_eq(getDerivedInstruction(test.(LogicalNotInstruction).getUnary()), left, right, k,
-      areEqual, dual)
+    // I think this is handled by forwarding in controlsBlock.
+    //or
+    //logical_comparison_eq(test, left, right, k, areEqual, testIsTrue)
+    /* a == b + k => b == a - k */
+    exists(int mk | k = -mk | compares_eq(derived, right, left, mk, areEqual, value))
+    or
+    complex_eq(derived, left, right, k, areEqual, value)
+    or
+    /* (x is true => (left == right + k)) => (!x is false => (left == right + k)) */
+    exists(AbstractValue dual | value = dual.getDualValue() |
+      compares_eq(derived.(LogicalNotInstruction).getUnary(), left, right, k, areEqual, dual)
+    )
+    or
+    compares_eq(derived.(BuiltinExpectCallInstruction).getCondition(), left, right, k, areEqual,
+      value)
   )
-  or
-  compares_eq(getDerivedInstruction(test.(BuiltinExpectCallInstruction).getCondition()), left,
-    right, k, areEqual, value)
 }
 
 /**
@@ -819,39 +829,41 @@ private predicate compares_eq(
 private predicate unary_compares_eq(
   Instruction test, Operand op, int k, boolean areEqual, boolean inNonZeroCase, AbstractValue value
 ) {
-  /* The simple case where the test *is* the comparison so areEqual = testIsTrue xor eq. */
-  exists(AbstractValue v |
-    unary_simple_comparison_eq(test, k, inNonZeroCase, v) and op.getDef() = test
-  |
-    areEqual = true and value = v
+  exists(Instruction derived | derived = getDerivedInstruction(test) |
+    /* The simple case where the test *is* the comparison so areEqual = testIsTrue xor eq. */
+    exists(AbstractValue v |
+      unary_simple_comparison_eq(derived, k, inNonZeroCase, v) and op.getDef() = test
+    |
+      areEqual = true and value = v
+      or
+      areEqual = false and value = v.getDualValue()
+    )
     or
-    areEqual = false and value = v.getDualValue()
-  )
-  or
-  unary_complex_eq(test, op, k, areEqual, inNonZeroCase, value)
-  or
-  /* (x is true => (op == k)) => (!x is false => (op == k)) */
-  exists(AbstractValue dual, boolean inNonZeroCase0 |
-    value = dual.getDualValue() and
-    unary_compares_eq(getDerivedInstruction(test.(LogicalNotInstruction).getUnary()), op, k,
-      inNonZeroCase0, areEqual, dual)
-  |
-    k = 0 and inNonZeroCase = inNonZeroCase0
+    unary_complex_eq(derived, op, k, areEqual, inNonZeroCase, value)
     or
-    k != 0 and inNonZeroCase = true
-  )
-  or
-  // ((test is `areEqual` => op == const + k2) and const == `k1`) =>
-  // test is `areEqual` => op == k1 + k2
-  inNonZeroCase = false and
-  exists(int k1, int k2, ConstantInstruction const |
-    compares_eq(test, op, const.getAUse(), k2, areEqual, value) and
-    int_value(const) = k1 and
-    k = k1 + k2
+    /* (x is true => (op == k)) => (!x is false => (op == k)) */
+    exists(AbstractValue dual, boolean inNonZeroCase0 |
+      value = dual.getDualValue() and
+      unary_compares_eq(derived.(LogicalNotInstruction).getUnary(), op, k, inNonZeroCase0, areEqual,
+        dual)
+    |
+      k = 0 and inNonZeroCase = inNonZeroCase0
+      or
+      k != 0 and inNonZeroCase = true
+    )
+    or
+    // ((test is `areEqual` => op == const + k2) and const == `k1`) =>
+    // test is `areEqual` => op == k1 + k2
+    inNonZeroCase = false and
+    exists(int k1, int k2, ConstantInstruction const |
+      compares_eq(derived, op, const.getAUse(), k2, areEqual, value) and
+      int_value(const) = k1 and
+      k = k1 + k2
+    )
+    or
+    unary_compares_eq(derived.(BuiltinExpectCallInstruction).getCondition(), op, k, areEqual,
+      inNonZeroCase, value)
   )
-  or
-  unary_compares_eq(test.(BuiltinExpectCallInstruction).getCondition(), op, k, areEqual,
-    inNonZeroCase, value)
 }
 
 /** Rearrange various simple comparisons into `left == right + k` form. */
@@ -954,7 +966,7 @@ private predicate builtin_expect_eq(
   exists(BuiltinExpectCallInstruction call, Instruction const, AbstractValue innerValue |
     int_value(const) = 0 and
     cmp.hasOperands(call.getAUse(), const.getAUse()) and
-    compares_eq(getDerivedInstruction(call.getCondition()), left, right, k, areEqual, innerValue)
+    compares_eq(call.getCondition(), left, right, k, areEqual, innerValue)
   |
     cmp instanceof CompareNEInstruction and
     value = innerValue
@@ -985,8 +997,7 @@ private predicate unary_builtin_expect_eq(
   exists(BuiltinExpectCallInstruction call, Instruction const, AbstractValue innerValue |
     int_value(const) = 0 and
     cmp.hasOperands(call.getAUse(), const.getAUse()) and
-    unary_compares_eq(getDerivedInstruction(call.getCondition()), op, k, areEqual, inNonZeroCase,
-      innerValue)
+    unary_compares_eq(call.getCondition(), op, k, areEqual, inNonZeroCase, innerValue)
   |
     cmp instanceof CompareNEInstruction and
     value = innerValue
@@ -1015,38 +1026,44 @@ private predicate unary_complex_eq(
 private predicate compares_lt(
   Instruction test, Operand left, Operand right, int k, boolean isLt, AbstractValue value
 ) {
-  /* In the simple case, the test is the comparison, so isLt = testIsTrue */
-  simple_comparison_lt(test, left, right, k) and
-  value.(BooleanValue).getValue() = isLt
-  or
-  complex_lt(test, left, right, k, isLt, value)
-  or
-  /* (not (left < right + k)) => (left >= right + k) */
-  exists(boolean isGe | isLt = isGe.booleanNot() | compares_ge(test, left, right, k, isGe, value))
-  or
-  /* (x is true => (left < right + k)) => (!x is false => (left < right + k)) */
-  exists(AbstractValue dual | value = dual.getDualValue() |
-    compares_lt(getDerivedInstruction(test.(LogicalNotInstruction).getUnary()), left, right, k,
-      isLt, dual)
+  exists(Instruction derived | derived = getDerivedInstruction(test) |
+    // exists(thing | thing = derived ... )
+    /* In the simple case, the test is the comparison, so isLt = testIsTrue */
+    simple_comparison_lt(derived, left, right, k) and
+    value.(BooleanValue).getValue() = isLt
+    or
+    complex_lt(derived, left, right, k, isLt, value)
+    or
+    /* (not (left < right + k)) => (left >= right + k) */
+    exists(boolean isGe | isLt = isGe.booleanNot() |
+      compares_ge(derived, left, right, k, isGe, value)
+    )
+    or
+    /* (x is true => (left < right + k)) => (!x is false => (left < right + k)) */
+    exists(AbstractValue dual | value = dual.getDualValue() |
+      compares_lt(derived.(LogicalNotInstruction).getUnary(), left, right, k, isLt, dual)
+    )
   )
 }
 
 /** Holds if `op < k` evaluates to `isLt` given that `test` evaluates to `value`. */
 private predicate compares_lt(Instruction test, Operand op, int k, boolean isLt, AbstractValue value) {
-  unary_simple_comparison_lt(test, k, isLt, value) and
-  op.getDef() = test
-  or
-  complex_lt(test, op, k, isLt, value)
-  or
-  /* (x is true => (op < k)) => (!x is false => (op < k)) */
-  exists(AbstractValue dual | value = dual.getDualValue() |
-    compares_lt(getDerivedInstruction(test.(LogicalNotInstruction).getUnary()), op, k, isLt, dual)
-  )
-  or
-  exists(int k1, int k2, ConstantInstruction const |
-    compares_lt(test, op, const.getAUse(), k2, isLt, value) and
-    int_value(const) = k1 and
-    k = k1 + k2
+  exists(Instruction derived | derived = getDerivedInstruction(test) |
+    unary_simple_comparison_lt(derived, k, isLt, value) and
+    op.getDef() = derived
+    or
+    complex_lt(derived, op, k, isLt, value)
+    or
+    /* (x is true => (op < k)) => (!x is false => (op < k)) */
+    exists(AbstractValue dual | value = dual.getDualValue() |
+      compares_lt(derived.(LogicalNotInstruction).getUnary(), op, k, isLt, dual)
+    )
+    or
+    exists(int k1, int k2, ConstantInstruction const |
+      compares_lt(derived, op, const.getAUse(), k2, isLt, value) and
+      int_value(const) = k1 and
+      k = k1 + k2
+    )
   )
 }
 

From 2dfa0293c4475fd883f3981dd65e0cbe1af554d1 Mon Sep 17 00:00:00 2001
From: "REDMOND\\brodes" <brodes@microsoft.com>
Date: Wed, 13 Nov 2024 15:38:55 -0500
Subject: [PATCH 10/10] Misc. poc updates to address performance issues with
 wireshark.

---
 .../semmle/code/cpp/controlflow/IRGuards.qll  | 56 ++++++++++++++++---
 1 file changed, 48 insertions(+), 8 deletions(-)

diff --git a/cpp/ql/lib/semmle/code/cpp/controlflow/IRGuards.qll b/cpp/ql/lib/semmle/code/cpp/controlflow/IRGuards.qll
index 045ee292ce37..054672741644 100644
--- a/cpp/ql/lib/semmle/code/cpp/controlflow/IRGuards.qll
+++ b/cpp/ql/lib/semmle/code/cpp/controlflow/IRGuards.qll
@@ -9,22 +9,48 @@ private import semmle.code.cpp.ir.ValueNumbering
 private import semmle.code.cpp.ir.implementation.raw.internal.TranslatedExpr
 private import semmle.code.cpp.ir.implementation.raw.internal.InstructionTag
 
+/**
+ * Variant of valueNumber that accounts for conversions.
+ * Note: this predicate is defined in IRGuards to take advantage of
+ * IRGuards definition of int_value.
+ */
+ValueNumber valueNumberWithConversions(Instruction instr) {
+  exists(ValueNumber vn | vn = valueNumber(instr) |
+    // GVN using valueNumber directly
+    result = vn
+    or
+    // GVN for the ConvertInstruction conversion
+    result = valueNumberWithConversions(vn.getAnInstruction().(ConvertInstruction).getUnary())
+    or
+    // GVN for the CompareNEInstruction conversion for a comparison with 0 (conversion of value to boolean in c)
+    exists(CompareNEInstruction cmp | cmp = vn.getAnInstruction() |
+      result = valueNumberWithConversions(cmp.getLeft()) and
+      int_value(cmp.getRight()) = 0
+    )
+  )
+}
+
 /**
  * Returns `instr` or any instruction used to define `instr`.
  */
+pragma[inline]
 private Instruction getDerivedInstruction(Instruction instr) {
-  // The instruction itself
   result = instr
   or
-  // or the GVN definition for the current instruction
   result = valueNumber(instr).getAnInstruction().(StoreInstruction).getSourceValue()
   or
-  // Special handling for conversions
-  result =
-    getDerivedInstruction(valueNumber(instr).getAnInstruction().(CompareNEInstruction).getLeft())
+  result = derivedThroughConversion(valueNumber(instr))
+}
+
+private Instruction derivedThroughConversion(ValueNumber vn) {
+  // GVN for the ConvertInstruction conversion
+  result = getDerivedInstruction(vn.getAnInstruction().(ConvertInstruction).getUnary())
   or
-  result =
-    getDerivedInstruction(valueNumber(instr).getAnInstruction().(ConvertInstruction).getUnary())
+  // GVN for the CompareNEInstruction conversion for a comparison with 0 (conversion of value to boolean in c)
+  exists(CompareNEInstruction comp | comp = vn.getAnInstruction() |
+    result = getDerivedInstruction(comp.getLeft()) and
+    int_value(comp.getRight()) = 0
+  )
 }
 
 /**
@@ -832,7 +858,7 @@ private predicate unary_compares_eq(
   exists(Instruction derived | derived = getDerivedInstruction(test) |
     /* The simple case where the test *is* the comparison so areEqual = testIsTrue xor eq. */
     exists(AbstractValue v |
-      unary_simple_comparison_eq(derived, k, inNonZeroCase, v) and op.getDef() = test
+      unary_simple_comparison_eq(derived, k, inNonZeroCase, v) and op.getDef() = derived
     |
       areEqual = true and value = v
       or
@@ -897,6 +923,18 @@ private predicate unary_simple_comparison_eq(
     inNonZeroCase = false
   )
   or
+  (
+    exists(BinaryLogicalOperation e | e.getAnOperand() = test.getUnconvertedResultExpression())
+    or
+    exists(UnaryLogicalOperation e | e.getAnOperand() = test.getUnconvertedResultExpression())
+    or
+    exists(IfStmt i | i.getCondition() = test.getUnconvertedResultExpression())
+    or
+    exists(Loop i | i.getCondition() = test.getUnconvertedResultExpression())
+    or
+    exists(ConditionalExpr c | c.getCondition() = test.getUnconvertedResultExpression())
+    // Todo recursive conditionalExpr in guard
+  ) and
   // Any instruction with an integral type could potentially be part of a
   // check for nullness when used in a guard. So we include all integral
   // typed instructions here. However, since some of these instructions are
@@ -941,6 +979,8 @@ private predicate unary_simple_comparison_eq(
     value.(BooleanValue).getValue() = false and
     inNonZeroCase = false
   )
+  // Has to be in a boolean operator (operanad of OR or AND)
+  // has to be a 'guard' (loop, if, switch, ternary)
 }
 
 /** A call to the builtin operation `__builtin_expect`. */