From 8213284d5ed7803036bd8f9ee5b2ec2a9eb5d112 Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Wed, 14 Aug 2019 13:48:34 -0700 Subject: [PATCH 01/13] C++: Make `duplicateOperand` query report function name --- .../aliased_ssa/Instruction.qll | 20 +++++++++++++------ .../cpp/ir/implementation/raw/Instruction.qll | 20 +++++++++++++------ .../unaliased_ssa/Instruction.qll | 20 +++++++++++++------ 3 files changed, 42 insertions(+), 18 deletions(-) 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 c6f0f8fa644a..b90f4cbd2a2a 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 @@ -78,12 +78,20 @@ module InstructionSanity { /** * Holds if instruction `instr` has multiple operands with tag `tag`. */ - query predicate duplicateOperand(Instruction instr, OperandTag tag) { - strictcount(NonPhiOperand operand | - operand = instr.getAnOperand() and - operand.getOperandTag() = tag - ) > 1 and - not tag instanceof UnmodeledUseOperandTag + query predicate duplicateOperand(Instruction instr, string message, IRFunction func, + string funcText) { + exists(OperandTag tag, int operandCount | + operandCount = strictcount(NonPhiOperand operand | + operand = instr.getAnOperand() and + operand.getOperandTag() = tag + ) and + operandCount > 1 and + not tag instanceof UnmodeledUseOperandTag and + message = "Instruction has " + operandCount + " operands with tag '" + tag.toString() + "'" + + " in function '$@'." and + func = instr.getEnclosingIRFunction() and + funcText = Language::getIdentityString(func.getFunction()) + ) } /** 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 c6f0f8fa644a..b90f4cbd2a2a 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 @@ -78,12 +78,20 @@ module InstructionSanity { /** * Holds if instruction `instr` has multiple operands with tag `tag`. */ - query predicate duplicateOperand(Instruction instr, OperandTag tag) { - strictcount(NonPhiOperand operand | - operand = instr.getAnOperand() and - operand.getOperandTag() = tag - ) > 1 and - not tag instanceof UnmodeledUseOperandTag + query predicate duplicateOperand(Instruction instr, string message, IRFunction func, + string funcText) { + exists(OperandTag tag, int operandCount | + operandCount = strictcount(NonPhiOperand operand | + operand = instr.getAnOperand() and + operand.getOperandTag() = tag + ) and + operandCount > 1 and + not tag instanceof UnmodeledUseOperandTag and + message = "Instruction has " + operandCount + " operands with tag '" + tag.toString() + "'" + + " in function '$@'." and + func = instr.getEnclosingIRFunction() and + funcText = Language::getIdentityString(func.getFunction()) + ) } /** 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 c6f0f8fa644a..b90f4cbd2a2a 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 @@ -78,12 +78,20 @@ module InstructionSanity { /** * Holds if instruction `instr` has multiple operands with tag `tag`. */ - query predicate duplicateOperand(Instruction instr, OperandTag tag) { - strictcount(NonPhiOperand operand | - operand = instr.getAnOperand() and - operand.getOperandTag() = tag - ) > 1 and - not tag instanceof UnmodeledUseOperandTag + query predicate duplicateOperand(Instruction instr, string message, IRFunction func, + string funcText) { + exists(OperandTag tag, int operandCount | + operandCount = strictcount(NonPhiOperand operand | + operand = instr.getAnOperand() and + operand.getOperandTag() = tag + ) and + operandCount > 1 and + not tag instanceof UnmodeledUseOperandTag and + message = "Instruction has " + operandCount + " operands with tag '" + tag.toString() + "'" + + " in function '$@'." and + func = instr.getEnclosingIRFunction() and + funcText = Language::getIdentityString(func.getFunction()) + ) } /** From d40e9f02cea9b3128527f53cf104aecdeaaaeb2c Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Wed, 14 Aug 2019 13:49:34 -0700 Subject: [PATCH 02/13] C++: Make `IRVariable` non-abstract --- .../implementation/aliased_ssa/IRVariable.qll | 28 +++++++++++++------ .../cpp/ir/implementation/raw/IRVariable.qll | 28 +++++++++++++------ .../unaliased_ssa/IRVariable.qll | 28 +++++++++++++------ 3 files changed, 60 insertions(+), 24 deletions(-) diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/IRVariable.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/IRVariable.qll index b8c6af20a607..aff794da9279 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/IRVariable.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/IRVariable.qll @@ -16,27 +16,35 @@ IRUserVariable getIRUserVariable(Language::Function func, Language::Variable var * be a user-declared variable (`IRUserVariable`) or a temporary variable * generated by the AST-to-IR translation (`IRTempVariable`). */ -abstract class IRVariable extends TIRVariable { +class IRVariable extends TIRVariable { Language::Function func; - abstract string toString(); + string toString() { + none() + } /** * Gets the type of the variable. */ - abstract Language::Type getType(); + Language::Type getType() { + none() + } /** * Gets the AST node that declared this variable, or that introduced this * variable as part of the AST-to-IR translation. */ - abstract Language::AST getAST(); + Language::AST getAST() { + none() + } /** * Gets an identifier string for the variable. This identifier is unique * within the function. */ - abstract string getUniqueId(); + string getUniqueId() { + none() + } /** * Gets the source location of this variable. @@ -100,10 +108,14 @@ class IRUserVariable extends IRVariable, TIRUserVariable { * stack. This includes all parameters, non-static local variables, and * temporary variables. */ -abstract class IRAutomaticVariable extends IRVariable { +class IRAutomaticVariable extends IRVariable { + IRAutomaticVariable() { + this instanceof IRAutomaticUserVariable or + this instanceof IRTempVariable + } } -class IRAutomaticUserVariable extends IRUserVariable, IRAutomaticVariable { +class IRAutomaticUserVariable extends IRUserVariable { override Language::AutomaticVariable var; IRAutomaticUserVariable() { @@ -132,7 +144,7 @@ IRTempVariable getIRTempVariable(Language::AST ast, TempVariableTag tag) { result.getTag() = tag } -class IRTempVariable extends IRVariable, IRAutomaticVariable, TIRTempVariable { +class IRTempVariable extends IRVariable, TIRTempVariable { Language::AST ast; TempVariableTag tag; Language::Type type; diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/IRVariable.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/IRVariable.qll index b8c6af20a607..aff794da9279 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/IRVariable.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/raw/IRVariable.qll @@ -16,27 +16,35 @@ IRUserVariable getIRUserVariable(Language::Function func, Language::Variable var * be a user-declared variable (`IRUserVariable`) or a temporary variable * generated by the AST-to-IR translation (`IRTempVariable`). */ -abstract class IRVariable extends TIRVariable { +class IRVariable extends TIRVariable { Language::Function func; - abstract string toString(); + string toString() { + none() + } /** * Gets the type of the variable. */ - abstract Language::Type getType(); + Language::Type getType() { + none() + } /** * Gets the AST node that declared this variable, or that introduced this * variable as part of the AST-to-IR translation. */ - abstract Language::AST getAST(); + Language::AST getAST() { + none() + } /** * Gets an identifier string for the variable. This identifier is unique * within the function. */ - abstract string getUniqueId(); + string getUniqueId() { + none() + } /** * Gets the source location of this variable. @@ -100,10 +108,14 @@ class IRUserVariable extends IRVariable, TIRUserVariable { * stack. This includes all parameters, non-static local variables, and * temporary variables. */ -abstract class IRAutomaticVariable extends IRVariable { +class IRAutomaticVariable extends IRVariable { + IRAutomaticVariable() { + this instanceof IRAutomaticUserVariable or + this instanceof IRTempVariable + } } -class IRAutomaticUserVariable extends IRUserVariable, IRAutomaticVariable { +class IRAutomaticUserVariable extends IRUserVariable { override Language::AutomaticVariable var; IRAutomaticUserVariable() { @@ -132,7 +144,7 @@ IRTempVariable getIRTempVariable(Language::AST ast, TempVariableTag tag) { result.getTag() = tag } -class IRTempVariable extends IRVariable, IRAutomaticVariable, TIRTempVariable { +class IRTempVariable extends IRVariable, TIRTempVariable { Language::AST ast; TempVariableTag tag; Language::Type type; diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/IRVariable.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/IRVariable.qll index b8c6af20a607..aff794da9279 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/IRVariable.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/IRVariable.qll @@ -16,27 +16,35 @@ IRUserVariable getIRUserVariable(Language::Function func, Language::Variable var * be a user-declared variable (`IRUserVariable`) or a temporary variable * generated by the AST-to-IR translation (`IRTempVariable`). */ -abstract class IRVariable extends TIRVariable { +class IRVariable extends TIRVariable { Language::Function func; - abstract string toString(); + string toString() { + none() + } /** * Gets the type of the variable. */ - abstract Language::Type getType(); + Language::Type getType() { + none() + } /** * Gets the AST node that declared this variable, or that introduced this * variable as part of the AST-to-IR translation. */ - abstract Language::AST getAST(); + Language::AST getAST() { + none() + } /** * Gets an identifier string for the variable. This identifier is unique * within the function. */ - abstract string getUniqueId(); + string getUniqueId() { + none() + } /** * Gets the source location of this variable. @@ -100,10 +108,14 @@ class IRUserVariable extends IRVariable, TIRUserVariable { * stack. This includes all parameters, non-static local variables, and * temporary variables. */ -abstract class IRAutomaticVariable extends IRVariable { +class IRAutomaticVariable extends IRVariable { + IRAutomaticVariable() { + this instanceof IRAutomaticUserVariable or + this instanceof IRTempVariable + } } -class IRAutomaticUserVariable extends IRUserVariable, IRAutomaticVariable { +class IRAutomaticUserVariable extends IRUserVariable { override Language::AutomaticVariable var; IRAutomaticUserVariable() { @@ -132,7 +144,7 @@ IRTempVariable getIRTempVariable(Language::AST ast, TempVariableTag tag) { result.getTag() = tag } -class IRTempVariable extends IRVariable, IRAutomaticVariable, TIRTempVariable { +class IRTempVariable extends IRVariable, TIRTempVariable { Language::AST ast; TempVariableTag tag; Language::Type type; From 448e4d47e4d9bdfc3e5a60b6fb55bd9fd04e23cd Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Wed, 14 Aug 2019 13:50:37 -0700 Subject: [PATCH 03/13] C++: Print memory accesses on the correct instruction in `PrintSSA` --- .../aliased_ssa/internal/PrintSSA.qll | 58 ++++++++++++------- .../unaliased_ssa/internal/PrintSSA.qll | 58 ++++++++++++------- 2 files changed, 72 insertions(+), 44 deletions(-) diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/PrintSSA.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/PrintSSA.qll index d2b510acecd2..6186e4ad11c4 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/PrintSSA.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/PrintSSA.qll @@ -4,31 +4,45 @@ private import Alias private import SSAConstruction private import DebugSSA +bindingset[offset] +private string getKeySuffixForOffset(int offset) { + if offset % 2 = 0 then + result = "" + else + result = "_Chi" +} + +bindingset[offset] +private int getIndexForOffset(int offset) { + result = offset / 2 +} + /** * Property provide that dumps the memory access of each result. Useful for debugging SSA * construction. */ class PropertyProvider extends IRPropertyProvider { override string getInstructionProperty(Instruction instruction, string key) { - exists(MemoryLocation location | - location = getResultMemoryLocation(instruction) and - ( - key = "ResultMemoryLocation" and result = location.toString() or - key = "ResultVirtualVariable" and result = location.getVirtualVariable().toString() - ) - ) - or - exists(MemoryLocation location | - location = getOperandMemoryLocation(instruction.getAnOperand()) and - ( - key = "OperandMemoryAccess" and result = location.toString() or - key = "OperandVirtualVariable" and result = location.getVirtualVariable().toString() - ) + ( + key = "ResultMemoryLocation" and + result = strictconcat(MemoryLocation loc | loc = getResultMemoryLocation(instruction) | loc.toString(), ",") + ) or + ( + key = "ResultVirtualVariable" and + result = strictconcat(MemoryLocation loc | loc = getResultMemoryLocation(instruction) | loc.getVirtualVariable().toString(), ",") + ) or + ( + key = "OperandMemoryLocation" and + result = strictconcat(MemoryLocation loc | loc = getOperandMemoryLocation(instruction.getAnOperand()) | loc.toString(), ",") + ) or + ( + key = "OperandVirtualVariable" and + result = strictconcat(MemoryLocation loc | loc = getOperandMemoryLocation(instruction.getAnOperand()) | loc.getVirtualVariable().toString(), ",") ) or - exists(MemoryLocation useLocation, IRBlock defBlock, int defRank, int defIndex | - hasDefinitionAtRank(useLocation, _, defBlock, defRank, defIndex) and - defBlock.getInstruction(defIndex) = instruction and - key = "DefinitionRank[" + useLocation.toString() + "]" and + exists(MemoryLocation useLocation, IRBlock defBlock, int defRank, int defOffset | + hasDefinitionAtRank(useLocation, _, defBlock, defRank, defOffset) and + defBlock.getInstruction(getIndexForOffset(defOffset)) = instruction and + key = "DefinitionRank" + getKeySuffixForOffset(defOffset) + "[" + useLocation.toString() + "]" and result = defRank.toString() ) or exists(MemoryLocation useLocation, IRBlock useBlock, int useRank | @@ -36,10 +50,10 @@ class PropertyProvider extends IRPropertyProvider { key = "UseRank[" + useLocation.toString() + "]" and result = useRank.toString() ) or - exists(MemoryLocation useLocation, IRBlock defBlock, int defRank, int defIndex | - hasDefinitionAtRank(useLocation, _, defBlock, defRank, defIndex) and - defBlock.getInstruction(defIndex) = instruction and - key = "DefinitionReachesUse[" + useLocation.toString() + "]" and + exists(MemoryLocation useLocation, IRBlock defBlock, int defRank, int defOffset | + hasDefinitionAtRank(useLocation, _, defBlock, defRank, defOffset) and + defBlock.getInstruction(getIndexForOffset(defOffset)) = instruction and + key = "DefinitionReachesUse" + getKeySuffixForOffset(defOffset) + "[" + useLocation.toString() + "]" and result = strictconcat(IRBlock useBlock, int useRank, int useIndex | exists(Instruction useInstruction | hasUseAtRank(useLocation, useBlock, useRank, useInstruction) and diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/PrintSSA.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/PrintSSA.qll index d2b510acecd2..6186e4ad11c4 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/PrintSSA.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/PrintSSA.qll @@ -4,31 +4,45 @@ private import Alias private import SSAConstruction private import DebugSSA +bindingset[offset] +private string getKeySuffixForOffset(int offset) { + if offset % 2 = 0 then + result = "" + else + result = "_Chi" +} + +bindingset[offset] +private int getIndexForOffset(int offset) { + result = offset / 2 +} + /** * Property provide that dumps the memory access of each result. Useful for debugging SSA * construction. */ class PropertyProvider extends IRPropertyProvider { override string getInstructionProperty(Instruction instruction, string key) { - exists(MemoryLocation location | - location = getResultMemoryLocation(instruction) and - ( - key = "ResultMemoryLocation" and result = location.toString() or - key = "ResultVirtualVariable" and result = location.getVirtualVariable().toString() - ) - ) - or - exists(MemoryLocation location | - location = getOperandMemoryLocation(instruction.getAnOperand()) and - ( - key = "OperandMemoryAccess" and result = location.toString() or - key = "OperandVirtualVariable" and result = location.getVirtualVariable().toString() - ) + ( + key = "ResultMemoryLocation" and + result = strictconcat(MemoryLocation loc | loc = getResultMemoryLocation(instruction) | loc.toString(), ",") + ) or + ( + key = "ResultVirtualVariable" and + result = strictconcat(MemoryLocation loc | loc = getResultMemoryLocation(instruction) | loc.getVirtualVariable().toString(), ",") + ) or + ( + key = "OperandMemoryLocation" and + result = strictconcat(MemoryLocation loc | loc = getOperandMemoryLocation(instruction.getAnOperand()) | loc.toString(), ",") + ) or + ( + key = "OperandVirtualVariable" and + result = strictconcat(MemoryLocation loc | loc = getOperandMemoryLocation(instruction.getAnOperand()) | loc.getVirtualVariable().toString(), ",") ) or - exists(MemoryLocation useLocation, IRBlock defBlock, int defRank, int defIndex | - hasDefinitionAtRank(useLocation, _, defBlock, defRank, defIndex) and - defBlock.getInstruction(defIndex) = instruction and - key = "DefinitionRank[" + useLocation.toString() + "]" and + exists(MemoryLocation useLocation, IRBlock defBlock, int defRank, int defOffset | + hasDefinitionAtRank(useLocation, _, defBlock, defRank, defOffset) and + defBlock.getInstruction(getIndexForOffset(defOffset)) = instruction and + key = "DefinitionRank" + getKeySuffixForOffset(defOffset) + "[" + useLocation.toString() + "]" and result = defRank.toString() ) or exists(MemoryLocation useLocation, IRBlock useBlock, int useRank | @@ -36,10 +50,10 @@ class PropertyProvider extends IRPropertyProvider { key = "UseRank[" + useLocation.toString() + "]" and result = useRank.toString() ) or - exists(MemoryLocation useLocation, IRBlock defBlock, int defRank, int defIndex | - hasDefinitionAtRank(useLocation, _, defBlock, defRank, defIndex) and - defBlock.getInstruction(defIndex) = instruction and - key = "DefinitionReachesUse[" + useLocation.toString() + "]" and + exists(MemoryLocation useLocation, IRBlock defBlock, int defRank, int defOffset | + hasDefinitionAtRank(useLocation, _, defBlock, defRank, defOffset) and + defBlock.getInstruction(getIndexForOffset(defOffset)) = instruction and + key = "DefinitionReachesUse" + getKeySuffixForOffset(defOffset) + "[" + useLocation.toString() + "]" and result = strictconcat(IRBlock useBlock, int useRank, int useIndex | exists(Instruction useInstruction | hasUseAtRank(useLocation, useBlock, useRank, useInstruction) and From 9c1eb7ea25d85b45f21a637398bf64d5c58113d1 Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Wed, 14 Aug 2019 13:53:48 -0700 Subject: [PATCH 04/13] C++: Add SSA sanity tests --- config/identical-files.json | 4 ++++ .../aliased_ssa/internal/SSAConstruction.qll | 14 ++++++++++++++ .../aliased_ssa/internal/SSASanity.ql | 8 ++++++++ .../aliased_ssa/internal/SSASanity.qll | 2 ++ .../unaliased_ssa/internal/SSAConstruction.qll | 14 ++++++++++++++ .../unaliased_ssa/internal/SSASanity.ql | 8 ++++++++ .../unaliased_ssa/internal/SSASanity.qll | 2 ++ .../ir/ssa/aliased_ssa_ssa_sanity.expected | 0 .../ir/ssa/aliased_ssa_ssa_sanity.qlref | 1 + .../ir/ssa/unaliased_ssa_ssa_sanity.expected | 0 .../ir/ssa/unaliased_ssa_ssa_sanity.qlref | 1 + 11 files changed, 54 insertions(+) create mode 100644 cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSASanity.ql create mode 100644 cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSASanity.qll create mode 100644 cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSASanity.ql create mode 100644 cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSASanity.qll create mode 100644 cpp/ql/test/library-tests/ir/ssa/aliased_ssa_ssa_sanity.expected create mode 100644 cpp/ql/test/library-tests/ir/ssa/aliased_ssa_ssa_sanity.qlref create mode 100644 cpp/ql/test/library-tests/ir/ssa/unaliased_ssa_ssa_sanity.expected create mode 100644 cpp/ql/test/library-tests/ir/ssa/unaliased_ssa_ssa_sanity.qlref diff --git a/config/identical-files.json b/config/identical-files.json index ebcfd3cb4f0b..3ab090225f57 100644 --- a/config/identical-files.json +++ b/config/identical-files.json @@ -61,6 +61,10 @@ "cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/PrintIR.qll", "cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/PrintIR.qll" ], + "IR SSASanity": [ + "cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSASanity.qll", + "cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSASanity.qll" + ], "C++ IR InstructionImports": [ "cpp/ql/src/semmle/code/cpp/ir/implementation/raw/internal/InstructionImports.qll", "cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/InstructionImports.qll", 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 d38943f10f7d..96d132cc6e02 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 @@ -1,5 +1,6 @@ import SSAConstructionInternal private import cpp +private import semmle.code.cpp.Print private import semmle.code.cpp.ir.implementation.Opcode private import semmle.code.cpp.ir.implementation.internal.OperandTag private import semmle.code.cpp.ir.internal.Overlap @@ -814,3 +815,16 @@ cached private module CachedForDebugging { result.getTag() = var.getTag() } } + +module SSASanity { + query predicate multipleOperandMemoryLocations(OldIR::MemoryOperand operand, string message, + OldIR::IRFunction func, string funcText) { + exists(int locationCount | + locationCount = strictcount(Alias::getOperandMemoryLocation(operand)) and + locationCount > 1 and + func = operand.getEnclosingIRFunction() and + funcText = getIdentityString(func.getFunction()) and + message = "Operand has " + locationCount.toString() + " memory accesses in function '$@'." + ) + } +} \ No newline at end of file diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSASanity.ql b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSASanity.ql new file mode 100644 index 000000000000..50368cdc6d9c --- /dev/null +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSASanity.ql @@ -0,0 +1,8 @@ +/** + * @name Aliased SSA Sanity Check + * @description Performs sanity checks on the SSA construction. This query should have no results. + * @kind table + * @id cpp/aliased-ssa-sanity-check + */ + +import SSASanity diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSASanity.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSASanity.qll new file mode 100644 index 000000000000..95e8443b2a38 --- /dev/null +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSASanity.qll @@ -0,0 +1,2 @@ +private import SSAConstruction as SSA +import SSA::SSASanity 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 d38943f10f7d..96d132cc6e02 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 @@ -1,5 +1,6 @@ import SSAConstructionInternal private import cpp +private import semmle.code.cpp.Print private import semmle.code.cpp.ir.implementation.Opcode private import semmle.code.cpp.ir.implementation.internal.OperandTag private import semmle.code.cpp.ir.internal.Overlap @@ -814,3 +815,16 @@ cached private module CachedForDebugging { result.getTag() = var.getTag() } } + +module SSASanity { + query predicate multipleOperandMemoryLocations(OldIR::MemoryOperand operand, string message, + OldIR::IRFunction func, string funcText) { + exists(int locationCount | + locationCount = strictcount(Alias::getOperandMemoryLocation(operand)) and + locationCount > 1 and + func = operand.getEnclosingIRFunction() and + funcText = getIdentityString(func.getFunction()) and + message = "Operand has " + locationCount.toString() + " memory accesses in function '$@'." + ) + } +} \ No newline at end of file diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSASanity.ql b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSASanity.ql new file mode 100644 index 000000000000..b67f6f281be8 --- /dev/null +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSASanity.ql @@ -0,0 +1,8 @@ +/** + * @name Unaliased SSA Sanity Check + * @description Performs sanity checks on the SSA construction. This query should have no results. + * @kind table + * @id cpp/unaliased-ssa-sanity-check + */ + +import SSASanity diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSASanity.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSASanity.qll new file mode 100644 index 000000000000..95e8443b2a38 --- /dev/null +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSASanity.qll @@ -0,0 +1,2 @@ +private import SSAConstruction as SSA +import SSA::SSASanity diff --git a/cpp/ql/test/library-tests/ir/ssa/aliased_ssa_ssa_sanity.expected b/cpp/ql/test/library-tests/ir/ssa/aliased_ssa_ssa_sanity.expected new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/cpp/ql/test/library-tests/ir/ssa/aliased_ssa_ssa_sanity.qlref b/cpp/ql/test/library-tests/ir/ssa/aliased_ssa_ssa_sanity.qlref new file mode 100644 index 000000000000..8e3480117851 --- /dev/null +++ b/cpp/ql/test/library-tests/ir/ssa/aliased_ssa_ssa_sanity.qlref @@ -0,0 +1 @@ +semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSASanity.ql \ No newline at end of file diff --git a/cpp/ql/test/library-tests/ir/ssa/unaliased_ssa_ssa_sanity.expected b/cpp/ql/test/library-tests/ir/ssa/unaliased_ssa_ssa_sanity.expected new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/cpp/ql/test/library-tests/ir/ssa/unaliased_ssa_ssa_sanity.qlref b/cpp/ql/test/library-tests/ir/ssa/unaliased_ssa_ssa_sanity.qlref new file mode 100644 index 000000000000..18bf9212dbf6 --- /dev/null +++ b/cpp/ql/test/library-tests/ir/ssa/unaliased_ssa_ssa_sanity.qlref @@ -0,0 +1 @@ +semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSASanity.ql \ No newline at end of file From 2abcaed416dadcf7d979b7f5ff90d76c818e8272 Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Wed, 14 Aug 2019 13:54:52 -0700 Subject: [PATCH 05/13] C++: Refactor some integer constant code Make `bitsToBytesAndBits` omit the leftover bits if zero. --- .../code/cpp/ir/internal/IntegerConstant.qll | 33 +++++++++++++++++++ .../code/cpp/ir/internal/IntegerInterval.qll | 2 +- 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/cpp/ql/src/semmle/code/cpp/ir/internal/IntegerConstant.qll b/cpp/ql/src/semmle/code/cpp/ir/internal/IntegerConstant.qll index d6e74a28414a..3c843dc497fc 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/internal/IntegerConstant.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/internal/IntegerConstant.qll @@ -253,3 +253,36 @@ bindingset[a, b] predicate isGE(IntValue a, IntValue b) { hasValue(a) and hasValue(b) and a >= b } + +/** + * Converts the bit count in `bits` to a byte count and a bit count in the form + * "bytes:bits". If `bits` represents an integer number of bytes, the ":bits" section is omitted. + * If `bits` does not have a known value, the result is "?". + */ +bindingset[bits] +string bitsToBytesAndBits(IntValue bits) { + exists(int bytes, int leftoverBits | + hasValue(bits) and + bytes = bits / 8 and + leftoverBits = bits % 8 and + if leftoverBits = 0 then + result = bytes.toString() + else + result = bytes + ":" + leftoverBits + ) or + not hasValue(bits) and result = "?" +} + +/** + * Gets a printable string for a bit offset with possibly unknown value. + */ +bindingset[bitOffset] +string getBitOffsetString(IntValue bitOffset) { + if hasValue(bitOffset) then + if bitOffset >= 0 then + result = "+" + bitsToBytesAndBits(bitOffset) + else + result = "-" + bitsToBytesAndBits(neg(bitOffset)) + else + result = "+?" +} diff --git a/cpp/ql/src/semmle/code/cpp/ir/internal/IntegerInterval.qll b/cpp/ql/src/semmle/code/cpp/ir/internal/IntegerInterval.qll index 986361ada588..c574f9816275 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/internal/IntegerInterval.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/internal/IntegerInterval.qll @@ -29,5 +29,5 @@ Overlap getOverlap(IntValue defStart, IntValue defEnd, IntValue useStart, IntVal bindingset[start, end] string getIntervalString(IntValue start, IntValue end) { // We represent an interval has half-open, so print it as "[start..end)". - result = "[" + intValueToString(start) + ".." + intValueToString(end) + ")" + result = "[" + bitsToBytesAndBits(start) + ".." + bitsToBytesAndBits(end) + ")" } From 336b7d324c24042b9ffd0896f50f801cca4c68d4 Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Wed, 14 Aug 2019 13:57:00 -0700 Subject: [PATCH 06/13] C++: Update points_to test for new alias analysis Adds `InlineExpectationsTest.qll`, which makes it easy to create a QL test where the expectations are provided as comments in the source code being tested. --- .../TestUtilities/InlineExpectationsTest.qll | 224 ++++++++++++++++++ .../ir/escape/points_to.expected | 47 ---- .../test/library-tests/ir/escape/points_to.ql | 34 --- .../library-tests/ir/points_to/points_to.cpp | 74 ++++++ .../ir/points_to/points_to.expected | 0 .../library-tests/ir/points_to/points_to.ql | 72 ++++++ 6 files changed, 370 insertions(+), 81 deletions(-) create mode 100644 cpp/ql/test/TestUtilities/InlineExpectationsTest.qll delete mode 100644 cpp/ql/test/library-tests/ir/escape/points_to.expected delete mode 100644 cpp/ql/test/library-tests/ir/escape/points_to.ql create mode 100644 cpp/ql/test/library-tests/ir/points_to/points_to.cpp create mode 100644 cpp/ql/test/library-tests/ir/points_to/points_to.expected create mode 100644 cpp/ql/test/library-tests/ir/points_to/points_to.ql diff --git a/cpp/ql/test/TestUtilities/InlineExpectationsTest.qll b/cpp/ql/test/TestUtilities/InlineExpectationsTest.qll new file mode 100644 index 000000000000..7da58b1f35b5 --- /dev/null +++ b/cpp/ql/test/TestUtilities/InlineExpectationsTest.qll @@ -0,0 +1,224 @@ +import cpp + +abstract class InlineExpectationsTest extends string { + bindingset[this] + InlineExpectationsTest() { + any() + } + + abstract string getARelevantTag(); + + abstract predicate hasActualResult(Location location, string element, string tag, string value); + + final predicate hasFailureMessage(FailureLocatable element, string message) { + exists(ActualResult actualResult | + actualResult.getTest() = this and + element = actualResult and + ( + exists(FalseNegativeExpectation falseNegative | + falseNegative.matchesActualResult(actualResult) and + message = "Fixed false negative:" + falseNegative.getExpectationText() + ) or + ( + not exists(ValidExpectation expectation | + expectation.matchesActualResult(actualResult) + ) and + message = "Unexpected result: " + actualResult.getExpectationText() + ) + ) + ) or + exists(ValidExpectation expectation | + not exists(ActualResult actualResult | + expectation.matchesActualResult(actualResult) + ) and + expectation.getTag() = getARelevantTag() and + element = expectation and + ( + ( + expectation instanceof GoodExpectation and + message = "Missing result:" + expectation.getExpectationText() + ) or + ( + expectation instanceof FalsePositiveExpectation and + message = "Fixed false positive:" + expectation.getExpectationText() + ) + ) + ) or + exists(InvalidExpectation expectation | + element = expectation and + message = "Invalid expectation syntax: " + expectation.getExpectation() + ) + } +} + +private string expectationCommentPattern() { + result = "//\\s*(\\$(?:[^/]|/[^/])*)(?://.*)?" +} + +private string expectationPattern() { + result = "(?:(f(?:\\+|-)):)?((?:[A-Za-z-_]+)(?:\\s*,\\s*[A-Za-z-_]+)*)(?:=(.*))?" +} + +private string getAnExpectation(CppStyleComment comment) { + result = comment.getContents().regexpCapture(expectationCommentPattern(), 1).splitAt("$").trim() and + result != "" +} + +private newtype TFailureLocatable = + TActualResult(InlineExpectationsTest test, Location location, string element, string tag, string value) { + test.hasActualResult(location, element, tag, value) + } or + TValidExpectation(CppStyleComment comment, string tag, string value, string knownFailure) { + exists(string expectation | + expectation = getAnExpectation(comment) and + expectation.regexpMatch(expectationPattern()) and + tag = expectation.regexpCapture(expectationPattern(), 2).splitAt(",").trim() and + ( + if exists(expectation.regexpCapture(expectationPattern(), 3)) then + value = expectation.regexpCapture(expectationPattern(), 3) + else + value = "" + ) and + ( + if exists(expectation.regexpCapture(expectationPattern(), 1)) then + knownFailure = expectation.regexpCapture(expectationPattern(), 1) + else + knownFailure = "" + ) + ) + } or + TInvalidExpectation(CppStyleComment comment, string expectation) { + expectation = getAnExpectation(comment) and + not expectation.regexpMatch(expectationPattern()) + } + +class FailureLocatable extends TFailureLocatable { + string toString() { + none() + } + + Location getLocation() { + none() + } + + final string getExpectationText() { + result = getTag() + "=" + getValue() + } + + string getTag() { + none() + } + + string getValue() { + none() + } +} + +class ActualResult extends FailureLocatable, TActualResult { + InlineExpectationsTest test; + Location location; + string element; + string tag; + string value; + + ActualResult() { + this = TActualResult(test, location, element, tag, value) + } + + override string toString() { + result = element + } + + override Location getLocation() { + result = location + } + + InlineExpectationsTest getTest() { + result = test + } + + override string getTag() { + result = tag + } + + override string getValue() { + result = value + } +} + +private abstract class Expectation extends FailureLocatable { + CppStyleComment comment; + + override string toString() { + result = comment.toString() + } + + override Location getLocation() { + result = comment.getLocation() + } +} + +private class ValidExpectation extends Expectation, TValidExpectation { + string tag; + string value; + string knownFailure; + + ValidExpectation() { + this = TValidExpectation(comment, tag, value, knownFailure) + } + + override string getTag() { + result = tag + } + + override string getValue() { + result = value + } + + string getKnownFailure() { + result = knownFailure + } + + predicate matchesActualResult(ActualResult actualResult) { + getLocation().getStartLine() = actualResult.getLocation().getStartLine() and + getLocation().getFile() = actualResult.getLocation().getFile() and + getTag() = actualResult.getTag() and + getValue() = actualResult.getValue() + } +} + +class GoodExpectation extends ValidExpectation { + GoodExpectation() { + getKnownFailure() = "" + } +} + +class FalsePositiveExpectation extends ValidExpectation { + FalsePositiveExpectation() { + getKnownFailure() = "f+" + } +} + +class FalseNegativeExpectation extends ValidExpectation { + FalseNegativeExpectation() { + getKnownFailure() = "f-" + } +} + +class InvalidExpectation extends Expectation, TInvalidExpectation { + string expectation; + + InvalidExpectation() { + this = TInvalidExpectation(comment, expectation) + } + + string getExpectation() { + result = expectation + } +} + +query predicate failures(FailureLocatable element, string message) { + exists(InlineExpectationsTest test | + test.hasFailureMessage(element, message) + ) +} diff --git a/cpp/ql/test/library-tests/ir/escape/points_to.expected b/cpp/ql/test/library-tests/ir/escape/points_to.expected deleted file mode 100644 index f40460f4d91b..000000000000 --- a/cpp/ql/test/library-tests/ir/escape/points_to.expected +++ /dev/null @@ -1,47 +0,0 @@ -| escape.cpp:115:19:115:28 | PointerAdd[4] | no_+0:0 | no_+0:0 | -| escape.cpp:116:19:116:28 | PointerSub[4] | no_+0:0 | no_+0:0 | -| escape.cpp:117:19:117:26 | PointerAdd[4] | no_+0:0 | no_+0:0 | -| escape.cpp:134:5:134:18 | Convert | no_Array+0:0 | no_Array+0:0 | -| escape.cpp:134:11:134:18 | Convert | no_Array+0:0 | no_Array+0:0 | -| escape.cpp:135:5:135:12 | Convert | no_Array+0:0 | no_Array+0:0 | -| escape.cpp:135:5:135:15 | PointerAdd[4] | no_Array+20:0 | no_Array+20:0 | -| escape.cpp:136:5:136:15 | PointerAdd[4] | no_Array+20:0 | no_Array+20:0 | -| escape.cpp:136:7:136:14 | Convert | no_Array+0:0 | no_Array+0:0 | -| escape.cpp:137:17:137:24 | Convert | no_Array+0:0 | no_Array+0:0 | -| escape.cpp:137:17:137:27 | PointerAdd[4] | no_Array+20:0 | no_Array+20:0 | -| escape.cpp:138:17:138:27 | PointerAdd[4] | no_Array+20:0 | no_Array+20:0 | -| escape.cpp:138:19:138:26 | Convert | no_Array+0:0 | no_Array+0:0 | -| escape.cpp:140:21:140:32 | FieldAddress[x] | no_Point+0:0 | no_Point+0:0 | -| escape.cpp:140:21:140:32 | FieldAddress[y] | no_Point+4:0 | no_Point+4:0 | -| escape.cpp:140:21:140:32 | FieldAddress[z] | no_Point+8:0 | no_Point+8:0 | -| escape.cpp:141:27:141:27 | FieldAddress[x] | no_Point+0:0 | no_Point+0:0 | -| escape.cpp:142:14:142:14 | FieldAddress[y] | no_Point+4:0 | no_Point+4:0 | -| escape.cpp:143:31:143:31 | FieldAddress[y] | no_Point+4:0 | no_Point+4:0 | -| escape.cpp:144:18:144:18 | FieldAddress[y] | no_Point+4:0 | no_Point+4:0 | -| escape.cpp:145:30:145:30 | FieldAddress[z] | no_Point+8:0 | no_Point+8:0 | -| escape.cpp:146:17:146:17 | FieldAddress[z] | no_Point+8:0 | no_Point+8:0 | -| escape.cpp:149:5:149:14 | ConvertToBase[Derived : Intermediate1] | no_Derived+0:0 | no_Derived+0:0 | -| escape.cpp:149:5:149:14 | ConvertToBase[Intermediate1 : Base] | no_Derived+0:0 | no_Derived+0:0 | -| escape.cpp:149:16:149:16 | FieldAddress[b] | no_Derived+0:0 | no_Derived+0:0 | -| escape.cpp:150:18:150:27 | ConvertToBase[Derived : Intermediate1] | no_Derived+0:0 | no_Derived+0:0 | -| escape.cpp:150:18:150:27 | ConvertToBase[Intermediate1 : Base] | no_Derived+0:0 | no_Derived+0:0 | -| escape.cpp:150:29:150:29 | FieldAddress[b] | no_Derived+0:0 | no_Derived+0:0 | -| escape.cpp:151:5:151:14 | ConvertToBase[Derived : Intermediate2] | no_Derived+12:0 | no_Derived+12:0 | -| escape.cpp:151:16:151:17 | FieldAddress[i2] | no_Derived+16:0 | no_Derived+16:0 | -| escape.cpp:152:19:152:28 | ConvertToBase[Derived : Intermediate2] | no_Derived+12:0 | no_Derived+12:0 | -| escape.cpp:152:30:152:31 | FieldAddress[i2] | no_Derived+16:0 | no_Derived+16:0 | -| escape.cpp:155:17:155:30 | Store | no_ssa_addrOf+0:0 | no_ssa_addrOf+0:0 | -| escape.cpp:158:17:158:28 | Store | no_ssa_refTo+0:0 | no_ssa_refTo+0:0 | -| escape.cpp:161:19:161:42 | Convert | no_ssa_refToArrayElement+0:0 | no_ssa_refToArrayElement+0:0 | -| escape.cpp:161:19:161:45 | PointerAdd[4] | no_ssa_refToArrayElement+20:0 | no_ssa_refToArrayElement+20:0 | -| escape.cpp:161:19:161:45 | Store | no_ssa_refToArrayElement+20:0 | no_ssa_refToArrayElement+20:0 | -| escape.cpp:164:24:164:40 | Store | no_ssa_refToArray+0:0 | no_ssa_refToArray+0:0 | -| escape.cpp:191:30:191:42 | Call | none | passByPtr3+0:0 | -| escape.cpp:194:32:194:46 | Call | none | passByRef3+0:0 | -| escape.cpp:202:5:202:19 | Call | none | passByRef6+0:0 | -| escape.cpp:205:5:205:19 | Call | none | no_ssa_passByRef7+0:0 | -| escape.cpp:209:14:209:25 | Call | none | no_ssa_c+0:0 | -| escape.cpp:221:8:221:19 | Call | none | c3+0:0 | -| escape.cpp:225:17:225:28 | Call | none | c4+0:0 | -| escape.cpp:247:2:247:27 | Store | condEscape1+0:0 | condEscape1+0:0 | -| escape.cpp:249:9:249:34 | Store | condEscape2+0:0 | condEscape2+0:0 | diff --git a/cpp/ql/test/library-tests/ir/escape/points_to.ql b/cpp/ql/test/library-tests/ir/escape/points_to.ql deleted file mode 100644 index 04dbf0e0060d..000000000000 --- a/cpp/ql/test/library-tests/ir/escape/points_to.ql +++ /dev/null @@ -1,34 +0,0 @@ -import default -import semmle.code.cpp.ir.implementation.unaliased_ssa.internal.AliasAnalysis as RawAA -import semmle.code.cpp.ir.implementation.raw.IR as Raw -import semmle.code.cpp.ir.implementation.aliased_ssa.internal.AliasAnalysis as UnAA -import semmle.code.cpp.ir.implementation.unaliased_ssa.IR as Un -import semmle.code.cpp.ir.implementation.unaliased_ssa.internal.SSAConstruction - -from Raw::Instruction rawInstr, Un::Instruction unInstr, string rawPointsTo, string unPointsTo -where - rawInstr = getOldInstruction(unInstr) and - not rawInstr instanceof Raw::VariableAddressInstruction and - ( - exists(Variable var, int rawBitOffset, int unBitOffset | - RawAA::resultPointsTo(rawInstr, Raw::getIRUserVariable(_, var), rawBitOffset) and - rawPointsTo = var.toString() + RawAA::getBitOffsetString(rawBitOffset) and - UnAA::resultPointsTo(unInstr, Un::getIRUserVariable(_, var), unBitOffset) and - unPointsTo = var.toString() + UnAA::getBitOffsetString(unBitOffset) - ) - or - exists(Variable var, int unBitOffset | - not RawAA::resultPointsTo(rawInstr, Raw::getIRUserVariable(_, var), _) and - rawPointsTo = "none" and - UnAA::resultPointsTo(unInstr, Un::getIRUserVariable(_, var), unBitOffset) and - unPointsTo = var.toString() + UnAA::getBitOffsetString(unBitOffset) - ) - or - exists(Variable var, int rawBitOffset | - RawAA::resultPointsTo(rawInstr, Raw::getIRUserVariable(_, var), rawBitOffset) and - rawPointsTo = var.toString() + RawAA::getBitOffsetString(rawBitOffset) and - not UnAA::resultPointsTo(unInstr, Un::getIRUserVariable(_, var), _) and - unPointsTo = "none" - ) - ) -select rawInstr.getLocation().toString(), rawInstr.getOperationString(), rawPointsTo, unPointsTo diff --git a/cpp/ql/test/library-tests/ir/points_to/points_to.cpp b/cpp/ql/test/library-tests/ir/points_to/points_to.cpp new file mode 100644 index 000000000000..87ed0166a91a --- /dev/null +++ b/cpp/ql/test/library-tests/ir/points_to/points_to.cpp @@ -0,0 +1,74 @@ +struct Point { + int x; + int y; +}; + +struct Base1 { + int b1; +}; + +struct Base2 { + int b2; +}; + +struct DerivedSI : Base1 { + int dsi; +}; + +struct DerivedMI : Base1, Base2 { + int dmi; +}; + +struct DerivedVI : virtual Base1 { + int dvi; +}; + +void PointsTo( + int a, //$raw,ussa=a + Point& b, //$raw,ussa=b + Point* c, //$raw,ussa=c + int* d, //$raw,ussa=d + DerivedSI* e, //$raw,ussa=e + DerivedMI* f, //$raw,ussa=f + DerivedVI* g //$raw,ussa=g +) { + + int i = a; //$raw,ussa=a + i = *&a; //$raw,ussa=a + i = *(&a + 0); //$raw,ussa=a + i = b.x; //$raw,ussa=b $ussa=??@'b'+[0..4) + i = b.y; //$raw,ussa=b $ussa=??@'b'+[4..8) + i = c->x; //$raw,ussa=c $ussa=??@'c'+[0..4) + i = c->y; //$raw,ussa=c $ussa=??@'c'+[4..8) + i = *d; //$raw,ussa=d $ussa=??@'d'+[0..4) + i = *(d + 0); //$raw,ussa=d $ussa=??@'d'+[0..4) + i = d[5]; //$raw,ussa=d $ussa=??@'d'+[20..24) + i = 5[d]; //$raw,ussa=d $ussa=??@'d'+[20..24) + i = d[a]; //$raw,ussa=d $raw,ussa=a $ussa=??@'PointerAdd: access to array'+[0..4) + i = a[d]; //$raw,ussa=d $raw,ussa=a $ussa=??@'PointerAdd: access to array'+[0..4) + + int* p = &b.x; //$raw,ussa=b + i = *p; //$ussa=??@'b'+[0..4) + p = &b.y; //$raw,ussa=b + i = *p; //$ussa=??@'b'+[4..8) + p = &c->x; //$raw,ussa=c + i = *p; //$ussa=??@'c'+[0..4) + p = &c->y; //$raw,ussa=c + i = *p; //$ussa=??@'c'+[4..8) + p = &d[5]; //$raw,ussa=d + i = *p; //$ussa=??@'d'+[20..24) + p = &d[a]; //$raw,ussa=d $raw,ussa=a + i = *p; //$ussa=??@'PointerAdd: access to array'+[0..4) + + Point* q = &c[a]; //$raw,ussa=c $raw,ussa=a + i = q->x; //$ussa=??@'PointerAdd: access to array'+[0..4) + i = q->y; //$ussa=??@'PointerAdd: access to array'+[4..8) + + i = e->b1; //$raw,ussa=e $ussa=??@'e'+[0..4) + i = e->dsi; //$raw,ussa=e $ussa=??@'e'+[4..8) + i = f->b1; //$raw,ussa=f $ussa=??@'f'+[0..4) + i = f->b2; //$raw,ussa=f $ussa=??@'f'+[4..8) + i = f->dmi; //$raw,ussa=f $ussa=??@'f'+[8..12) + i = g->b1; //$raw,ussa=g $ussa=??@'ConvertToVirtualBase: (Base1 *)...'+[0..4) + i = g->dvi; //$raw,ussa=g $ussa=??@'g'+[8..12) +} \ No newline at end of file diff --git a/cpp/ql/test/library-tests/ir/points_to/points_to.expected b/cpp/ql/test/library-tests/ir/points_to/points_to.expected new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/cpp/ql/test/library-tests/ir/points_to/points_to.ql b/cpp/ql/test/library-tests/ir/points_to/points_to.ql new file mode 100644 index 000000000000..cd1eada1c1ab --- /dev/null +++ b/cpp/ql/test/library-tests/ir/points_to/points_to.ql @@ -0,0 +1,72 @@ +import cpp +private import TestUtilities.InlineExpectationsTest +private import semmle.code.cpp.ir.internal.IntegerConstant as Ints + +private predicate ignoreAllocation(string name) { + name = "i" or + name = "p" or + name = "q" +} + +module Raw { + private import semmle.code.cpp.ir.implementation.raw.IR + private import semmle.code.cpp.ir.implementation.unaliased_ssa.internal.SimpleSSA + + private MemoryLocation getAMemoryAccess(Instruction instr) { + result = getResultMemoryLocation(instr) or + result = getOperandMemoryLocation(instr.getAnOperand()) + } + + class RawPointsToTest extends InlineExpectationsTest { + RawPointsToTest() { + this = "RawPointsToTest" + } + + override string getARelevantTag() { + result = "raw" + } + + override predicate hasActualResult(Location location, string element, string tag, string value) { + exists(Instruction instr, MemoryLocation memLocation | + memLocation = getAMemoryAccess(instr) and + tag = "raw" and + not ignoreAllocation(memLocation.getAllocation().getAllocationString()) and + value = memLocation.toString() and + element = instr.toString() and + location = instr.getLocation() + ) + } + } +} + +module UnaliasedSSA { + private import semmle.code.cpp.ir.implementation.unaliased_ssa.IR + private import semmle.code.cpp.ir.implementation.aliased_ssa.internal.AliasedSSA + + private MemoryLocation getAMemoryAccess(Instruction instr) { + result = getResultMemoryLocation(instr) or + result = getOperandMemoryLocation(instr.getAnOperand()) + } + + class UnaliasedSSAPointsToTest extends InlineExpectationsTest { + UnaliasedSSAPointsToTest() { + this = "UnaliasedSSAPointsToTest" + } + + override string getARelevantTag() { + result = "ussa" + } + + override predicate hasActualResult(Location location, string element, string tag, string value) { + exists(Instruction instr, MemoryLocation memLocation | + memLocation = getAMemoryAccess(instr) and + not memLocation instanceof UnknownVirtualVariable and + tag = "ussa" and + not ignoreAllocation(memLocation.getAllocation().getAllocationString()) and + value = memLocation.toString() and + element = instr.toString() and + location = instr.getLocation() + ) + } + } +} From 22b776dc01ca743092e5e4952076a1633513af15 Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Wed, 14 Aug 2019 13:57:59 -0700 Subject: [PATCH 07/13] C++: Fix escape tests for alias API changes --- cpp/ql/test/library-tests/ir/escape/escape.ql | 4 ++-- cpp/ql/test/library-tests/ir/escape/ssa_escape.ql | 10 ++++++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/cpp/ql/test/library-tests/ir/escape/escape.ql b/cpp/ql/test/library-tests/ir/escape/escape.ql index 68a308e65d12..b85b116eed16 100644 --- a/cpp/ql/test/library-tests/ir/escape/escape.ql +++ b/cpp/ql/test/library-tests/ir/escape/escape.ql @@ -15,8 +15,8 @@ where exists(IRFunction irFunc | irFunc = var.getEnclosingIRFunction() and ( - (shouldEscape(var) and variableAddressEscapes(var)) or - (not shouldEscape(var) and not variableAddressEscapes(var)) + (shouldEscape(var) and allocationEscapes(var)) or + (not shouldEscape(var) and not allocationEscapes(var)) ) ) select var diff --git a/cpp/ql/test/library-tests/ir/escape/ssa_escape.ql b/cpp/ql/test/library-tests/ir/escape/ssa_escape.ql index c1cc2e408e9e..d247d8127e5d 100644 --- a/cpp/ql/test/library-tests/ir/escape/ssa_escape.ql +++ b/cpp/ql/test/library-tests/ir/escape/ssa_escape.ql @@ -1,5 +1,6 @@ import default import semmle.code.cpp.ir.implementation.aliased_ssa.internal.AliasAnalysis +import semmle.code.cpp.ir.implementation.aliased_ssa.internal.AliasConfiguration import semmle.code.cpp.ir.implementation.unaliased_ssa.IR predicate shouldEscape(IRAutomaticUserVariable var) { @@ -11,11 +12,12 @@ predicate shouldEscape(IRAutomaticUserVariable var) { from IRAutomaticUserVariable var where - exists(IRFunction irFunc | - irFunc = var.getEnclosingIRFunction() and + exists(VariableAddressInstruction instr, Allocation allocation | + instr.getVariable() = var and + allocation.getAnInstruction() = instr and ( - (shouldEscape(var) and variableAddressEscapes(var)) or - (not shouldEscape(var) and not variableAddressEscapes(var)) + (shouldEscape(var) and allocationEscapes(allocation)) or + (not shouldEscape(var) and not allocationEscapes(allocation)) ) ) select var From 5b2baa4d0631b1b9ac2f3fb2b2e0d432977d61ac Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Wed, 14 Aug 2019 13:59:41 -0700 Subject: [PATCH 08/13] C++: Add SSA sanity tests to IR tests --- cpp/ql/test/library-tests/ir/ir/aliased_ssa_ssa_sanity.expected | 0 cpp/ql/test/library-tests/ir/ir/aliased_ssa_ssa_sanity.qlref | 1 + .../test/library-tests/ir/ir/unaliased_ssa_ssa_sanity.expected | 0 cpp/ql/test/library-tests/ir/ir/unaliased_ssa_ssa_sanity.qlref | 1 + 4 files changed, 2 insertions(+) create mode 100644 cpp/ql/test/library-tests/ir/ir/aliased_ssa_ssa_sanity.expected create mode 100644 cpp/ql/test/library-tests/ir/ir/aliased_ssa_ssa_sanity.qlref create mode 100644 cpp/ql/test/library-tests/ir/ir/unaliased_ssa_ssa_sanity.expected create mode 100644 cpp/ql/test/library-tests/ir/ir/unaliased_ssa_ssa_sanity.qlref diff --git a/cpp/ql/test/library-tests/ir/ir/aliased_ssa_ssa_sanity.expected b/cpp/ql/test/library-tests/ir/ir/aliased_ssa_ssa_sanity.expected new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/cpp/ql/test/library-tests/ir/ir/aliased_ssa_ssa_sanity.qlref b/cpp/ql/test/library-tests/ir/ir/aliased_ssa_ssa_sanity.qlref new file mode 100644 index 000000000000..8e3480117851 --- /dev/null +++ b/cpp/ql/test/library-tests/ir/ir/aliased_ssa_ssa_sanity.qlref @@ -0,0 +1 @@ +semmle/code/cpp/ir/implementation/aliased_ssa/internal/SSASanity.ql \ No newline at end of file diff --git a/cpp/ql/test/library-tests/ir/ir/unaliased_ssa_ssa_sanity.expected b/cpp/ql/test/library-tests/ir/ir/unaliased_ssa_ssa_sanity.expected new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/cpp/ql/test/library-tests/ir/ir/unaliased_ssa_ssa_sanity.qlref b/cpp/ql/test/library-tests/ir/ir/unaliased_ssa_ssa_sanity.qlref new file mode 100644 index 000000000000..18bf9212dbf6 --- /dev/null +++ b/cpp/ql/test/library-tests/ir/ir/unaliased_ssa_ssa_sanity.qlref @@ -0,0 +1 @@ +semmle/code/cpp/ir/implementation/unaliased_ssa/internal/SSASanity.ql \ No newline at end of file From 4aab88e4a5141832dc602c13af75117135f28280 Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Wed, 14 Aug 2019 14:00:17 -0700 Subject: [PATCH 09/13] C++: Add SSA test case for new alias analysis --- .../ir/ssa/aliased_ssa_ir.expected | 140 +++++++++++++++--- cpp/ql/test/library-tests/ir/ssa/ssa.cpp | 20 ++- .../ir/ssa/unaliased_ssa_ir.expected | 91 ++++++++++++ 3 files changed, 230 insertions(+), 21 deletions(-) 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 fe11d99f31f6..94d07f1a40f5 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 @@ -40,9 +40,11 @@ ssa.cpp: # 21| Block 3 # 21| m3_0(unknown) = Phi : from 1:~m1_7, from 2:~m2_7 -# 21| r3_1(glval) = VariableAddress[which2] : -# 21| r3_2(bool) = Load : &:r3_1, m0_8 -# 21| v3_3(void) = ConditionalBranch : r3_2 +# 21| m3_1(int) = Phi : from 1:m1_6, from 2:~m0_1 +# 21| m3_2(int) = Phi : from 1:~m0_1, from 2:m2_6 +# 21| r3_3(glval) = VariableAddress[which2] : +# 21| r3_4(bool) = Load : &:r3_3, m0_8 +# 21| v3_5(void) = ConditionalBranch : r3_4 #-----| False -> Block 5 #-----| True -> Block 4 @@ -50,7 +52,7 @@ ssa.cpp: # 22| r4_0(glval) = VariableAddress[p] : # 22| r4_1(Point *) = Load : &:r4_0, m0_4 # 22| r4_2(glval) = FieldAddress[x] : r4_1 -# 22| r4_3(int) = Load : &:r4_2, ~m3_0 +# 22| r4_3(int) = Load : &:r4_2, m3_1 # 22| r4_4(int) = Constant[1] : # 22| r4_5(int) = Add : r4_3, r4_4 # 22| m4_6(int) = Store : &:r4_2, r4_5 @@ -61,7 +63,7 @@ ssa.cpp: # 25| r5_0(glval) = VariableAddress[p] : # 25| r5_1(Point *) = Load : &:r5_0, m0_4 # 25| r5_2(glval) = FieldAddress[y] : r5_1 -# 25| r5_3(int) = Load : &:r5_2, ~m3_0 +# 25| r5_3(int) = Load : &:r5_2, m3_2 # 25| r5_4(int) = Constant[1] : # 25| r5_5(int) = Add : r5_3, r5_4 # 25| m5_6(int) = Store : &:r5_2, r5_5 @@ -70,21 +72,23 @@ ssa.cpp: # 28| Block 6 # 28| m6_0(unknown) = Phi : from 4:~m4_7, from 5:~m5_7 -# 28| r6_1(glval) = VariableAddress[#return] : -# 28| r6_2(glval) = VariableAddress[p] : -# 28| r6_3(Point *) = Load : &:r6_2, m0_4 -# 28| r6_4(glval) = FieldAddress[x] : r6_3 -# 28| r6_5(int) = Load : &:r6_4, ~m6_0 -# 28| r6_6(glval) = VariableAddress[p] : -# 28| r6_7(Point *) = Load : &:r6_6, m0_4 -# 28| r6_8(glval) = FieldAddress[y] : r6_7 -# 28| r6_9(int) = Load : &:r6_8, ~m6_0 -# 28| r6_10(int) = Add : r6_5, r6_9 -# 28| m6_11(int) = Store : &:r6_1, r6_10 -# 13| r6_12(glval) = VariableAddress[#return] : -# 13| v6_13(void) = ReturnValue : &:r6_12, m6_11 -# 13| v6_14(void) = UnmodeledUse : mu* -# 13| v6_15(void) = ExitFunction : +# 28| m6_1(int) = Phi : from 4:m4_6, from 5:m3_1 +# 28| m6_2(int) = Phi : from 4:m3_2, from 5:m5_6 +# 28| r6_3(glval) = VariableAddress[#return] : +# 28| r6_4(glval) = VariableAddress[p] : +# 28| r6_5(Point *) = Load : &:r6_4, m0_4 +# 28| r6_6(glval) = FieldAddress[x] : r6_5 +# 28| r6_7(int) = Load : &:r6_6, m6_1 +# 28| r6_8(glval) = VariableAddress[p] : +# 28| r6_9(Point *) = Load : &:r6_8, m0_4 +# 28| r6_10(glval) = FieldAddress[y] : r6_9 +# 28| r6_11(int) = Load : &:r6_10, m6_2 +# 28| r6_12(int) = Add : r6_7, r6_11 +# 28| m6_13(int) = Store : &:r6_3, r6_12 +# 13| r6_14(glval) = VariableAddress[#return] : +# 13| v6_15(void) = ReturnValue : &:r6_14, m6_13 +# 13| v6_16(void) = UnmodeledUse : mu* +# 13| v6_17(void) = ExitFunction : # 31| int UnreachableViaGoto() # 31| Block 0 @@ -807,3 +811,99 @@ ssa.cpp: # 198| v0_43(void) = ReturnValue : &:r0_42, m0_41 # 198| v0_44(void) = UnmodeledUse : mu* # 198| v0_45(void) = ExitFunction : + +# 205| void IndirectParams(int*, Point*, Point*) +# 205| Block 0 +# 205| v0_0(void) = EnterFunction : +# 205| m0_1(unknown) = AliasedDefinition : +# 205| mu0_2(unknown) = UnmodeledDefinition : +# 205| r0_3(glval) = VariableAddress[pi] : +# 205| m0_4(int *) = InitializeParameter[pi] : &:r0_3 +# 205| r0_5(glval) = VariableAddress[ppt] : +# 205| m0_6(Point *) = InitializeParameter[ppt] : &:r0_5 +# 205| r0_7(glval) = VariableAddress[apt] : +# 205| m0_8(Point *) = InitializeParameter[apt] : &:r0_7 +# 206| r0_9(glval) = VariableAddress[x] : +# 206| r0_10(glval) = VariableAddress[pi] : +# 206| r0_11(int *) = Load : &:r0_10, m0_4 +# 206| r0_12(int) = Load : &:r0_11, ~m0_1 +# 206| m0_13(int) = Store : &:r0_9, r0_12 +# 207| r0_14(int) = Constant[5] : +# 207| r0_15(glval) = VariableAddress[pi] : +# 207| r0_16(int *) = Load : &:r0_15, m0_4 +# 207| m0_17(int) = Store : &:r0_16, r0_14 +# 207| m0_18(unknown) = Chi : total:m0_1, partial:m0_17 +# 208| r0_19(glval) = VariableAddress[y] : +# 208| r0_20(glval) = VariableAddress[pi] : +# 208| r0_21(int *) = Load : &:r0_20, m0_4 +# 208| r0_22(int) = Load : &:r0_21, m0_17 +# 208| m0_23(int) = Store : &:r0_19, r0_22 +# 210| r0_24(glval) = VariableAddress[ppt] : +# 210| r0_25(Point *) = Load : &:r0_24, m0_6 +# 210| r0_26(glval) = FieldAddress[x] : r0_25 +# 210| r0_27(int) = Load : &:r0_26, ~m0_18 +# 210| r0_28(glval) = VariableAddress[x] : +# 210| m0_29(int) = Store : &:r0_28, r0_27 +# 211| r0_30(int) = Constant[2] : +# 211| r0_31(glval) = VariableAddress[ppt] : +# 211| r0_32(Point *) = Load : &:r0_31, m0_6 +# 211| r0_33(glval) = FieldAddress[x] : r0_32 +# 211| m0_34(int) = Store : &:r0_33, r0_30 +# 211| m0_35(unknown) = Chi : total:m0_18, partial:m0_34 +# 212| r0_36(int) = Constant[4] : +# 212| r0_37(glval) = VariableAddress[ppt] : +# 212| r0_38(Point *) = Load : &:r0_37, m0_6 +# 212| r0_39(glval) = FieldAddress[y] : r0_38 +# 212| m0_40(int) = Store : &:r0_39, r0_36 +# 212| m0_41(unknown) = Chi : total:m0_35, partial:m0_40 +# 213| r0_42(glval) = VariableAddress[ppt] : +# 213| r0_43(Point *) = Load : &:r0_42, m0_6 +# 213| r0_44(glval) = FieldAddress[x] : r0_43 +# 213| r0_45(int) = Load : &:r0_44, m0_34 +# 213| r0_46(glval) = VariableAddress[x] : +# 213| m0_47(int) = Store : &:r0_46, r0_45 +# 214| r0_48(glval) = VariableAddress[ppt] : +# 214| r0_49(Point *) = Load : &:r0_48, m0_6 +# 214| r0_50(glval) = FieldAddress[y] : r0_49 +# 214| r0_51(int) = Load : &:r0_50, m0_40 +# 214| r0_52(glval) = VariableAddress[y] : +# 214| m0_53(int) = Store : &:r0_52, r0_51 +# 216| r0_54(glval) = VariableAddress[i] : +# 216| r0_55(glval) = VariableAddress[pi] : +# 216| r0_56(int *) = Load : &:r0_55, m0_4 +# 216| r0_57(int) = Load : &:r0_56, ~m0_41 +# 216| m0_58(int) = Store : &:r0_54, r0_57 +# 217| r0_59(glval) = VariableAddress[pt] : +# 217| r0_60(glval) = VariableAddress[apt] : +# 217| r0_61(Point *) = Load : &:r0_60, m0_8 +# 217| r0_62(glval) = VariableAddress[i] : +# 217| r0_63(int) = Load : &:r0_62, m0_58 +# 217| r0_64(Point *) = PointerAdd[8] : r0_61, r0_63 +# 217| m0_65(Point &) = Store : &:r0_59, r0_64 +# 218| r0_66(int) = Constant[3] : +# 218| r0_67(glval) = VariableAddress[pt] : +# 218| r0_68(Point &) = Load : &:r0_67, m0_65 +# 218| r0_69(glval) = FieldAddress[x] : r0_68 +# 218| m0_70(int) = Store : &:r0_69, r0_66 +# 218| m0_71(unknown) = Chi : total:m0_41, partial:m0_70 +# 219| r0_72(int) = Constant[7] : +# 219| r0_73(glval) = VariableAddress[pt] : +# 219| r0_74(Point &) = Load : &:r0_73, m0_65 +# 219| r0_75(glval) = FieldAddress[y] : r0_74 +# 219| m0_76(int) = Store : &:r0_75, r0_72 +# 219| m0_77(unknown) = Chi : total:m0_71, partial:m0_76 +# 220| r0_78(glval) = VariableAddress[pt] : +# 220| r0_79(Point &) = Load : &:r0_78, m0_65 +# 220| r0_80(glval) = FieldAddress[x] : r0_79 +# 220| r0_81(int) = Load : &:r0_80, m0_70 +# 220| r0_82(glval) = VariableAddress[pt] : +# 220| r0_83(Point &) = Load : &:r0_82, m0_65 +# 220| r0_84(glval) = FieldAddress[y] : r0_83 +# 220| r0_85(int) = Load : &:r0_84, m0_76 +# 220| r0_86(int) = Add : r0_81, r0_85 +# 220| r0_87(glval) = VariableAddress[x] : +# 220| m0_88(int) = Store : &:r0_87, r0_86 +# 221| v0_89(void) = NoOp : +# 205| v0_90(void) = ReturnVoid : +# 205| v0_91(void) = UnmodeledUse : mu* +# 205| v0_92(void) = ExitFunction : diff --git a/cpp/ql/test/library-tests/ir/ssa/ssa.cpp b/cpp/ql/test/library-tests/ir/ssa/ssa.cpp index 8ada4a691f9c..df84d7bc2924 100644 --- a/cpp/ql/test/library-tests/ir/ssa/ssa.cpp +++ b/cpp/ql/test/library-tests/ir/ssa/ssa.cpp @@ -200,4 +200,22 @@ int PureFunctions(char *str1, char *str2, int x) { ret += strlen(str1); ret += abs(x); return ret; -} \ No newline at end of file +} + +void IndirectParams(int* pi, Point* ppt, Point* apt) { + int x = *pi; + *pi = 5; + int y = *pi; + + x = ppt->x; + ppt->x = 2; + ppt->y = 4; + x = ppt->x; + y = ppt->y; + + int i = *pi; + Point& pt = apt[i]; + pt.x = 3; + pt.y = 7; + x = pt.x + pt.y; +} diff --git a/cpp/ql/test/library-tests/ir/ssa/unaliased_ssa_ir.expected b/cpp/ql/test/library-tests/ir/ssa/unaliased_ssa_ir.expected index ce0d93233404..2237e6ac65ec 100644 --- a/cpp/ql/test/library-tests/ir/ssa/unaliased_ssa_ir.expected +++ b/cpp/ql/test/library-tests/ir/ssa/unaliased_ssa_ir.expected @@ -773,3 +773,94 @@ ssa.cpp: # 198| v0_43(void) = ReturnValue : &:r0_42, m0_41 # 198| v0_44(void) = UnmodeledUse : mu* # 198| v0_45(void) = ExitFunction : + +# 205| void IndirectParams(int*, Point*, Point*) +# 205| Block 0 +# 205| v0_0(void) = EnterFunction : +# 205| mu0_1(unknown) = AliasedDefinition : +# 205| mu0_2(unknown) = UnmodeledDefinition : +# 205| r0_3(glval) = VariableAddress[pi] : +# 205| m0_4(int *) = InitializeParameter[pi] : &:r0_3 +# 205| r0_5(glval) = VariableAddress[ppt] : +# 205| m0_6(Point *) = InitializeParameter[ppt] : &:r0_5 +# 205| r0_7(glval) = VariableAddress[apt] : +# 205| m0_8(Point *) = InitializeParameter[apt] : &:r0_7 +# 206| r0_9(glval) = VariableAddress[x] : +# 206| r0_10(glval) = VariableAddress[pi] : +# 206| r0_11(int *) = Load : &:r0_10, m0_4 +# 206| r0_12(int) = Load : &:r0_11, ~mu0_2 +# 206| m0_13(int) = Store : &:r0_9, r0_12 +# 207| r0_14(int) = Constant[5] : +# 207| r0_15(glval) = VariableAddress[pi] : +# 207| r0_16(int *) = Load : &:r0_15, m0_4 +# 207| mu0_17(int) = Store : &:r0_16, r0_14 +# 208| r0_18(glval) = VariableAddress[y] : +# 208| r0_19(glval) = VariableAddress[pi] : +# 208| r0_20(int *) = Load : &:r0_19, m0_4 +# 208| r0_21(int) = Load : &:r0_20, ~mu0_2 +# 208| m0_22(int) = Store : &:r0_18, r0_21 +# 210| r0_23(glval) = VariableAddress[ppt] : +# 210| r0_24(Point *) = Load : &:r0_23, m0_6 +# 210| r0_25(glval) = FieldAddress[x] : r0_24 +# 210| r0_26(int) = Load : &:r0_25, ~mu0_2 +# 210| r0_27(glval) = VariableAddress[x] : +# 210| m0_28(int) = Store : &:r0_27, r0_26 +# 211| r0_29(int) = Constant[2] : +# 211| r0_30(glval) = VariableAddress[ppt] : +# 211| r0_31(Point *) = Load : &:r0_30, m0_6 +# 211| r0_32(glval) = FieldAddress[x] : r0_31 +# 211| mu0_33(int) = Store : &:r0_32, r0_29 +# 212| r0_34(int) = Constant[4] : +# 212| r0_35(glval) = VariableAddress[ppt] : +# 212| r0_36(Point *) = Load : &:r0_35, m0_6 +# 212| r0_37(glval) = FieldAddress[y] : r0_36 +# 212| mu0_38(int) = Store : &:r0_37, r0_34 +# 213| r0_39(glval) = VariableAddress[ppt] : +# 213| r0_40(Point *) = Load : &:r0_39, m0_6 +# 213| r0_41(glval) = FieldAddress[x] : r0_40 +# 213| r0_42(int) = Load : &:r0_41, ~mu0_2 +# 213| r0_43(glval) = VariableAddress[x] : +# 213| m0_44(int) = Store : &:r0_43, r0_42 +# 214| r0_45(glval) = VariableAddress[ppt] : +# 214| r0_46(Point *) = Load : &:r0_45, m0_6 +# 214| r0_47(glval) = FieldAddress[y] : r0_46 +# 214| r0_48(int) = Load : &:r0_47, ~mu0_2 +# 214| r0_49(glval) = VariableAddress[y] : +# 214| m0_50(int) = Store : &:r0_49, r0_48 +# 216| r0_51(glval) = VariableAddress[i] : +# 216| r0_52(glval) = VariableAddress[pi] : +# 216| r0_53(int *) = Load : &:r0_52, m0_4 +# 216| r0_54(int) = Load : &:r0_53, ~mu0_2 +# 216| m0_55(int) = Store : &:r0_51, r0_54 +# 217| r0_56(glval) = VariableAddress[pt] : +# 217| r0_57(glval) = VariableAddress[apt] : +# 217| r0_58(Point *) = Load : &:r0_57, m0_8 +# 217| r0_59(glval) = VariableAddress[i] : +# 217| r0_60(int) = Load : &:r0_59, m0_55 +# 217| r0_61(Point *) = PointerAdd[8] : r0_58, r0_60 +# 217| m0_62(Point &) = Store : &:r0_56, r0_61 +# 218| r0_63(int) = Constant[3] : +# 218| r0_64(glval) = VariableAddress[pt] : +# 218| r0_65(Point &) = Load : &:r0_64, m0_62 +# 218| r0_66(glval) = FieldAddress[x] : r0_65 +# 218| mu0_67(int) = Store : &:r0_66, r0_63 +# 219| r0_68(int) = Constant[7] : +# 219| r0_69(glval) = VariableAddress[pt] : +# 219| r0_70(Point &) = Load : &:r0_69, m0_62 +# 219| r0_71(glval) = FieldAddress[y] : r0_70 +# 219| mu0_72(int) = Store : &:r0_71, r0_68 +# 220| r0_73(glval) = VariableAddress[pt] : +# 220| r0_74(Point &) = Load : &:r0_73, m0_62 +# 220| r0_75(glval) = FieldAddress[x] : r0_74 +# 220| r0_76(int) = Load : &:r0_75, ~mu0_2 +# 220| r0_77(glval) = VariableAddress[pt] : +# 220| r0_78(Point &) = Load : &:r0_77, m0_62 +# 220| r0_79(glval) = FieldAddress[y] : r0_78 +# 220| r0_80(int) = Load : &:r0_79, ~mu0_2 +# 220| r0_81(int) = Add : r0_76, r0_80 +# 220| r0_82(glval) = VariableAddress[x] : +# 220| m0_83(int) = Store : &:r0_82, r0_81 +# 221| v0_84(void) = NoOp : +# 205| v0_85(void) = ReturnVoid : +# 205| v0_86(void) = UnmodeledUse : mu* +# 205| v0_87(void) = ExitFunction : From 5385f13f41bd697d31859676508f5ea3c7580978 Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Wed, 14 Aug 2019 14:01:46 -0700 Subject: [PATCH 10/13] C++: Fix dataflow test expectations for new alias analysis --- .../library-tests/dataflow/dataflow-tests/test_diff.expected | 1 - .../test/library-tests/dataflow/dataflow-tests/test_ir.expected | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test_diff.expected b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test_diff.expected index b6d11257315d..87ddd1c12d74 100644 --- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test_diff.expected +++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test_diff.expected @@ -1,7 +1,6 @@ | test.cpp:89:28:89:34 | test.cpp:92:8:92:14 | IR only | | test.cpp:100:13:100:18 | test.cpp:103:10:103:12 | AST only | | test.cpp:109:9:109:14 | test.cpp:110:10:110:12 | IR only | -| test.cpp:136:27:136:32 | test.cpp:137:27:137:28 | AST only | | test.cpp:136:27:136:32 | test.cpp:140:22:140:23 | AST only | | test.cpp:142:32:142:37 | test.cpp:145:10:145:11 | IR only | | test.cpp:151:35:151:40 | test.cpp:153:17:153:18 | IR only | diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test_ir.expected b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test_ir.expected index a7666de428ac..049ec4268cff 100644 --- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test_ir.expected +++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test_ir.expected @@ -17,6 +17,7 @@ | test.cpp:110:10:110:12 | Load: (reference dereference) | test.cpp:109:9:109:14 | Call: call to source | | test.cpp:126:8:126:19 | Convert: (const int *)... | test.cpp:120:9:120:20 | InitializeParameter: sourceArray1 | | test.cpp:126:8:126:19 | Load: sourceArray1 | test.cpp:120:9:120:20 | InitializeParameter: sourceArray1 | +| test.cpp:137:27:137:28 | Load: m1 | test.cpp:136:27:136:32 | Call: call to source | | test.cpp:145:10:145:11 | Load: m2 | test.cpp:142:32:142:37 | Call: call to source | | test.cpp:153:17:153:18 | Load: m2 | test.cpp:151:35:151:40 | Call: call to source | | test.cpp:188:8:188:8 | Load: y | test.cpp:186:27:186:32 | Call: call to source | From d4ae9dd8f28eec91962547dee9ad63a354d84a6a Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Wed, 14 Aug 2019 14:02:42 -0700 Subject: [PATCH 11/13] C++: Alias analysis changes to support precise indirections --- .../aliased_ssa/internal/AliasAnalysis.qll | 122 +++++++++--------- .../unaliased_ssa/internal/AliasAnalysis.qll | 122 +++++++++--------- 2 files changed, 120 insertions(+), 124 deletions(-) diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasAnalysis.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasAnalysis.qll index 3c50cea62068..7fce1d0a7121 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasAnalysis.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasAnalysis.qll @@ -2,34 +2,10 @@ private import AliasAnalysisInternal private import cpp private import InputIR private import semmle.code.cpp.ir.internal.IntegerConstant as Ints - private import semmle.code.cpp.models.interfaces.Alias private class IntValue = Ints::IntValue; -/** - * Converts the bit count in `bits` to a byte count and a bit count in the form - * bytes:bits. - */ -bindingset[bits] -string bitsToBytesAndBits(int bits) { - result = (bits / 8).toString() + ":" + (bits % 8).toString() -} - -/** - * Gets a printable string for a bit offset with possibly unknown value. - */ -bindingset[bitOffset] -string getBitOffsetString(IntValue bitOffset) { - if Ints::hasValue(bitOffset) then - if bitOffset >= 0 then - result = "+" + bitsToBytesAndBits(bitOffset) - else - result = "-" + bitsToBytesAndBits(Ints::neg(bitOffset)) - else - result = "+?" -} - /** * Gets the offset of field `field` in bits. */ @@ -142,7 +118,11 @@ private predicate operandIsPropagated(Operand operand, IntValue bitOffset) { ) or // Adding an integer to or subtracting an integer from a pointer propagates // the address with an offset. - bitOffset = getPointerBitOffset(instr.(PointerOffsetInstruction)) or + exists(PointerOffsetInstruction ptrOffset | + ptrOffset = instr and + operand = ptrOffset.getLeftOperand() and + bitOffset = getPointerBitOffset(ptrOffset) + ) or // Computing a field address from a pointer propagates the address plus the // offset of the field. bitOffset = getFieldBitOffset(instr.(FieldAddressInstruction).getField()) or @@ -278,52 +258,70 @@ private predicate resultEscapesNonReturn(Instruction instr) { } /** - * Holds if the address of the specified local variable or parameter escapes the - * domain of the analysis. + * Holds if the address of `allocation` escapes outside the domain of the analysis. This can occur + * either because the allocation's address is taken within the function and escapes, or because the + * allocation is marked as always escaping via `alwaysEscapes()`. + */ +predicate allocationEscapes(Configuration::Allocation allocation) { + allocation.alwaysEscapes() or + resultEscapesNonReturn(allocation.getABaseInstruction()) +} + +/** + * Equivalent to `operandIsPropagated()`, but includes interprocedural propagation. */ -private predicate automaticVariableAddressEscapes(IRAutomaticVariable var) { - // The variable's address escapes if the result of any - // VariableAddressInstruction that computes the variable's address escapes. - exists(VariableAddressInstruction instr | - instr.getVariable() = var and - resultEscapesNonReturn(instr) +private predicate operandIsPropagatedIncludingByCall(Operand operand, IntValue bitOffset) { + operandIsPropagated(operand, bitOffset) or + exists(CallInstruction call, Instruction init | + isArgumentForParameter(call, operand, init) and + resultReturned(init, bitOffset) ) } /** - * Holds if the address of the specified variable escapes the domain of the - * analysis. + * Holds if `addrOperand` is at offset `bitOffset` from the value of instruction `base`. The offset + * may be `unknown()`. */ -predicate variableAddressEscapes(IRVariable var) { - automaticVariableAddressEscapes(var.(IRAutomaticVariable)) or - // All variables with static storage duration have their address escape. - not var instanceof IRAutomaticVariable +private predicate hasBaseAndOffset(AddressOperand addrOperand, Instruction base, + IntValue bitOffset) { + base = addrOperand.getDef() and bitOffset = 0 or // Base case + exists(Instruction middle, int previousBitOffset, Operand middleOperand, + IntValue additionalBitOffset | + // We already have an offset from `middle`. + hasBaseAndOffset(addrOperand, middle, previousBitOffset) and + // `middle` is propagated from `base`. + middleOperand = middle.getAnOperand() and + operandIsPropagatedIncludingByCall(middleOperand, additionalBitOffset) and + base = middleOperand.getDef() and + bitOffset = Ints::add(previousBitOffset, additionalBitOffset) + ) } /** - * Holds if the result of instruction `instr` points within variable `var`, at - * bit offset `bitOffset` within the variable. If the result points within - * `var`, but at an unknown or non-constant offset, then `bitOffset` is unknown. + * Holds if `addrOperand` is at constant offset `bitOffset` from the value of instruction `base`. + * Only holds for the `base` with the longest chain of propagation to `addrOperand`. */ -predicate resultPointsTo(Instruction instr, IRVariable var, IntValue bitOffset) { - ( - // The address of a variable points to that variable, at offset 0. - instr.(VariableAddressInstruction).getVariable() = var and - bitOffset = 0 - ) or - exists(Operand operand, IntValue originalBitOffset, IntValue propagatedBitOffset | - operand = instr.getAnOperand() and - // If an operand is propagated, then the result points to the same variable, - // offset by the bit offset from the propagation. - resultPointsTo(operand.getAnyDef(), var, originalBitOffset) and - ( - operandIsPropagated(operand, propagatedBitOffset) - or - exists(CallInstruction ci, Instruction init | - isArgumentForParameter(ci, operand, init) and - resultReturned(init, propagatedBitOffset) - ) - ) and - bitOffset = Ints::add(originalBitOffset, propagatedBitOffset) +predicate addressOperandBaseAndConstantOffset(AddressOperand addrOperand, Instruction base, + int bitOffset) { + hasBaseAndOffset(addrOperand, base, bitOffset) and + Ints::hasValue(bitOffset) and + not exists(Instruction previousBase, int previousBitOffset | + hasBaseAndOffset(addrOperand, previousBase, previousBitOffset) and + previousBase = base.getAnOperand().getDef() and + Ints::hasValue(previousBitOffset) + ) +} + +/** + * Gets the allocation into which `addrOperand` points, if known. + */ +Configuration::Allocation getAddressOperandAllocation(AddressOperand addrOperand) { + exists(Instruction base | + result.getABaseInstruction() = base and + hasBaseAndOffset(addrOperand, base, _) and + not exists(Instruction previousBase | + hasBaseAndOffset(addrOperand, previousBase, _) and + previousBase = base.getAnOperand().getDef() + ) ) } diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/AliasAnalysis.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/AliasAnalysis.qll index 3c50cea62068..7fce1d0a7121 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/AliasAnalysis.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/AliasAnalysis.qll @@ -2,34 +2,10 @@ private import AliasAnalysisInternal private import cpp private import InputIR private import semmle.code.cpp.ir.internal.IntegerConstant as Ints - private import semmle.code.cpp.models.interfaces.Alias private class IntValue = Ints::IntValue; -/** - * Converts the bit count in `bits` to a byte count and a bit count in the form - * bytes:bits. - */ -bindingset[bits] -string bitsToBytesAndBits(int bits) { - result = (bits / 8).toString() + ":" + (bits % 8).toString() -} - -/** - * Gets a printable string for a bit offset with possibly unknown value. - */ -bindingset[bitOffset] -string getBitOffsetString(IntValue bitOffset) { - if Ints::hasValue(bitOffset) then - if bitOffset >= 0 then - result = "+" + bitsToBytesAndBits(bitOffset) - else - result = "-" + bitsToBytesAndBits(Ints::neg(bitOffset)) - else - result = "+?" -} - /** * Gets the offset of field `field` in bits. */ @@ -142,7 +118,11 @@ private predicate operandIsPropagated(Operand operand, IntValue bitOffset) { ) or // Adding an integer to or subtracting an integer from a pointer propagates // the address with an offset. - bitOffset = getPointerBitOffset(instr.(PointerOffsetInstruction)) or + exists(PointerOffsetInstruction ptrOffset | + ptrOffset = instr and + operand = ptrOffset.getLeftOperand() and + bitOffset = getPointerBitOffset(ptrOffset) + ) or // Computing a field address from a pointer propagates the address plus the // offset of the field. bitOffset = getFieldBitOffset(instr.(FieldAddressInstruction).getField()) or @@ -278,52 +258,70 @@ private predicate resultEscapesNonReturn(Instruction instr) { } /** - * Holds if the address of the specified local variable or parameter escapes the - * domain of the analysis. + * Holds if the address of `allocation` escapes outside the domain of the analysis. This can occur + * either because the allocation's address is taken within the function and escapes, or because the + * allocation is marked as always escaping via `alwaysEscapes()`. + */ +predicate allocationEscapes(Configuration::Allocation allocation) { + allocation.alwaysEscapes() or + resultEscapesNonReturn(allocation.getABaseInstruction()) +} + +/** + * Equivalent to `operandIsPropagated()`, but includes interprocedural propagation. */ -private predicate automaticVariableAddressEscapes(IRAutomaticVariable var) { - // The variable's address escapes if the result of any - // VariableAddressInstruction that computes the variable's address escapes. - exists(VariableAddressInstruction instr | - instr.getVariable() = var and - resultEscapesNonReturn(instr) +private predicate operandIsPropagatedIncludingByCall(Operand operand, IntValue bitOffset) { + operandIsPropagated(operand, bitOffset) or + exists(CallInstruction call, Instruction init | + isArgumentForParameter(call, operand, init) and + resultReturned(init, bitOffset) ) } /** - * Holds if the address of the specified variable escapes the domain of the - * analysis. + * Holds if `addrOperand` is at offset `bitOffset` from the value of instruction `base`. The offset + * may be `unknown()`. */ -predicate variableAddressEscapes(IRVariable var) { - automaticVariableAddressEscapes(var.(IRAutomaticVariable)) or - // All variables with static storage duration have their address escape. - not var instanceof IRAutomaticVariable +private predicate hasBaseAndOffset(AddressOperand addrOperand, Instruction base, + IntValue bitOffset) { + base = addrOperand.getDef() and bitOffset = 0 or // Base case + exists(Instruction middle, int previousBitOffset, Operand middleOperand, + IntValue additionalBitOffset | + // We already have an offset from `middle`. + hasBaseAndOffset(addrOperand, middle, previousBitOffset) and + // `middle` is propagated from `base`. + middleOperand = middle.getAnOperand() and + operandIsPropagatedIncludingByCall(middleOperand, additionalBitOffset) and + base = middleOperand.getDef() and + bitOffset = Ints::add(previousBitOffset, additionalBitOffset) + ) } /** - * Holds if the result of instruction `instr` points within variable `var`, at - * bit offset `bitOffset` within the variable. If the result points within - * `var`, but at an unknown or non-constant offset, then `bitOffset` is unknown. + * Holds if `addrOperand` is at constant offset `bitOffset` from the value of instruction `base`. + * Only holds for the `base` with the longest chain of propagation to `addrOperand`. */ -predicate resultPointsTo(Instruction instr, IRVariable var, IntValue bitOffset) { - ( - // The address of a variable points to that variable, at offset 0. - instr.(VariableAddressInstruction).getVariable() = var and - bitOffset = 0 - ) or - exists(Operand operand, IntValue originalBitOffset, IntValue propagatedBitOffset | - operand = instr.getAnOperand() and - // If an operand is propagated, then the result points to the same variable, - // offset by the bit offset from the propagation. - resultPointsTo(operand.getAnyDef(), var, originalBitOffset) and - ( - operandIsPropagated(operand, propagatedBitOffset) - or - exists(CallInstruction ci, Instruction init | - isArgumentForParameter(ci, operand, init) and - resultReturned(init, propagatedBitOffset) - ) - ) and - bitOffset = Ints::add(originalBitOffset, propagatedBitOffset) +predicate addressOperandBaseAndConstantOffset(AddressOperand addrOperand, Instruction base, + int bitOffset) { + hasBaseAndOffset(addrOperand, base, bitOffset) and + Ints::hasValue(bitOffset) and + not exists(Instruction previousBase, int previousBitOffset | + hasBaseAndOffset(addrOperand, previousBase, previousBitOffset) and + previousBase = base.getAnOperand().getDef() and + Ints::hasValue(previousBitOffset) + ) +} + +/** + * Gets the allocation into which `addrOperand` points, if known. + */ +Configuration::Allocation getAddressOperandAllocation(AddressOperand addrOperand) { + exists(Instruction base | + result.getABaseInstruction() = base and + hasBaseAndOffset(addrOperand, base, _) and + not exists(Instruction previousBase | + hasBaseAndOffset(addrOperand, previousBase, _) and + previousBase = base.getAnOperand().getDef() + ) ) } From 2bc350612e90455011bcb6556d03ba2728f1b48e Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Wed, 14 Aug 2019 14:03:20 -0700 Subject: [PATCH 12/13] C++: Update `SimpleSSA` to use the new alias analysis API --- .../internal/AliasAnalysisInternal.qll | 1 + .../internal/AliasConfiguration.qll | 20 +++++ .../unaliased_ssa/internal/SimpleSSA.qll | 79 ++++++++----------- 3 files changed, 56 insertions(+), 44 deletions(-) create mode 100644 cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/AliasConfiguration.qll diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/AliasAnalysisInternal.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/AliasAnalysisInternal.qll index 8a9e43e14a37..6e9e055c3814 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/AliasAnalysisInternal.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/AliasAnalysisInternal.qll @@ -1 +1,2 @@ import semmle.code.cpp.ir.implementation.raw.IR as InputIR +import AliasConfiguration as Configuration \ No newline at end of file diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/AliasConfiguration.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/AliasConfiguration.qll new file mode 100644 index 000000000000..8b360aa7e179 --- /dev/null +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/unaliased_ssa/internal/AliasConfiguration.qll @@ -0,0 +1,20 @@ +private import semmle.code.cpp.ir.implementation.raw.IR + +/** + * A memory allocation that can be tracked by the SimpleSSA alias analysis. + * All automatic variables are tracked. + */ +class Allocation extends IRAutomaticVariable { + VariableAddressInstruction getABaseInstruction() { + result.getVariable() = this + } + + final string getAllocationString() { + result = toString() + } + + predicate alwaysEscapes() { + // An automatic variable only escapes if its address is taken and escapes. + none() + } +} 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 9bfff0076b41..05bde150e77f 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 @@ -1,64 +1,61 @@ import AliasAnalysis private import cpp +private import AliasConfiguration private import semmle.code.cpp.ir.implementation.raw.IR -private import semmle.code.cpp.ir.internal.IntegerConstant as Ints -private import semmle.code.cpp.ir.implementation.internal.OperandTag private import semmle.code.cpp.ir.internal.Overlap - -private class IntValue = Ints::IntValue; - -private predicate hasResultMemoryAccess(Instruction instr, IRVariable var, Type type, IntValue bitOffset) { - resultPointsTo(instr.getResultAddressOperand().getAnyDef(), var, bitOffset) and - type = instr.getResultType() -} - -private predicate hasOperandMemoryAccess(MemoryOperand operand, IRVariable var, Type type, IntValue bitOffset) { - resultPointsTo(operand.getAddressOperand().getAnyDef(), var, bitOffset) and - type = operand.getType() -} +private import semmle.code.cpp.ir.internal.IntegerConstant as Ints /** - * Holds if the specified variable should be modeled in SSA form. For unaliased SSA, we only model a variable if its - * address never escapes and all reads and writes of that variable access the entire variable using the original type - * of the variable. + * Holds if the specified variable should be modeled in SSA form. For unaliased SSA, we only model a + * variable if its address never escapes and all reads and writes of that variable access the entire + * variable using the original type of the variable. */ -private predicate isVariableModeled(IRVariable var) { - not variableAddressEscapes(var) and - // 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, Type type, IntValue bitOffset | - hasResultMemoryAccess(instr, var, type, bitOffset) | - bitOffset = 0 and - type = var.getType() - ) and - forall(MemoryOperand operand, Type type, IntValue bitOffset | - hasOperandMemoryAccess(operand, var, type, bitOffset) | - bitOffset = 0 and - type = var.getType() +private predicate isVariableModeled(Allocation var) { + not allocationEscapes(var) and + forall(AddressOperand addrOperand, Type type | + ( + exists(Instruction instr | + addrOperand = instr.getResultAddressOperand() and + type = instr.getResultType() + ) or + exists(MemoryOperand operand | + addrOperand = operand.getAddressOperand() and + type = operand.getType() + ) + ) and + getAddressOperandAllocation(addrOperand) = var | + exists(Instruction constantBase, int bitOffset | + addressOperandBaseAndConstantOffset(addrOperand, constantBase, bitOffset) and + bitOffset = 0 and + constantBase = var.getABaseInstruction() and + // There's no need to check for the right size. An `IRVariable` never has an `UnknownType`, so + // the test for the right type is sufficient. + type = var.getType() + ) ) } private newtype TMemoryLocation = - MkMemoryLocation(IRVariable var) { + MkMemoryLocation(Allocation var) { isVariableModeled(var) } -private MemoryLocation getMemoryLocation(IRVariable var) { - result.getIRVariable() = var +private MemoryLocation getMemoryLocation(Allocation var) { + result.getAllocation() = var } class MemoryLocation extends TMemoryLocation { - IRVariable var; + Allocation var; MemoryLocation() { this = MkMemoryLocation(var) } final string toString() { - result = var.toString() + result = var.getAllocationString() } - final IRVariable getIRVariable() { + final Allocation getAllocation() { result = var } @@ -85,15 +82,9 @@ Overlap getOverlap(MemoryLocation def, MemoryLocation use) { } MemoryLocation getResultMemoryLocation(Instruction instr) { - exists(IRVariable var | - hasResultMemoryAccess(instr, var, _, _) and - result = getMemoryLocation(var) - ) + result = getMemoryLocation(getAddressOperandAllocation(instr.getResultAddressOperand())) } MemoryLocation getOperandMemoryLocation(MemoryOperand operand) { - exists(IRVariable var | - hasOperandMemoryAccess(operand, var, _, _) and - result = getMemoryLocation(var) - ) + result = getMemoryLocation(getAddressOperandAllocation(operand.getAddressOperand())) } From 352e750e45fc08e70e833985bfa90d8a1c7eab54 Mon Sep 17 00:00:00 2001 From: Dave Bartolomeo Date: Wed, 14 Aug 2019 14:03:46 -0700 Subject: [PATCH 13/13] C++: Implement precise analysis of indirections in `AliasedSSA` --- .../internal/AliasAnalysisInternal.qll | 1 + .../internal/AliasConfiguration.qll | 51 +++ .../aliased_ssa/internal/AliasedSSA.qll | 305 ++++++++++++------ 3 files changed, 252 insertions(+), 105 deletions(-) create mode 100644 cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasConfiguration.qll diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasAnalysisInternal.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasAnalysisInternal.qll index 003b7008619b..d718a9184394 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasAnalysisInternal.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasAnalysisInternal.qll @@ -1 +1,2 @@ import semmle.code.cpp.ir.implementation.unaliased_ssa.IR as InputIR +import AliasConfiguration as Configuration diff --git a/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasConfiguration.qll b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasConfiguration.qll new file mode 100644 index 000000000000..bdf23a61a015 --- /dev/null +++ b/cpp/ql/src/semmle/code/cpp/ir/implementation/aliased_ssa/internal/AliasConfiguration.qll @@ -0,0 +1,51 @@ +private import cpp +private import semmle.code.cpp.ir.implementation.unaliased_ssa.IR +private import semmle.code.cpp.ir.implementation.unaliased_ssa.gvn.ValueNumbering +private import AliasAnalysis + +/** + * A memory allocation that can be tracked by the AliasedSSA alias analysis. + * For now, we track all variables accessed within the function, including both local variables + * and global variables. In the future, we will track indirect parameters as well. + */ +class Allocation extends ValueNumber { + IRVariable var; + + Allocation() { + // For now, we only track variables. + var = this.getAnInstruction().(VariableAddressInstruction).getVariable() + } + + final string getAllocationString() { + exists(string suffix | + result = var.toString() + suffix and + if isUnaliased() then + suffix = "" + else + suffix = "*" + ) + } + + final Type getType() { + result = var.getType() + } + + final int getBitSize() { + result = getType().getSize() * 8 + } + + final predicate alwaysEscapes() { + // An automatic variable only escapes if its address is taken and escapes, but we assume that + // any other kind of variable always escapes. + not var instanceof IRAutomaticVariable + } + + final predicate isUnaliased() { + not allocationEscapes(this) + } + + final Instruction getABaseInstruction() { + // Any instruction with this value number serves as a base address for this allocation. + result = getAnInstruction() + } +} 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 2b0f339e006b..fad3a4fda83a 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 @@ -1,49 +1,81 @@ private import cpp import AliasAnalysis import semmle.code.cpp.ir.internal.Overlap +private import AliasConfiguration private import semmle.code.cpp.Print private import semmle.code.cpp.ir.implementation.unaliased_ssa.IR private import semmle.code.cpp.ir.internal.IntegerConstant as Ints private import semmle.code.cpp.ir.internal.IntegerInterval as Interval private import semmle.code.cpp.ir.implementation.internal.OperandTag +private import semmle.code.cpp.ir.implementation.unaliased_ssa.gvn.ValueNumbering private class IntValue = Ints::IntValue; -private predicate hasResultMemoryAccess(Instruction instr, IRVariable var, Type type, IntValue startBitOffset, - IntValue endBitOffset) { - resultPointsTo(instr.getResultAddress(), var, startBitOffset) and - type = instr.getResultType() and - if exists(instr.getResultSize()) then - endBitOffset = Ints::add(startBitOffset, Ints::mul(instr.getResultSize(), 8)) - else - endBitOffset = Ints::unknown() +/** + * Gets a printable string for the specified base address. + */ +private string getBaseAddressString(ValueNumber baseAddress) { + // If it's the value of a parameter, just use the parameter name. + result = baseAddress.getAnInstruction().(InitializeParameterInstruction).getParameter().toString() or + ( + // Otherwise, use the `toString()` of the example instruction. + not exists(baseAddress.getAnInstruction().(InitializeParameterInstruction)) and + result = baseAddress.getExampleInstruction().toString() + ) +} + +/** + * Holds if the indirect memory access via `addrOperand` has the specified type and size (in bytes). + */ +private predicate memoryAccessTypeAndSize(AddressOperand addrOperand, Type type, + IntValue byteSize) { + exists(Instruction instr | + instr = addrOperand.getUse() and + instr.getResultAddressOperand() = addrOperand and + type = instr.getResultType() and + ( + byteSize = instr.getResultSize() or + not exists(instr.getResultSize()) and byteSize = Ints::unknown() + ) + ) or + exists(MemoryOperand operand | + operand.getAddressOperand() = addrOperand and + type = operand.getType() and + ( + byteSize = operand.getSize() or + not exists(operand.getSize()) and byteSize = Ints::unknown() + ) + ) } -private predicate hasOperandMemoryAccess(MemoryOperand operand, IRVariable var, Type type, IntValue startBitOffset, - IntValue endBitOffset) { - resultPointsTo(operand.getAddressOperand().getAnyDef(), var, startBitOffset) and - type = operand.getType() and - if exists(operand.getSize()) then - endBitOffset = Ints::add(startBitOffset, Ints::mul(operand.getSize(), 8)) - else - endBitOffset = Ints::unknown() +/** + * Holds if the indirect memory access via `addrOperand` has the specified type, base address, and + * offset interval. + */ +private predicate hasMemoryAccess(AddressOperand addrOperand, Type type, ValueNumber baseAddress, + IntValue startBitOffset, IntValue endBitOffset) { + exists(Instruction baseInstr, IntValue byteSize | + addressOperandBaseAndConstantOffset(addrOperand, baseInstr, startBitOffset) and + memoryAccessTypeAndSize(addrOperand, type, byteSize) and + endBitOffset = Ints::add(startBitOffset, Ints::mul(byteSize, 8)) and + baseAddress = valueNumber(baseInstr) + ) } private newtype TMemoryLocation = - TVariableMemoryLocation(IRVariable var, Type type, IntValue startBitOffset, IntValue endBitOffset) { - hasResultMemoryAccess(_, var, type, startBitOffset, endBitOffset) or - hasOperandMemoryAccess(_, var, type, startBitOffset, endBitOffset) + TIndirectMemoryLocation(Type type, ValueNumber baseAddress, IntValue startBitOffset, + IntValue endBitOffset) { + hasMemoryAccess(_, type, baseAddress, startBitOffset, endBitOffset) } or TUnknownMemoryLocation(IRFunction irFunc) or TUnknownVirtualVariable(IRFunction irFunc) /** - * Represents the memory location accessed by a memory operand or memory result. In this implementation, the location is - * one of the following: - * - `VariableMemoryLocation` - A location within a known `IRVariable`, at an offset that is either a constant or is - * unknown. - * - `UnknownMemoryLocation` - A location not known to be within a specific `IRVariable`. + * Represents the memory location accessed by a memory operand or memory result. In this + * implementation, the location is one of the following: + * - `IndirectMemoryLocation` - A location at a constant offset from a known base address. + * - `UnknownMemoryLocation` - A location in aliased memory, without a specific address. */ abstract class MemoryLocation extends TMemoryLocation { abstract string toString(); @@ -52,6 +84,8 @@ abstract class MemoryLocation extends TMemoryLocation { abstract Type getType(); + abstract Allocation getAllocation(); + abstract string getUniqueId(); } @@ -59,49 +93,102 @@ abstract class VirtualVariable extends MemoryLocation { } /** - * An access to memory within a single known `IRVariable`. The variable may be either an unescaped variable - * (with its own `VirtualIRVariable`) or an escaped variable (assigned to `UnknownVirtualVariable`). + * An access to memory at a constant offset from a known base address. The address may or may not be + * in a known allocation. */ -class VariableMemoryLocation extends TVariableMemoryLocation, MemoryLocation { - IRVariable var; +class IndirectMemoryLocation extends TIndirectMemoryLocation, MemoryLocation { + ValueNumber baseAddress; Type type; IntValue startBitOffset; IntValue endBitOffset; - VariableMemoryLocation() { - this = TVariableMemoryLocation(var, type, startBitOffset, endBitOffset) + IndirectMemoryLocation() { + this = TIndirectMemoryLocation(type, baseAddress, startBitOffset, endBitOffset) + } + + private string getAllocationString() { + result = getAllocation().getAllocationString() or + not exists(getAllocation()) and result = "??" + } + + private string getBaseAddressStringIfNeeded() { + if getAllocation() = baseAddress then + result = "" + else + result = "@'" + getBaseAddressString(baseAddress) + "'+" + } + + private string getIntervalString() { + if coversEntireVariable() then + result = "" + else + result = Interval::getIntervalString(startBitOffset, endBitOffset) + } + + private string getTypeString() { + if coversEntireVariable() and type = getAllocation().getType() then + result = "" + else + result = "<" + type.toString() + ">" } override final string toString() { - result = var.toString() + Interval::getIntervalString(startBitOffset, endBitOffset) + "<" + type.toString() + ">" + result = getAllocationString() + getBaseAddressStringIfNeeded() + getIntervalString() + + getTypeString() } override final Type getType() { result = type } + /** + * Gets the bit offset from the base address at which the access starts. + */ final IntValue getStartBitOffset() { result = startBitOffset } + /** + * Gets the bit offset from the base address at which the access ends. This offset is one bit past + * the last bit actually accessed. + */ final IntValue getEndBitOffset() { result = endBitOffset } - final IRVariable getVariable() { - result = var + /** + * Gets the base address to which the offsets are relative. + */ + final ValueNumber getBaseAddress() { + result = baseAddress + } + + override Allocation getAllocation() { + exists(AddressOperand addrOperand | + this = getMemoryLocation(addrOperand) and + result = getAddressOperandAllocation(addrOperand) + ) } override final string getUniqueId() { - result = var.getUniqueId() + Interval::getIntervalString(startBitOffset, endBitOffset) + "<" + + result = baseAddress.getExampleInstruction().getUniqueId() + + Interval::getIntervalString(startBitOffset, endBitOffset) + "<" + getTypeIdentityString(type) + ">" } override final VirtualVariable getVirtualVariable() { - if variableAddressEscapes(var) then - result = TUnknownVirtualVariable(var.getEnclosingIRFunction()) - else - result = TVariableMemoryLocation(var, var.getType(), 0, var.getType().getSize() * 8) + exists(Allocation allocation | + // In an unaliased allocation, get the memory access for the entire allocation. + allocation = getAllocation() and + allocation.isUnaliased() and + result = TIndirectMemoryLocation(allocation.getType(), allocation, 0, + allocation.getBitSize()) + ) or + ( + // All aliased allocations are grouped into the `UnknownVirtualVariable`. + not getAllocation().isUnaliased() and + result = TUnknownVirtualVariable(baseAddress.getAnInstruction().getEnclosingIRFunction()) + ) } /** @@ -109,25 +196,28 @@ class VariableMemoryLocation extends TVariableMemoryLocation, MemoryLocation { */ final predicate coversEntireVariable() { startBitOffset = 0 and - endBitOffset = var.getType().getSize() * 8 + endBitOffset = getAllocation().getBitSize() } } /** - * Represents the `MemoryLocation` for an `IRVariable` that acts as its own `VirtualVariable`. Includes any - * `VariableMemoryLocation` that exactly overlaps its entire `IRVariable`, and only if that `IRVariable` does not - * escape. + * Represents the `MemoryLocation` for an entire unaliased allocation. This is also the + * `IndirectMemoryLocation` for accesses that directly read or write the whole allocation with the + * correct type. */ -class VariableVirtualVariable extends VariableMemoryLocation, VirtualVariable { - VariableVirtualVariable() { - not variableAddressEscapes(var) and - type = var.getType() and - coversEntireVariable() +class UnaliasedVirtualVariable extends IndirectMemoryLocation, VirtualVariable { + UnaliasedVirtualVariable() { + exists(Allocation allocation | + allocation = getAllocation() and + allocation.isUnaliased() and + type = allocation.getType() and + coversEntireVariable() + ) } } /** - * An access to memory that is not known to be confined to a specific `IRVariable`. + * An access to memory that is not known to be confined to a specific allocation. */ class UnknownMemoryLocation extends TUnknownMemoryLocation, MemoryLocation { IRFunction irFunc; @@ -148,6 +238,10 @@ class UnknownMemoryLocation extends TUnknownMemoryLocation, MemoryLocation { result instanceof UnknownType } + override final Allocation getAllocation() { + none() + } + override final string getUniqueId() { result = "{Unknown}" } @@ -171,6 +265,10 @@ class UnknownVirtualVariable extends TUnknownVirtualVariable, VirtualVariable { result instanceof UnknownType } + override final Allocation getAllocation() { + none() + } + override final string getUniqueId() { result = " " + toString() } @@ -191,19 +289,19 @@ Overlap getOverlap(MemoryLocation def, MemoryLocation use) { def.getVirtualVariable() = use.getVirtualVariable() and def instanceof UnknownMemoryLocation and result instanceof MayPartiallyOverlap or - exists(VariableMemoryLocation defVariableLocation | - defVariableLocation = def and + exists(IndirectMemoryLocation defIndirectLocation | + defIndirectLocation = def and ( ( - // A VariableMemoryLocation may partially overlap an unknown location within the same virtual variable. + // An IndirectMemoryLocation may partially overlap an unknown location within the same virtual variable. def.getVirtualVariable() = use.getVirtualVariable() and ((use instanceof UnknownMemoryLocation) or (use instanceof UnknownVirtualVariable)) and result instanceof MayPartiallyOverlap ) or - // A VariableMemoryLocation overlaps another location within the same variable based on the relationship - // of the two offset intervals. + // An IndirectMemoryLocation overlaps another location with the same constant base address + // based on the relationship of the two offset intervals. exists(Overlap intervalOverlap | - intervalOverlap = getVariableMemoryLocationOverlap(def, use) and + intervalOverlap = getIndirectMemoryLocationOverlap(def, use) and if intervalOverlap instanceof MustExactlyOverlap then ( if def.getType() = use.getType() then ( // The def and use types match, so it's an exact overlap. @@ -214,7 +312,7 @@ Overlap getOverlap(MemoryLocation def, MemoryLocation use) { result instanceof MustTotallyOverlap ) ) - else if defVariableLocation.coversEntireVariable() then ( + else if defIndirectLocation.coversEntireVariable() then ( // The definition covers the entire variable, so assume that it totally overlaps the use, even if the // interval for the use is unknown or outside the bounds of the variable. result instanceof MustTotallyOverlap @@ -230,24 +328,24 @@ Overlap getOverlap(MemoryLocation def, MemoryLocation use) { } /* - * The following predicates compute the overlap relation between `VariableMemoryLocation`s in the + * The following predicates compute the overlap relation between `IndirectMemoryLocation`s in the * same `VirtualVariable` as follows: * 1. In `isRelevantOffset`, compute the set of offsets within each virtual variable (linear in - * the number of VMLs) + * the number of IMLs) * 2. In `isCoveredOffset`, rank the offsets within each virtual variable (linear in the number - * of VMLs) - * 3. In `isCoveredOffset`, compute the set of ranks that each VML with known start and end + * of IMLs) + * 3. In `isCoveredOffset`, compute the set of ranks that each IML with known start and end * offsets covers (linear in the size of the overlap set) - * 4. In `overlappingVariableMemoryLocations`, compute the set of overlapping pairs of VMLs using a + * 4. In `overlappingIndirectMemoryLocations`, compute the set of overlapping pairs of IMLs using a * join on `isCoveredOffset` (linear in the size of the overlap set) - * 5. In `overlappingIRVariableMemoryLocations`, restrict to only the pairs that share an - * `IRVariable` (linear in the size of the overlap set) - * 5. In `getVariableMemoryLocationOverlap`, compute the precise overlap relation for each - * overlapping pair of VMLs (linear in the size of the overlap set) + * 5. In `overlappingIndirectMemoryLocationsWithBase`, restrict to only the pairs that share the + * same base address. (linear in the size of the overlap set) + * 6. In `getIndirectMemoryLocationOverlap`, compute the precise overlap relation for each + * overlapping pair of IMLs (linear in the size of the overlap set) */ -private predicate isRelevantOffset(VirtualVariable vv, IntValue offset) { - exists(VariableMemoryLocation ml | - ml.getVirtualVariable() = vv +private predicate isRelevantOffset(ValueNumber baseAddress, IntValue offset) { + exists(IndirectMemoryLocation ml | + ml.getBaseAddress() = baseAddress | ml.getStartBitOffset() = offset or @@ -255,22 +353,23 @@ private predicate isRelevantOffset(VirtualVariable vv, IntValue offset) { ) } -private predicate isRelatableMemoryLocation(VariableMemoryLocation vml) { +private predicate isRelatableMemoryLocation(IndirectMemoryLocation vml) { vml.getEndBitOffset() != Ints::unknown() and vml.getStartBitOffset() != Ints::unknown() } -private predicate isCoveredOffset(VariableMemoryLocation vml, VirtualVariable vv, int offsetRank) { +private predicate isCoveredOffset(IndirectMemoryLocation vml, ValueNumber baseAddress, + int offsetRank) { exists(int startRank, int endRank | - vml.getStartBitOffset() = rank[startRank](IntValue offset_ | isRelevantOffset(vv, offset_)) and - vml.getEndBitOffset() = rank[endRank](IntValue offset_ | isRelevantOffset(vv, offset_)) and - vv = vml.getVirtualVariable() and + vml.getStartBitOffset() = rank[startRank](IntValue offset_ | isRelevantOffset(baseAddress, offset_)) and + vml.getEndBitOffset() = rank[endRank](IntValue offset_ | isRelevantOffset(baseAddress, offset_)) and + baseAddress = vml.getBaseAddress() and isRelatableMemoryLocation(vml) and offsetRank in [startRank .. endRank] ) } -private predicate hasUnknownOffset(VariableMemoryLocation vml, VirtualVariable vv) { +private predicate hasUnknownOffset(IndirectMemoryLocation vml, VirtualVariable vv) { vml.getVirtualVariable() = vv and ( vml.getStartBitOffset() = Ints::unknown() or @@ -278,8 +377,9 @@ private predicate hasUnknownOffset(VariableMemoryLocation vml, VirtualVariable v ) } -private predicate overlappingVariableMemoryLocations(VariableMemoryLocation def, VariableMemoryLocation use) { - exists(VirtualVariable vv, int offsetRank | isCoveredOffset(def, vv, offsetRank) and isCoveredOffset(use, vv, offsetRank)) +private predicate overlappingIndirectMemoryLocations(IndirectMemoryLocation def, IndirectMemoryLocation use) { + exists(ValueNumber baseAddress, int offsetRank | + isCoveredOffset(def, baseAddress, offsetRank) and isCoveredOffset(use, baseAddress, offsetRank)) or hasUnknownOffset(def, use.getVirtualVariable()) or @@ -287,33 +387,39 @@ private predicate overlappingVariableMemoryLocations(VariableMemoryLocation def, } pragma[noopt] // Internal ticket: QL-937 -private predicate overlappingIRVariableMemoryLocations(VariableMemoryLocation def, VariableMemoryLocation use) { - overlappingVariableMemoryLocations(def, use) and - def.getVariable() = use.getVariable() +private predicate overlappingIndirectMemoryLocationsWithBase(IndirectMemoryLocation def, IndirectMemoryLocation use) { + overlappingIndirectMemoryLocations(def, use) and + def.getBaseAddress() = use.getBaseAddress() } -private Overlap getVariableMemoryLocationOverlap(VariableMemoryLocation def, VariableMemoryLocation use) { - overlappingIRVariableMemoryLocations(def, use) and - result = Interval::getOverlap(def.getStartBitOffset(), def.getEndBitOffset(), use.getStartBitOffset(), use.getEndBitOffset()) +private Overlap getIndirectMemoryLocationOverlap(IndirectMemoryLocation def, IndirectMemoryLocation use) { + ( + // If the two locations have the same base address, they overlap based on their offset ranges. + overlappingIndirectMemoryLocationsWithBase(def, use) and + result = Interval::getOverlap(def.getStartBitOffset(), def.getEndBitOffset(), use.getStartBitOffset(), use.getEndBitOffset()) + ) or + ( + // If they do not share the same base address, then they may partially overlap unless they both + // have an allocation and the two allocations are different. + def.getVirtualVariable() = use.getVirtualVariable() and + def.getBaseAddress() != use.getBaseAddress() and + not (def.getAllocation() != use.getAllocation()) and + result instanceof MayPartiallyOverlap + ) } +private MemoryLocation getMemoryLocation(AddressOperand addrOperand) { + exists(Type type, ValueNumber baseAddress, IntValue startBitOffset, IntValue endBitOffset | + hasMemoryAccess(addrOperand, type, baseAddress, startBitOffset, endBitOffset) and + result = TIndirectMemoryLocation(type, baseAddress, startBitOffset, endBitOffset) + ) +} MemoryLocation getResultMemoryLocation(Instruction instr) { + result = getMemoryLocation(instr.getResultAddressOperand()) or exists(MemoryAccessKind kind | kind = instr.getResultMemoryAccess() and ( - ( - kind.usesAddressOperand() and - if hasResultMemoryAccess(instr, _, _, _, _) then ( - exists(IRVariable var, Type type, IntValue startBitOffset, IntValue endBitOffset | - hasResultMemoryAccess(instr, var, type, startBitOffset, endBitOffset) and - result = TVariableMemoryLocation(var, type, startBitOffset, endBitOffset) - ) - ) - else ( - result = TUnknownMemoryLocation(instr.getEnclosingIRFunction()) - ) - ) or ( kind instanceof EscapedMemoryAccess and result = TUnknownVirtualVariable(instr.getEnclosingIRFunction()) @@ -327,21 +433,10 @@ MemoryLocation getResultMemoryLocation(Instruction instr) { } MemoryLocation getOperandMemoryLocation(MemoryOperand operand) { + result = getMemoryLocation(operand.getAddressOperand()) or exists(MemoryAccessKind kind | kind = operand.getMemoryAccess() and ( - ( - kind.usesAddressOperand() and - if hasOperandMemoryAccess(operand, _, _, _, _) then ( - exists(IRVariable var, Type type, IntValue startBitOffset, IntValue endBitOffset | - hasOperandMemoryAccess(operand, var, type, startBitOffset, endBitOffset) and - result = TVariableMemoryLocation(var, type, startBitOffset, endBitOffset) - ) - ) - else ( - result = TUnknownMemoryLocation(operand.getEnclosingIRFunction()) - ) - ) or ( kind instanceof EscapedMemoryAccess and result = TUnknownVirtualVariable(operand.getEnclosingIRFunction())