C++/C#: Treat string literals like read-only global variables for alias purposes.

Previously, we didn't track string literals as known memory locations at all, so they all just got marked as `UnknownMemoryLocation`, just like an aribtrary read from a random pointer. This led to some confusing def-use chains, where it would look like the contents of a string literal were being written to by the side effect of an earlier function call, which of course is impossible.

To fix this, I've made two changes. First, each string literal is now given a corresponding `IRVariable` (specifically `IRStringLiteral`), since a string literal behaves more or less as a read-only global variable. Second, the `IRVariable` for each string literal is now marked `isReadOnly()`, which the alias analysis uses to determine that an arbitrary write to aliased memory will not overwrite the contents of a string literal.

I originally planned to treat all string literals with the same value as being the same memory location, since this is the usual behavior of modern compilers. However, this made implementing `IRVariable.getAST()` tricky for string literals, so I left them unpooled.

Author: Dave Bartolomeo

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

@@ -22,6 +22,12 @@ abstract class IRVariable extends TIRVariable {
 
   abstract string toString();
 
+  /**
+   * Holds if this variable's value cannot be changed within a function. Currently used for string
+   * literals, but could also apply to `const` global and static variables.
+   */
+  predicate isReadOnly() { none() }
+
   /**
    * Gets the type of the variable.
    */
@@ -113,34 +119,43 @@ class IRStaticUserVariable extends IRUserVariable {
   final override Language::StaticVariable getVariable() { result = var }
 }
 
+abstract class IRGeneratedVariable extends IRVariable {
+  Language::AST ast;
+  Language::LanguageType type;
+
+  final override Language::LanguageType getLanguageType() { result = type }
+
+  final override Language::AST getAST() { result = ast }
+
+  override string toString() { result = getBaseString() + getLocationString() }
+
+  override string getUniqueId() { none() }
+
+  final string getLocationString() {
+    result = ast.getLocation().getStartLine().toString() + ":" +
+        ast.getLocation().getStartColumn().toString()
+  }
+
+  string getBaseString() { none() }
+}
+
 IRTempVariable getIRTempVariable(Language::AST ast, TempVariableTag tag) {
   result.getAST() = ast and
   result.getTag() = tag
 }
 
-class IRTempVariable extends IRVariable, IRAutomaticVariable, TIRTempVariable {
-  Language::AST ast;
+class IRTempVariable extends IRGeneratedVariable, IRAutomaticVariable, TIRTempVariable {
   TempVariableTag tag;
-  Language::LanguageType type;
 
   IRTempVariable() { this = TIRTempVariable(func, ast, tag, type) }
 
-  final override Language::LanguageType getLanguageType() { result = type }
-
-  final override Language::AST getAST() { result = ast }
-
   final override string getUniqueId() {
     result = "Temp: " + Construction::getTempVariableUniqueId(this)
   }
 
   final TempVariableTag getTag() { result = tag }
 
-  override string toString() {
-    result = getBaseString() + ast.getLocation().getStartLine().toString() + ":" +
-        ast.getLocation().getStartColumn().toString()
-  }
-
-  string getBaseString() { result = "#temp" }
+  override string getBaseString() { result = "#temp" }
 }
 
 class IRReturnVariable extends IRTempVariable {
@@ -154,3 +169,19 @@ class IRThrowVariable extends IRTempVariable {
 
   override string getBaseString() { result = "#throw" }
 }
+
+class IRStringLiteral extends IRGeneratedVariable, TIRStringLiteral {
+  Language::StringLiteral literal;
+
+  IRStringLiteral() { this = TIRStringLiteral(func, ast, type, literal) }
+
+  final override predicate isReadOnly() { any() }
+
+  final override string getUniqueId() {
+    result = "String: " + getLocationString() + "=" + Language::getStringLiteralText(literal)
+  }
+
+  override string getBaseString() { result = "#string" }
+
+  final Language::StringLiteral getLiteral() { result = literal }
+}

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

@@ -808,14 +808,12 @@ class FloatConstantInstruction extends ConstantInstruction {
   FloatConstantInstruction() { getResultType() instanceof Language::FloatingPointType }
 }
 
-class StringConstantInstruction extends Instruction {
-  Language::StringLiteral value;
+class StringConstantInstruction extends VariableInstruction {
+  override IRStringLiteral var;
 
-  StringConstantInstruction() { value = Construction::getInstructionStringLiteral(this) }
+  final override string getImmediateString() { result = Language::getStringLiteralText(getValue()) }
 
-  final override string getImmediateString() { result = Language::getStringLiteralText(value) }
-
-  final Language::StringLiteral getValue() { result = value }
+  final Language::StringLiteral getValue() { result = var.getLiteral() }
 }
 
 class BinaryInstruction extends Instruction {

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

@@ -308,6 +308,10 @@ predicate resultPointsTo(Instruction instr, IRVariable var, IntValue bitOffset)
   instr.(VariableAddressInstruction).getIRVariable() = var and
   bitOffset = 0
   or
+  // A string literal is just a special read-only global variable.
+  instr.(StringConstantInstruction).getIRVariable() = var and
+  bitOffset = 0
+  or
   exists(Operand operand, IntValue originalBitOffset, IntValue propagatedBitOffset |
     operand = instr.getAnOperand() and
     // If an operand is propagated, then the result points to the same variable,

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

@@ -210,17 +210,21 @@ Overlap getOverlap(MemoryLocation def, MemoryLocation use) {
     def instanceof UnknownVirtualVariable and
     result instanceof MustTotallyOverlap
     or
-    // An UnknownMemoryLocation may partially overlap any Location within the same virtual variable.
+    // An UnknownMemoryLocation may partially overlap any Location within the same virtual variable,
+    // unless the location is read-only.
     def.getVirtualVariable() = use.getVirtualVariable() and
     def instanceof UnknownMemoryLocation and
-    result instanceof MayPartiallyOverlap
+    result instanceof MayPartiallyOverlap and
+    not use.(VariableMemoryLocation).getVariable().isReadOnly()
     or
     // An UnknownNonLocalMemoryLocation may partially overlap any location within the same virtual
-    // variable, except a local variable.
+    // variable, except a local variable or read-only variable.
     def.getVirtualVariable() = use.getVirtualVariable() and
     def instanceof UnknownNonLocalMemoryLocation and
     result instanceof MayPartiallyOverlap and
-    not use.(VariableMemoryLocation).getVariable() instanceof IRAutomaticVariable
+    not exists(IRVariable var | var = use.(VariableMemoryLocation).getVariable() |
+      var instanceof IRAutomaticVariable or var.isReadOnly()
+    )
     or
     exists(VariableMemoryLocation defVariableLocation |
       defVariableLocation = def and

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

@@ -341,11 +341,6 @@ private module Cached {
     result = getOldInstruction(instruction).(OldIR::ConstantValueInstruction).getValue()
   }
 
-  cached
-  Language::StringLiteral getInstructionStringLiteral(Instruction instruction) {
-    result = getOldInstruction(instruction).(OldIR::StringConstantInstruction).getValue()
-  }
-
   cached
   Language::BuiltInOperation getInstructionBuiltInOperation(Instruction instruction) {
     result = getOldInstruction(instruction)

cpp/ql/src/semmle/code/cpp/ir/implementation/internal/TIRVariable.qll

@@ -9,4 +9,10 @@ newtype TIRVariable =
     Language::Function func, Language::AST ast, TempVariableTag tag, Language::LanguageType type
   ) {
     Construction::hasTempVariable(func, ast, tag, type)
+  } or
+  TIRStringLiteral(
+    Language::Function func, Language::AST ast, Language::LanguageType type,
+    Language::StringLiteral literal
+  ) {
+    Construction::hasStringLiteral(func, ast, type, literal)
   }

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

@@ -22,6 +22,12 @@ abstract class IRVariable extends TIRVariable {
 
   abstract string toString();
 
+  /**
+   * Holds if this variable's value cannot be changed within a function. Currently used for string
+   * literals, but could also apply to `const` global and static variables.
+   */
+  predicate isReadOnly() { none() }
+
   /**
    * Gets the type of the variable.
    */
@@ -113,34 +119,43 @@ class IRStaticUserVariable extends IRUserVariable {
   final override Language::StaticVariable getVariable() { result = var }
 }
 
+abstract class IRGeneratedVariable extends IRVariable {
+  Language::AST ast;
+  Language::LanguageType type;
+
+  final override Language::LanguageType getLanguageType() { result = type }
+
+  final override Language::AST getAST() { result = ast }
+
+  override string toString() { result = getBaseString() + getLocationString() }
+
+  override string getUniqueId() { none() }
+
+  final string getLocationString() {
+    result = ast.getLocation().getStartLine().toString() + ":" +
+        ast.getLocation().getStartColumn().toString()
+  }
+
+  string getBaseString() { none() }
+}
+
 IRTempVariable getIRTempVariable(Language::AST ast, TempVariableTag tag) {
   result.getAST() = ast and
   result.getTag() = tag
 }
 
-class IRTempVariable extends IRVariable, IRAutomaticVariable, TIRTempVariable {
-  Language::AST ast;
+class IRTempVariable extends IRGeneratedVariable, IRAutomaticVariable, TIRTempVariable {
   TempVariableTag tag;
-  Language::LanguageType type;
 
   IRTempVariable() { this = TIRTempVariable(func, ast, tag, type) }
 
-  final override Language::LanguageType getLanguageType() { result = type }
-
-  final override Language::AST getAST() { result = ast }
-
   final override string getUniqueId() {
     result = "Temp: " + Construction::getTempVariableUniqueId(this)
   }
 
   final TempVariableTag getTag() { result = tag }
 
-  override string toString() {
-    result = getBaseString() + ast.getLocation().getStartLine().toString() + ":" +
-        ast.getLocation().getStartColumn().toString()
-  }
-
-  string getBaseString() { result = "#temp" }
+  override string getBaseString() { result = "#temp" }
 }
 
 class IRReturnVariable extends IRTempVariable {
@@ -154,3 +169,19 @@ class IRThrowVariable extends IRTempVariable {
 
   override string getBaseString() { result = "#throw" }
 }
+
+class IRStringLiteral extends IRGeneratedVariable, TIRStringLiteral {
+  Language::StringLiteral literal;
+
+  IRStringLiteral() { this = TIRStringLiteral(func, ast, type, literal) }
+
+  final override predicate isReadOnly() { any() }
+
+  final override string getUniqueId() {
+    result = "String: " + getLocationString() + "=" + Language::getStringLiteralText(literal)
+  }
+
+  override string getBaseString() { result = "#string" }
+
+  final Language::StringLiteral getLiteral() { result = literal }
+}

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

@@ -808,14 +808,12 @@ class FloatConstantInstruction extends ConstantInstruction {
   FloatConstantInstruction() { getResultType() instanceof Language::FloatingPointType }
 }
 
-class StringConstantInstruction extends Instruction {
-  Language::StringLiteral value;
+class StringConstantInstruction extends VariableInstruction {
+  override IRStringLiteral var;
 
-  StringConstantInstruction() { value = Construction::getInstructionStringLiteral(this) }
+  final override string getImmediateString() { result = Language::getStringLiteralText(getValue()) }
 
-  final override string getImmediateString() { result = Language::getStringLiteralText(value) }
-
-  final Language::StringLiteral getValue() { result = value }
+  final Language::StringLiteral getValue() { result = var.getLiteral() }
 }
 
 class BinaryInstruction extends Instruction {

cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/IRConstruction.qll

@@ -44,6 +44,13 @@ private module Cached {
     )
   }
 
+  cached
+  predicate hasStringLiteral(Function func, Locatable ast, CppType type, StringLiteral literal) {
+    literal = ast and
+    literal.getEnclosingFunction() = func and
+    getTypeForPRValue(literal.getType()) = type
+  }
+
   cached
   predicate hasModeledMemoryResult(Instruction instruction) { none() }
 
@@ -231,8 +238,14 @@ private module Cached {
 
   cached
   IRVariable getInstructionVariable(Instruction instruction) {
-    result = getInstructionTranslatedElement(instruction)
-          .getInstructionVariable(getInstructionTag(instruction))
+    exists(TranslatedElement element, InstructionTag tag |
+      element = getInstructionTranslatedElement(instruction) and
+      tag = getInstructionTag(instruction) and
+      (
+        result = element.getInstructionVariable(tag) or
+        result.(IRStringLiteral).getAST() = element.getInstructionStringLiteral(tag)
+      )
+    )
   }
 
   cached
@@ -263,12 +276,6 @@ private module Cached {
     )
   }
 
-  cached
-  StringLiteral getInstructionStringLiteral(Instruction instruction) {
-    result = getInstructionTranslatedElement(instruction)
-          .getInstructionStringLiteral(getInstructionTag(instruction))
-  }
-
   cached
   BuiltInOperation getInstructionBuiltInOperation(Instruction instruction) {
     result = getInstructionTranslatedElement(instruction)