C++/C#: Fix missing virtual variables

The aliased SSA code was assuming that, for every automatic variable, there would be at least one memory access that reads or writes the entire variable. We've encountered a couple cases where that isn't true due to extractor issues. As a workaround, we now always create the `VariableMemoryLocation` for every local variable.

I've also added a sanity test to detect this condition in the future.

Along the way, I had to fix a perf issue in the PrintIR code. When determining the ID of a result based on line number, we were considering all `Instruction`s generated for a particular line, regardless of whether they were all in the same `IRFunction`. In addition, the predicate had what appeared to be a bad join order that made it take forever on large snapshots. I've scoped it down to just consider `Instruction`s in the same function, and outlined that predicate to fix the join order issue. This causes some numbering changes, but they're for the better. I don't think there was actually any nondeterminism there before, but now the numbering won't depend on the number of instantiations of a template, either.

Author: Dave Bartolomeo

cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Instruction.qll

@@ -260,6 +260,19 @@ module InstructionSanity {
   }
 }
 
+/**
+ * Gets an `Instruction` that is contained in `IRFunction`, and has a location with the specified
+ * `File` and line number. Used for assigning register names when printing IR.
+ */
+private Instruction getAnInstructionAtLine(IRFunction irFunc, Language::File file, int line) {
+  exists(Language::Location location |
+    irFunc = result.getEnclosingIRFunction() and
+    location = result.getLocation() and
+    file = location.getFile() and
+    line = location.getStartLine()
+  )
+}
+
 /**
  * Represents a single operation in the IR.
  */
@@ -324,8 +337,8 @@ class Instruction extends Construction::TInstruction {
 
   private int getLineRank() {
     this = rank[result](Instruction instr |
-        instr.getAST().getFile() = getAST().getFile() and
-        instr.getAST().getLocation().getStartLine() = getAST().getLocation().getStartLine()
+        instr = getAnInstructionAtLine(getEnclosingIRFunction(), getLocation().getFile(),
+            getLocation().getStartLine())
       |
         instr order by instr.getBlock().getDisplayIndex(), instr.getDisplayIndexInBlock()
       )

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

@@ -41,8 +41,18 @@ private newtype TMemoryLocation =
     IntValue endBitOffset, boolean isMayAccess
   ) {
     (
-      hasResultMemoryAccess(_, var, type, _, startBitOffset, endBitOffset, isMayAccess) or
+      hasResultMemoryAccess(_, var, type, _, startBitOffset, endBitOffset, isMayAccess)
+      or
       hasOperandMemoryAccess(_, var, type, _, startBitOffset, endBitOffset, isMayAccess)
+      or
+      exists(IRAutomaticVariable autoVar |
+        // Always create a memory location for the entire variable.
+        autoVar = var and
+        type = autoVar.getIRType() and
+        startBitOffset = 0 and
+        endBitOffset = type.getByteSize() * 8 and
+        isMayAccess = false
+      )
     ) and
     languageType = type.getCanonicalLanguageType()
   } or
@@ -78,6 +88,8 @@ abstract class MemoryLocation extends TMemoryLocation {
 
   abstract IRFunction getIRFunction();
 
+  abstract Location getLocation();
+
   final IRType getIRType() { result = getType().getIRType() }
 
   abstract predicate isMayAccess();
@@ -141,6 +153,8 @@ class VariableMemoryLocation extends TVariableMemoryLocation, MemoryLocation {
 
   final override IRFunction getIRFunction() { result = var.getEnclosingIRFunction() }
 
+  final override Location getLocation() { result = var.getLocation() }
+
   final IntValue getStartBitOffset() { result = startBitOffset }
 
   final IntValue getEndBitOffset() { result = endBitOffset }
@@ -208,6 +222,8 @@ class UnknownMemoryLocation extends TUnknownMemoryLocation, MemoryLocation {
 
   final override IRFunction getIRFunction() { result = irFunc }
 
+  final override Location getLocation() { result = irFunc.getLocation() }
+
   final override string getUniqueId() { result = "{Unknown}" }
 
   final override predicate isMayAccess() { isMayAccess = true }
@@ -233,6 +249,8 @@ class AllNonLocalMemory extends TAllNonLocalMemory, MemoryLocation {
 
   final override IRFunction getIRFunction() { result = irFunc }
 
+  final override Location getLocation() { result = irFunc.getLocation() }
+
   final override string getUniqueId() { result = "{AllNonLocal}" }
 
   final override predicate isMayAccess() { isMayAccess = true }
@@ -255,6 +273,8 @@ class AllAliasedMemory extends TAllAliasedMemory, MemoryLocation {
 
   final override IRFunction getIRFunction() { result = irFunc }
 
+  final override Location getLocation() { result = irFunc.getLocation() }
+
   final override string getUniqueId() { result = " " + toString() }
 
   final override VirtualVariable getVirtualVariable() { result = TAllAliasedMemory(irFunc, false) }

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

@@ -885,4 +885,13 @@ module SSASanity {
       message = "Operand has " + locationCount.toString() + " memory accesses in function '$@'."
     )
   }
+
+  query predicate missingVirtualVariableForMemoryLocation(
+    Alias::MemoryLocation location, string message, OldIR::IRFunction func, string funcText
+  ) {
+    not exists(location.getVirtualVariable()) and
+    func = location.getIRFunction() and
+    funcText = Language::getIdentityString(func.getFunction()) and
+    message = "Memory location has no virtual variable in function '$@'."
+  }
 }

cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Instruction.qll

@@ -260,6 +260,19 @@ module InstructionSanity {
   }
 }
 
+/**
+ * Gets an `Instruction` that is contained in `IRFunction`, and has a location with the specified
+ * `File` and line number. Used for assigning register names when printing IR.
+ */
+private Instruction getAnInstructionAtLine(IRFunction irFunc, Language::File file, int line) {
+  exists(Language::Location location |
+    irFunc = result.getEnclosingIRFunction() and
+    location = result.getLocation() and
+    file = location.getFile() and
+    line = location.getStartLine()
+  )
+}
+
 /**
  * Represents a single operation in the IR.
  */
@@ -324,8 +337,8 @@ class Instruction extends Construction::TInstruction {
 
   private int getLineRank() {
     this = rank[result](Instruction instr |
-        instr.getAST().getFile() = getAST().getFile() and
-        instr.getAST().getLocation().getStartLine() = getAST().getLocation().getStartLine()
+        instr = getAnInstructionAtLine(getEnclosingIRFunction(), getLocation().getFile(),
+            getLocation().getStartLine())
       |
         instr order by instr.getBlock().getDisplayIndex(), instr.getDisplayIndexInBlock()
       )

cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/Instruction.qll

@@ -260,6 +260,19 @@ module InstructionSanity {
   }
 }
 
+/**
+ * Gets an `Instruction` that is contained in `IRFunction`, and has a location with the specified
+ * `File` and line number. Used for assigning register names when printing IR.
+ */
+private Instruction getAnInstructionAtLine(IRFunction irFunc, Language::File file, int line) {
+  exists(Language::Location location |
+    irFunc = result.getEnclosingIRFunction() and
+    location = result.getLocation() and
+    file = location.getFile() and
+    line = location.getStartLine()
+  )
+}
+
 /**
  * Represents a single operation in the IR.
  */
@@ -324,8 +337,8 @@ class Instruction extends Construction::TInstruction {
 
   private int getLineRank() {
     this = rank[result](Instruction instr |
-        instr.getAST().getFile() = getAST().getFile() and
-        instr.getAST().getLocation().getStartLine() = getAST().getLocation().getStartLine()
+        instr = getAnInstructionAtLine(getEnclosingIRFunction(), getLocation().getFile(),
+            getLocation().getStartLine())
       |
         instr order by instr.getBlock().getDisplayIndex(), instr.getDisplayIndexInBlock()
       )

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

@@ -885,4 +885,13 @@ module SSASanity {
       message = "Operand has " + locationCount.toString() + " memory accesses in function '$@'."
     )
   }
+
+  query predicate missingVirtualVariableForMemoryLocation(
+    Alias::MemoryLocation location, string message, OldIR::IRFunction func, string funcText
+  ) {
+    not exists(location.getVirtualVariable()) and
+    func = location.getIRFunction() and
+    funcText = Language::getIdentityString(func.getFunction()) and
+    message = "Memory location has no virtual variable in function '$@'."
+  }
 }

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

@@ -55,6 +55,10 @@ class MemoryLocation extends TMemoryLocation {
 
   final string toString() { result = var.toString() }
 
+  final Language::Location getLocation() { result = var.getLocation() }
+
+  final IRFunction getIRFunction() { result = var.getEnclosingIRFunction() }
+
   final IRVariable getIRVariable() { result = var }
 
   final VirtualVariable getVirtualVariable() { result = this }

cpp/ql/src/semmle/code/cpp/ir/internal/IRCppLanguage.qll

@@ -13,6 +13,8 @@ class Function = Cpp::Function;
 
 class Location = Cpp::Location;
 
+class File = Cpp::File;
+
 class AST = Cpp::Locatable;
 
 class Type = Cpp::Type;

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

@@ -0,0 +1,2 @@
+multipleOperandMemoryLocations
+missingVirtualVariableForMemoryLocation