Skip to content

Commit dbcd4d6

Browse files
committed
C++: Remove 'ReferenceToInstruction' from the list of instructions we interpret as a load. This makes use lose a bunch of flow, and we'll restore this flow in the next commit.
1 parent 10bca35 commit dbcd4d6

File tree

10 files changed

+105
-104
lines changed

10 files changed

+105
-104
lines changed

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

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -244,17 +244,6 @@ Instruction getDestinationAddress(Instruction instr) {
244244
]
245245
}
246246

247-
class ReferenceToInstruction extends CopyValueInstruction {
248-
ReferenceToInstruction() {
249-
this.getResultType() instanceof Cpp::ReferenceType and
250-
not this.getUnary().getResultType() instanceof Cpp::ReferenceType
251-
}
252-
253-
Instruction getSourceAddress() { result = getSourceAddressOperand().getDef() }
254-
255-
Operand getSourceAddressOperand() { result = this.getUnaryOperand() }
256-
}
257-
258247
/** Gets the source address of `instr` if it is an instruction that behaves like a `LoadInstruction`. */
259248
Instruction getSourceAddress(Instruction instr) { result = getSourceAddressOperand(instr).getDef() }
260249

@@ -266,11 +255,7 @@ Operand getSourceAddressOperand(Instruction instr) {
266255
result =
267256
[
268257
instr.(LoadInstruction).getSourceAddressOperand(),
269-
instr.(ReadSideEffectInstruction).getArgumentOperand(),
270-
// `ReferenceToInstruction` is really more of an address-of operation,
271-
// but by including it in this list we break out of `flowOutOfAddressStep` at an
272-
// instruction that, at the source level, looks like a use of a variable.
273-
instr.(ReferenceToInstruction).getSourceAddressOperand()
258+
instr.(ReadSideEffectInstruction).getArgumentOperand()
274259
]
275260
}
276261

@@ -295,10 +280,6 @@ Operand getSourceValueOperand(Instruction instr) {
295280
result = instr.(LoadInstruction).getSourceValueOperand()
296281
or
297282
result = instr.(ReadSideEffectInstruction).getSideEffectOperand()
298-
or
299-
// See the comment on the `ReferenceToInstruction` disjunct in `getSourceAddressOperand` for why
300-
// this case is included.
301-
result = instr.(ReferenceToInstruction).getSourceValueOperand()
302283
}
303284

304285
/**

cpp/ql/test/library-tests/dataflow/dataflow-tests/lambdas.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,27 +18,27 @@ void test_lambdas()
1818
sink(a()); // $ ast,ir
1919

2020
auto b = [&] {
21-
sink(t); // $ ast,ir
21+
sink(t); // $ ast MISSING: ir
2222
sink(u);
2323
v = source(); // (v is reference captured)
2424
};
2525
b();
2626
sink(v); // $ MISSING: ast,ir
2727

2828
auto c = [=] {
29-
sink(t); // $ ast,ir
29+
sink(t); // $ ast MISSING: ir
3030
sink(u);
3131
};
3232
c();
3333

3434
auto d = [](int a, int b) {
35-
sink(a); // $ ast,ir
35+
sink(a); // $ ast MISSING: ir
3636
sink(b);
3737
};
3838
d(t, u);
3939

4040
auto e = [](int &a, int &b, int &c) {
41-
sink(a); // $ ast,ir
41+
sink(a); // $ ast MISSING: ir
4242
sink(b);
4343
c = source();
4444
};

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ void local_references(int &source1, int clean1) {
100100
int t = source();
101101
int &ref = t;
102102
t = clean1;
103-
sink(ref); // $ SPURIOUS: ast,ir
103+
sink(ref); // $ SPURIOUS: ast
104104
}
105105

106106
{

cpp/ql/test/library-tests/dataflow/fields/ir-path-flow.expected

Lines changed: 81 additions & 61 deletions
Large diffs are not rendered by default.

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

cpp/ql/test/library-tests/dataflow/taint-tests/string.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -119,20 +119,20 @@ void test_string_constructors_assignments()
119119
void test_range_based_for_loop_string() {
120120
std::string s(source());
121121
for(char c : s) {
122-
sink(c); // $ ast,ir
122+
sink(c); // $ ast MISSING: ir
123123
}
124124

125125
for(std::string::iterator it = s.begin(); it != s.end(); ++it) {
126-
sink(*it); // $ ast,ir
126+
sink(*it); // $ ast MISSING: ir
127127
}
128128

129129
for(char& c : s) {
130-
sink(c); // $ ast,ir
130+
sink(c); // $ ast MISSING: ir
131131
}
132132

133133
const std::string const_s(source());
134134
for(const char& c : const_s) {
135-
sink(c); // $ ast,ir
135+
sink(c); // $ ast MISSING: ir
136136
}
137137
}
138138

cpp/ql/test/library-tests/dataflow/taint-tests/swap1.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ void test_move_assignment_operator()
100100
y = std::move(x);
101101

102102
sink(y.data1); // $ ir ast=95:15 SPURIOUS: ast=93:23
103-
sink(x.data1); // $ ast,ir
103+
sink(x.data1); // $ ast MISSING: ir
104104
}
105105

106106
void test_move_constructor()
@@ -142,7 +142,7 @@ void test_move_assignment_method()
142142
y.move_assign(std::move(x));
143143

144144
sink(y.data1); // $ ir ast=137:15 SPURIOUS: ast=135:23
145-
sink(x.data1); // $ ast,ir
145+
sink(x.data1); // $ ast MISSING: ir
146146
}
147147

148148
}

cpp/ql/test/library-tests/dataflow/taint-tests/swap2.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ void test_move_assignment_operator()
100100
y = std::move(x);
101101

102102
sink(y.data1); // $ ir ast=95:15 SPURIOUS: ast=93:23
103-
sink(x.data1); // $ ast,ir
103+
sink(x.data1); // $ ast MISSING: ir
104104
}
105105

106106
void test_move_constructor()
@@ -142,7 +142,7 @@ void test_move_assignment_method()
142142
y.move_assign(std::move(x));
143143

144144
sink(y.data1); // $ ir ast=137:15 SPURIOUS: ast=135:23
145-
sink(x.data1); // $ ast,ir
145+
sink(x.data1); // $ ast MISSING: ir
146146
}
147147

148148
}

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -233,27 +233,27 @@ void test_lambdas()
233233
sink(a()); // $ ast,ir
234234

235235
auto b = [&] {
236-
sink(t); // $ ast,ir
236+
sink(t); // $ ast MISSING: ir
237237
sink(u); // clean
238238
v = source(); // (v is reference captured)
239239
};
240240
b();
241241
sink(v); // $ MISSING: ast,ir
242242

243243
auto c = [=] {
244-
sink(t); // $ ast,ir
244+
sink(t); // $ ast MISSING: ir
245245
sink(u); // clean
246246
};
247247
c();
248248

249249
auto d = [](int a, int b) {
250-
sink(a); // $ ast,ir
250+
sink(a); // $ ast MISSING: ir
251251
sink(b); // clean
252252
};
253253
d(t, u);
254254

255255
auto e = [](int &a, int &b, int &c) {
256-
sink(a); // $ ast,ir
256+
sink(a); // $ ast MISSING: ir
257257
sink(b); // clean
258258
c = source();
259259
};

cpp/ql/test/library-tests/dataflow/taint-tests/vector.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,20 +17,20 @@ void test_range_based_for_loop_vector(int source1) {
1717
std::vector<int> v(100, source1);
1818

1919
for(int x : v) {
20-
sink(x); // $ ast,ir
20+
sink(x); // $ ast MISSING: ir
2121
}
2222

2323
for(std::vector<int>::iterator it = v.begin(); it != v.end(); ++it) {
24-
sink(*it); // $ ast,ir
24+
sink(*it); // $ ast MISSING: ir
2525
}
2626

2727
for(int& x : v) {
28-
sink(x); // $ ast,ir
28+
sink(x); // $ ast MISSING: ir
2929
}
3030

3131
const std::vector<int> const_v(100, source1);
3232
for(const int& x : const_v) {
33-
sink(x); // $ ast,ir
33+
sink(x); // $ ast MISSING: ir
3434
}
3535
}
3636

0 commit comments

Comments
 (0)