diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/MemoryAccessKind.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/MemoryAccessKind.qll index a71608ee855..f5c6de314ab 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/MemoryAccessKind.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/MemoryAccessKind.qll @@ -1,11 +1,8 @@ private newtype TMemoryAccessKind = TIndirectMemoryAccess() or - TIndirectMayMemoryAccess() or TBufferMemoryAccess() or - TBufferMayMemoryAccess() or TEscapedMemoryAccess() or - TEscapedMayMemoryAccess() or - TNonLocalMayMemoryAccess() or + TNonLocalMemoryAccess() or TPhiMemoryAccess() or TUnmodeledMemoryAccess() or TChiTotalMemoryAccess() or @@ -35,16 +32,6 @@ class IndirectMemoryAccess extends MemoryAccessKind, TIndirectMemoryAccess { final override predicate usesAddressOperand() { any() } } -/** - * The operand or result may access some, all, or none of the memory at the address specified by the - * `AddressOperand` on the same instruction. - */ -class IndirectMayMemoryAccess extends MemoryAccessKind, TIndirectMayMemoryAccess { - override string toString() { result = "indirect(may)" } - - final override predicate usesAddressOperand() { any() } -} - /** * The operand or result accesses memory starting at the address specified by the `AddressOperand` * on the same instruction, accessing a number of consecutive elements given by the @@ -56,17 +43,6 @@ class BufferMemoryAccess extends MemoryAccessKind, TBufferMemoryAccess { final override predicate usesAddressOperand() { any() } } -/** - * The operand or result may access some, all, or none of the memory starting at the address - * specified by the `AddressOperand` on the same instruction, accessing a number of consecutive - * elements given by the `BufferSizeOperand`. - */ -class BufferMayMemoryAccess extends MemoryAccessKind, TBufferMayMemoryAccess { - override string toString() { result = "buffer(may)" } - - final override predicate usesAddressOperand() { any() } -} - /** * The operand or result accesses all memory whose address has escaped. */ @@ -75,18 +51,11 @@ class EscapedMemoryAccess extends MemoryAccessKind, TEscapedMemoryAccess { } /** - * The operand or result may access all memory whose address has escaped. + * The operand or result access all memory whose address has escaped, other than data on the stack + * frame of the current function. */ -class EscapedMayMemoryAccess extends MemoryAccessKind, TEscapedMayMemoryAccess { - override string toString() { result = "escaped(may)" } -} - -/** - * The operand or result may access all memory whose address has escaped, other than data on the - * stack frame of the current function. - */ -class NonLocalMayMemoryAccess extends MemoryAccessKind, TNonLocalMayMemoryAccess { - override string toString() { result = "nonlocal(may)" } +class NonLocalMemoryAccess extends MemoryAccessKind, TNonLocalMemoryAccess { + override string toString() { result = "nonlocal" } } /** diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Instruction.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Instruction.qll index 8819826d5c3..b325aeb1bf1 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Instruction.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Instruction.qll @@ -550,6 +550,16 @@ class Instruction extends Construction::TInstruction { */ MemoryAccessKind getResultMemoryAccess() { none() } + /** + * Holds if the memory access performed by this instruction's result will not always write to + * every bit in the memory location. This is most commonly used for memory accesses that may or + * may not actually occur depending on runtime state (for example, the write side effect of an + * output parameter that is not written to on all paths), or for accesses where the memory + * location is a conservative estimate of the memory that might actually be accessed at runtime + * (for example, the global side effects of a function call). + */ + predicate hasResultMayMemoryAccess() { none() } + /** * Gets the operand that holds the memory address to which this instruction stores its * result, if any. For example, in `m3 = Store r1, r2`, the result of `getResultAddressOperand()` @@ -1206,9 +1216,9 @@ class SideEffectInstruction extends Instruction { class CallSideEffectInstruction extends SideEffectInstruction { CallSideEffectInstruction() { getOpcode() instanceof Opcode::CallSideEffect } - final override MemoryAccessKind getResultMemoryAccess() { - result instanceof EscapedMayMemoryAccess - } + final override MemoryAccessKind getResultMemoryAccess() { result instanceof EscapedMemoryAccess } + + final override predicate hasResultMayMemoryAccess() { any() } } /** @@ -1306,9 +1316,9 @@ class IndirectMayWriteSideEffectInstruction extends WriteSideEffectInstruction { getOpcode() instanceof Opcode::IndirectMayWriteSideEffect } - final override MemoryAccessKind getResultMemoryAccess() { - result instanceof IndirectMayMemoryAccess - } + final override MemoryAccessKind getResultMemoryAccess() { result instanceof IndirectMemoryAccess } + + final override predicate hasResultMayMemoryAccess() { any() } } /** @@ -1318,9 +1328,9 @@ class IndirectMayWriteSideEffectInstruction extends WriteSideEffectInstruction { class BufferMayWriteSideEffectInstruction extends WriteSideEffectInstruction { BufferMayWriteSideEffectInstruction() { getOpcode() instanceof Opcode::BufferMayWriteSideEffect } - final override MemoryAccessKind getResultMemoryAccess() { - result instanceof BufferMayMemoryAccess - } + final override MemoryAccessKind getResultMemoryAccess() { result instanceof BufferMemoryAccess } + + final override predicate hasResultMayMemoryAccess() { any() } } /** @@ -1332,9 +1342,9 @@ class SizedBufferMayWriteSideEffectInstruction extends WriteSideEffectInstructio getOpcode() instanceof Opcode::SizedBufferMayWriteSideEffect } - final override MemoryAccessKind getResultMemoryAccess() { - result instanceof BufferMayMemoryAccess - } + final override MemoryAccessKind getResultMemoryAccess() { result instanceof BufferMemoryAccess } + + final override predicate hasResultMayMemoryAccess() { any() } Instruction getSizeDef() { result = getAnOperand().(BufferSizeOperand).getDef() } } @@ -1345,9 +1355,9 @@ class SizedBufferMayWriteSideEffectInstruction extends WriteSideEffectInstructio class InlineAsmInstruction extends Instruction { InlineAsmInstruction() { getOpcode() instanceof Opcode::InlineAsm } - final override MemoryAccessKind getResultMemoryAccess() { - result instanceof EscapedMayMemoryAccess - } + final override MemoryAccessKind getResultMemoryAccess() { result instanceof EscapedMemoryAccess } + + final override predicate hasResultMayMemoryAccess() { any() } } /** diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Operand.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Operand.qll index 07dd107bf12..3e63e86a738 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Operand.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/Operand.qll @@ -194,6 +194,16 @@ class MemoryOperand extends Operand { */ MemoryAccessKind getMemoryAccess() { none() } + /** + * Holds if the memory access performed by this operand will not always read from every bit in the + * memory location. This is most commonly used for memory accesses that may or may not actually + * occur depending on runtime state (for example, the write side effect of an output parameter + * that is not written to on all paths), or for accesses where the memory location is a + * conservative estimate of the memory that might actually be accessed at runtime (for example, + * the global side effects of a function call). + */ + predicate hasMayMemoryAccess() { none() } + /** * Returns the operand that holds the memory address from which the current operand loads its * value, if any. For example, in `r3 = Load r1, m2`, the result of `getAddressOperand()` for `m2` @@ -397,13 +407,13 @@ class SideEffectOperand extends TypedOperand { override MemoryAccessKind getMemoryAccess() { useInstr instanceof AliasedUseInstruction and - result instanceof NonLocalMayMemoryAccess + result instanceof NonLocalMemoryAccess or useInstr instanceof CallSideEffectInstruction and - result instanceof EscapedMayMemoryAccess + result instanceof EscapedMemoryAccess or useInstr instanceof CallReadSideEffectInstruction and - result instanceof EscapedMayMemoryAccess + result instanceof EscapedMemoryAccess or useInstr instanceof IndirectReadSideEffectInstruction and result instanceof IndirectMemoryAccess @@ -418,10 +428,22 @@ class SideEffectOperand extends TypedOperand { result instanceof BufferMemoryAccess or useInstr instanceof IndirectMayWriteSideEffectInstruction and - result instanceof IndirectMayMemoryAccess + result instanceof IndirectMemoryAccess or useInstr instanceof BufferMayWriteSideEffectInstruction and - result instanceof BufferMayMemoryAccess + result instanceof BufferMemoryAccess + } + + final override predicate hasMayMemoryAccess() { + useInstr instanceof AliasedUseInstruction + or + useInstr instanceof CallSideEffectInstruction + or + useInstr instanceof CallReadSideEffectInstruction + or + useInstr instanceof IndirectMayWriteSideEffectInstruction + or + useInstr instanceof BufferMayWriteSideEffectInstruction } } diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasedSSA.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasedSSA.qll index 0835ffb7da6..6d9c314c836 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasedSSA.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasedSSA.qll @@ -11,11 +11,12 @@ private class IntValue = Ints::IntValue; private predicate hasResultMemoryAccess( Instruction instr, IRVariable var, IRType type, Language::LanguageType languageType, - IntValue startBitOffset, IntValue endBitOffset + IntValue startBitOffset, IntValue endBitOffset, boolean isMayAccess ) { resultPointsTo(instr.getResultAddress(), var, startBitOffset) and languageType = instr.getResultLanguageType() and type = languageType.getIRType() and + (if instr.hasResultMayMemoryAccess() then isMayAccess = true else isMayAccess = false) and if exists(type.getByteSize()) then endBitOffset = Ints::add(startBitOffset, Ints::mul(type.getByteSize(), 8)) else endBitOffset = Ints::unknown() @@ -23,11 +24,12 @@ private predicate hasResultMemoryAccess( private predicate hasOperandMemoryAccess( MemoryOperand operand, IRVariable var, IRType type, Language::LanguageType languageType, - IntValue startBitOffset, IntValue endBitOffset + IntValue startBitOffset, IntValue endBitOffset, boolean isMayAccess ) { resultPointsTo(operand.getAddressOperand().getAnyDef(), var, startBitOffset) and languageType = operand.getLanguageType() and type = languageType.getIRType() and + (if operand.hasMayMemoryAccess() then isMayAccess = true else isMayAccess = false) and if exists(type.getByteSize()) then endBitOffset = Ints::add(startBitOffset, Ints::mul(type.getByteSize(), 8)) else endBitOffset = Ints::unknown() @@ -36,17 +38,23 @@ private predicate hasOperandMemoryAccess( private newtype TMemoryLocation = TVariableMemoryLocation( IRVariable var, IRType type, Language::LanguageType languageType, IntValue startBitOffset, - IntValue endBitOffset + IntValue endBitOffset, boolean isMayAccess ) { ( - hasResultMemoryAccess(_, var, type, _, startBitOffset, endBitOffset) or - hasOperandMemoryAccess(_, var, type, _, startBitOffset, endBitOffset) + hasResultMemoryAccess(_, var, type, _, startBitOffset, endBitOffset, isMayAccess) or + hasOperandMemoryAccess(_, var, type, _, startBitOffset, endBitOffset, isMayAccess) ) and languageType = type.getCanonicalLanguageType() } or - TUnknownMemoryLocation(IRFunction irFunc) or - TUnknownNonLocalMemoryLocation(IRFunction irFunc) or - TUnknownVirtualVariable(IRFunction irFunc) + TUnknownMemoryLocation(IRFunction irFunc, boolean isMayAccess) { + isMayAccess = false or isMayAccess = true + } or + TAllNonLocalMemory(IRFunction irFunc, boolean isMayAccess) { + isMayAccess = false or isMayAccess = true + } or + TAllAliasedMemory(IRFunction irFunc, boolean isMayAccess) { + isMayAccess = false or isMayAccess = true + } /** * Represents the memory location accessed by a memory operand or memory result. In this implementation, the location is @@ -56,7 +64,11 @@ private newtype TMemoryLocation = * - `UnknownMemoryLocation` - A location not known to be within a specific `IRVariable`. */ abstract class MemoryLocation extends TMemoryLocation { - abstract string toString(); + final string toString() { + if isMayAccess() then result = "?" + toStringInternal() else result = toStringInternal() + } + + abstract string toStringInternal(); abstract VirtualVariable getVirtualVariable(); @@ -64,7 +76,28 @@ abstract class MemoryLocation extends TMemoryLocation { abstract string getUniqueId(); + abstract IRFunction getIRFunction(); + final IRType getIRType() { result = getType().getIRType() } + + abstract predicate isMayAccess(); + + /** + * Holds if the location cannot be overwritten except by definition of a `MemoryLocation` for + * which `def.canDefineReadOnly()` holds. + */ + predicate isReadOnly() { none() } + + /** + * Holds if a definition of this location can be the definition of a read-only use location. + */ + predicate canDefineReadOnly() { none() } + + /** + * Holds if the location always represents memory allocated on the stack (for example, a variable + * with automatic storage duration). + */ + predicate isAlwaysAllocatedOnStack() { none() } } abstract class VirtualVariable extends MemoryLocation { } @@ -79,12 +112,14 @@ class VariableMemoryLocation extends TVariableMemoryLocation, MemoryLocation { Language::LanguageType languageType; IntValue startBitOffset; IntValue endBitOffset; + boolean isMayAccess; VariableMemoryLocation() { - this = TVariableMemoryLocation(var, type, languageType, startBitOffset, endBitOffset) + this = TVariableMemoryLocation(var, type, languageType, startBitOffset, endBitOffset, + isMayAccess) } - final override string toString() { + final override string toStringInternal() { result = var.toString() + Interval::getIntervalString(startBitOffset, endBitOffset) + "<" + type.toString() + ", " + languageType.toString() + ">" } @@ -92,18 +127,20 @@ class VariableMemoryLocation extends TVariableMemoryLocation, MemoryLocation { final override Language::LanguageType getType() { if strictcount(Language::LanguageType accessType | - hasResultMemoryAccess(_, var, type, accessType, startBitOffset, endBitOffset) or - hasOperandMemoryAccess(_, var, type, accessType, startBitOffset, endBitOffset) + hasResultMemoryAccess(_, var, type, accessType, startBitOffset, endBitOffset, _) or + hasOperandMemoryAccess(_, var, type, accessType, startBitOffset, endBitOffset, _) ) = 1 then // All of the accesses have the same `LanguageType`, so just use that. - hasResultMemoryAccess(_, var, type, result, startBitOffset, endBitOffset) or - hasOperandMemoryAccess(_, var, type, result, startBitOffset, endBitOffset) + hasResultMemoryAccess(_, var, type, result, startBitOffset, endBitOffset, _) or + hasOperandMemoryAccess(_, var, type, result, startBitOffset, endBitOffset, _) else // There is no single type for all accesses, so just use the canonical one for this `IRType`. result = type.getCanonicalLanguageType() } + final override IRFunction getIRFunction() { result = var.getEnclosingIRFunction() } + final IntValue getStartBitOffset() { result = startBitOffset } final IntValue getEndBitOffset() { result = endBitOffset } @@ -117,11 +154,18 @@ class VariableMemoryLocation extends TVariableMemoryLocation, MemoryLocation { final override VirtualVariable getVirtualVariable() { if variableAddressEscapes(var) - then result = TUnknownVirtualVariable(var.getEnclosingIRFunction()) + then result = TAllAliasedMemory(var.getEnclosingIRFunction(), false) else - result = TVariableMemoryLocation(var, var.getIRType(), _, 0, var.getIRType().getByteSize() * 8) + result = TVariableMemoryLocation(var, var.getIRType(), _, 0, + var.getIRType().getByteSize() * 8, false) } + final override predicate isMayAccess() { isMayAccess = true } + + final override predicate isReadOnly() { var.isReadOnly() } + + final override predicate isAlwaysAllocatedOnStack() { var instanceof IRAutomaticVariable } + /** * Holds if this memory location covers the entire variable. */ @@ -140,7 +184,8 @@ class VariableVirtualVariable extends VariableMemoryLocation, VirtualVariable { VariableVirtualVariable() { not variableAddressEscapes(var) and type = var.getIRType() and - coversEntireVariable() + coversEntireVariable() and + not isMayAccess() } } @@ -149,111 +194,168 @@ class VariableVirtualVariable extends VariableMemoryLocation, VirtualVariable { */ class UnknownMemoryLocation extends TUnknownMemoryLocation, MemoryLocation { IRFunction irFunc; + boolean isMayAccess; - UnknownMemoryLocation() { this = TUnknownMemoryLocation(irFunc) } + UnknownMemoryLocation() { this = TUnknownMemoryLocation(irFunc, isMayAccess) } - final override string toString() { result = "{Unknown}" } + final override string toStringInternal() { result = "{Unknown}" } - final override VirtualVariable getVirtualVariable() { result = TUnknownVirtualVariable(irFunc) } + final override VirtualVariable getVirtualVariable() { result = TAllAliasedMemory(irFunc, false) } final override Language::LanguageType getType() { result = any(IRUnknownType type).getCanonicalLanguageType() } + final override IRFunction getIRFunction() { result = irFunc } + final override string getUniqueId() { result = "{Unknown}" } + + final override predicate isMayAccess() { isMayAccess = true } } /** * An access to memory that is not known to be confined to a specific `IRVariable`, but is known to * not access memory on the current function's stack frame. */ -class UnknownNonLocalMemoryLocation extends TUnknownNonLocalMemoryLocation, MemoryLocation { +class AllNonLocalMemory extends TAllNonLocalMemory, MemoryLocation { IRFunction irFunc; + boolean isMayAccess; - UnknownNonLocalMemoryLocation() { this = TUnknownNonLocalMemoryLocation(irFunc) } + AllNonLocalMemory() { this = TAllNonLocalMemory(irFunc, isMayAccess) } - final override string toString() { result = "{UnknownNonLocal}" } + final override string toStringInternal() { result = "{AllNonLocal}" } - final override VirtualVariable getVirtualVariable() { result = TUnknownVirtualVariable(irFunc) } + final override VirtualVariable getVirtualVariable() { result = TAllAliasedMemory(irFunc, false) } final override Language::LanguageType getType() { result = any(IRUnknownType type).getCanonicalLanguageType() } - final override string getUniqueId() { result = "{UnknownNonLocal}" } + final override IRFunction getIRFunction() { result = irFunc } + + final override string getUniqueId() { result = "{AllNonLocal}" } + + final override predicate isMayAccess() { isMayAccess = true } } /** * An access to all aliased memory. */ -class UnknownVirtualVariable extends TUnknownVirtualVariable, VirtualVariable { +class AllAliasedMemory extends TAllAliasedMemory, MemoryLocation { IRFunction irFunc; + boolean isMayAccess; - UnknownVirtualVariable() { this = TUnknownVirtualVariable(irFunc) } + AllAliasedMemory() { this = TAllAliasedMemory(irFunc, isMayAccess) } - final override string toString() { result = "{AllAliased}" } + final override string toStringInternal() { result = "{AllAliased}" } final override Language::LanguageType getType() { result = any(IRUnknownType type).getCanonicalLanguageType() } + final override IRFunction getIRFunction() { result = irFunc } + final override string getUniqueId() { result = " " + toString() } - final override VirtualVariable getVirtualVariable() { result = this } + final override VirtualVariable getVirtualVariable() { result = TAllAliasedMemory(irFunc, false) } + + final override predicate isMayAccess() { isMayAccess = true } } +class AliasedVirtualVariable extends AllAliasedMemory, VirtualVariable { + AliasedVirtualVariable() { not isMayAccess() } + + override predicate canDefineReadOnly() { + // A must-def of all aliased memory is only used in two places: + // 1. In the prologue of the function, to provide a definition for all memory defined before the + // function was called. In this case, it needs to provide a definition even for read-only + // non-local variables. + // 2. As the result of a `Chi` instruction. These don't participate in overlap analysis, so it's + // OK if we let this predicate hold in that case. + any() + } +} + +/** + * Gets the overlap relationship between the definition location `def` and the use location `use`. + */ Overlap getOverlap(MemoryLocation def, MemoryLocation use) { + exists(Overlap overlap | + // Compute the overlap based only on the extent. + overlap = getExtentOverlap(def, use) and + // Filter out attempts to write to read-only memory. + (def.canDefineReadOnly() or not use.isReadOnly()) and + if def.isMayAccess() + then + // No matter what kind of extent overlap we have, the final overlap is still + // `MayPartiallyOverlap`, because the def might not have written all of the bits of the use + // location. + result instanceof MayPartiallyOverlap + else + if + overlap instanceof MustExactlyOverlap and + (use.isMayAccess() or not def.getIRType() = use.getIRType()) + then + // Can't exactly overlap with a "may" use or a use of a different type. + result instanceof MustTotallyOverlap + else result = overlap + ) +} + +/** + * Gets the overlap relationship between the definition location `def` and the use location `use`, + * based only on the set of memory locations accessed. Handling of "may" accesses and read-only + * locations occurs in `getOverlap()`. + */ +private Overlap getExtentOverlap(MemoryLocation def, MemoryLocation use) { // The def and the use must have the same virtual variable, or no overlap is possible. ( - // An UnknownVirtualVariable must totally overlap any location within the same virtual variable. + // AllAliasedMemory must totally overlap any location within the same virtual variable. def.getVirtualVariable() = use.getVirtualVariable() and - def instanceof UnknownVirtualVariable and + def instanceof AllAliasedMemory and result instanceof MustTotallyOverlap or // An UnknownMemoryLocation may partially overlap any Location within the same virtual variable, - // unless the location is read-only. + // even itself. def.getVirtualVariable() = use.getVirtualVariable() and def instanceof UnknownMemoryLocation and - result instanceof MayPartiallyOverlap and - not use.(VariableMemoryLocation).getVariable().isReadOnly() + result instanceof MayPartiallyOverlap or - // An UnknownNonLocalMemoryLocation may partially overlap any location within the same virtual - // variable, except a local variable or read-only variable. def.getVirtualVariable() = use.getVirtualVariable() and - def instanceof UnknownNonLocalMemoryLocation and - result instanceof MayPartiallyOverlap and - not exists(IRVariable var | var = use.(VariableMemoryLocation).getVariable() | - var instanceof IRAutomaticVariable or var.isReadOnly() + def instanceof AllNonLocalMemory and + ( + // AllNonLocalMemory exactly overlaps itself. + use instanceof AllNonLocalMemory and + result instanceof MustExactlyOverlap + or + // AllNonLocalMemory may partially overlap any location within the same virtual variable, + // except a local variable. + result instanceof MayPartiallyOverlap and + not use.isAlwaysAllocatedOnStack() ) or exists(VariableMemoryLocation defVariableLocation | defVariableLocation = def and ( - // A VariableMemoryLocation may partially overlap an unknown location within the same virtual variable. + // A VariableMemoryLocation may partially overlap an unknown location within the same + // virtual variable. def.getVirtualVariable() = use.getVirtualVariable() and - (use instanceof UnknownMemoryLocation or use instanceof UnknownVirtualVariable) and + (use instanceof UnknownMemoryLocation or use instanceof AllAliasedMemory) and result instanceof MayPartiallyOverlap or - // A VariableMemoryLocation that is not a local variable may partially overlap an unknown - // non-local location within the same virtual variable. + // A VariableMemoryLocation that is not a local variable may partially overlap an + // AllNonLocalMemory within the same virtual variable. def.getVirtualVariable() = use.getVirtualVariable() and - use instanceof UnknownNonLocalMemoryLocation and + use instanceof AllNonLocalMemory and result instanceof MayPartiallyOverlap and - not defVariableLocation.getVariable() instanceof IRAutomaticVariable + not defVariableLocation.isAlwaysAllocatedOnStack() or - // A VariableMemoryLocation overlaps another location within the same variable based on the relationship - // of the two offset intervals. + // A VariableMemoryLocation overlaps another location within the same variable based on the + // relationship of the two offset intervals. exists(Overlap intervalOverlap | intervalOverlap = getVariableMemoryLocationOverlap(def, use) and if intervalOverlap instanceof MustExactlyOverlap - then - if def.getIRType() = use.getIRType() - then - // The def and use types match, so it's an exact overlap. - result instanceof MustExactlyOverlap - else - // The def and use types are not the same, so it's just a total overlap. - result instanceof MustTotallyOverlap + then result instanceof MustExactlyOverlap else if defVariableLocation.coversEntireVariable() then @@ -347,55 +449,51 @@ private Overlap getVariableMemoryLocationOverlap( } MemoryLocation getResultMemoryLocation(Instruction instr) { - exists(MemoryAccessKind kind | + exists(MemoryAccessKind kind, boolean isMayAccess | kind = instr.getResultMemoryAccess() and + (if instr.hasResultMayMemoryAccess() then isMayAccess = true else isMayAccess = false) and ( ( kind.usesAddressOperand() and - if hasResultMemoryAccess(instr, _, _, _, _, _) + if hasResultMemoryAccess(instr, _, _, _, _, _, _) then exists(IRVariable var, IRType type, IntValue startBitOffset, IntValue endBitOffset | - hasResultMemoryAccess(instr, var, type, _, startBitOffset, endBitOffset) and - result = TVariableMemoryLocation(var, type, _, startBitOffset, endBitOffset) + hasResultMemoryAccess(instr, var, type, _, startBitOffset, endBitOffset, isMayAccess) and + result = TVariableMemoryLocation(var, type, _, startBitOffset, endBitOffset, isMayAccess) ) - else result = TUnknownMemoryLocation(instr.getEnclosingIRFunction()) + else result = TUnknownMemoryLocation(instr.getEnclosingIRFunction(), isMayAccess) ) or kind instanceof EscapedMemoryAccess and - result = TUnknownVirtualVariable(instr.getEnclosingIRFunction()) + result = TAllAliasedMemory(instr.getEnclosingIRFunction(), isMayAccess) or - kind instanceof EscapedMayMemoryAccess and - result = TUnknownMemoryLocation(instr.getEnclosingIRFunction()) - or - kind instanceof NonLocalMayMemoryAccess and - result = TUnknownNonLocalMemoryLocation(instr.getEnclosingIRFunction()) + kind instanceof NonLocalMemoryAccess and + result = TAllNonLocalMemory(instr.getEnclosingIRFunction(), isMayAccess) ) ) } MemoryLocation getOperandMemoryLocation(MemoryOperand operand) { - exists(MemoryAccessKind kind | + exists(MemoryAccessKind kind, boolean isMayAccess | kind = operand.getMemoryAccess() and + (if operand.hasMayMemoryAccess() then isMayAccess = true else isMayAccess = false) and ( ( kind.usesAddressOperand() and - if hasOperandMemoryAccess(operand, _, _, _, _, _) + if hasOperandMemoryAccess(operand, _, _, _, _, _, _) then exists(IRVariable var, IRType type, IntValue startBitOffset, IntValue endBitOffset | - hasOperandMemoryAccess(operand, var, type, _, startBitOffset, endBitOffset) and - result = TVariableMemoryLocation(var, type, _, startBitOffset, endBitOffset) + hasOperandMemoryAccess(operand, var, type, _, startBitOffset, endBitOffset, isMayAccess) and + result = TVariableMemoryLocation(var, type, _, startBitOffset, endBitOffset, isMayAccess) ) - else result = TUnknownMemoryLocation(operand.getEnclosingIRFunction()) + else result = TUnknownMemoryLocation(operand.getEnclosingIRFunction(), isMayAccess) ) or kind instanceof EscapedMemoryAccess and - result = TUnknownVirtualVariable(operand.getEnclosingIRFunction()) + result = TAllAliasedMemory(operand.getEnclosingIRFunction(), isMayAccess) or - kind instanceof EscapedMayMemoryAccess and - result = TUnknownMemoryLocation(operand.getEnclosingIRFunction()) - or - kind instanceof NonLocalMayMemoryAccess and - result = TUnknownNonLocalMemoryLocation(operand.getEnclosingIRFunction()) + kind instanceof NonLocalMemoryAccess and + result = TAllNonLocalMemory(operand.getEnclosingIRFunction(), isMayAccess) ) ) } diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSAConstruction.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSAConstruction.qll index 96b4c28db82..883e2f58c96 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSAConstruction.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSAConstruction.qll @@ -96,7 +96,7 @@ private module Cached { hasUseAtRank(useLocation, useBlock, useRank, oldInstruction) and definitionReachesUse(useLocation, defBlock, defRank, useBlock, useRank) and overlap = Alias::getOverlap(defLocation, useLocation) and - result = getDefinitionOrChiInstruction(defBlock, defOffset, defLocation) + result = getDefinitionOrChiInstruction(defBlock, defOffset, defLocation, _) ) else ( result = instruction.getEnclosingIRFunction().getUnmodeledDefinitionInstruction() and @@ -149,13 +149,13 @@ private module Cached { ) { exists( Alias::MemoryLocation defLocation, Alias::MemoryLocation useLocation, OldBlock phiBlock, - OldBlock predBlock, OldBlock defBlock, int defOffset + OldBlock predBlock, OldBlock defBlock, int defOffset, Alias::MemoryLocation actualDefLocation | - hasPhiOperandDefinition(defLocation, useLocation, phiBlock, predBlock, defBlock, defOffset, - overlap) and + hasPhiOperandDefinition(defLocation, useLocation, phiBlock, predBlock, defBlock, defOffset) and instr = Phi(phiBlock, useLocation) and newPredecessorBlock = getNewBlock(predBlock) and - result = getDefinitionOrChiInstruction(defBlock, defOffset, defLocation) + result = getDefinitionOrChiInstruction(defBlock, defOffset, defLocation, actualDefLocation) and + overlap = Alias::getOverlap(actualDefLocation, useLocation) ) } @@ -170,7 +170,7 @@ private module Cached { hasDefinitionAtRank(vvar, defLocation, defBlock, defRank, defOffset) and hasUseAtRank(vvar, useBlock, useRank, oldInstr) and definitionReachesUse(vvar, defBlock, defRank, useBlock, useRank) and - result = getDefinitionOrChiInstruction(defBlock, defOffset, vvar) + result = getDefinitionOrChiInstruction(defBlock, defOffset, vvar, _) ) } @@ -396,11 +396,7 @@ private predicate hasChiNode(Alias::VirtualVariable vvar, OldInstruction def) { defLocation.getVirtualVariable() = vvar and // If the definition totally (or exactly) overlaps the virtual variable, then there's no need for a `Chi` // instruction. - ( - Alias::getOverlap(defLocation, vvar) instanceof MayPartiallyOverlap or - def.getResultMemoryAccess() instanceof IndirectMayMemoryAccess or - def.getResultMemoryAccess() instanceof BufferMayMemoryAccess - ) + Alias::getOverlap(defLocation, vvar) instanceof MayPartiallyOverlap ) } @@ -560,22 +556,27 @@ module DefUse { bindingset[defOffset, defLocation] pragma[inline] Instruction getDefinitionOrChiInstruction( - OldBlock defBlock, int defOffset, Alias::MemoryLocation defLocation + OldBlock defBlock, int defOffset, Alias::MemoryLocation defLocation, + Alias::MemoryLocation actualDefLocation ) { defOffset >= 0 and exists(OldInstruction oldInstr | oldInstr = defBlock.getInstruction(defOffset / 2) and if (defOffset % 2) > 0 - then + then ( // An odd offset corresponds to the `Chi` instruction. - result = Chi(oldInstr) - else + result = Chi(oldInstr) and + actualDefLocation = defLocation.getVirtualVariable() + ) else ( // An even offset corresponds to the original instruction. - result = getNewInstruction(oldInstr) + result = getNewInstruction(oldInstr) and + actualDefLocation = defLocation + ) ) or defOffset < 0 and - result = Phi(defBlock, defLocation) + result = Phi(defBlock, defLocation) and + actualDefLocation = defLocation } /** @@ -713,10 +714,7 @@ module DefUse { defLocation = Alias::getResultMemoryLocation(def) and block.getInstruction(index) = def and overlap = Alias::getOverlap(defLocation, useLocation) and - if - overlap instanceof MayPartiallyOverlap or - def.getResultMemoryAccess() instanceof IndirectMayMemoryAccess or - def.getResultMemoryAccess() instanceof BufferMayMemoryAccess + if overlap instanceof MayPartiallyOverlap then offset = (index * 2) + 1 // The use will be connected to the definition on the `Chi` instruction. else offset = index * 2 // The use will be connected to the definition on the original instruction. ) @@ -801,20 +799,19 @@ module DefUse { /** * Holds if the `Phi` instruction for location `useLocation` at the beginning of block `phiBlock` has an operand along - * the incoming edge from `predBlock`, where that operand's definition is at offset `defOffset` in block `defBlock`, - * and overlaps the use operand with overlap relationship `overlap`. + * the incoming edge from `predBlock`, where that operand's definition is at offset `defOffset` in block `defBlock`. */ pragma[inline] predicate hasPhiOperandDefinition( Alias::MemoryLocation defLocation, Alias::MemoryLocation useLocation, OldBlock phiBlock, - OldBlock predBlock, OldBlock defBlock, int defOffset, Overlap overlap + OldBlock predBlock, OldBlock defBlock, int defOffset ) { exists(int defRank | definitionHasPhiNode(useLocation, phiBlock) and predBlock = phiBlock.getAFeasiblePredecessor() and hasDefinitionAtRank(useLocation, defLocation, defBlock, defRank, defOffset) and definitionReachesEndOfBlock(useLocation, defBlock, defRank, predBlock) and - overlap = Alias::getOverlap(defLocation, useLocation) + exists(Alias::getOverlap(defLocation, useLocation)) ) } } diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Instruction.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Instruction.qll index 8819826d5c3..b325aeb1bf1 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Instruction.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Instruction.qll @@ -550,6 +550,16 @@ class Instruction extends Construction::TInstruction { */ MemoryAccessKind getResultMemoryAccess() { none() } + /** + * Holds if the memory access performed by this instruction's result will not always write to + * every bit in the memory location. This is most commonly used for memory accesses that may or + * may not actually occur depending on runtime state (for example, the write side effect of an + * output parameter that is not written to on all paths), or for accesses where the memory + * location is a conservative estimate of the memory that might actually be accessed at runtime + * (for example, the global side effects of a function call). + */ + predicate hasResultMayMemoryAccess() { none() } + /** * Gets the operand that holds the memory address to which this instruction stores its * result, if any. For example, in `m3 = Store r1, r2`, the result of `getResultAddressOperand()` @@ -1206,9 +1216,9 @@ class SideEffectInstruction extends Instruction { class CallSideEffectInstruction extends SideEffectInstruction { CallSideEffectInstruction() { getOpcode() instanceof Opcode::CallSideEffect } - final override MemoryAccessKind getResultMemoryAccess() { - result instanceof EscapedMayMemoryAccess - } + final override MemoryAccessKind getResultMemoryAccess() { result instanceof EscapedMemoryAccess } + + final override predicate hasResultMayMemoryAccess() { any() } } /** @@ -1306,9 +1316,9 @@ class IndirectMayWriteSideEffectInstruction extends WriteSideEffectInstruction { getOpcode() instanceof Opcode::IndirectMayWriteSideEffect } - final override MemoryAccessKind getResultMemoryAccess() { - result instanceof IndirectMayMemoryAccess - } + final override MemoryAccessKind getResultMemoryAccess() { result instanceof IndirectMemoryAccess } + + final override predicate hasResultMayMemoryAccess() { any() } } /** @@ -1318,9 +1328,9 @@ class IndirectMayWriteSideEffectInstruction extends WriteSideEffectInstruction { class BufferMayWriteSideEffectInstruction extends WriteSideEffectInstruction { BufferMayWriteSideEffectInstruction() { getOpcode() instanceof Opcode::BufferMayWriteSideEffect } - final override MemoryAccessKind getResultMemoryAccess() { - result instanceof BufferMayMemoryAccess - } + final override MemoryAccessKind getResultMemoryAccess() { result instanceof BufferMemoryAccess } + + final override predicate hasResultMayMemoryAccess() { any() } } /** @@ -1332,9 +1342,9 @@ class SizedBufferMayWriteSideEffectInstruction extends WriteSideEffectInstructio getOpcode() instanceof Opcode::SizedBufferMayWriteSideEffect } - final override MemoryAccessKind getResultMemoryAccess() { - result instanceof BufferMayMemoryAccess - } + final override MemoryAccessKind getResultMemoryAccess() { result instanceof BufferMemoryAccess } + + final override predicate hasResultMayMemoryAccess() { any() } Instruction getSizeDef() { result = getAnOperand().(BufferSizeOperand).getDef() } } @@ -1345,9 +1355,9 @@ class SizedBufferMayWriteSideEffectInstruction extends WriteSideEffectInstructio class InlineAsmInstruction extends Instruction { InlineAsmInstruction() { getOpcode() instanceof Opcode::InlineAsm } - final override MemoryAccessKind getResultMemoryAccess() { - result instanceof EscapedMayMemoryAccess - } + final override MemoryAccessKind getResultMemoryAccess() { result instanceof EscapedMemoryAccess } + + final override predicate hasResultMayMemoryAccess() { any() } } /** diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Operand.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Operand.qll index 07dd107bf12..3e63e86a738 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Operand.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/Operand.qll @@ -194,6 +194,16 @@ class MemoryOperand extends Operand { */ MemoryAccessKind getMemoryAccess() { none() } + /** + * Holds if the memory access performed by this operand will not always read from every bit in the + * memory location. This is most commonly used for memory accesses that may or may not actually + * occur depending on runtime state (for example, the write side effect of an output parameter + * that is not written to on all paths), or for accesses where the memory location is a + * conservative estimate of the memory that might actually be accessed at runtime (for example, + * the global side effects of a function call). + */ + predicate hasMayMemoryAccess() { none() } + /** * Returns the operand that holds the memory address from which the current operand loads its * value, if any. For example, in `r3 = Load r1, m2`, the result of `getAddressOperand()` for `m2` @@ -397,13 +407,13 @@ class SideEffectOperand extends TypedOperand { override MemoryAccessKind getMemoryAccess() { useInstr instanceof AliasedUseInstruction and - result instanceof NonLocalMayMemoryAccess + result instanceof NonLocalMemoryAccess or useInstr instanceof CallSideEffectInstruction and - result instanceof EscapedMayMemoryAccess + result instanceof EscapedMemoryAccess or useInstr instanceof CallReadSideEffectInstruction and - result instanceof EscapedMayMemoryAccess + result instanceof EscapedMemoryAccess or useInstr instanceof IndirectReadSideEffectInstruction and result instanceof IndirectMemoryAccess @@ -418,10 +428,22 @@ class SideEffectOperand extends TypedOperand { result instanceof BufferMemoryAccess or useInstr instanceof IndirectMayWriteSideEffectInstruction and - result instanceof IndirectMayMemoryAccess + result instanceof IndirectMemoryAccess or useInstr instanceof BufferMayWriteSideEffectInstruction and - result instanceof BufferMayMemoryAccess + result instanceof BufferMemoryAccess + } + + final override predicate hasMayMemoryAccess() { + useInstr instanceof AliasedUseInstruction + or + useInstr instanceof CallSideEffectInstruction + or + useInstr instanceof CallReadSideEffectInstruction + or + useInstr instanceof IndirectMayWriteSideEffectInstruction + or + useInstr instanceof BufferMayWriteSideEffectInstruction } } diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/Instruction.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/Instruction.qll index 8819826d5c3..b325aeb1bf1 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/Instruction.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/Instruction.qll @@ -550,6 +550,16 @@ class Instruction extends Construction::TInstruction { */ MemoryAccessKind getResultMemoryAccess() { none() } + /** + * Holds if the memory access performed by this instruction's result will not always write to + * every bit in the memory location. This is most commonly used for memory accesses that may or + * may not actually occur depending on runtime state (for example, the write side effect of an + * output parameter that is not written to on all paths), or for accesses where the memory + * location is a conservative estimate of the memory that might actually be accessed at runtime + * (for example, the global side effects of a function call). + */ + predicate hasResultMayMemoryAccess() { none() } + /** * Gets the operand that holds the memory address to which this instruction stores its * result, if any. For example, in `m3 = Store r1, r2`, the result of `getResultAddressOperand()` @@ -1206,9 +1216,9 @@ class SideEffectInstruction extends Instruction { class CallSideEffectInstruction extends SideEffectInstruction { CallSideEffectInstruction() { getOpcode() instanceof Opcode::CallSideEffect } - final override MemoryAccessKind getResultMemoryAccess() { - result instanceof EscapedMayMemoryAccess - } + final override MemoryAccessKind getResultMemoryAccess() { result instanceof EscapedMemoryAccess } + + final override predicate hasResultMayMemoryAccess() { any() } } /** @@ -1306,9 +1316,9 @@ class IndirectMayWriteSideEffectInstruction extends WriteSideEffectInstruction { getOpcode() instanceof Opcode::IndirectMayWriteSideEffect } - final override MemoryAccessKind getResultMemoryAccess() { - result instanceof IndirectMayMemoryAccess - } + final override MemoryAccessKind getResultMemoryAccess() { result instanceof IndirectMemoryAccess } + + final override predicate hasResultMayMemoryAccess() { any() } } /** @@ -1318,9 +1328,9 @@ class IndirectMayWriteSideEffectInstruction extends WriteSideEffectInstruction { class BufferMayWriteSideEffectInstruction extends WriteSideEffectInstruction { BufferMayWriteSideEffectInstruction() { getOpcode() instanceof Opcode::BufferMayWriteSideEffect } - final override MemoryAccessKind getResultMemoryAccess() { - result instanceof BufferMayMemoryAccess - } + final override MemoryAccessKind getResultMemoryAccess() { result instanceof BufferMemoryAccess } + + final override predicate hasResultMayMemoryAccess() { any() } } /** @@ -1332,9 +1342,9 @@ class SizedBufferMayWriteSideEffectInstruction extends WriteSideEffectInstructio getOpcode() instanceof Opcode::SizedBufferMayWriteSideEffect } - final override MemoryAccessKind getResultMemoryAccess() { - result instanceof BufferMayMemoryAccess - } + final override MemoryAccessKind getResultMemoryAccess() { result instanceof BufferMemoryAccess } + + final override predicate hasResultMayMemoryAccess() { any() } Instruction getSizeDef() { result = getAnOperand().(BufferSizeOperand).getDef() } } @@ -1345,9 +1355,9 @@ class SizedBufferMayWriteSideEffectInstruction extends WriteSideEffectInstructio class InlineAsmInstruction extends Instruction { InlineAsmInstruction() { getOpcode() instanceof Opcode::InlineAsm } - final override MemoryAccessKind getResultMemoryAccess() { - result instanceof EscapedMayMemoryAccess - } + final override MemoryAccessKind getResultMemoryAccess() { result instanceof EscapedMemoryAccess } + + final override predicate hasResultMayMemoryAccess() { any() } } /** diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/Operand.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/Operand.qll index 07dd107bf12..3e63e86a738 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/Operand.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/Operand.qll @@ -194,6 +194,16 @@ class MemoryOperand extends Operand { */ MemoryAccessKind getMemoryAccess() { none() } + /** + * Holds if the memory access performed by this operand will not always read from every bit in the + * memory location. This is most commonly used for memory accesses that may or may not actually + * occur depending on runtime state (for example, the write side effect of an output parameter + * that is not written to on all paths), or for accesses where the memory location is a + * conservative estimate of the memory that might actually be accessed at runtime (for example, + * the global side effects of a function call). + */ + predicate hasMayMemoryAccess() { none() } + /** * Returns the operand that holds the memory address from which the current operand loads its * value, if any. For example, in `r3 = Load r1, m2`, the result of `getAddressOperand()` for `m2` @@ -397,13 +407,13 @@ class SideEffectOperand extends TypedOperand { override MemoryAccessKind getMemoryAccess() { useInstr instanceof AliasedUseInstruction and - result instanceof NonLocalMayMemoryAccess + result instanceof NonLocalMemoryAccess or useInstr instanceof CallSideEffectInstruction and - result instanceof EscapedMayMemoryAccess + result instanceof EscapedMemoryAccess or useInstr instanceof CallReadSideEffectInstruction and - result instanceof EscapedMayMemoryAccess + result instanceof EscapedMemoryAccess or useInstr instanceof IndirectReadSideEffectInstruction and result instanceof IndirectMemoryAccess @@ -418,10 +428,22 @@ class SideEffectOperand extends TypedOperand { result instanceof BufferMemoryAccess or useInstr instanceof IndirectMayWriteSideEffectInstruction and - result instanceof IndirectMayMemoryAccess + result instanceof IndirectMemoryAccess or useInstr instanceof BufferMayWriteSideEffectInstruction and - result instanceof BufferMayMemoryAccess + result instanceof BufferMemoryAccess + } + + final override predicate hasMayMemoryAccess() { + useInstr instanceof AliasedUseInstruction + or + useInstr instanceof CallSideEffectInstruction + or + useInstr instanceof CallReadSideEffectInstruction + or + useInstr instanceof IndirectMayWriteSideEffectInstruction + or + useInstr instanceof BufferMayWriteSideEffectInstruction } } diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSAConstruction.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSAConstruction.qll index 96b4c28db82..883e2f58c96 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSAConstruction.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSAConstruction.qll @@ -96,7 +96,7 @@ private module Cached { hasUseAtRank(useLocation, useBlock, useRank, oldInstruction) and definitionReachesUse(useLocation, defBlock, defRank, useBlock, useRank) and overlap = Alias::getOverlap(defLocation, useLocation) and - result = getDefinitionOrChiInstruction(defBlock, defOffset, defLocation) + result = getDefinitionOrChiInstruction(defBlock, defOffset, defLocation, _) ) else ( result = instruction.getEnclosingIRFunction().getUnmodeledDefinitionInstruction() and @@ -149,13 +149,13 @@ private module Cached { ) { exists( Alias::MemoryLocation defLocation, Alias::MemoryLocation useLocation, OldBlock phiBlock, - OldBlock predBlock, OldBlock defBlock, int defOffset + OldBlock predBlock, OldBlock defBlock, int defOffset, Alias::MemoryLocation actualDefLocation | - hasPhiOperandDefinition(defLocation, useLocation, phiBlock, predBlock, defBlock, defOffset, - overlap) and + hasPhiOperandDefinition(defLocation, useLocation, phiBlock, predBlock, defBlock, defOffset) and instr = Phi(phiBlock, useLocation) and newPredecessorBlock = getNewBlock(predBlock) and - result = getDefinitionOrChiInstruction(defBlock, defOffset, defLocation) + result = getDefinitionOrChiInstruction(defBlock, defOffset, defLocation, actualDefLocation) and + overlap = Alias::getOverlap(actualDefLocation, useLocation) ) } @@ -170,7 +170,7 @@ private module Cached { hasDefinitionAtRank(vvar, defLocation, defBlock, defRank, defOffset) and hasUseAtRank(vvar, useBlock, useRank, oldInstr) and definitionReachesUse(vvar, defBlock, defRank, useBlock, useRank) and - result = getDefinitionOrChiInstruction(defBlock, defOffset, vvar) + result = getDefinitionOrChiInstruction(defBlock, defOffset, vvar, _) ) } @@ -396,11 +396,7 @@ private predicate hasChiNode(Alias::VirtualVariable vvar, OldInstruction def) { defLocation.getVirtualVariable() = vvar and // If the definition totally (or exactly) overlaps the virtual variable, then there's no need for a `Chi` // instruction. - ( - Alias::getOverlap(defLocation, vvar) instanceof MayPartiallyOverlap or - def.getResultMemoryAccess() instanceof IndirectMayMemoryAccess or - def.getResultMemoryAccess() instanceof BufferMayMemoryAccess - ) + Alias::getOverlap(defLocation, vvar) instanceof MayPartiallyOverlap ) } @@ -560,22 +556,27 @@ module DefUse { bindingset[defOffset, defLocation] pragma[inline] Instruction getDefinitionOrChiInstruction( - OldBlock defBlock, int defOffset, Alias::MemoryLocation defLocation + OldBlock defBlock, int defOffset, Alias::MemoryLocation defLocation, + Alias::MemoryLocation actualDefLocation ) { defOffset >= 0 and exists(OldInstruction oldInstr | oldInstr = defBlock.getInstruction(defOffset / 2) and if (defOffset % 2) > 0 - then + then ( // An odd offset corresponds to the `Chi` instruction. - result = Chi(oldInstr) - else + result = Chi(oldInstr) and + actualDefLocation = defLocation.getVirtualVariable() + ) else ( // An even offset corresponds to the original instruction. - result = getNewInstruction(oldInstr) + result = getNewInstruction(oldInstr) and + actualDefLocation = defLocation + ) ) or defOffset < 0 and - result = Phi(defBlock, defLocation) + result = Phi(defBlock, defLocation) and + actualDefLocation = defLocation } /** @@ -713,10 +714,7 @@ module DefUse { defLocation = Alias::getResultMemoryLocation(def) and block.getInstruction(index) = def and overlap = Alias::getOverlap(defLocation, useLocation) and - if - overlap instanceof MayPartiallyOverlap or - def.getResultMemoryAccess() instanceof IndirectMayMemoryAccess or - def.getResultMemoryAccess() instanceof BufferMayMemoryAccess + if overlap instanceof MayPartiallyOverlap then offset = (index * 2) + 1 // The use will be connected to the definition on the `Chi` instruction. else offset = index * 2 // The use will be connected to the definition on the original instruction. ) @@ -801,20 +799,19 @@ module DefUse { /** * Holds if the `Phi` instruction for location `useLocation` at the beginning of block `phiBlock` has an operand along - * the incoming edge from `predBlock`, where that operand's definition is at offset `defOffset` in block `defBlock`, - * and overlaps the use operand with overlap relationship `overlap`. + * the incoming edge from `predBlock`, where that operand's definition is at offset `defOffset` in block `defBlock`. */ pragma[inline] predicate hasPhiOperandDefinition( Alias::MemoryLocation defLocation, Alias::MemoryLocation useLocation, OldBlock phiBlock, - OldBlock predBlock, OldBlock defBlock, int defOffset, Overlap overlap + OldBlock predBlock, OldBlock defBlock, int defOffset ) { exists(int defRank | definitionHasPhiNode(useLocation, phiBlock) and predBlock = phiBlock.getAFeasiblePredecessor() and hasDefinitionAtRank(useLocation, defLocation, defBlock, defRank, defOffset) and definitionReachesEndOfBlock(useLocation, defBlock, defRank, predBlock) and - overlap = Alias::getOverlap(defLocation, useLocation) + exists(Alias::getOverlap(defLocation, useLocation)) ) } } diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SimpleSSA.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SimpleSSA.qll index 49bb908265c..c6a6bc3fec9 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SimpleSSA.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SimpleSSA.qll @@ -27,20 +27,19 @@ private predicate isVariableModeled(IRVariable var) { // There's no need to check for the right size. An `IRVariable` never has an `UnknownType`, so the test for // `type = var.getType()` is sufficient. forall(Instruction instr, Language::LanguageType type, IntValue bitOffset | - hasResultMemoryAccess(instr, var, type, bitOffset) + hasResultMemoryAccess(instr, var, type, bitOffset) and + not instr.hasResultMayMemoryAccess() | bitOffset = 0 and type.getIRType() = var.getIRType() and - not ( - instr.getResultMemoryAccess() instanceof IndirectMayMemoryAccess or - instr.getResultMemoryAccess() instanceof BufferMayMemoryAccess - ) + not instr.hasResultMayMemoryAccess() ) and forall(MemoryOperand operand, Language::LanguageType type, IntValue bitOffset | hasOperandMemoryAccess(operand, var, type, bitOffset) | bitOffset = 0 and - type.getIRType() = var.getIRType() + type.getIRType() = var.getIRType() and + not operand.hasMayMemoryAccess() ) } diff --git a/cpp/ql/test/library-tests/ir/ssa/aliased_ssa_ir.expected b/cpp/ql/test/library-tests/ir/ssa/aliased_ssa_ir.expected index 6332253cbe3..1e79d13fd3d 100644 --- a/cpp/ql/test/library-tests/ir/ssa/aliased_ssa_ir.expected +++ b/cpp/ql/test/library-tests/ir/ssa/aliased_ssa_ir.expected @@ -521,7 +521,7 @@ ssa.cpp: #-----| Goto -> Block 3 # 130| Block 3 -# 130| m3_0(Point) = Phi : from 1:~m1_5, from 2:~m2_5 +# 130| m3_0(Point) = Phi : from 1:m1_5, from 2:m2_5 # 130| m3_1(int) = Phi : from 1:m1_4, from 2:m2_4 # 130| r3_2(glval) = VariableAddress[x] : # 130| r3_3(glval) = VariableAddress[a] : @@ -582,7 +582,7 @@ ssa.cpp: #-----| Goto -> Block 3 # 142| Block 3 -# 142| m3_0(Point) = Phi : from 1:~m1_5, from 2:m2_3 +# 142| m3_0(Point) = Phi : from 1:m1_5, from 2:m2_3 # 142| m3_1(int) = Phi : from 1:m1_4, from 2:~m2_3 # 142| r3_2(glval) = VariableAddress[x] : # 142| r3_3(glval) = VariableAddress[a] : @@ -639,7 +639,7 @@ ssa.cpp: #-----| Goto -> Block 3 # 153| Block 3 -# 153| m3_0(Point) = Phi : from 1:~m1_5, from 2:m2_3 +# 153| m3_0(Point) = Phi : from 1:m1_5, from 2:m2_3 # 153| r3_1(glval) = VariableAddress[b] : # 153| r3_2(glval) = VariableAddress[a] : # 153| r3_3(Point) = Load : &:r3_2, m3_0 @@ -695,7 +695,7 @@ ssa.cpp: #-----| Goto -> Block 3 # 164| Block 3 -# 164| m3_0(Rect) = Phi : from 1:~m1_6, from 2:m2_3 +# 164| m3_0(Rect) = Phi : from 1:m1_6, from 2:m2_3 # 164| r3_1(glval) = VariableAddress[b] : # 164| r3_2(glval) = VariableAddress[a] : # 164| r3_3(glval) = FieldAddress[topLeft] : r3_2 diff --git a/csharp/ql/src/semmle/code/csharp/ir/implementation/MemoryAccessKind.qll b/csharp/ql/src/semmle/code/csharp/ir/implementation/MemoryAccessKind.qll index a71608ee855..f5c6de314ab 100644 --- a/csharp/ql/src/semmle/code/csharp/ir/implementation/MemoryAccessKind.qll +++ b/csharp/ql/src/semmle/code/csharp/ir/implementation/MemoryAccessKind.qll @@ -1,11 +1,8 @@ private newtype TMemoryAccessKind = TIndirectMemoryAccess() or - TIndirectMayMemoryAccess() or TBufferMemoryAccess() or - TBufferMayMemoryAccess() or TEscapedMemoryAccess() or - TEscapedMayMemoryAccess() or - TNonLocalMayMemoryAccess() or + TNonLocalMemoryAccess() or TPhiMemoryAccess() or TUnmodeledMemoryAccess() or TChiTotalMemoryAccess() or @@ -35,16 +32,6 @@ class IndirectMemoryAccess extends MemoryAccessKind, TIndirectMemoryAccess { final override predicate usesAddressOperand() { any() } } -/** - * The operand or result may access some, all, or none of the memory at the address specified by the - * `AddressOperand` on the same instruction. - */ -class IndirectMayMemoryAccess extends MemoryAccessKind, TIndirectMayMemoryAccess { - override string toString() { result = "indirect(may)" } - - final override predicate usesAddressOperand() { any() } -} - /** * The operand or result accesses memory starting at the address specified by the `AddressOperand` * on the same instruction, accessing a number of consecutive elements given by the @@ -56,17 +43,6 @@ class BufferMemoryAccess extends MemoryAccessKind, TBufferMemoryAccess { final override predicate usesAddressOperand() { any() } } -/** - * The operand or result may access some, all, or none of the memory starting at the address - * specified by the `AddressOperand` on the same instruction, accessing a number of consecutive - * elements given by the `BufferSizeOperand`. - */ -class BufferMayMemoryAccess extends MemoryAccessKind, TBufferMayMemoryAccess { - override string toString() { result = "buffer(may)" } - - final override predicate usesAddressOperand() { any() } -} - /** * The operand or result accesses all memory whose address has escaped. */ @@ -75,18 +51,11 @@ class EscapedMemoryAccess extends MemoryAccessKind, TEscapedMemoryAccess { } /** - * The operand or result may access all memory whose address has escaped. + * The operand or result access all memory whose address has escaped, other than data on the stack + * frame of the current function. */ -class EscapedMayMemoryAccess extends MemoryAccessKind, TEscapedMayMemoryAccess { - override string toString() { result = "escaped(may)" } -} - -/** - * The operand or result may access all memory whose address has escaped, other than data on the - * stack frame of the current function. - */ -class NonLocalMayMemoryAccess extends MemoryAccessKind, TNonLocalMayMemoryAccess { - override string toString() { result = "nonlocal(may)" } +class NonLocalMemoryAccess extends MemoryAccessKind, TNonLocalMemoryAccess { + override string toString() { result = "nonlocal" } } /** diff --git a/csharp/ql/src/semmle/code/csharp/ir/implementation/raw/Instruction.qll b/csharp/ql/src/semmle/code/csharp/ir/implementation/raw/Instruction.qll index 8819826d5c3..b325aeb1bf1 100644 --- a/csharp/ql/src/semmle/code/csharp/ir/implementation/raw/Instruction.qll +++ b/csharp/ql/src/semmle/code/csharp/ir/implementation/raw/Instruction.qll @@ -550,6 +550,16 @@ class Instruction extends Construction::TInstruction { */ MemoryAccessKind getResultMemoryAccess() { none() } + /** + * Holds if the memory access performed by this instruction's result will not always write to + * every bit in the memory location. This is most commonly used for memory accesses that may or + * may not actually occur depending on runtime state (for example, the write side effect of an + * output parameter that is not written to on all paths), or for accesses where the memory + * location is a conservative estimate of the memory that might actually be accessed at runtime + * (for example, the global side effects of a function call). + */ + predicate hasResultMayMemoryAccess() { none() } + /** * Gets the operand that holds the memory address to which this instruction stores its * result, if any. For example, in `m3 = Store r1, r2`, the result of `getResultAddressOperand()` @@ -1206,9 +1216,9 @@ class SideEffectInstruction extends Instruction { class CallSideEffectInstruction extends SideEffectInstruction { CallSideEffectInstruction() { getOpcode() instanceof Opcode::CallSideEffect } - final override MemoryAccessKind getResultMemoryAccess() { - result instanceof EscapedMayMemoryAccess - } + final override MemoryAccessKind getResultMemoryAccess() { result instanceof EscapedMemoryAccess } + + final override predicate hasResultMayMemoryAccess() { any() } } /** @@ -1306,9 +1316,9 @@ class IndirectMayWriteSideEffectInstruction extends WriteSideEffectInstruction { getOpcode() instanceof Opcode::IndirectMayWriteSideEffect } - final override MemoryAccessKind getResultMemoryAccess() { - result instanceof IndirectMayMemoryAccess - } + final override MemoryAccessKind getResultMemoryAccess() { result instanceof IndirectMemoryAccess } + + final override predicate hasResultMayMemoryAccess() { any() } } /** @@ -1318,9 +1328,9 @@ class IndirectMayWriteSideEffectInstruction extends WriteSideEffectInstruction { class BufferMayWriteSideEffectInstruction extends WriteSideEffectInstruction { BufferMayWriteSideEffectInstruction() { getOpcode() instanceof Opcode::BufferMayWriteSideEffect } - final override MemoryAccessKind getResultMemoryAccess() { - result instanceof BufferMayMemoryAccess - } + final override MemoryAccessKind getResultMemoryAccess() { result instanceof BufferMemoryAccess } + + final override predicate hasResultMayMemoryAccess() { any() } } /** @@ -1332,9 +1342,9 @@ class SizedBufferMayWriteSideEffectInstruction extends WriteSideEffectInstructio getOpcode() instanceof Opcode::SizedBufferMayWriteSideEffect } - final override MemoryAccessKind getResultMemoryAccess() { - result instanceof BufferMayMemoryAccess - } + final override MemoryAccessKind getResultMemoryAccess() { result instanceof BufferMemoryAccess } + + final override predicate hasResultMayMemoryAccess() { any() } Instruction getSizeDef() { result = getAnOperand().(BufferSizeOperand).getDef() } } @@ -1345,9 +1355,9 @@ class SizedBufferMayWriteSideEffectInstruction extends WriteSideEffectInstructio class InlineAsmInstruction extends Instruction { InlineAsmInstruction() { getOpcode() instanceof Opcode::InlineAsm } - final override MemoryAccessKind getResultMemoryAccess() { - result instanceof EscapedMayMemoryAccess - } + final override MemoryAccessKind getResultMemoryAccess() { result instanceof EscapedMemoryAccess } + + final override predicate hasResultMayMemoryAccess() { any() } } /** diff --git a/csharp/ql/src/semmle/code/csharp/ir/implementation/raw/Operand.qll b/csharp/ql/src/semmle/code/csharp/ir/implementation/raw/Operand.qll index 07dd107bf12..3e63e86a738 100644 --- a/csharp/ql/src/semmle/code/csharp/ir/implementation/raw/Operand.qll +++ b/csharp/ql/src/semmle/code/csharp/ir/implementation/raw/Operand.qll @@ -194,6 +194,16 @@ class MemoryOperand extends Operand { */ MemoryAccessKind getMemoryAccess() { none() } + /** + * Holds if the memory access performed by this operand will not always read from every bit in the + * memory location. This is most commonly used for memory accesses that may or may not actually + * occur depending on runtime state (for example, the write side effect of an output parameter + * that is not written to on all paths), or for accesses where the memory location is a + * conservative estimate of the memory that might actually be accessed at runtime (for example, + * the global side effects of a function call). + */ + predicate hasMayMemoryAccess() { none() } + /** * Returns the operand that holds the memory address from which the current operand loads its * value, if any. For example, in `r3 = Load r1, m2`, the result of `getAddressOperand()` for `m2` @@ -397,13 +407,13 @@ class SideEffectOperand extends TypedOperand { override MemoryAccessKind getMemoryAccess() { useInstr instanceof AliasedUseInstruction and - result instanceof NonLocalMayMemoryAccess + result instanceof NonLocalMemoryAccess or useInstr instanceof CallSideEffectInstruction and - result instanceof EscapedMayMemoryAccess + result instanceof EscapedMemoryAccess or useInstr instanceof CallReadSideEffectInstruction and - result instanceof EscapedMayMemoryAccess + result instanceof EscapedMemoryAccess or useInstr instanceof IndirectReadSideEffectInstruction and result instanceof IndirectMemoryAccess @@ -418,10 +428,22 @@ class SideEffectOperand extends TypedOperand { result instanceof BufferMemoryAccess or useInstr instanceof IndirectMayWriteSideEffectInstruction and - result instanceof IndirectMayMemoryAccess + result instanceof IndirectMemoryAccess or useInstr instanceof BufferMayWriteSideEffectInstruction and - result instanceof BufferMayMemoryAccess + result instanceof BufferMemoryAccess + } + + final override predicate hasMayMemoryAccess() { + useInstr instanceof AliasedUseInstruction + or + useInstr instanceof CallSideEffectInstruction + or + useInstr instanceof CallReadSideEffectInstruction + or + useInstr instanceof IndirectMayWriteSideEffectInstruction + or + useInstr instanceof BufferMayWriteSideEffectInstruction } } diff --git a/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/Instruction.qll b/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/Instruction.qll index 8819826d5c3..b325aeb1bf1 100644 --- a/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/Instruction.qll +++ b/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/Instruction.qll @@ -550,6 +550,16 @@ class Instruction extends Construction::TInstruction { */ MemoryAccessKind getResultMemoryAccess() { none() } + /** + * Holds if the memory access performed by this instruction's result will not always write to + * every bit in the memory location. This is most commonly used for memory accesses that may or + * may not actually occur depending on runtime state (for example, the write side effect of an + * output parameter that is not written to on all paths), or for accesses where the memory + * location is a conservative estimate of the memory that might actually be accessed at runtime + * (for example, the global side effects of a function call). + */ + predicate hasResultMayMemoryAccess() { none() } + /** * Gets the operand that holds the memory address to which this instruction stores its * result, if any. For example, in `m3 = Store r1, r2`, the result of `getResultAddressOperand()` @@ -1206,9 +1216,9 @@ class SideEffectInstruction extends Instruction { class CallSideEffectInstruction extends SideEffectInstruction { CallSideEffectInstruction() { getOpcode() instanceof Opcode::CallSideEffect } - final override MemoryAccessKind getResultMemoryAccess() { - result instanceof EscapedMayMemoryAccess - } + final override MemoryAccessKind getResultMemoryAccess() { result instanceof EscapedMemoryAccess } + + final override predicate hasResultMayMemoryAccess() { any() } } /** @@ -1306,9 +1316,9 @@ class IndirectMayWriteSideEffectInstruction extends WriteSideEffectInstruction { getOpcode() instanceof Opcode::IndirectMayWriteSideEffect } - final override MemoryAccessKind getResultMemoryAccess() { - result instanceof IndirectMayMemoryAccess - } + final override MemoryAccessKind getResultMemoryAccess() { result instanceof IndirectMemoryAccess } + + final override predicate hasResultMayMemoryAccess() { any() } } /** @@ -1318,9 +1328,9 @@ class IndirectMayWriteSideEffectInstruction extends WriteSideEffectInstruction { class BufferMayWriteSideEffectInstruction extends WriteSideEffectInstruction { BufferMayWriteSideEffectInstruction() { getOpcode() instanceof Opcode::BufferMayWriteSideEffect } - final override MemoryAccessKind getResultMemoryAccess() { - result instanceof BufferMayMemoryAccess - } + final override MemoryAccessKind getResultMemoryAccess() { result instanceof BufferMemoryAccess } + + final override predicate hasResultMayMemoryAccess() { any() } } /** @@ -1332,9 +1342,9 @@ class SizedBufferMayWriteSideEffectInstruction extends WriteSideEffectInstructio getOpcode() instanceof Opcode::SizedBufferMayWriteSideEffect } - final override MemoryAccessKind getResultMemoryAccess() { - result instanceof BufferMayMemoryAccess - } + final override MemoryAccessKind getResultMemoryAccess() { result instanceof BufferMemoryAccess } + + final override predicate hasResultMayMemoryAccess() { any() } Instruction getSizeDef() { result = getAnOperand().(BufferSizeOperand).getDef() } } @@ -1345,9 +1355,9 @@ class SizedBufferMayWriteSideEffectInstruction extends WriteSideEffectInstructio class InlineAsmInstruction extends Instruction { InlineAsmInstruction() { getOpcode() instanceof Opcode::InlineAsm } - final override MemoryAccessKind getResultMemoryAccess() { - result instanceof EscapedMayMemoryAccess - } + final override MemoryAccessKind getResultMemoryAccess() { result instanceof EscapedMemoryAccess } + + final override predicate hasResultMayMemoryAccess() { any() } } /** diff --git a/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/Operand.qll b/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/Operand.qll index 07dd107bf12..3e63e86a738 100644 --- a/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/Operand.qll +++ b/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/Operand.qll @@ -194,6 +194,16 @@ class MemoryOperand extends Operand { */ MemoryAccessKind getMemoryAccess() { none() } + /** + * Holds if the memory access performed by this operand will not always read from every bit in the + * memory location. This is most commonly used for memory accesses that may or may not actually + * occur depending on runtime state (for example, the write side effect of an output parameter + * that is not written to on all paths), or for accesses where the memory location is a + * conservative estimate of the memory that might actually be accessed at runtime (for example, + * the global side effects of a function call). + */ + predicate hasMayMemoryAccess() { none() } + /** * Returns the operand that holds the memory address from which the current operand loads its * value, if any. For example, in `r3 = Load r1, m2`, the result of `getAddressOperand()` for `m2` @@ -397,13 +407,13 @@ class SideEffectOperand extends TypedOperand { override MemoryAccessKind getMemoryAccess() { useInstr instanceof AliasedUseInstruction and - result instanceof NonLocalMayMemoryAccess + result instanceof NonLocalMemoryAccess or useInstr instanceof CallSideEffectInstruction and - result instanceof EscapedMayMemoryAccess + result instanceof EscapedMemoryAccess or useInstr instanceof CallReadSideEffectInstruction and - result instanceof EscapedMayMemoryAccess + result instanceof EscapedMemoryAccess or useInstr instanceof IndirectReadSideEffectInstruction and result instanceof IndirectMemoryAccess @@ -418,10 +428,22 @@ class SideEffectOperand extends TypedOperand { result instanceof BufferMemoryAccess or useInstr instanceof IndirectMayWriteSideEffectInstruction and - result instanceof IndirectMayMemoryAccess + result instanceof IndirectMemoryAccess or useInstr instanceof BufferMayWriteSideEffectInstruction and - result instanceof BufferMayMemoryAccess + result instanceof BufferMemoryAccess + } + + final override predicate hasMayMemoryAccess() { + useInstr instanceof AliasedUseInstruction + or + useInstr instanceof CallSideEffectInstruction + or + useInstr instanceof CallReadSideEffectInstruction + or + useInstr instanceof IndirectMayWriteSideEffectInstruction + or + useInstr instanceof BufferMayWriteSideEffectInstruction } } diff --git a/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/internal/SSAConstruction.qll b/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/internal/SSAConstruction.qll index 96b4c28db82..883e2f58c96 100644 --- a/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/internal/SSAConstruction.qll +++ b/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/internal/SSAConstruction.qll @@ -96,7 +96,7 @@ private module Cached { hasUseAtRank(useLocation, useBlock, useRank, oldInstruction) and definitionReachesUse(useLocation, defBlock, defRank, useBlock, useRank) and overlap = Alias::getOverlap(defLocation, useLocation) and - result = getDefinitionOrChiInstruction(defBlock, defOffset, defLocation) + result = getDefinitionOrChiInstruction(defBlock, defOffset, defLocation, _) ) else ( result = instruction.getEnclosingIRFunction().getUnmodeledDefinitionInstruction() and @@ -149,13 +149,13 @@ private module Cached { ) { exists( Alias::MemoryLocation defLocation, Alias::MemoryLocation useLocation, OldBlock phiBlock, - OldBlock predBlock, OldBlock defBlock, int defOffset + OldBlock predBlock, OldBlock defBlock, int defOffset, Alias::MemoryLocation actualDefLocation | - hasPhiOperandDefinition(defLocation, useLocation, phiBlock, predBlock, defBlock, defOffset, - overlap) and + hasPhiOperandDefinition(defLocation, useLocation, phiBlock, predBlock, defBlock, defOffset) and instr = Phi(phiBlock, useLocation) and newPredecessorBlock = getNewBlock(predBlock) and - result = getDefinitionOrChiInstruction(defBlock, defOffset, defLocation) + result = getDefinitionOrChiInstruction(defBlock, defOffset, defLocation, actualDefLocation) and + overlap = Alias::getOverlap(actualDefLocation, useLocation) ) } @@ -170,7 +170,7 @@ private module Cached { hasDefinitionAtRank(vvar, defLocation, defBlock, defRank, defOffset) and hasUseAtRank(vvar, useBlock, useRank, oldInstr) and definitionReachesUse(vvar, defBlock, defRank, useBlock, useRank) and - result = getDefinitionOrChiInstruction(defBlock, defOffset, vvar) + result = getDefinitionOrChiInstruction(defBlock, defOffset, vvar, _) ) } @@ -396,11 +396,7 @@ private predicate hasChiNode(Alias::VirtualVariable vvar, OldInstruction def) { defLocation.getVirtualVariable() = vvar and // If the definition totally (or exactly) overlaps the virtual variable, then there's no need for a `Chi` // instruction. - ( - Alias::getOverlap(defLocation, vvar) instanceof MayPartiallyOverlap or - def.getResultMemoryAccess() instanceof IndirectMayMemoryAccess or - def.getResultMemoryAccess() instanceof BufferMayMemoryAccess - ) + Alias::getOverlap(defLocation, vvar) instanceof MayPartiallyOverlap ) } @@ -560,22 +556,27 @@ module DefUse { bindingset[defOffset, defLocation] pragma[inline] Instruction getDefinitionOrChiInstruction( - OldBlock defBlock, int defOffset, Alias::MemoryLocation defLocation + OldBlock defBlock, int defOffset, Alias::MemoryLocation defLocation, + Alias::MemoryLocation actualDefLocation ) { defOffset >= 0 and exists(OldInstruction oldInstr | oldInstr = defBlock.getInstruction(defOffset / 2) and if (defOffset % 2) > 0 - then + then ( // An odd offset corresponds to the `Chi` instruction. - result = Chi(oldInstr) - else + result = Chi(oldInstr) and + actualDefLocation = defLocation.getVirtualVariable() + ) else ( // An even offset corresponds to the original instruction. - result = getNewInstruction(oldInstr) + result = getNewInstruction(oldInstr) and + actualDefLocation = defLocation + ) ) or defOffset < 0 and - result = Phi(defBlock, defLocation) + result = Phi(defBlock, defLocation) and + actualDefLocation = defLocation } /** @@ -713,10 +714,7 @@ module DefUse { defLocation = Alias::getResultMemoryLocation(def) and block.getInstruction(index) = def and overlap = Alias::getOverlap(defLocation, useLocation) and - if - overlap instanceof MayPartiallyOverlap or - def.getResultMemoryAccess() instanceof IndirectMayMemoryAccess or - def.getResultMemoryAccess() instanceof BufferMayMemoryAccess + if overlap instanceof MayPartiallyOverlap then offset = (index * 2) + 1 // The use will be connected to the definition on the `Chi` instruction. else offset = index * 2 // The use will be connected to the definition on the original instruction. ) @@ -801,20 +799,19 @@ module DefUse { /** * Holds if the `Phi` instruction for location `useLocation` at the beginning of block `phiBlock` has an operand along - * the incoming edge from `predBlock`, where that operand's definition is at offset `defOffset` in block `defBlock`, - * and overlaps the use operand with overlap relationship `overlap`. + * the incoming edge from `predBlock`, where that operand's definition is at offset `defOffset` in block `defBlock`. */ pragma[inline] predicate hasPhiOperandDefinition( Alias::MemoryLocation defLocation, Alias::MemoryLocation useLocation, OldBlock phiBlock, - OldBlock predBlock, OldBlock defBlock, int defOffset, Overlap overlap + OldBlock predBlock, OldBlock defBlock, int defOffset ) { exists(int defRank | definitionHasPhiNode(useLocation, phiBlock) and predBlock = phiBlock.getAFeasiblePredecessor() and hasDefinitionAtRank(useLocation, defLocation, defBlock, defRank, defOffset) and definitionReachesEndOfBlock(useLocation, defBlock, defRank, predBlock) and - overlap = Alias::getOverlap(defLocation, useLocation) + exists(Alias::getOverlap(defLocation, useLocation)) ) } } diff --git a/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/internal/SimpleSSA.qll b/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/internal/SimpleSSA.qll index 49bb908265c..c6a6bc3fec9 100644 --- a/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/internal/SimpleSSA.qll +++ b/csharp/ql/src/semmle/code/csharp/ir/implementation/unaliased_ssa/internal/SimpleSSA.qll @@ -27,20 +27,19 @@ private predicate isVariableModeled(IRVariable var) { // There's no need to check for the right size. An `IRVariable` never has an `UnknownType`, so the test for // `type = var.getType()` is sufficient. forall(Instruction instr, Language::LanguageType type, IntValue bitOffset | - hasResultMemoryAccess(instr, var, type, bitOffset) + hasResultMemoryAccess(instr, var, type, bitOffset) and + not instr.hasResultMayMemoryAccess() | bitOffset = 0 and type.getIRType() = var.getIRType() and - not ( - instr.getResultMemoryAccess() instanceof IndirectMayMemoryAccess or - instr.getResultMemoryAccess() instanceof BufferMayMemoryAccess - ) + not instr.hasResultMayMemoryAccess() ) and forall(MemoryOperand operand, Language::LanguageType type, IntValue bitOffset | hasOperandMemoryAccess(operand, var, type, bitOffset) | bitOffset = 0 and - type.getIRType() = var.getIRType() + type.getIRType() = var.getIRType() and + not operand.hasMayMemoryAccess() ) }