Skip to content

Commit efa133f

Browse files
author
Dave Bartolomeo
authored
Merge pull request #3463 from MathiasVP/fix-field-flow-annotation
C++: Annotate field flow tests
2 parents 49ebb31 + 1c2b856 commit efa133f

File tree

17 files changed

+147
-1049
lines changed

17 files changed

+147
-1049
lines changed

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

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -40,21 +40,21 @@ class A
4040
cc.insert(nullptr);
4141
ct.insert(new C());
4242
sink(&cc); // no flow
43-
sink(&ct); // flow
43+
sink(&ct); // $ast $f-:ir
4444
}
4545
void f1()
4646
{
4747
C *c = new C();
4848
B *b = B::make(c);
49-
sink(b->c); // flow
49+
sink(b->c); // $ast $f-:ir
5050
}
5151

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

6060
void f3()
@@ -63,7 +63,7 @@ class A
6363
B *b2;
6464
b2 = setOnB(b1, new C2());
6565
sink(b1->c); // no flow
66-
sink(b2->c); // flow
66+
sink(b2->c); // $ast $f-:ir
6767
}
6868

6969
void f4()
@@ -72,7 +72,7 @@ class A
7272
B *b2;
7373
b2 = setOnBWrap(b1, new C2());
7474
sink(b1->c); // no flow
75-
sink(b2->c); // flow
75+
sink(b2->c); // $ast $f-:ir
7676
}
7777

7878
B *setOnBWrap(B *b1, C *c)
@@ -104,7 +104,7 @@ class A
104104
{
105105
if (C1 *c1 = dynamic_cast<C1 *>(c))
106106
{
107-
sink(c1->a); // flow
107+
sink(c1->a); // $ast $f-:ir
108108
}
109109
C *cc;
110110
if (C2 *c2 = dynamic_cast<C2 *>(c))
@@ -117,7 +117,7 @@ class A
117117
}
118118
if (C1 *c1 = dynamic_cast<C1 *>(cc))
119119
{
120-
sink(c1->a); // no flow, stopped by cast to C2 [FALSE POSITIVE]
120+
sink(c1->a); //$f+:ast
121121
}
122122
}
123123

@@ -129,7 +129,7 @@ class A
129129
{
130130
B *b = new B();
131131
f7(b);
132-
sink(b->c); // flow
132+
sink(b->c); // $ast $f-:ir
133133
}
134134

135135
class D
@@ -149,9 +149,9 @@ class A
149149
{
150150
B *b = new B();
151151
D *d = new D(b, r());
152-
sink(d->b); // flow x2
153-
sink(d->b->c); // flow
154-
sink(b->c); // flow
152+
sink(d->b); // $ast=143:25 $ast=150:12 $f-:ir
153+
sink(d->b->c); // $ast $f-:ir
154+
sink(b->c); // $ast,ir
155155
}
156156

157157
void f10()
@@ -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); // flow
165+
sink(l3->next->next->head); // $ast $f-: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); // flow
169+
sink(l->head); // $ast $f-: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); // flow
9+
sink(b2->box1->elem1); // $ast $f-: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); // flow
19+
sink(b2->box1->elem2); // $ast $f-:ir
2020
}
2121

2222
static void sink(void *o) {}

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,10 @@ class C
2626

2727
void func()
2828
{
29-
sink(s1); // flow
30-
sink(s2); // flow [NOT DETECTED]
31-
sink(s3); // flow
32-
sink(s4); // flow [NOT DETECTED]
29+
sink(s1); // $ast $f-:ir
30+
sink(s2); // $f-:ast $f-:ir
31+
sink(s3); // $ast $f-:ir
32+
sink(s4); // $f-:ast $f-:ir
3333
}
3434

3535
static void sink(const void *o) {}

‎cpp/ql/test/library-tests/dataflow/fields/D.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ class D {
1919
};
2020

2121
static void sinkWrap(Box2* b2) {
22-
sink(b2->getBox1()->getElem());
22+
sink(b2->getBox1()->getElem()); // $ast=28:15 $ast=35:15 $ast=42:15 $ast=49:15 $f-:ir
2323
}
2424

2525
Box2* boxfield;
@@ -61,6 +61,6 @@ class D {
6161

6262
private:
6363
void f5b() {
64-
sink(boxfield->box->elem);
64+
sink(boxfield->box->elem); // $ast $f-:ir
6565
}
6666
};

‎cpp/ql/test/library-tests/dataflow/fields/E.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ void sink(char *b);
1818

1919
void handlePacket(packet *p)
2020
{
21-
sink(p->data.buffer);
21+
sink(p->data.buffer); // $ast $f-:ir
2222
}
2323

2424
void f(buf* b)
@@ -28,7 +28,7 @@ void f(buf* b)
2828
argument_source(raw);
2929
argument_source(b->buffer);
3030
argument_source(p.data.buffer);
31-
sink(raw);
32-
sink(b->buffer);
31+
sink(raw); // $ast $f-:ir
32+
sink(b->buffer); // $ast $f-:ir
3333
handlePacket(&p);
3434
}

‎cpp/ql/test/library-tests/dataflow/fields/aliasing.cpp‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,21 +26,21 @@ void callSetters() {
2626
referenceSetter(s2);
2727
copySetter(s3);
2828

29-
sink(s1.m1); // flow
30-
sink(s2.m1); // flow
29+
sink(s1.m1); // $ast,ir
30+
sink(s2.m1); // $ast,ir
3131
sink(s3.m1); // no flow
3232
}
3333

3434
void assignAfterAlias() {
3535
S s1 = { 0, 0 };
3636
S &ref1 = s1;
3737
ref1.m1 = user_input();
38-
sink(s1.m1); // flow [FALSE NEGATIVE]
38+
sink(s1.m1); // $f-:ast $ir
3939

4040
S s2 = { 0, 0 };
4141
S &ref2 = s2;
4242
s2.m1 = user_input();
43-
sink(ref2.m1); // flow [FALSE NEGATIVE]
43+
sink(ref2.m1); // $f-:ast $ir
4444
}
4545

4646
void assignAfterCopy() {
@@ -59,7 +59,7 @@ void assignBeforeCopy() {
5959
S s2 = { 0, 0 };
6060
s2.m1 = user_input();
6161
S copy2 = s2;
62-
sink(copy2.m1); // flow
62+
sink(copy2.m1); // $ast,ir
6363
}
6464

6565
struct Wrapper {
@@ -77,18 +77,18 @@ void pointerIntermediate() {
7777
Wrapper w = { { 0, 0 } };
7878
S *s = &w.s;
7979
s->m1 = user_input();
80-
sink(w.s.m1); // flow [FALSE NEGATIVE]
80+
sink(w.s.m1); // $f-:ast $ir
8181
}
8282

8383
void referenceIntermediate() {
8484
Wrapper w = { { 0, 0 } };
8585
S &s = w.s;
8686
s.m1 = user_input();
87-
sink(w.s.m1); // flow [FALSE NEGATIVE]
87+
sink(w.s.m1); // $f-:ast $ir
8888
}
8989

9090
void nestedAssign() {
9191
Wrapper w = { { 0, 0 } };
9292
w.s.m1 = user_input();
93-
sink(w.s.m1); // flow
93+
sink(w.s.m1); // $ast,ir
9494
}

‎cpp/ql/test/library-tests/dataflow/fields/by_reference.cpp‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -48,25 +48,25 @@ struct S {
4848
void test_setDirectly() {
4949
S s;
5050
s.setDirectly(user_input());
51-
sink(s.getDirectly()); // flow
51+
sink(s.getDirectly()); // $ast $f-:ir
5252
}
5353

5454
void test_setIndirectly() {
5555
S s;
5656
s.setIndirectly(user_input());
57-
sink(s.getIndirectly()); // flow
57+
sink(s.getIndirectly()); // $ast $f-:ir
5858
}
5959

6060
void test_setThroughNonMember() {
6161
S s;
6262
s.setThroughNonMember(user_input());
63-
sink(s.getThroughNonMember()); // flow
63+
sink(s.getThroughNonMember()); // $ast $f-:ir
6464
}
6565

6666
void test_nonMemberSetA() {
6767
S s;
6868
nonMemberSetA(&s, user_input());
69-
sink(nonMemberGetA(&s)); // flow
69+
sink(nonMemberGetA(&s)); // $ast $f-:ir
7070
}
7171

7272
////////////////////
@@ -107,13 +107,13 @@ void test_outer_with_ptr(Outer *pouter) {
107107
taint_inner_a_ptr(pouter->inner_ptr);
108108
taint_a_ptr(&pouter->a);
109109

110-
sink(outer.inner_nested.a); // flow
111-
sink(outer.inner_ptr->a); // flow [NOT DETECTED by IR]
112-
sink(outer.a); // flow [NOT DETECTED]
110+
sink(outer.inner_nested.a); // $ast,ir
111+
sink(outer.inner_ptr->a); // $ast $f-:ir
112+
sink(outer.a); // $f-:ast $f-:ir
113113

114-
sink(pouter->inner_nested.a); // flow
115-
sink(pouter->inner_ptr->a); // flow [NOT DETECTED by IR]
116-
sink(pouter->a); // flow [NOT DETECTED]
114+
sink(pouter->inner_nested.a); // $ast,ir
115+
sink(pouter->inner_ptr->a); // $ast $f-:ir
116+
sink(pouter->a); // $f-:ast $f-:ir
117117
}
118118

119119
void test_outer_with_ref(Outer *pouter) {
@@ -127,11 +127,11 @@ void test_outer_with_ref(Outer *pouter) {
127127
taint_inner_a_ref(*pouter->inner_ptr);
128128
taint_a_ref(pouter->a);
129129

130-
sink(outer.inner_nested.a); // flow
131-
sink(outer.inner_ptr->a); // flow [NOT DETECTED by IR]
132-
sink(outer.a); // flow [NOT DETECTED by IR]
130+
sink(outer.inner_nested.a); // $ast,ir
131+
sink(outer.inner_ptr->a); // $ast $f-:ir
132+
sink(outer.a); // $ast $f-:ir
133133

134-
sink(pouter->inner_nested.a); // flow
135-
sink(pouter->inner_ptr->a); // flow [NOT DETECTED by IR]
136-
sink(pouter->a); // flow [NOT DETECTED by IR]
134+
sink(pouter->inner_nested.a); // $ast,ir
135+
sink(pouter->inner_ptr->a); // $ast $f-:ir
136+
sink(pouter->a); // $ast $f-:ir
137137
}

‎cpp/ql/test/library-tests/dataflow/fields/complex.cpp‎

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,12 @@ class Bar
2222
Bar() : f(0, 0) {}
2323
};
2424

25+
class Outer
26+
{
27+
public:
28+
Bar inner;
29+
};
30+
2531
int user_input()
2632
{
2733
return 42;
@@ -31,31 +37,32 @@ void sink(int x)
3137
{
3238
}
3339

34-
void bar(Bar &b)
40+
void bar(Outer &b)
3541
{
3642
// The library correctly finds that the four `user_input` sources can make it
3743
// to the `sink` calls, but it also finds some source/sink combinations that
3844
// are impossible. Those false positives here are a consequence of how the
3945
// shared data flow library overapproximates field flow. The library only
40-
// tracks the head (`f`) and the length (2) of the field access path, and
41-
// then it tracks that both `a_` and `b_` have followed `f` in _some_ access
42-
// path somewhere in the search. That makes the library conclude that there
43-
// could be flow to `b.f.a_` even when the flow was actually to `b.f.b_`.
44-
sink(b.f.a()); // flow [FALSE POSITIVE through `b2.f.setB` and `b3.f.setB`]
45-
sink(b.f.b()); // flow [FALSE POSITIVE through `b1.f.setA` and `b3.f.setA`]
46+
// tracks the final two fields (`f` and `inner`) and the length (3) of the field
47+
// access path, and then it tracks that both `a_` and `b_` have followed `f.inner`
48+
// in _some_ access path somewhere in the search. That makes the library conclude
49+
// that there could be flow to `b.inner.f.a_` even when the flow was actually to
50+
// `b.inner.f.b_`.
51+
sink(b.inner.f.a()); // $ast=62:19 $f+:ast=63:19 $ast=64:19 $f+:ast=65:19 $f-:ir
52+
sink(b.inner.f.b()); // $f+:ast=62:19 $ast=63:19 $f+:ast=64:19 $ast=65:19 $f-:ir
4653
}
4754

4855
void foo()
4956
{
50-
Bar b1;
51-
Bar b2;
52-
Bar b3;
53-
Bar b4;
57+
Outer b1;
58+
Outer b2;
59+
Outer b3;
60+
Outer b4;
5461

55-
b1.f.setA(user_input());
56-
b2.f.setB(user_input());
57-
b3.f.setA(user_input());
58-
b3.f.setB(user_input());
62+
b1.inner.f.setA(user_input());
63+
b2.inner.f.setB(user_input());
64+
b3.inner.f.setA(user_input());
65+
b3.inner.f.setB(user_input());
5966

6067
// Only a() should alert
6168
bar(b1);

‎cpp/ql/test/library-tests/dataflow/fields/constructors.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ class Foo
2525

2626
void bar(Foo &f)
2727
{
28-
sink(f.a()); // flow (through `f` and `h`)
29-
sink(f.b()); // flow (through `g` and `h`)
28+
sink(f.a()); //$ast=34:11 $ast=36:11 $f-:ir
29+
sink(f.b()); //$ast=35:14 $ast=36:25 $f-:ir
3030
}
3131

3232
void foo()

‎cpp/ql/test/library-tests/dataflow/fields/dataflow-consistency.expected‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,13 @@ uniqueEnclosingCallable
44
uniqueTypeBound
55
| complex.cpp:22:11:22:17 | constructor init of field f [post-this] | Node should have one type bound but has 0. |
66
| complex.cpp:22:11:22:17 | constructor init of field f [pre-this] | Node should have one type bound but has 0. |
7+
| complex.cpp:25:7:25:7 | constructor init of field inner [post-this] | Node should have one type bound but has 0. |
8+
| complex.cpp:25:7:25:7 | constructor init of field inner [pre-this] | Node should have one type bound but has 0. |
79
uniqueTypeRepr
810
| complex.cpp:22:11:22:17 | constructor init of field f [post-this] | Node should have one type representation but has 0. |
911
| complex.cpp:22:11:22:17 | constructor init of field f [pre-this] | Node should have one type representation but has 0. |
12+
| complex.cpp:25:7:25:7 | constructor init of field inner [post-this] | Node should have one type representation but has 0. |
13+
| complex.cpp:25:7:25:7 | constructor init of field inner [pre-this] | Node should have one type representation but has 0. |
1014
uniqueNodeLocation
1115
| A.cpp:38:7:38:8 | call to C | Node should have one location but has 2. |
1216
| A.cpp:39:7:39:8 | call to C | Node should have one location but has 2. |

0 commit comments

Comments
 (0)