C++: Fix more inconsistencies.

Author: MathiasVP

cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasAnalysis.qll

@@ -338,6 +338,57 @@ private predicate resultEscapesNonReturn(Instruction instr) {
   not instr.isResultModeled()
 }
 
+/** Holds if `operand` may (transitively) flow to an `AddressOperand` whose use is unmodeled. */
+private predicate consumedAsAddressOperand(Operand operand) {
+  operand instanceof AddressOperand and
+  exists(Instruction use | not use.isResultModeled())
+  or
+  exists(Operand address |
+    consumedAsAddressOperand(address) and
+    operandIsPropagated(operand, _, address.getDef())
+  )
+}
+
+/**
+ * Holds if `operand` may originate from a base instruction of an allocation,
+ * and that operand may transitively flow to an `AddressOperand` whose use is unmodeled.
+ */
+private predicate propagatedFromAllocationBase(Operand operand, Configuration::Allocation allocation) {
+  consumedAsAddressOperand(operand) and
+  (
+    not exists(Configuration::getOldAllocation(allocation)) and
+    operand.getDef() = allocation.getABaseInstruction()
+    or
+    exists(Operand address |
+      operandIsPropagated(address, _, operand.getDef()) and
+      propagatedFromAllocationBase(address, allocation)
+    )
+  )
+}
+
+private predicate propagatedFromNonAllocationBase(Operand operand) {
+  exists(Instruction def |
+    def = operand.getDef() and
+    not operandIsPropagated(_, _, def) and
+    not def = any(Configuration::Allocation allocation).getABaseInstruction()
+  )
+  or
+  exists(Operand address |
+    operandIsPropagated(address, _, operand.getDef()) and
+    propagatedFromNonAllocationBase(address)
+  )
+}
+
+/**
+ * Holds if we cannot see all producers of an operand for which allocation also flows into.
+ */
+private predicate operandConsumesEscaped(Configuration::Allocation allocation) {
+  exists(AddressOperand address |
+    propagatedFromAllocationBase(address, allocation) and
+    propagatedFromNonAllocationBase(address)
+  )
+}
+
 /**
  * Holds if the address of `allocation` escapes outside the domain of the analysis. This can occur
  * either because the allocation's address is taken within the function and escapes, or because the
@@ -346,12 +397,14 @@ private predicate resultEscapesNonReturn(Instruction instr) {
 predicate allocationEscapes(Configuration::Allocation allocation) {
   allocation.alwaysEscapes()
   or
-  exists(IREscapeAnalysisConfiguration config |
-    config.useSoundEscapeAnalysis() and resultEscapesNonReturn(allocation.getABaseInstruction())
+  exists(IREscapeAnalysisConfiguration config | config.useSoundEscapeAnalysis() |
+    resultEscapesNonReturn(allocation.getABaseInstruction())
+    or
+    operandConsumesEscaped(allocation)
   )
   or
   Configuration::phaseNeedsSoundEscapeAnalysis() and
-  resultEscapesNonReturn(allocation.getABaseInstruction())
+  (resultEscapesNonReturn(allocation.getABaseInstruction()) or operandConsumesEscaped(allocation))
 }
 
 /**

cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasConfiguration.qll

@@ -146,3 +146,8 @@ class DynamicAllocation extends Allocation, TDynamicAllocation {
 }
 
 predicate phaseNeedsSoundEscapeAnalysis() { none() }
+
+UnaliasedSsa::Allocation getOldAllocation(VariableAllocation allocation) {
+  UnaliasedSsa::canReuseSsaForVariable(allocation.getIRVariable()) and
+  result = allocation.getIRVariable()
+}

cpp/ql/lib/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasedSSA.qll

@@ -7,7 +7,7 @@ private import semmle.code.cpp.ir.implementation.unaliased_ssa.internal.SSAConst
 private import semmle.code.cpp.ir.internal.IntegerConstant as Ints
 private import semmle.code.cpp.ir.internal.IntegerInterval as Interval
 private import semmle.code.cpp.ir.implementation.internal.OperandTag
-private import AliasConfiguration
+import AliasConfiguration
 private import codeql.util.Boolean
 
 private class IntValue = Ints::IntValue;

cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/AliasAnalysis.qll

@@ -338,6 +338,57 @@ private predicate resultEscapesNonReturn(Instruction instr) {
   not instr.isResultModeled()
 }
 
+/** Holds if `operand` may (transitively) flow to an `AddressOperand` whose use is unmodeled. */
+private predicate consumedAsAddressOperand(Operand operand) {
+  operand instanceof AddressOperand and
+  exists(Instruction use | not use.isResultModeled())
+  or
+  exists(Operand address |
+    consumedAsAddressOperand(address) and
+    operandIsPropagated(operand, _, address.getDef())
+  )
+}
+
+/**
+ * Holds if `operand` may originate from a base instruction of an allocation,
+ * and that operand may transitively flow to an `AddressOperand` whose use is unmodeled.
+ */
+private predicate propagatedFromAllocationBase(Operand operand, Configuration::Allocation allocation) {
+  consumedAsAddressOperand(operand) and
+  (
+    not exists(Configuration::getOldAllocation(allocation)) and
+    operand.getDef() = allocation.getABaseInstruction()
+    or
+    exists(Operand address |
+      operandIsPropagated(address, _, operand.getDef()) and
+      propagatedFromAllocationBase(address, allocation)
+    )
+  )
+}
+
+private predicate propagatedFromNonAllocationBase(Operand operand) {
+  exists(Instruction def |
+    def = operand.getDef() and
+    not operandIsPropagated(_, _, def) and
+    not def = any(Configuration::Allocation allocation).getABaseInstruction()
+  )
+  or
+  exists(Operand address |
+    operandIsPropagated(address, _, operand.getDef()) and
+    propagatedFromNonAllocationBase(address)
+  )
+}
+
+/**
+ * Holds if we cannot see all producers of an operand for which allocation also flows into.
+ */
+private predicate operandConsumesEscaped(Configuration::Allocation allocation) {
+  exists(AddressOperand address |
+    propagatedFromAllocationBase(address, allocation) and
+    propagatedFromNonAllocationBase(address)
+  )
+}
+
 /**
  * Holds if the address of `allocation` escapes outside the domain of the analysis. This can occur
  * either because the allocation's address is taken within the function and escapes, or because the
@@ -346,12 +397,14 @@ private predicate resultEscapesNonReturn(Instruction instr) {
 predicate allocationEscapes(Configuration::Allocation allocation) {
   allocation.alwaysEscapes()
   or
-  exists(IREscapeAnalysisConfiguration config |
-    config.useSoundEscapeAnalysis() and resultEscapesNonReturn(allocation.getABaseInstruction())
+  exists(IREscapeAnalysisConfiguration config | config.useSoundEscapeAnalysis() |
+    resultEscapesNonReturn(allocation.getABaseInstruction())
+    or
+    operandConsumesEscaped(allocation)
   )
   or
   Configuration::phaseNeedsSoundEscapeAnalysis() and
-  resultEscapesNonReturn(allocation.getABaseInstruction())
+  (resultEscapesNonReturn(allocation.getABaseInstruction()) or operandConsumesEscaped(allocation))
 }
 
 /**

cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/AliasConfiguration.qll

@@ -1,4 +1,5 @@
 private import AliasConfigurationImports
+private import codeql.util.Unit
 
 /**
  * A memory allocation that can be tracked by the SimpleSSA alias analysis.
@@ -16,3 +17,5 @@ class Allocation extends IRAutomaticVariable {
 }
 
 predicate phaseNeedsSoundEscapeAnalysis() { any() }
+
+Unit getOldAllocation(Allocation allocation) { any() }

cpp/ql/lib/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SimpleSSA.qll

@@ -1,7 +1,7 @@
 import AliasAnalysis
 private import SimpleSSAImports
 import SimpleSSAPublicImports
-private import AliasConfiguration
+import AliasConfiguration
 private import codeql.util.Unit
 
 private predicate isTotalAccess(Allocation var, AddressOperand addrOperand, IRType type) {

cpp/ql/test/library-tests/ir/ir/aliased_ir.expected

@@ -18827,22 +18827,23 @@ ir.cpp:
 # 2667|     r2667_1(glval<int>)   = VariableAddress[intBuffer] : 
 # 2667|     r2667_2(int)          = Constant[8]                : 
 # 2667|     m2667_3(int)          = Store[intBuffer]           : &:r2667_1, r2667_2
+# 2667|     m2667_4(unknown)      = Chi                        : total:m2663_4, partial:m2667_3
 # 2668|     r2668_1(glval<int>)   = VariableAddress[intBuffer] : 
 # 2668|     r2668_2(int *)        = CopyValue                  : r2668_1
 # 2668|     r2668_3(glval<int *>) = VariableAddress[data]      : 
 # 2668|     m2668_4(int *)        = Store[data]                : &:r2668_3, r2668_2
 #-----|   Goto -> Block 2
 
 # 2670|   Block 2
-# 2670|     m2670_1(int)            = Phi                      : from 1:m2667_3
+# 2670|     m2670_1(unknown)        = Phi                      : from 0:~m2663_4, from 1:~m2667_4
 # 2670|     m2670_2(int *)          = Phi                      : from 0:m2665_3, from 1:m2668_4
 # 2670|     r2670_3(glval<unknown>) = FunctionAddress[use_int] : 
 # 2670|     r2670_4(glval<int *>)   = VariableAddress[data]    : 
 # 2670|     r2670_5(int *)          = Load[data]               : &:r2670_4, m2670_2
 # 2670|     r2670_6(int)            = Load[?]                  : &:r2670_5, ~m2670_1
 # 2670|     v2670_7(void)           = Call[use_int]            : func:r2670_3, 0:r2670_6
-# 2670|     m2670_8(unknown)        = ^CallSideEffect          : ~m2663_4
-# 2670|     m2670_9(unknown)        = Chi                      : total:m2663_4, partial:m2670_8
+# 2670|     m2670_8(unknown)        = ^CallSideEffect          : ~m2670_1
+# 2670|     m2670_9(unknown)        = Chi                      : total:m2670_1, partial:m2670_8
 # 2671|     v2671_1(void)           = NoOp                     : 
 # 2663|     v2663_7(void)           = ReturnVoid               : 
 # 2663|     v2663_8(void)           = AliasedUse               : ~m2670_9

cpp/ql/test/library-tests/ir/ir/aliased_ssa_consistency.expected

@@ -2,7 +2,6 @@ missingOperand
 unexpectedOperand
 duplicateOperand
 missingPhiOperand
-| ir.cpp:2670:3:2670:9 | Phi: call to use_int | Instruction 'Phi: call to use_int' is missing an operand for predecessor block 'EnterFunction: phi_with_single_input_at_merge' in function '$@'. | ir.cpp:2663:13:2663:42 | void phi_with_single_input_at_merge(bool) | void phi_with_single_input_at_merge(bool) |
 missingOperandType
 duplicateChiOperand
 sideEffectWithoutPrimary

cpp/ql/test/library-tests/ir/ir/aliased_ssa_consistency_unsound.expected

@@ -2,7 +2,6 @@ missingOperand
 unexpectedOperand
 duplicateOperand
 missingPhiOperand
-| ir.cpp:2670:3:2670:9 | Phi: call to use_int | Instruction 'Phi: call to use_int' is missing an operand for predecessor block 'EnterFunction: phi_with_single_input_at_merge' in function '$@'. | ir.cpp:2663:13:2663:42 | void phi_with_single_input_at_merge(bool) | void phi_with_single_input_at_merge(bool) |
 missingOperandType
 duplicateChiOperand
 sideEffectWithoutPrimary

cpp/ql/test/query-tests/Security/CWE/CWE-457/semmle/tests/test.cpp

@@ -565,4 +565,19 @@ void test45() {
 
   *rP = NULL;
   use(r); // GOOD
-}
+}
+
+void test46()
+{
+  LinkedList *r, **rP = &r;
+
+  while (getBool())
+  {
+    LinkedList *s = nullptr;
+    *rP = s;
+    rP = &s->next;
+  }
+
+  *rP = nullptr;
+  use(r);
+}