Merge pull request github#16714 from MathiasVP/handle-unlikely-in-guards-2

MathiasVP · web-flow · commit ec34007a88c5 · 2024-06-10T16:54:01.000+01:00
C++: Support `__builtin_expect` in `IRGuards`
diff --git a/cpp/ql/lib/change-notes/2024-10-06-builtin-expect.md b/cpp/ql/lib/change-notes/2024-10-06-builtin-expect.md
@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* The "Guards" library (`semmle.code.cpp.controlflow.Guards`) now also infers guards from calls to the builtin operation `__builtin_expect`. As a result, some queries may produce fewer false positives.
diff --git a/cpp/ql/lib/semmle/code/cpp/controlflow/IRGuards.qll b/cpp/ql/lib/semmle/code/cpp/controlflow/IRGuards.qll
@@ -762,6 +762,8 @@ private predicate compares_eq(
   exists(AbstractValue dual | value = dual.getDualValue() |
     compares_eq(test.(LogicalNotInstruction).getUnary(), left, right, k, areEqual, dual)
   )
+  or
+  compares_eq(test.(BuiltinExpectCallInstruction).getCondition(), left, right, k, areEqual, value)
 }
 
 /**
@@ -831,6 +833,9 @@ private predicate unary_compares_eq(
     int_value(const) = k1 and
     k = k1 + k2
   )
+  or
+  unary_compares_eq(test.(BuiltinExpectCallInstruction).getCondition(), op, k, areEqual,
+    inNonZeroCase, value)
 }
 
 /** Rearrange various simple comparisons into `left == right + k` form. */
@@ -910,12 +915,68 @@ private predicate unary_simple_comparison_eq(
   )
 }
 
+/** A call to the builtin operation `__builtin_expect`. */
+private class BuiltinExpectCallInstruction extends CallInstruction {
+  BuiltinExpectCallInstruction() { this.getStaticCallTarget().hasName("__builtin_expect") }
+
+  /** Gets the condition of this call. */
+  Instruction getCondition() {
+    // The first parameter of `__builtin_expect` has type `long`. So we skip
+    // the conversion when inferring guards.
+    result = this.getArgument(0).(ConvertInstruction).getUnary()
+  }
+}
+
+/**
+ * Holds if `left == right + k` is `areEqual` if `cmp` evaluates to `value`,
+ * and `cmp` is an instruction that compares the value of
+ * `__builtin_expect(left == right + k, _)` to `0`.
+ */
+private predicate builtin_expect_eq(
+  CompareInstruction cmp, Operand left, Operand right, int k, boolean areEqual, AbstractValue value
+) {
+  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)
+  |
+    cmp instanceof CompareNEInstruction and
+    value = innerValue
+    or
+    cmp instanceof CompareEQInstruction and
+    value.getDualValue() = innerValue
+  )
+}
+
 private predicate complex_eq(
   CompareInstruction cmp, Operand left, Operand right, int k, boolean areEqual, AbstractValue value
 ) {
   sub_eq(cmp, left, right, k, areEqual, value)
   or
   add_eq(cmp, left, right, k, areEqual, value)
+  or
+  builtin_expect_eq(cmp, left, right, k, areEqual, value)
+}
+
+/**
+ * Holds if `op == k` is `areEqual` if `cmp` evaluates to `value`, and `cmp` is
+ * an instruction that compares the value of `__builtin_expect(op == k, _)` to `0`.
+ */
+private predicate unary_builtin_expect_eq(
+  CompareInstruction cmp, Operand op, int k, boolean areEqual, boolean inNonZeroCase,
+  AbstractValue value
+) {
+  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)
+  |
+    cmp instanceof CompareNEInstruction and
+    value = innerValue
+    or
+    cmp instanceof CompareEQInstruction and
+    value.getDualValue() = innerValue
+  )
 }
 
 private predicate unary_complex_eq(
@@ -924,6 +985,8 @@ private predicate unary_complex_eq(
   unary_sub_eq(test, op, k, areEqual, inNonZeroCase, value)
   or
   unary_add_eq(test, op, k, areEqual, inNonZeroCase, value)
+  or
+  unary_builtin_expect_eq(test, op, k, areEqual, inNonZeroCase, value)
 }
 
 /*
diff --git a/cpp/ql/test/library-tests/controlflow/guards/Guards.expected b/cpp/ql/test/library-tests/controlflow/guards/Guards.expected
@@ -45,3 +45,7 @@
 | test.cpp:122:9:122:9 | b |
 | test.cpp:125:13:125:20 | ! ... |
 | test.cpp:125:14:125:17 | call to safe |
+| test.cpp:131:6:131:21 | call to __builtin_expect |
+| test.cpp:135:6:135:21 | call to __builtin_expect |
+| test.cpp:141:6:141:21 | call to __builtin_expect |
+| test.cpp:145:6:145:21 | call to __builtin_expect |
diff --git a/cpp/ql/test/library-tests/controlflow/guards/GuardsCompare.expected b/cpp/ql/test/library-tests/controlflow/guards/GuardsCompare.expected
@@ -164,10 +164,46 @@
 | 126 | call to test3_condition != 0 when ... && ... is true |
 | 126 | call to test3_condition != 0 when call to test3_condition is true |
 | 126 | call to test3_condition == 0 when call to test3_condition is false |
+| 131 | ... + ... != a+0 when call to __builtin_expect is false |
+| 131 | ... + ... == a+0 when call to __builtin_expect is true |
+| 131 | a != ... + ...+0 when call to __builtin_expect is false |
+| 131 | a != b+42 when call to __builtin_expect is false |
+| 131 | a == ... + ...+0 when call to __builtin_expect is true |
+| 131 | a == b+42 when call to __builtin_expect is true |
 | 131 | b != 0 when b is true |
+| 131 | b != a+-42 when call to __builtin_expect is false |
 | 131 | b == 0 when b is false |
+| 131 | b == a+-42 when call to __builtin_expect is true |
+| 131 | call to __builtin_expect != 0 when call to __builtin_expect is true |
+| 131 | call to __builtin_expect == 0 when call to __builtin_expect is false |
+| 135 | ... + ... != a+0 when call to __builtin_expect is true |
+| 135 | ... + ... == a+0 when call to __builtin_expect is false |
+| 135 | a != ... + ...+0 when call to __builtin_expect is true |
+| 135 | a != b+42 when call to __builtin_expect is true |
+| 135 | a == ... + ...+0 when call to __builtin_expect is false |
+| 135 | a == b+42 when call to __builtin_expect is false |
+| 135 | b != a+-42 when call to __builtin_expect is true |
+| 135 | b == a+-42 when call to __builtin_expect is false |
+| 135 | call to __builtin_expect != 0 when call to __builtin_expect is true |
+| 135 | call to __builtin_expect == 0 when call to __builtin_expect is false |
 | 137 | 0 != 0 when 0 is true |
 | 137 | 0 == 0 when 0 is false |
+| 141 | 42 != a+0 when call to __builtin_expect is false |
+| 141 | 42 == a+0 when call to __builtin_expect is true |
+| 141 | a != 42 when call to __builtin_expect is false |
+| 141 | a != 42+0 when call to __builtin_expect is false |
+| 141 | a == 42 when call to __builtin_expect is true |
+| 141 | a == 42+0 when call to __builtin_expect is true |
+| 141 | call to __builtin_expect != 0 when call to __builtin_expect is true |
+| 141 | call to __builtin_expect == 0 when call to __builtin_expect is false |
+| 145 | 42 != a+0 when call to __builtin_expect is true |
+| 145 | 42 == a+0 when call to __builtin_expect is false |
+| 145 | a != 42 when call to __builtin_expect is true |
+| 145 | a != 42+0 when call to __builtin_expect is true |
+| 145 | a == 42 when call to __builtin_expect is false |
+| 145 | a == 42+0 when call to __builtin_expect is false |
+| 145 | call to __builtin_expect != 0 when call to __builtin_expect is true |
+| 145 | call to __builtin_expect == 0 when call to __builtin_expect is false |
 | 146 | ! ... != 0 when ! ... is true |
 | 146 | ! ... == 0 when ! ... is false |
 | 146 | x != 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
@@ -104,3 +104,7 @@
 | test.cpp:122:9:122:9 | b | true | 125 | 125 |
 | test.cpp:125:13:125:20 | ! ... | true | 125 | 125 |
 | test.cpp:125:14:125:17 | call to safe | false | 125 | 125 |
+| test.cpp:131:6:131:21 | call to __builtin_expect | true | 131 | 132 |
+| test.cpp:135:6:135:21 | call to __builtin_expect | true | 135 | 136 |
+| test.cpp:141:6:141:21 | call to __builtin_expect | true | 141 | 142 |
+| test.cpp:145:6:145:21 | call to __builtin_expect | true | 145 | 146 |
diff --git a/cpp/ql/test/library-tests/controlflow/guards/GuardsEnsure.expected b/cpp/ql/test/library-tests/controlflow/guards/GuardsEnsure.expected
@@ -159,6 +159,18 @@ binary
 | test.cpp:105:6:105:14 | ... != ... | test.cpp:105:11:105:14 | 0.0 | != | test.cpp:105:6:105:6 | f | 0 | 105 | 106 |
 | test.cpp:111:6:111:14 | ... != ... | test.cpp:111:6:111:6 | i | != | test.cpp:111:11:111:14 | 0.0 | 0 | 111 | 112 |
 | test.cpp:111:6:111:14 | ... != ... | test.cpp:111:11:111:14 | 0.0 | != | test.cpp:111:6:111:6 | i | 0 | 111 | 112 |
+| test.cpp:131:6:131:21 | call to __builtin_expect | test.cpp:131:23:131:23 | a | == | test.cpp:131:28:131:28 | b | 42 | 131 | 132 |
+| test.cpp:131:6:131:21 | call to __builtin_expect | test.cpp:131:23:131:23 | a | == | test.cpp:131:28:131:33 | ... + ... | 0 | 131 | 132 |
+| test.cpp:131:6:131:21 | call to __builtin_expect | test.cpp:131:28:131:28 | b | == | test.cpp:131:23:131:23 | a | -42 | 131 | 132 |
+| test.cpp:131:6:131:21 | call to __builtin_expect | test.cpp:131:28:131:33 | ... + ... | == | test.cpp:131:23:131:23 | a | 0 | 131 | 132 |
+| test.cpp:135:6:135:21 | call to __builtin_expect | test.cpp:135:23:135:23 | a | != | test.cpp:135:28:135:28 | b | 42 | 135 | 136 |
+| test.cpp:135:6:135:21 | call to __builtin_expect | test.cpp:135:23:135:23 | a | != | test.cpp:135:28:135:33 | ... + ... | 0 | 135 | 136 |
+| test.cpp:135:6:135:21 | call to __builtin_expect | test.cpp:135:28:135:28 | b | != | test.cpp:135:23:135:23 | a | -42 | 135 | 136 |
+| test.cpp:135:6:135:21 | call to __builtin_expect | test.cpp:135:28:135:33 | ... + ... | != | test.cpp:135:23:135:23 | a | 0 | 135 | 136 |
+| test.cpp:141:6:141:21 | call to __builtin_expect | test.cpp:141:23:141:23 | a | == | test.cpp:141:28:141:29 | 42 | 0 | 141 | 142 |
+| test.cpp:141:6:141:21 | call to __builtin_expect | test.cpp:141:28:141:29 | 42 | == | test.cpp:141:23:141:23 | a | 0 | 141 | 142 |
+| test.cpp:145:6:145:21 | call to __builtin_expect | test.cpp:145:23:145:23 | a | != | test.cpp:145:28:145:29 | 42 | 0 | 145 | 146 |
+| test.cpp:145:6:145:21 | call to __builtin_expect | test.cpp:145:28:145:29 | 42 | != | test.cpp:145:23:145:23 | a | 0 | 145 | 146 |
 unary
 | test.c:7:9:7:13 | ... > ... | test.c:7:9:7:9 | x | < | 1 | 10 | 11 |
 | test.c:7:9:7:13 | ... > ... | test.c:7:9:7:9 | x | >= | 1 | 7 | 9 |
@@ -270,3 +282,9 @@ unary
 | test.cpp:122:9:122:9 | b | test.cpp:122:9:122:9 | b | != | 0 | 125 | 125 |
 | test.cpp:125:13:125:20 | ! ... | test.cpp:125:13:125:20 | ! ... | != | 0 | 125 | 125 |
 | test.cpp:125:14:125:17 | call to safe | test.cpp:125:14:125:17 | call to safe | == | 0 | 125 | 125 |
+| test.cpp:131:6:131:21 | call to __builtin_expect | test.cpp:131:6:131:21 | call to __builtin_expect | != | 0 | 131 | 132 |
+| test.cpp:135:6:135:21 | call to __builtin_expect | test.cpp:135:6:135:21 | call to __builtin_expect | != | 0 | 135 | 136 |
+| test.cpp:141:6:141:21 | call to __builtin_expect | test.cpp:141:6:141:21 | call to __builtin_expect | != | 0 | 141 | 142 |
+| test.cpp:141:6:141:21 | call to __builtin_expect | test.cpp:141:23:141:23 | a | == | 42 | 141 | 142 |
+| test.cpp:145:6:145:21 | call to __builtin_expect | test.cpp:145:6:145:21 | call to __builtin_expect | != | 0 | 145 | 146 |
+| test.cpp:145:6:145:21 | call to __builtin_expect | test.cpp:145:23:145:23 | a | != | 42 | 145 | 146 |
diff --git a/cpp/ql/test/library-tests/controlflow/guards/test.cpp b/cpp/ql/test/library-tests/controlflow/guards/test.cpp
@@ -125,4 +125,24 @@ void test(bool b)
         if (!safe(x)) return;
     }
     use(x);
+}
+
+void binary_test_builtin_expected(int a, int b) {
+  if(__builtin_expect(a == b + 42, 0)) {
+      use(a);
+  }
+
+  if(__builtin_expect(a != b + 42, 0)) {
+      use(a);
+  }
+}
+
+void unary_test_builtin_expected(int a) {
+  if(__builtin_expect(a == 42, 0)) {
+      use(a);
+  }
+
+  if(__builtin_expect(a != 42, 0)) {
+      use(a);
+  }
 }

-Original file line number
+Diff line change
@@ @@ -0,0 +1,4 @@ @@
 +---
 +category: minorAnalysis
 +---
 +* The "Guards" library (`semmle.code.cpp.controlflow.Guards`) now also infers guards from calls to the builtin operation `__builtin_expect`. As a result, some queries may produce fewer false positives.