diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaImpl.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaImpl.qll index c7f622f81d3..7877c55d200 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaImpl.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaImpl.qll @@ -15,17 +15,79 @@ private import DataFlowPrivate import SsaImplCommon private module SourceVariables { + /** + * Holds if `store` is the `StoreInstruction` generated by an postfix + * increment or decrement operation `e`, and `postCrement` is the operand + * that represents the use of the evaluated value of `e`. + */ + private predicate isUseAfterPostfixCrement0(StoreInstruction store, Operand postCrement) { + exists( + BinaryInstruction binary, IRBlock b, int iPre, int iPost, int iStore, Operand preCrement, + Instruction left + | + binary instanceof AddInstruction + or + binary instanceof PointerAddInstruction + or + binary instanceof SubInstruction + or + binary instanceof PointerSubInstruction + | + store.getSourceValue() = binary and + left = binary.getLeft() and + strictcount(left.getAUse()) = 2 and + left.getAUse() = preCrement and + left.getAUse() = postCrement and + b.getInstruction(iPre) = preCrement.getUse() and + b.getInstruction(iPost) = postCrement.getUse() and + b.getInstruction(iStore) = store and + iPre < iStore and + iStore < iPost + ) + } + + /** + * Holds if `store` is the `StoreInstruction` generated by an postfix + * increment or decrement operation `e`, and `postCrement` is the fully + * converted operand that represents the use of the evaluated value of `e`. + */ + private predicate isUseAfterPostfixCrement(StoreInstruction store, Operand postCrement) { + isUseAfterPostfixCrement0(store, postCrement) and + conversionFlow(postCrement, _, false, _) + or + exists(Instruction instr, Operand postCrement0 | + isUseAfterPostfixCrement(store, postCrement0) and + conversionFlow(postCrement0, instr, false, _) and + instr = postCrement.getDef() + ) + } + + private predicate hasSavedPostfixCrementSourceVariable( + BaseSourceVariable base, StoreInstruction store, int ind + ) { + exists(BaseSourceVariableInstruction inst, int ind0 | + isUseAfterPostfixCrement(store, _) and + inst.getBaseSourceVariable() = base and + isDef(_, _, store.getDestinationAddressOperand(), inst, ind0, 0) and + ind = [ind0 .. countIndirectionsForCppType(base.getLanguageType()) + 1] + ) + } + cached private newtype TSourceVariable = - TMkSourceVariable(BaseSourceVariable base, int ind) { + TNormalSourceVariable(BaseSourceVariable base, int ind) { ind = [0 .. countIndirectionsForCppType(base.getLanguageType()) + 1] + } or + TSavedPostfixCrementSourceVariable(StoreInstruction store, int ind) { + hasSavedPostfixCrementSourceVariable(_, store, ind) } - class SourceVariable extends TSourceVariable { + abstract private class AbstractSourceVariable extends TSourceVariable { BaseSourceVariable base; int ind; - SourceVariable() { this = TMkSourceVariable(base, ind) } + bindingset[ind] + AbstractSourceVariable() { any() } /** Gets the IR variable associated with this `SourceVariable`, if any. */ IRVariable getIRVariable() { result = base.(BaseIRVariable).getIRVariable() } @@ -37,7 +99,7 @@ private module SourceVariables { BaseSourceVariable getBaseVariable() { result = base } /** Gets a textual representation of this element. */ - string toString() { result = repeatStars(this.getIndirection()) + base.toString() } + abstract string toString(); /** * Gets the number of loads performed on the base source variable @@ -62,6 +124,53 @@ private module SourceVariables { /** Gets the location of this variable. */ Location getLocation() { result = this.getBaseVariable().getLocation() } } + + final class SourceVariable = AbstractSourceVariable; + + /** + * A regular source variable. Most source variables are instances of this + * class. + */ + class NormalSourceVariable extends AbstractSourceVariable, TNormalSourceVariable { + NormalSourceVariable() { this = TNormalSourceVariable(base, ind) } + + final override string toString() { + result = repeatStars(this.getIndirection()) + base.toString() + } + } + + /** + * Before a value is postfix incremented (or decremented) we "save" its + * current value so that the pre-incremented value can be returned to the + * enclosing expression. We use the source variables represented by this + * class to represent the "saved value". + */ + class SavedPostfixCrementSourceVariable extends AbstractSourceVariable, + TSavedPostfixCrementSourceVariable + { + StoreInstruction store; + + SavedPostfixCrementSourceVariable() { + this = TSavedPostfixCrementSourceVariable(store, ind) and + hasSavedPostfixCrementSourceVariable(base, store, ind) + } + + final override string toString() { + result = repeatStars(this.getIndirection()) + base.toString() + " [before crement]" + } + + /** + * Gets the `StoreInstruction` that writes the incremented (or decremented) + * value. + */ + StoreInstruction getStoreInstruction() { result = store } + + /** + * Gets the fully converted `Operand` that represents the use of the + * value before the increment. + */ + Operand getOperand() { isUseAfterPostfixCrement(store, result) } + } } import SourceVariables @@ -109,17 +218,43 @@ private newtype TDefImpl = TDirectDefImpl(Operand address, int indirectionIndex) { isDef(_, _, address, _, _, indirectionIndex) } or + TSavedPostfixCrementDefImpl(SavedPostfixCrementSourceVariable sv, int indirectionIndex) { + isDef(_, _, sv.getStoreInstruction().getDestinationAddressOperand(), _, sv.getIndirection(), + indirectionIndex) + } or TGlobalDefImpl(GlobalLikeVariable v, IRFunction f, int indirectionIndex) { // Represents the initial "definition" of a global variable when entering // a function body. isGlobalDefImpl(v, f, _, indirectionIndex) } +pragma[nomagic] +private predicate hasOperandAndIndirection( + SavedPostfixCrementSourceVariable sv, Operand operand, int indirection +) { + sv.getOperand() = operand and + sv.getIndirection() = indirection +} + +private predicate hasBeforePostCrementUseImpl( + SavedPostfixCrementSourceVariable sv, Operand operand, int indirectionIndex +) { + not isDef(true, _, operand, _, _, _) and + exists(int indirection | + hasOperandAndIndirection(sv, operand, indirection) and + isUse(_, operand, _, indirection, indirectionIndex) + ) +} + cached private newtype TUseImpl = TDirectUseImpl(Operand operand, int indirectionIndex) { isUse(_, operand, _, _, indirectionIndex) and - not isDef(true, _, operand, _, _, _) + not isDef(true, _, operand, _, _, _) and + not hasBeforePostCrementUseImpl(_, operand, indirectionIndex) + } or + TSavedPostfixCrementUseImpl(SavedPostfixCrementSourceVariable sv, int indirectionIndex) { + hasBeforePostCrementUseImpl(sv, _, indirectionIndex) } or TGlobalUse(GlobalLikeVariable v, IRFunction f, int indirectionIndex) { // Represents a final "use" of a global variable to ensure that @@ -326,7 +461,7 @@ abstract private class DefAddressImpl extends DefImpl, TDefAddressImpl { override Cpp::Location getLocation() { result = v.getLocation() } - final override SourceVariable getSourceVariable() { + final override NormalSourceVariable getSourceVariable() { result.getBaseVariable() = v and result.getIndirection() = 0 } @@ -381,7 +516,7 @@ private class DirectDef extends DefImpl, TDirectDefImpl { indirection = this.getIndirection() } - final override SourceVariable getSourceVariable() { + final override NormalSourceVariable getSourceVariable() { exists(BaseSourceVariable v, int indirection | sourceVariableHasBaseAndIndex(result, v, indirection) and this.hasBaseSourceVariableAndIndirection(v, indirection) @@ -395,6 +530,32 @@ private class DirectDef extends DefImpl, TDirectDefImpl { override predicate isCertain() { isDef(true, _, address, _, _, indirectionIndex) } } +/** + * A definition that "saves" the value of a variable before it is incremented + * or decremented. + */ +private class SavedPostfixCrementDefImpl extends DefImpl, TSavedPostfixCrementDefImpl { + SavedPostfixCrementSourceVariable sv; + + SavedPostfixCrementDefImpl() { this = TSavedPostfixCrementDefImpl(sv, indirectionIndex) } + + override Cpp::Location getLocation() { result = sv.getStoreInstruction().getLocation() } + + final override predicate hasIndexInBlock(IRBlock block, int index) { + sv.getStoreInstruction() = block.getInstruction(index) + } + + override string toString() { result = "Def of " + this.getSourceVariable() } + + override SourceVariable getSourceVariable() { result = sv } + + override int getIndirection() { result = sv.getIndirection() } + + override predicate isCertain() { + isDef(true, _, sv.getStoreInstruction().getDestinationAddressOperand(), _, _, indirectionIndex) + } +} + private class DirectUseImpl extends UseImpl, TDirectUseImpl { Operand operand; @@ -414,7 +575,7 @@ private class DirectUseImpl extends UseImpl, TDirectUseImpl { this.getIndirection() = indirection } - override SourceVariable getSourceVariable() { + override NormalSourceVariable getSourceVariable() { exists(BaseSourceVariable v, int indirection | sourceVariableHasBaseAndIndex(result, v, indirection) and this.hasBaseSourceVariableAndIndirection(v, indirection) @@ -432,6 +593,34 @@ private class DirectUseImpl extends UseImpl, TDirectUseImpl { override Node getNode() { nodeHasOperand(result, operand, indirectionIndex) } } +/** + * The use of the original "saved" variable after the variable has been incremented + * or decremented. + */ +private class SavedPostfixCrementUseImpl extends UseImpl, TSavedPostfixCrementUseImpl { + SavedPostfixCrementSourceVariable sv; + + SavedPostfixCrementUseImpl() { this = TSavedPostfixCrementUseImpl(sv, indirectionIndex) } + + override string toString() { result = "Use of " + this.getSourceVariable() } + + final override predicate hasIndexInBlock(IRBlock block, int index) { + this.getOperand().getUse() = block.getInstruction(index) + } + + override SourceVariable getSourceVariable() { result = sv } + + final Operand getOperand() { result = sv.getOperand() } + + final override Cpp::Location getLocation() { result = this.getOperand().getLocation() } + + override int getIndirection() { result = sv.getIndirection() } + + override predicate isCertain() { isUse(true, this.getOperand(), _, _, indirectionIndex) } + + override Node getNode() { nodeHasOperand(result, this.getOperand(), indirectionIndex) } +} + pragma[nomagic] private predicate finalParameterNodeHasParameterAndIndex( FinalParameterNode n, Parameter p, int indirectionIndex @@ -502,7 +691,7 @@ class FinalParameterUse extends UseImpl, TFinalParameterUse { indirection = this.getIndirection() } - override SourceVariable getSourceVariable() { + override NormalSourceVariable getSourceVariable() { exists(BaseIRVariable v, int indirection | sourceVariableHasBaseAndIndex(result, v, indirection) and this.hasBaseSourceVariableAndIndirectrion(v, indirection) @@ -593,7 +782,7 @@ class GlobalUse extends UseImpl, TGlobalUse { indirection = this.getIndirection() } - override SourceVariable getSourceVariable() { + override NormalSourceVariable getSourceVariable() { exists(BaseIRVariable v, int indirection | sourceVariableHasBaseAndIndex(result, v, indirection) and this.hasBaseSourceVariableAndIndirection(v, indirection) @@ -642,7 +831,7 @@ class GlobalDefImpl extends DefImpl, TGlobalDefImpl { ) } - final override SourceVariable getSourceVariable() { + final override NormalSourceVariable getSourceVariable() { exists(BaseSourceVariable v | sourceVariableHasBaseAndIndex(result, v, indirectionIndex) and baseSourceVariableIsGlobal(v, global, f) @@ -688,9 +877,15 @@ predicate defToNode(Node node, Definition def, SourceVariable sv) { } private predicate defToNode(Node node, Definition def) { - nodeHasOperand(node, def.getValue().asOperand(), def.getIndirectionIndex()) - or - nodeHasInstruction(node, def.getValue().asInstruction(), def.getIndirectionIndex()) + // Only definitions of `NormalSourceVariable` need to be converted into + // dataflow nodes. The other case, `SavedPostfixCrementSourceVariable`, + // are internal definitions that don't have a dataflow node representation. + def.getSourceVariable() instanceof NormalSourceVariable and + ( + nodeHasOperand(node, def.getValue().asOperand(), def.getIndirectionIndex()) + or + nodeHasInstruction(node, def.getValue().asInstruction(), def.getIndirectionIndex()) + ) or node.(InitialGlobalValue).getGlobalDef() = def }