From ab8c49a35f72bd29ebeea9aec997c35a3c600c63 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Thu, 7 Dec 2023 19:49:17 +0000 Subject: [PATCH] C++: Merge 'PostUpdateFieldNode' and 'IndirectArgumentOutNode' into a single IPA branch. --- .../cpp/ir/dataflow/internal/DataFlowUtil.qll | 96 +++++++++---------- 1 file changed, 43 insertions(+), 53 deletions(-) diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll index 717c0532473c..95f2412f8e5f 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll @@ -18,17 +18,17 @@ private import codeql.util.Unit /** * The IR dataflow graph consists of the following nodes: - * - `Node0`, which injects most instructions and operands directly into the dataflow graph. + * - `Node0`, which injects most instructions and operands directly into the + * dataflow graph. * - `VariableNode`, which is used to model flow through global variables. - * - `PostFieldUpdateNode`, which is used to model the state of a field after a value has been stored - * into an address after a number of loads. - * - `SsaPhiNode`, which represents phi nodes as computed by the shared SSA library. - * - `IndirectArgumentOutNode`, which represents the value of an argument (and its indirections) after - * it leaves a function call. - * - `RawIndirectOperand`, which represents the value of `operand` after loading the address a number - * of times. - * - `RawIndirectInstruction`, which represents the value of `instr` after loading the address a number - * of times. + * - `PostUpdateNodeImpl`, which is used to model the state of an object after + * an update after a number of loads. + * - `SsaPhiNode`, which represents phi nodes as computed by the shared SSA + * library. + * - `RawIndirectOperand`, which represents the value of `operand` after + * loading the address a number of times. + * - `RawIndirectInstruction`, which represents the value of `instr` after + * loading the address a number of times. */ cached private newtype TIRDataFlowNode = @@ -37,14 +37,13 @@ private newtype TIRDataFlowNode = indirectionIndex = [getMinIndirectionsForType(var.getUnspecifiedType()) .. Ssa::getMaxIndirectionsForType(var.getUnspecifiedType())] } or - TPostFieldUpdateNode(FieldAddress operand, int indirectionIndex) { - indirectionIndex = - [1 .. Ssa::countIndirectionsForCppType(operand.getObjectAddress().getResultLanguageType())] - } or - TSsaPhiNode(Ssa::PhiNode phi) or - TIndirectArgumentOutNode(ArgumentOperand operand, int indirectionIndex) { + TPostUpdateNodeImpl(Operand operand, int indirectionIndex) { + operand = any(FieldAddress fa).getObjectAddressOperand() and + indirectionIndex = [1 .. Ssa::countIndirectionsForCppType(Ssa::getLanguageType(operand))] + or Ssa::isModifiableByCall(operand, indirectionIndex) } or + TSsaPhiNode(Ssa::PhiNode phi) or TRawIndirectOperand0(Node0Impl node, int indirectionIndex) { Ssa::hasRawIndirectOperand(node.asOperand(), indirectionIndex) } or @@ -84,7 +83,7 @@ private predicate parameterIsRedefined(Parameter p) { class FieldAddress extends Operand { FieldAddressInstruction fai; - FieldAddress() { fai = this.getDef() } + FieldAddress() { fai = this.getDef() and not Ssa::ignoreOperand(this) } /** Gets the field associated with this instruction. */ Field getField() { result = fai.getField() } @@ -550,37 +549,43 @@ Type stripPointer(Type t) { result = t.(FunctionPointerIshType).getBaseType() } -/** - * INTERNAL: do not use. - * - * The node representing the value of a field after it has been updated. - */ -class PostFieldUpdateNode extends TPostFieldUpdateNode, PartialDefinitionNode { +private class PostUpdateNodeImpl extends PartialDefinitionNode, TPostUpdateNodeImpl { int indirectionIndex; - FieldAddress fieldAddress; + Operand operand; - PostFieldUpdateNode() { this = TPostFieldUpdateNode(fieldAddress, indirectionIndex) } + PostUpdateNodeImpl() { this = TPostUpdateNodeImpl(operand, indirectionIndex) } - override Declaration getFunction() { result = fieldAddress.getUse().getEnclosingFunction() } + override Declaration getFunction() { result = operand.getUse().getEnclosingFunction() } override Declaration getEnclosingCallable() { result = this.getFunction() } - FieldAddress getFieldAddress() { result = fieldAddress } - - Field getUpdatedField() { result = fieldAddress.getField() } + /** Gets the operand associated with this node. */ + Operand getOperand() { result = operand } int getIndirectionIndex() { result = indirectionIndex } - override Node getPreUpdateNode() { - hasOperandAndIndex(result, pragma[only_bind_into](fieldAddress).getObjectAddressOperand(), - indirectionIndex) - } + override Location getLocationImpl() { result = operand.getLocation() } - override Expr getDefinedExpr() { - result = fieldAddress.getObjectAddress().getUnconvertedResultExpression() + final override Node getPreUpdateNode() { hasOperandAndIndex(result, operand, indirectionIndex) } + + final override Expr getDefinedExpr() { + result = operand.getDef().getUnconvertedResultExpression() } +} + +/** + * INTERNAL: do not use. + * + * The node representing the value of a field after it has been updated. + */ +class PostFieldUpdateNode extends PostUpdateNodeImpl { + FieldAddress fieldAddress; - override Location getLocationImpl() { result = fieldAddress.getLocation() } + PostFieldUpdateNode() { operand = fieldAddress.getObjectAddressOperand() } + + FieldAddress getFieldAddress() { result = fieldAddress } + + Field getUpdatedField() { result = this.getFieldAddress().getField() } override string toStringImpl() { result = this.getPreUpdateNode() + " [post update]" } } @@ -816,13 +821,8 @@ class IndirectReturnNode extends Node { * A node representing the indirection of a value after it * has been returned from a function. */ -class IndirectArgumentOutNode extends Node, TIndirectArgumentOutNode, PartialDefinitionNode { - ArgumentOperand operand; - int indirectionIndex; - - IndirectArgumentOutNode() { this = TIndirectArgumentOutNode(operand, indirectionIndex) } - - int getIndirectionIndex() { result = indirectionIndex } +class IndirectArgumentOutNode extends PostUpdateNodeImpl { + override ArgumentOperand operand; int getArgumentIndex() { exists(CallInstruction call | call.getArgumentOperand(result) = operand) @@ -834,12 +834,6 @@ class IndirectArgumentOutNode extends Node, TIndirectArgumentOutNode, PartialDef Function getStaticCallTarget() { result = this.getCallInstruction().getStaticCallTarget() } - override Declaration getEnclosingCallable() { result = this.getFunction() } - - override Declaration getFunction() { result = this.getCallInstruction().getEnclosingFunction() } - - override Node getPreUpdateNode() { hasOperandAndIndex(result, operand, indirectionIndex) } - override string toStringImpl() { // This string should be unique enough to be helpful but common enough to // avoid storing too many different strings. @@ -848,10 +842,6 @@ class IndirectArgumentOutNode extends Node, TIndirectArgumentOutNode, PartialDef not exists(this.getStaticCallTarget()) and result = "output argument" } - - override Location getLocationImpl() { result = operand.getLocation() } - - override Expr getDefinedExpr() { result = operand.getDef().getUnconvertedResultExpression() } } /**