Skip to content

Commit 9888de8

Browse files
MathiasVPMathiasVP
committed
Merge branch 'replace-ast-with-ir-use-usedataflow' into rdmarsh2/repair-365-days-per-year
2 parents a334dc9 + 42e8ec3 commit 9888de8

24 files changed

Lines changed: 27614 additions & 27387 deletions

File tree

‎cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll‎

Lines changed: 110 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -142,11 +142,7 @@ class Node extends TIRDataFlowNode {
142142
* Gets the non-conversion expression that's indirectly tracked by this node
143143
* under `index` number of indirections.
144144
*/
145-
Expr asIndirectExpr(int index) {
146-
exists(Operand operand | hasOperandAndIndex(this, operand, index) |
147-
result = operand.getDef().getUnconvertedResultExpression()
148-
)
149-
}
145+
Expr asIndirectExpr(int index) { result = this.(IndirectExprNode).getExpr(index) }
150146

151147
/**
152148
* Gets the non-conversion expression that's indirectly tracked by this node
@@ -165,9 +161,7 @@ class Node extends TIRDataFlowNode {
165161
* behind `index` number of indirections.
166162
*/
167163
Expr asIndirectConvertedExpr(int index) {
168-
exists(Operand operand | hasOperandAndIndex(this, operand, index) |
169-
result = operand.getDef().getConvertedResultExpression()
170-
)
164+
result = this.(IndirectExprNode).getConvertedExpr(index)
171165
}
172166

173167
/**
@@ -309,14 +303,25 @@ class Node extends TIRDataFlowNode {
309303

310304
/** Gets a textual representation of this element. */
311305
cached
312-
final string toString() { result = this.toStringImpl() }
306+
final string toString() {
307+
result = toExprString(this)
308+
or
309+
not exists(toExprString(this)) and
310+
result = this.toStringImpl()
311+
}
313312

314313
/** INTERNAL: Do not use. */
315314
string toStringImpl() {
316315
none() // overridden by subclasses
317316
}
318317
}
319318

319+
private string toExprString(Node n) {
320+
result = n.asExpr().toString()
321+
or
322+
result = n.asIndirectExpr().toString() + " indirection"
323+
}
324+
320325
/**
321326
* An instruction, viewed as a node in a data flow graph.
322327
*/
@@ -738,6 +743,19 @@ private predicate exprNodeShouldBeIndirectOperand(IndirectOperand node, Expr e,
738743
not convertedExprMustBeOperand(e)
739744
}
740745

746+
/** Holds if `node` should be an `IndirectOperand` that maps `node.asIndirectExpr()` to `e`. */
747+
private predicate indirectExprNodeShouldBeIndirectOperand(IndirectOperand node, Expr e) {
748+
exists(Instruction instr |
749+
instr = node.getOperand().getDef() and
750+
not node instanceof ExprNode
751+
|
752+
e = instr.(VariableAddressInstruction).getAst().(Expr).getFullyConverted()
753+
or
754+
not instr instanceof VariableAddressInstruction and
755+
e = instr.getConvertedResultExpression()
756+
)
757+
}
758+
741759
private predicate exprNodeShouldBeIndirectOutNode(IndirectArgumentOutNode node, Expr e) {
742760
exists(CallInstruction call |
743761
call.getStaticCallTarget() instanceof Constructor and
@@ -754,18 +772,35 @@ predicate exprNodeShouldBeInstruction(Node node, Expr e) {
754772
not exprNodeShouldBeIndirectOutNode(_, e)
755773
}
756774

757-
private class ExprNodeBase extends Node {
758-
Expr getConvertedExpr() { none() } // overridden by subclasses
775+
/** Holds if `node` should be an `IndirectInstruction` that maps `node.asIndirectExpr()` to `e`. */
776+
predicate indirectExprNodeShouldBeIndirectInstruction(IndirectInstruction node, Expr e) {
777+
exists(Instruction instr |
778+
instr = node.getInstruction() and not indirectExprNodeShouldBeIndirectOperand(_, e)
779+
|
780+
e = instr.(VariableAddressInstruction).getAst().(Expr).getFullyConverted()
781+
or
782+
not instr instanceof VariableAddressInstruction and
783+
e = instr.getConvertedResultExpression()
784+
)
785+
}
786+
787+
abstract private class ExprNodeBase extends Node {
788+
/**
789+
* Gets the expression corresponding to this node, if any. The returned
790+
* expression may be a `Conversion`.
791+
*/
792+
abstract Expr getConvertedExpr();
759793

760-
Expr getExpr() { none() } // overridden by subclasses
794+
/** Gets the non-conversion expression corresponding to this node, if any. */
795+
abstract Expr getExpr();
761796
}
762797

763798
private class InstructionExprNode extends ExprNodeBase, InstructionNode {
764799
InstructionExprNode() { exprNodeShouldBeInstruction(this, _) }
765800

766801
final override Expr getConvertedExpr() { exprNodeShouldBeInstruction(this, result) }
767802

768-
final override Expr getExpr() { result = this.getInstruction().getUnconvertedResultExpression() }
803+
final override Expr getExpr() { result = this.getConvertedExpr().getUnconverted() }
769804

770805
final override string toStringImpl() { result = this.getConvertedExpr().toString() }
771806
}
@@ -775,9 +810,7 @@ private class OperandExprNode extends ExprNodeBase, OperandNode {
775810

776811
final override Expr getConvertedExpr() { exprNodeShouldBeOperand(this, result) }
777812

778-
final override Expr getExpr() {
779-
result = this.getOperand().getDef().getUnconvertedResultExpression()
780-
}
813+
final override Expr getExpr() { result = this.getConvertedExpr().getUnconverted() }
781814

782815
final override string toStringImpl() {
783816
// Avoid generating multiple `toString` results for `ArgumentNode`s
@@ -790,17 +823,54 @@ private class OperandExprNode extends ExprNodeBase, OperandNode {
790823
}
791824

792825
private class IndirectOperandExprNode extends ExprNodeBase, IndirectOperand {
793-
LoadInstruction load;
794-
795-
IndirectOperandExprNode() { exprNodeShouldBeIndirectOperand(this, _, load) }
826+
IndirectOperandExprNode() { exprNodeShouldBeIndirectOperand(this, _, _) }
796827

797-
final override Expr getConvertedExpr() { exprNodeShouldBeIndirectOperand(this, result, load) }
828+
final override Expr getConvertedExpr() { exprNodeShouldBeIndirectOperand(this, result, _) }
798829

799-
final override Expr getExpr() { result = load.getUnconvertedResultExpression() }
830+
final override Expr getExpr() { result = this.getConvertedExpr().getUnconverted() }
800831

801832
final override string toStringImpl() { result = this.getConvertedExpr().toString() }
802833
}
803834

835+
abstract private class IndirectExprNodeBase extends Node {
836+
/**
837+
* Gets the expression corresponding to this node, if any. The returned
838+
* expression may be a `Conversion`.
839+
*/
840+
abstract Expr getConvertedExpr(int indirectionIndex);
841+
842+
/** Gets the non-conversion expression corresponding to this node, if any. */
843+
abstract Expr getExpr(int indirectionIndex);
844+
}
845+
846+
private class IndirectOperandIndirectExprNode extends IndirectExprNodeBase, IndirectOperand {
847+
IndirectOperandIndirectExprNode() { indirectExprNodeShouldBeIndirectOperand(this, _) }
848+
849+
final override Expr getConvertedExpr(int index) {
850+
this.getIndirectionIndex() = index and
851+
indirectExprNodeShouldBeIndirectOperand(this, result)
852+
}
853+
854+
final override Expr getExpr(int index) {
855+
this.getIndirectionIndex() = index and
856+
result = this.getConvertedExpr(index).getUnconverted()
857+
}
858+
}
859+
860+
private class IndirectInstructionIndirectExprNode extends IndirectExprNodeBase, IndirectInstruction {
861+
IndirectInstructionIndirectExprNode() { indirectExprNodeShouldBeIndirectInstruction(this, _) }
862+
863+
final override Expr getConvertedExpr(int index) {
864+
this.getIndirectionIndex() = index and
865+
indirectExprNodeShouldBeIndirectInstruction(this, result)
866+
}
867+
868+
final override Expr getExpr(int index) {
869+
this.getIndirectionIndex() = index and
870+
result = this.getConvertedExpr(index).getUnconverted()
871+
}
872+
}
873+
804874
private class IndirectArgumentOutExprNode extends ExprNodeBase, IndirectArgumentOutNode {
805875
IndirectArgumentOutExprNode() { exprNodeShouldBeIndirectOutNode(this, _) }
806876

@@ -830,6 +900,25 @@ class ExprNode extends Node instanceof ExprNodeBase {
830900
Expr getConvertedExpr() { result = super.getConvertedExpr() }
831901
}
832902

903+
/**
904+
* An indirect expression, viewed as a node in a data flow graph.
905+
*/
906+
class IndirectExprNode extends Node instanceof IndirectExprNodeBase {
907+
/**
908+
* Gets the non-conversion expression corresponding to this node, if any. If
909+
* this node strictly (in the sense of `getConvertedExpr`) corresponds to a
910+
* `Conversion`, then the result is that `Conversion`'s non-`Conversion` base
911+
* expression.
912+
*/
913+
Expr getExpr(int indirectionIndex) { result = super.getExpr(indirectionIndex) }
914+
915+
/**
916+
* Gets the expression corresponding to this node, if any. The returned
917+
* expression may be a `Conversion`.
918+
*/
919+
Expr getConvertedExpr(int indirectionIndex) { result = super.getConvertedExpr(indirectionIndex) }
920+
}
921+
833922
/**
834923
* The value of a parameter at function entry, viewed as a node in a data
835924
* flow graph. This includes both explicit parameters such as `x` in `f(x)`

‎cpp/ql/test/experimental/query-tests/Security/CWE/CWE-190/AllocMultiplicationOverflow/AllocMultiplicationOverflow.expected‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,20 @@
11
edges
22
| test.cpp:13:33:13:37 | ... * ... | test.cpp:13:33:13:37 | ... * ... |
3-
| test.cpp:15:31:15:35 | ... * ... | test.cpp:15:31:15:35 | (unsigned long)... |
3+
| test.cpp:15:31:15:35 | ... * ... | test.cpp:15:31:15:35 | ... * ... |
44
| test.cpp:19:34:19:38 | ... * ... | test.cpp:19:34:19:38 | ... * ... |
5-
| test.cpp:22:17:22:21 | (size_t)... | test.cpp:23:33:23:37 | size1 |
5+
| test.cpp:22:17:22:21 | ... * ... | test.cpp:23:33:23:37 | size1 |
66
| test.cpp:22:17:22:21 | ... * ... | test.cpp:23:33:23:37 | size1 |
77
nodes
88
| test.cpp:13:33:13:37 | ... * ... | semmle.label | ... * ... |
99
| test.cpp:13:33:13:37 | ... * ... | semmle.label | ... * ... |
1010
| test.cpp:13:33:13:37 | ... * ... | semmle.label | ... * ... |
11-
| test.cpp:15:31:15:35 | (unsigned long)... | semmle.label | (unsigned long)... |
11+
| test.cpp:15:31:15:35 | ... * ... | semmle.label | ... * ... |
1212
| test.cpp:15:31:15:35 | ... * ... | semmle.label | ... * ... |
1313
| test.cpp:15:31:15:35 | ... * ... | semmle.label | ... * ... |
1414
| test.cpp:19:34:19:38 | ... * ... | semmle.label | ... * ... |
1515
| test.cpp:19:34:19:38 | ... * ... | semmle.label | ... * ... |
1616
| test.cpp:19:34:19:38 | ... * ... | semmle.label | ... * ... |
17-
| test.cpp:22:17:22:21 | (size_t)... | semmle.label | (size_t)... |
17+
| test.cpp:22:17:22:21 | ... * ... | semmle.label | ... * ... |
1818
| test.cpp:22:17:22:21 | ... * ... | semmle.label | ... * ... |
1919
| test.cpp:23:33:23:37 | size1 | semmle.label | size1 |
2020
| test.cpp:30:27:30:31 | ... * ... | semmle.label | ... * ... |
@@ -24,13 +24,13 @@ subpaths
2424
| test.cpp:13:33:13:37 | ... * ... | test.cpp:13:33:13:37 | ... * ... | test.cpp:13:33:13:37 | ... * ... | Potentially overflowing value from $@ is used in the size of this allocation. | test.cpp:13:33:13:37 | ... * ... | multiplication |
2525
| test.cpp:13:33:13:37 | ... * ... | test.cpp:13:33:13:37 | ... * ... | test.cpp:13:33:13:37 | ... * ... | Potentially overflowing value from $@ is used in the size of this allocation. | test.cpp:13:33:13:37 | ... * ... | multiplication |
2626
| test.cpp:13:33:13:37 | ... * ... | test.cpp:13:33:13:37 | ... * ... | test.cpp:13:33:13:37 | ... * ... | Potentially overflowing value from $@ is used in the size of this allocation. | test.cpp:13:33:13:37 | ... * ... | multiplication |
27-
| test.cpp:15:31:15:35 | (unsigned long)... | test.cpp:15:31:15:35 | (unsigned long)... | test.cpp:15:31:15:35 | (unsigned long)... | Potentially overflowing value from $@ is used in the size of this allocation. | test.cpp:15:31:15:35 | (unsigned long)... | multiplication |
28-
| test.cpp:15:31:15:35 | (unsigned long)... | test.cpp:15:31:15:35 | ... * ... | test.cpp:15:31:15:35 | (unsigned long)... | Potentially overflowing value from $@ is used in the size of this allocation. | test.cpp:15:31:15:35 | ... * ... | multiplication |
27+
| test.cpp:15:31:15:35 | ... * ... | test.cpp:15:31:15:35 | ... * ... | test.cpp:15:31:15:35 | ... * ... | Potentially overflowing value from $@ is used in the size of this allocation. | test.cpp:15:31:15:35 | ... * ... | multiplication |
28+
| test.cpp:15:31:15:35 | ... * ... | test.cpp:15:31:15:35 | ... * ... | test.cpp:15:31:15:35 | ... * ... | Potentially overflowing value from $@ is used in the size of this allocation. | test.cpp:15:31:15:35 | ... * ... | multiplication |
2929
| test.cpp:15:31:15:35 | ... * ... | test.cpp:15:31:15:35 | ... * ... | test.cpp:15:31:15:35 | ... * ... | Potentially overflowing value from $@ is used in the size of this allocation. | test.cpp:15:31:15:35 | ... * ... | multiplication |
3030
| test.cpp:19:34:19:38 | ... * ... | test.cpp:19:34:19:38 | ... * ... | test.cpp:19:34:19:38 | ... * ... | Potentially overflowing value from $@ is used in the size of this allocation. | test.cpp:19:34:19:38 | ... * ... | multiplication |
3131
| test.cpp:19:34:19:38 | ... * ... | test.cpp:19:34:19:38 | ... * ... | test.cpp:19:34:19:38 | ... * ... | Potentially overflowing value from $@ is used in the size of this allocation. | test.cpp:19:34:19:38 | ... * ... | multiplication |
3232
| test.cpp:19:34:19:38 | ... * ... | test.cpp:19:34:19:38 | ... * ... | test.cpp:19:34:19:38 | ... * ... | Potentially overflowing value from $@ is used in the size of this allocation. | test.cpp:19:34:19:38 | ... * ... | multiplication |
33-
| test.cpp:23:33:23:37 | size1 | test.cpp:22:17:22:21 | (size_t)... | test.cpp:23:33:23:37 | size1 | Potentially overflowing value from $@ is used in the size of this allocation. | test.cpp:22:17:22:21 | (size_t)... | multiplication |
33+
| test.cpp:23:33:23:37 | size1 | test.cpp:22:17:22:21 | ... * ... | test.cpp:23:33:23:37 | size1 | Potentially overflowing value from $@ is used in the size of this allocation. | test.cpp:22:17:22:21 | ... * ... | multiplication |
3434
| test.cpp:23:33:23:37 | size1 | test.cpp:22:17:22:21 | ... * ... | test.cpp:23:33:23:37 | size1 | Potentially overflowing value from $@ is used in the size of this allocation. | test.cpp:22:17:22:21 | ... * ... | multiplication |
3535
| test.cpp:30:27:30:31 | ... * ... | test.cpp:30:27:30:31 | ... * ... | test.cpp:30:27:30:31 | ... * ... | Potentially overflowing value from $@ is used in the size of this allocation. | test.cpp:30:27:30:31 | ... * ... | multiplication |
3636
| test.cpp:31:27:31:31 | ... * ... | test.cpp:31:27:31:31 | ... * ... | test.cpp:31:27:31:31 | ... * ... | Potentially overflowing value from $@ is used in the size of this allocation. | test.cpp:31:27:31:31 | ... * ... | multiplication |

0 commit comments

Comments
 (0)