Merge pull request #2154 from rdmarsh2/rdmarsh/cpp/ir-callee-side-effects

C++: add InitializeIndirection for pointer params

Author: Dave Bartolomeo

cpp/ql/src/semmle/code/cpp/ir/implementation/Opcode.qll

@@ -3,11 +3,13 @@ private newtype TOpcode =
   TUninitialized() or
   TError() or
   TInitializeParameter() or
+  TInitializeIndirection() or
   TInitializeThis() or
   TEnterFunction() or
   TExitFunction() or
   TReturnValue() or
   TReturnVoid() or
+  TReturnIndirection() or
   TCopyValue() or
   TLoad() or
   TStore() or
@@ -180,6 +182,10 @@ module Opcode {
     final override string toString() { result = "InitializeParameter" }
   }
 
+  class InitializeIndirection extends MemoryAccessOpcode, TInitializeIndirection {
+    final override string toString() { result = "InitializeIndirection" }
+  }
+
   class InitializeThis extends Opcode, TInitializeThis {
     final override string toString() { result = "InitializeThis" }
   }
@@ -200,6 +206,10 @@ module Opcode {
     final override string toString() { result = "ReturnVoid" }
   }
 
+  class ReturnIndirection extends MemoryAccessOpcode, TReturnIndirection {
+    final override string toString() { result = "ReturnIndirection" }
+  }
+
   class CopyValue extends UnaryOpcode, CopyOpcode, TCopyValue {
     final override string toString() { result = "CopyValue" }
   }

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

@@ -51,6 +51,7 @@ module InstructionSanity {
           opcode instanceof ReadSideEffectOpcode or
           opcode instanceof Opcode::InlineAsm or
           opcode instanceof Opcode::CallSideEffect or
+          opcode instanceof Opcode::ReturnIndirection or
           opcode instanceof Opcode::AliasedUse
         ) and
         tag instanceof SideEffectOperandTag
@@ -713,6 +714,14 @@ class InitializeParameterInstruction extends VariableInstruction {
   final override MemoryAccessKind getResultMemoryAccess() { result instanceof IndirectMemoryAccess }
 }
 
+class InitializeIndirectionInstruction extends VariableInstruction {
+  InitializeIndirectionInstruction() { getOpcode() instanceof Opcode::InitializeIndirection }
+
+  final Language::Parameter getParameter() { result = var.(IRUserVariable).getVariable() }
+
+  final override MemoryAccessKind getResultMemoryAccess() { result instanceof IndirectMemoryAccess }
+}
+
 /**
  * An instruction that initializes the `this` pointer parameter of the enclosing function.
  */
@@ -773,6 +782,18 @@ class ReturnValueInstruction extends ReturnInstruction {
   final Instruction getReturnValue() { result = getReturnValueOperand().getDef() }
 }
 
+class ReturnIndirectionInstruction extends Instruction {
+  ReturnIndirectionInstruction() { getOpcode() instanceof Opcode::ReturnIndirection }
+
+  final SideEffectOperand getSideEffectOperand() { result = getAnOperand() }
+
+  final Instruction getSideEffect() { result = getSideEffectOperand().getDef() }
+
+  final AddressOperand getSourceAddressOperand() { result = getAnOperand() }
+
+  final Instruction getSourceAddress() { result = getSourceAddressOperand().getDef() }
+}
+
 class CopyInstruction extends Instruction {
   CopyInstruction() { getOpcode() instanceof CopyOpcode }
 

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

@@ -410,6 +410,9 @@ class SideEffectOperand extends TypedOperand {
     or
     useInstr instanceof BufferMayWriteSideEffectInstruction and
     result instanceof BufferMemoryAccess
+    or
+    useInstr instanceof ReturnIndirectionInstruction and
+    result instanceof BufferMemoryAccess
   }
 
   final override predicate hasMayMemoryAccess() {

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

@@ -49,6 +49,7 @@ private predicate operandEscapesDomain(Operand operand) {
   not isArgumentForParameter(_, operand, _) and
   not isOnlyEscapesViaReturnArgument(operand) and
   not operand.getUse() instanceof ReturnValueInstruction and
+  not operand.getUse() instanceof ReturnIndirectionInstruction and
   not operand instanceof PhiInputOperand
 }
 

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

@@ -51,6 +51,7 @@ module InstructionSanity {
           opcode instanceof ReadSideEffectOpcode or
           opcode instanceof Opcode::InlineAsm or
           opcode instanceof Opcode::CallSideEffect or
+          opcode instanceof Opcode::ReturnIndirection or
           opcode instanceof Opcode::AliasedUse
         ) and
         tag instanceof SideEffectOperandTag
@@ -713,6 +714,14 @@ class InitializeParameterInstruction extends VariableInstruction {
   final override MemoryAccessKind getResultMemoryAccess() { result instanceof IndirectMemoryAccess }
 }
 
+class InitializeIndirectionInstruction extends VariableInstruction {
+  InitializeIndirectionInstruction() { getOpcode() instanceof Opcode::InitializeIndirection }
+
+  final Language::Parameter getParameter() { result = var.(IRUserVariable).getVariable() }
+
+  final override MemoryAccessKind getResultMemoryAccess() { result instanceof IndirectMemoryAccess }
+}
+
 /**
  * An instruction that initializes the `this` pointer parameter of the enclosing function.
  */
@@ -773,6 +782,18 @@ class ReturnValueInstruction extends ReturnInstruction {
   final Instruction getReturnValue() { result = getReturnValueOperand().getDef() }
 }
 
+class ReturnIndirectionInstruction extends Instruction {
+  ReturnIndirectionInstruction() { getOpcode() instanceof Opcode::ReturnIndirection }
+
+  final SideEffectOperand getSideEffectOperand() { result = getAnOperand() }
+
+  final Instruction getSideEffect() { result = getSideEffectOperand().getDef() }
+
+  final AddressOperand getSourceAddressOperand() { result = getAnOperand() }
+
+  final Instruction getSourceAddress() { result = getSourceAddressOperand().getDef() }
+}
+
 class CopyInstruction extends Instruction {
   CopyInstruction() { getOpcode() instanceof CopyOpcode }
 

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

@@ -410,6 +410,9 @@ class SideEffectOperand extends TypedOperand {
     or
     useInstr instanceof BufferMayWriteSideEffectInstruction and
     result instanceof BufferMemoryAccess
+    or
+    useInstr instanceof ReturnIndirectionInstruction and
+    result instanceof BufferMemoryAccess
   }
 
   final override predicate hasMayMemoryAccess() {

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

@@ -6,6 +6,8 @@ newtype TInstructionTag =
   InitializerVariableAddressTag() or
   InitializerLoadStringTag() or
   InitializerStoreTag() or
+  InitializerIndirectAddressTag() or
+  InitializerIndirectStoreTag() or
   ZeroPadStringConstantTag() or
   ZeroPadStringElementIndexTag() or
   ZeroPadStringElementAddressTag() or
@@ -80,6 +82,10 @@ string getInstructionTagId(TInstructionTag tag) {
   or
   tag = InitializerUninitializedTag() and result = "InitUninit"
   or
+  tag = InitializerIndirectAddressTag() and result = "InitIndirectAddr"
+  or
+  tag = InitializerIndirectStoreTag() and result = "InitIndirectStore"
+  or
   tag = ZeroPadStringConstantTag() and result = "ZeroPadConst"
   or
   tag = ZeroPadStringElementIndexTag() and result = "ZeroPadElemIndex"

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

@@ -355,6 +355,16 @@ newtype TTranslatedElement =
       translateFunction(func)
     )
   } or
+  TTranslatedReadEffects(Function func) { translateFunction(func) } or
+  // The read side effects in a function's return block
+  TTranslatedReadEffect(Parameter param) {
+    translateFunction(param.getFunction()) and
+    exists(Type t | t = param.getUnspecifiedType() |
+      t instanceof ArrayType or
+      t instanceof PointerType or
+      t instanceof ReferenceType
+    )
+  } or
   // A local declaration
   TTranslatedDeclarationEntry(DeclarationEntry entry) {
     exists(DeclStmt declStmt |

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

@@ -35,6 +35,8 @@ class TranslatedFunction extends TranslatedElement, TTranslatedFunction {
   final override Function getFunction() { result = func }
 
   final override TranslatedElement getChild(int id) {
+    id = -4 and result = getReadEffects()
+    or
     id = -3 and result = getConstructorInitList()
     or
     id = -2 and result = getBody()
@@ -54,6 +56,8 @@ class TranslatedFunction extends TranslatedElement, TTranslatedFunction {
 
   final private TranslatedStmt getBody() { result = getTranslatedStmt(func.getEntryPoint()) }
 
+  final private TranslatedReadEffects getReadEffects() { result = getTranslatedReadEffects(func) }
+
   final private TranslatedParameter getParameter(int index) {
     result = getTranslatedParameter(func.getParameter(index))
   }
@@ -117,12 +121,13 @@ class TranslatedFunction extends TranslatedElement, TTranslatedFunction {
     child = getBody() and
     result = getReturnSuccessorInstruction()
     or
-    (
-      child = getDestructorDestructionList() and
-      if hasReturnValue()
-      then result = getInstruction(ReturnValueAddressTag())
-      else result = getInstruction(ReturnTag())
-    )
+    child = getDestructorDestructionList() and
+    result = getReadEffects().getFirstInstruction()
+    or
+    child = getReadEffects() and
+    if hasReturnValue()
+    then result = getInstruction(ReturnValueAddressTag())
+    else result = getInstruction(ReturnTag())
   }
 
   final override predicate hasInstruction(Opcode opcode, InstructionTag tag, CppType resultType) {
@@ -339,6 +344,14 @@ class TranslatedParameter extends TranslatedElement, TTranslatedParameter {
       result = getInstruction(InitializerStoreTag())
       or
       tag = InitializerStoreTag() and
+      if hasIndirection()
+      then result = getInstruction(InitializerIndirectAddressTag())
+      else result = getParent().getChildSuccessor(this)
+      or
+      tag = InitializerIndirectAddressTag() and
+      result = getInstruction(InitializerIndirectStoreTag())
+      or
+      tag = InitializerIndirectStoreTag() and
       result = getParent().getChildSuccessor(this)
     )
   }
@@ -353,12 +366,23 @@ class TranslatedParameter extends TranslatedElement, TTranslatedParameter {
     tag = InitializerStoreTag() and
     opcode instanceof Opcode::InitializeParameter and
     resultType = getTypeForPRValue(getVariableType(param))
+    or
+    hasIndirection() and
+    tag = InitializerIndirectAddressTag() and
+    opcode instanceof Opcode::Load and
+    resultType = getTypeForPRValue(getVariableType(param))
+    or
+    hasIndirection() and
+    tag = InitializerIndirectStoreTag() and
+    opcode instanceof Opcode::InitializeIndirection and
+    resultType = getUnknownType()
   }
 
   final override IRVariable getInstructionVariable(InstructionTag tag) {
     (
       tag = InitializerStoreTag() or
-      tag = InitializerVariableAddressTag()
+      tag = InitializerVariableAddressTag() or
+      tag = InitializerIndirectStoreTag()
     ) and
     result = getIRUserVariable(getFunction(), param)
   }
@@ -369,6 +393,28 @@ class TranslatedParameter extends TranslatedElement, TTranslatedParameter {
       operandTag instanceof AddressOperandTag and
       result = getInstruction(InitializerVariableAddressTag())
     )
+    or
+    // this feels a little strange, but I think it's the best we can do
+    tag = InitializerIndirectAddressTag() and
+    (
+      operandTag instanceof AddressOperandTag and
+      result = getInstruction(InitializerVariableAddressTag())
+      or
+      operandTag instanceof LoadOperandTag and
+      result = getInstruction(InitializerStoreTag())
+    )
+    or
+    tag = InitializerIndirectStoreTag() and
+    operandTag instanceof AddressOperandTag and
+    result = getInstruction(InitializerIndirectAddressTag())
+  }
+
+  predicate hasIndirection() {
+    exists(Type t | t = param.getUnspecifiedType() |
+      t instanceof ArrayType or
+      t instanceof PointerType or
+      t instanceof ReferenceType
+    )
   }
 }
 
@@ -490,3 +536,96 @@ class TranslatedDestructorDestructionList extends TranslatedElement,
     )
   }
 }
+
+TranslatedReadEffects getTranslatedReadEffects(Function func) { result.getAST() = func }
+
+class TranslatedReadEffects extends TranslatedElement, TTranslatedReadEffects {
+  Function func;
+
+  TranslatedReadEffects() { this = TTranslatedReadEffects(func) }
+
+  override Locatable getAST() { result = func }
+
+  override Function getFunction() { result = func }
+
+  override string toString() { result = "read effects: " + func.toString() }
+
+  override TranslatedElement getChild(int id) {
+    result = getTranslatedReadEffect(func.getParameter(id))
+  }
+
+  override Instruction getFirstInstruction() {
+    if exists(getAChild())
+    then
+      result = min(TranslatedReadEffect child, int id | child = getChild(id) | child order by id)
+            .getFirstInstruction()
+    else result = getParent().getChildSuccessor(this)
+  }
+
+  override Instruction getChildSuccessor(TranslatedElement child) {
+    exists(int id | child = getChild(id) |
+      if exists(TranslatedReadEffect child2, int id2 | id2 > id and child2 = getChild(id2))
+      then
+        result = min(TranslatedReadEffect child2, int id2 |
+            child2 = getChild(id2) and id2 > id
+          |
+            child2 order by id2
+          ).getFirstInstruction()
+      else result = getParent().getChildSuccessor(this)
+    )
+  }
+
+  override predicate hasInstruction(Opcode opcode, InstructionTag tag, CppType resultType) {
+    none()
+  }
+
+  override Instruction getInstructionSuccessor(InstructionTag tag, EdgeKind kind) { none() }
+}
+
+private TranslatedReadEffect getTranslatedReadEffect(Parameter param) { result.getAST() = param }
+
+class TranslatedReadEffect extends TranslatedElement, TTranslatedReadEffect {
+  Parameter param;
+
+  TranslatedReadEffect() { this = TTranslatedReadEffect(param) }
+
+  override Locatable getAST() { result = param }
+
+  override string toString() { result = "read effect: " + param.toString() }
+
+  override TranslatedElement getChild(int id) { none() }
+
+  override Instruction getChildSuccessor(TranslatedElement child) { none() }
+
+  override Instruction getInstructionSuccessor(InstructionTag tag, EdgeKind edge) {
+    tag = OnlyInstructionTag() and
+    edge = gotoEdge() and
+    result = getParent().getChildSuccessor(this)
+  }
+
+  override Instruction getFirstInstruction() { result = getInstruction(OnlyInstructionTag()) }
+
+  override Function getFunction() { result = param.getFunction() }
+
+  override predicate hasInstruction(Opcode opcode, InstructionTag tag, CppType resultType) {
+    opcode instanceof Opcode::ReturnIndirection and
+    tag = OnlyInstructionTag() and
+    resultType = getVoidType()
+  }
+
+  final override Instruction getInstructionOperand(InstructionTag tag, OperandTag operandTag) {
+    tag = OnlyInstructionTag() and
+    operandTag = sideEffectOperand() and
+    result = getTranslatedFunction(getFunction()).getUnmodeledDefinitionInstruction()
+    or
+    tag = OnlyInstructionTag() and
+    operandTag = addressOperand() and
+    result = getTranslatedParameter(param).getInstruction(InitializerIndirectAddressTag())
+  }
+
+  final override CppType getInstructionOperandType(InstructionTag tag, TypedOperandTag operandTag) {
+    tag = OnlyInstructionTag() and
+    operandTag = sideEffectOperand() and
+    result = getUnknownType()
+  }
+}