Merge pull request github#3577 from dbartol/github/codeql-c-analysis-team/69

C++: Fix `MemoryLocation` with multiple `VirtualVariables`

Author: MathiasVP

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

@@ -90,7 +90,7 @@ class IndirectParameterAllocation extends Allocation, TIndirectParameterAllocati
 
   final override string getUniqueId() { result = var.getUniqueId() }
 
-  final override IRType getIRType() { result = var.getIRType() }
+  final override IRType getIRType() { result instanceof IRUnknownType }
 
   final override predicate isReadOnly() { none() }
 

cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSAConstruction.qll

@@ -913,6 +913,9 @@ private module CachedForDebugging {
 }
 
 module SSAConsistency {
+  /**
+   * Holds if a `MemoryOperand` has more than one `MemoryLocation` assigned by alias analysis.
+   */
   query predicate multipleOperandMemoryLocations(
     OldIR::MemoryOperand operand, string message, OldIR::IRFunction func, string funcText
   ) {
@@ -925,6 +928,9 @@ module SSAConsistency {
     )
   }
 
+  /**
+   * Holds if a `MemoryLocation` does not have an associated `VirtualVariable`.
+   */
   query predicate missingVirtualVariableForMemoryLocation(
     Alias::MemoryLocation location, string message, OldIR::IRFunction func, string funcText
   ) {
@@ -933,4 +939,25 @@ module SSAConsistency {
     funcText = Language::getIdentityString(func.getFunction()) and
     message = "Memory location has no virtual variable in function '$@'."
   }
+
+  /**
+   * Holds if a `MemoryLocation` is a member of more than one `VirtualVariable`.
+   */
+  query predicate multipleVirtualVariablesForMemoryLocation(
+    Alias::MemoryLocation location, string message, OldIR::IRFunction func, string funcText
+  ) {
+    exists(int vvarCount |
+      vvarCount = strictcount(location.getVirtualVariable()) and
+      vvarCount > 1 and
+      func = location.getIRFunction() and
+      funcText = Language::getIdentityString(func.getFunction()) and
+      message =
+        "Memory location has " + vvarCount.toString() + " virtual variables in function '$@': (" +
+          concat(Alias::VirtualVariable vvar |
+            vvar = location.getVirtualVariable()
+          |
+            vvar.toString(), ", "
+          ) + ")."
+    )
+  }
 }

cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSAConstruction.qll

@@ -913,6 +913,9 @@ private module CachedForDebugging {
 }
 
 module SSAConsistency {
+  /**
+   * Holds if a `MemoryOperand` has more than one `MemoryLocation` assigned by alias analysis.
+   */
   query predicate multipleOperandMemoryLocations(
     OldIR::MemoryOperand operand, string message, OldIR::IRFunction func, string funcText
   ) {
@@ -925,6 +928,9 @@ module SSAConsistency {
     )
   }
 
+  /**
+   * Holds if a `MemoryLocation` does not have an associated `VirtualVariable`.
+   */
   query predicate missingVirtualVariableForMemoryLocation(
     Alias::MemoryLocation location, string message, OldIR::IRFunction func, string funcText
   ) {
@@ -933,4 +939,25 @@ module SSAConsistency {
     funcText = Language::getIdentityString(func.getFunction()) and
     message = "Memory location has no virtual variable in function '$@'."
   }
+
+  /**
+   * Holds if a `MemoryLocation` is a member of more than one `VirtualVariable`.
+   */
+  query predicate multipleVirtualVariablesForMemoryLocation(
+    Alias::MemoryLocation location, string message, OldIR::IRFunction func, string funcText
+  ) {
+    exists(int vvarCount |
+      vvarCount = strictcount(location.getVirtualVariable()) and
+      vvarCount > 1 and
+      func = location.getIRFunction() and
+      funcText = Language::getIdentityString(func.getFunction()) and
+      message =
+        "Memory location has " + vvarCount.toString() + " virtual variables in function '$@': (" +
+          concat(Alias::VirtualVariable vvar |
+            vvar = location.getVirtualVariable()
+          |
+            vvar.toString(), ", "
+          ) + ")."
+    )
+  }
 }

cpp/ql/test/library-tests/dataflow/fields/dataflow-ir-consistency.expected

@@ -1,8 +1,5 @@
 uniqueEnclosingCallable
 uniqueTypeBound
-| by_reference.cpp:106:21:106:41 | Chi | Node should have one type bound but has 2. |
-| by_reference.cpp:126:21:126:40 | Chi | Node should have one type bound but has 2. |
-| file://:0:0:0:0 | Chi | Node should have one type bound but has 2. |
 uniqueTypeRepr
 uniqueNodeLocation
 | D.cpp:1:17:1:17 | o | Node should have one location but has 3. |

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

@@ -1,2 +1,3 @@
 multipleOperandMemoryLocations
 missingVirtualVariableForMemoryLocation
+multipleVirtualVariablesForMemoryLocation

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

@@ -1,2 +1,3 @@
 multipleOperandMemoryLocations
 missingVirtualVariableForMemoryLocation
+multipleVirtualVariablesForMemoryLocation

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

@@ -1,2 +1,3 @@
 multipleOperandMemoryLocations
 missingVirtualVariableForMemoryLocation
+multipleVirtualVariablesForMemoryLocation

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

@@ -1,2 +1,3 @@
 multipleOperandMemoryLocations
 missingVirtualVariableForMemoryLocation
+multipleVirtualVariablesForMemoryLocation

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

@@ -20,7 +20,6 @@ switchInstructionWithoutDefaultEdge
 notMarkedAsConflated
 wronglyMarkedAsConflated
 invalidOverlap
-| ssa.cpp:301:27:301:30 | SideEffect | MemoryOperand 'SideEffect' has a `getDefinitionOverlap()` of 'MayPartiallyOverlap'. | ssa.cpp:301:5:301:8 | IR: main | int main(int, char**) |
 missingCanonicalLanguageType
 multipleCanonicalLanguageTypes
 missingIRType

cpp/ql/test/library-tests/ir/ssa/aliased_ssa_ir_unsound.expected

@@ -1441,7 +1441,7 @@ ssa.cpp:
 #  302|     m302_8(unknown)        = Chi                              : total:m301_4, partial:m302_7
 #  302|     v302_9(void)           = ^BufferReadSideEffect[1]         : &:r302_5, ~m301_10
 #  302|     m302_10(unknown)       = ^BufferMayWriteSideEffect[1]     : &:r302_5
-#  302|     m302_11(char *)        = Chi                              : total:m301_10, partial:m302_10
+#  302|     m302_11(unknown)       = Chi                              : total:m301_10, partial:m302_10
 #  303|     r303_1(glval<unknown>) = FunctionAddress[unknownFunction] : 
 #  303|     r303_2(glval<int>)     = VariableAddress[argc]            : 
 #  303|     r303_3(int)            = Load                             : &:r303_2, m301_6
@@ -1452,7 +1452,7 @@ ssa.cpp:
 #  303|     m303_8(unknown)        = Chi                              : total:m302_8, partial:m303_7
 #  303|     v303_9(void)           = ^BufferReadSideEffect[1]         : &:r303_5, ~m302_11
 #  303|     m303_10(unknown)       = ^BufferMayWriteSideEffect[1]     : &:r303_5
-#  303|     m303_11(char *)        = Chi                              : total:m302_11, partial:m303_10
+#  303|     m303_11(unknown)       = Chi                              : total:m302_11, partial:m303_10
 #  304|     r304_1(glval<int>)     = VariableAddress[#return]         : 
 #  304|     r304_2(glval<char **>) = VariableAddress[argv]            : 
 #  304|     r304_3(char **)        = Load                             : &:r304_2, m301_8