Merge pull request #2268 from dave-bartolomeo/dbartol/StringLiteralAlias

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

Author: Robert Marsh

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() }
 
@@ -234,8 +241,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
@@ -266,12 +279,6 @@ private module Cached {
     )
   }
 
-  cached
-  StringLiteral getInstructionStringLiteral(Instruction instruction) {
-    result = getInstructionTranslatedElement(instruction)
-          .getInstructionStringLiteral(getInstructionTag(instruction))
-  }
-
   cached
   BuiltInOperation getInstructionBuiltInOperation(Instruction instruction) {
     result = getInstructionTranslatedElement(instruction)