Skip to content

Commit 21a1ee7

Browse files
committed
C++: Add annoying case in SSA.qll related to 'NewExpr' and accept test changes.
1 parent 3efe60f commit 21a1ee7

File tree

5 files changed

+45
-11
lines changed

5 files changed

+45
-11
lines changed

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/Ssa.qll

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,30 @@ private module Cached {
324324
use.hasRankInBlock(bb2, i2) and
325325
flowOutOfAddressStep(use.getOperand(), nodeTo)
326326
)
327+
or
328+
// This final case is a bit annoying. The write side effect on an expression like `a = new A;` writes
329+
// to a fresh address returned by `operator new`, and there's no easy way to use the shared SSA
330+
// library to hook that up to the assignment to `a`. So instead we flow to the _first_ use of the
331+
// value computed by `operator new` that occurs after `nodeFrom` (to avoid a loop in the
332+
// dataflow graph).
333+
exists(WriteSideEffectInstruction write, IRBlock bb, int i1, int i2, Operand op |
334+
nodeFrom.getInstruction().(CallInstruction).getStaticCallTarget() instanceof
335+
Alloc::OperatorNewAllocationFunction and
336+
write = nodeFrom.getStoreInstruction() and
337+
bb.getInstruction(i1) = write and
338+
bb.getInstruction(i2) = op.getUse() and
339+
// Flow to an instruction that occurs later in the block.
340+
valueFlow*(nodeFrom.getInstruction(), op.getDef()) and
341+
nodeTo.asOperand() = op and
342+
i2 > i1 and
343+
// There is no previous instruction that also occurs after `nodeFrom`.
344+
not exists(Instruction instr, int i |
345+
bb.getInstruction(i) = instr and
346+
valueFlow(instr, op.getDef()) and
347+
i1 < i and
348+
i < i2
349+
)
350+
)
327351
}
328352

329353
private predicate fromReadNode(ReadNode nodeFrom, Node nodeTo) {
@@ -402,6 +426,16 @@ private module Cached {
402426
)
403427
}
404428

429+
private predicate valueFlow(Instruction iFrom, Instruction iTo) {
430+
iTo.(CopyValueInstruction).getSourceValue() = iFrom
431+
or
432+
iTo.(ConvertInstruction).getUnary() = iFrom
433+
or
434+
iTo.(CheckedConvertOrNullInstruction).getUnary() = iFrom
435+
or
436+
iTo.(InheritanceConversionInstruction).getUnary() = iFrom
437+
}
438+
405439
private predicate flowOutOfAddressStep(Operand operand, Node nTo) {
406440
// Flow into a read node
407441
exists(ReadNode readNode | readNode = nTo |

cpp/ql/test/library-tests/dataflow/fields/A.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,15 +46,15 @@ class A
4646
{
4747
C *c = new C();
4848
B *b = B::make(c);
49-
sink(b->c); // $ast MISSING: ir
49+
sink(b->c); // $ast,ir
5050
}
5151

5252
void f2()
5353
{
5454
B *b = new B();
5555
b->set(new C1());
5656
sink(b->get()); // $ ast ir=55:12
57-
sink((new B(new C()))->get()); // $ ast MISSING: ir
57+
sink((new B(new C()))->get()); // $ ast,ir
5858
}
5959

6060
void f3()
@@ -149,8 +149,8 @@ class A
149149
{
150150
B *b = new B();
151151
D *d = new D(b, r());
152-
sink(d->b); // $ ast MISSING: ir
153-
sink(d->b->c); // $ ast MISSING: ir
152+
sink(d->b); // $ ast,ir=143:25 ast,ir=150:12
153+
sink(d->b->c); // $ ast,ir
154154
sink(b->c); // $ ast,ir
155155
}
156156

@@ -162,11 +162,11 @@ class A
162162
MyList *l3 = new MyList(nullptr, l2);
163163
sink(l3->head); // no flow, b is nested beneath at least one ->next
164164
sink(l3->next->head); // no flow
165-
sink(l3->next->next->head); // $ ast MISSING: ir
165+
sink(l3->next->next->head); // $ ast,ir
166166
sink(l3->next->next->next->head); // no flow
167167
for (MyList *l = l3; l != nullptr; l = l->next)
168168
{
169-
sink(l->head); // $ ast MISSING: ir
169+
sink(l->head); // $ ast,ir
170170
}
171171
}
172172

cpp/ql/test/library-tests/dataflow/fields/B.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ class B
66
Elem *e = new Elem();
77
Box1 *b1 = new Box1(e, nullptr);
88
Box2 *b2 = new Box2(b1);
9-
sink(b2->box1->elem1); // $ ast MISSING: ir
9+
sink(b2->box1->elem1); // $ ast,ir
1010
sink(b2->box1->elem2); // no flow
1111
}
1212

@@ -16,7 +16,7 @@ class B
1616
Box1 *b1 = new B::Box1(nullptr, e);
1717
Box2 *b2 = new Box2(b1);
1818
sink(b2->box1->elem1); // no flow
19-
sink(b2->box1->elem2); // $ ast MISSING: ir
19+
sink(b2->box1->elem2); // $ ast,ir
2020
}
2121

2222
static void sink(void *o) {}

cpp/ql/test/library-tests/dataflow/fields/C.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ class C
2626

2727
void func()
2828
{
29-
sink(s1); // $ast MISSING: ir
29+
sink(s1); // $ast,ir
3030
sink(s2); // $ MISSING: ast,ir
3131
sink(s3); // $ast MISSING: ir
3232
sink(s4); // $ MISSING: ast,ir

cpp/ql/test/library-tests/dataflow/smart-pointers-taint/test.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,9 @@ void test_unique_ptr_struct() {
2121
std::unique_ptr<A> p1(new A{source(), 0});
2222
std::unique_ptr<A> p2 = std::make_unique<A>(source(), 0);
2323

24-
sink(p1->x); // $ MISSING: ast,ir
24+
sink(p1->x); // $ ir MISSING: ast
2525
sink(p1->y);
26-
sink(p2->x); // $ MISSING: ast,ir
26+
sink(p2->x); // $ ir=22:46 MISSING: ast
2727
sink(p2->y);
2828
}
2929

0 commit comments

Comments
 (0)