Skip to content

Commit c1c1f60

Browse files
committed
C++: Delete incorrect comment and add a bunch of barrier guard tests.
1 parent 04ce405 commit c1c1f60

File tree

3 files changed

+59
-23
lines changed

3 files changed

+59
-23
lines changed

cpp/ql/lib/semmle/code/cpp/controlflow/IRGuards.qll

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -342,25 +342,7 @@ module GuardsInput implements SharedGuards::InputSig<Cpp::Location, Instruction,
342342
/** Gets an expression returned from this function. */
343343
GuardsInput::Expr getAReturnExpr() {
344344
exists(StoreInstruction store |
345-
// We use the `Store` instruction that writes the return value instead of the
346-
// `ReturnValue` instruction since the `ReturnValue` instruction is not always
347-
// dominated by certain guards. For example:
348-
// ```
349-
// if(b) {
350-
// return true;
351-
// } else {
352-
// return false;
353-
// }
354-
// ```
355-
// this will be translated into IR like:
356-
// ```
357-
// if(b) {
358-
// x = true;
359-
// } else {
360-
// x = false;
361-
// }
362-
// return x;
363-
// ```
345+
// A write to the `IRVariable` which represents the return value.
364346
store.getDestinationAddress().(VariableAddressInstruction).getIRVariable() instanceof
365347
IRReturnVariable and
366348
store.getEnclosingFunction() = this and

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

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,32 @@ bool guarded_wrapper(int x) {
135135
}
136136
}
137137

138+
bool guarded_wrapper_2(int x) {
139+
bool b;
140+
if(guarded(x)) {
141+
b = true;
142+
} else {
143+
b = false;
144+
}
145+
return b;
146+
}
147+
148+
bool guarded_wrapper_3(int x) {
149+
bool b = false;
150+
if(guarded(x)) {
151+
b = true;
152+
}
153+
return b;
154+
}
155+
156+
bool guarded_wrapper_4(int x) {
157+
bool b = false;
158+
if(guarded(x)) {
159+
return true;
160+
}
161+
return b;
162+
}
163+
138164
void test_guarded_wrapper() {
139165
int x = source();
140166

@@ -143,4 +169,23 @@ void test_guarded_wrapper() {
143169
} else {
144170
sink(x); // $ ast,ir
145171
}
146-
}
172+
173+
if(guarded_wrapper_2(x)) {
174+
sink(x); // $ SPURIOUS: ast
175+
} else {
176+
sink(x); // $ ast,ir
177+
}
178+
179+
if(guarded_wrapper_3(x)) {
180+
sink(x); // $ SPURIOUS: ast
181+
} else {
182+
sink(x); // $ ast,ir
183+
}
184+
185+
if(guarded_wrapper_4(x)) {
186+
sink(x); // $ SPURIOUS: ast
187+
} else {
188+
sink(x); // $ ast,ir
189+
}
190+
}
191+

cpp/ql/test/library-tests/dataflow/dataflow-tests/test-source-sink.expected

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,14 @@ astFlow
1616
| BarrierGuard.cpp:90:11:90:16 | call to source | BarrierGuard.cpp:101:8:101:8 | x |
1717
| BarrierGuard.cpp:107:11:107:16 | call to source | BarrierGuard.cpp:112:8:112:8 | x |
1818
| BarrierGuard.cpp:116:11:116:16 | call to source | BarrierGuard.cpp:127:8:127:8 | x |
19-
| BarrierGuard.cpp:139:11:139:16 | call to source | BarrierGuard.cpp:142:10:142:10 | x |
20-
| BarrierGuard.cpp:139:11:139:16 | call to source | BarrierGuard.cpp:144:10:144:10 | x |
19+
| BarrierGuard.cpp:165:11:165:16 | call to source | BarrierGuard.cpp:168:10:168:10 | x |
20+
| BarrierGuard.cpp:165:11:165:16 | call to source | BarrierGuard.cpp:170:10:170:10 | x |
21+
| BarrierGuard.cpp:165:11:165:16 | call to source | BarrierGuard.cpp:174:10:174:10 | x |
22+
| BarrierGuard.cpp:165:11:165:16 | call to source | BarrierGuard.cpp:176:10:176:10 | x |
23+
| BarrierGuard.cpp:165:11:165:16 | call to source | BarrierGuard.cpp:180:10:180:10 | x |
24+
| BarrierGuard.cpp:165:11:165:16 | call to source | BarrierGuard.cpp:182:10:182:10 | x |
25+
| BarrierGuard.cpp:165:11:165:16 | call to source | BarrierGuard.cpp:186:10:186:10 | x |
26+
| BarrierGuard.cpp:165:11:165:16 | call to source | BarrierGuard.cpp:188:10:188:10 | x |
2127
| acrossLinkTargets.cpp:19:27:19:32 | call to source | acrossLinkTargets.cpp:12:8:12:8 | x |
2228
| clang.cpp:12:9:12:20 | sourceArray1 | clang.cpp:18:8:18:19 | sourceArray1 |
2329
| clang.cpp:12:9:12:20 | sourceArray1 | clang.cpp:22:8:22:20 | & ... |
@@ -158,7 +164,10 @@ irFlow
158164
| BarrierGuard.cpp:49:10:49:15 | call to source | BarrierGuard.cpp:55:13:55:13 | x |
159165
| BarrierGuard.cpp:60:11:60:16 | call to source | BarrierGuard.cpp:64:14:64:14 | x |
160166
| BarrierGuard.cpp:60:11:60:16 | call to source | BarrierGuard.cpp:66:14:66:14 | x |
161-
| BarrierGuard.cpp:139:11:139:16 | call to source | BarrierGuard.cpp:144:10:144:10 | x |
167+
| BarrierGuard.cpp:165:11:165:16 | call to source | BarrierGuard.cpp:170:10:170:10 | x |
168+
| BarrierGuard.cpp:165:11:165:16 | call to source | BarrierGuard.cpp:176:10:176:10 | x |
169+
| BarrierGuard.cpp:165:11:165:16 | call to source | BarrierGuard.cpp:182:10:182:10 | x |
170+
| BarrierGuard.cpp:165:11:165:16 | call to source | BarrierGuard.cpp:188:10:188:10 | x |
162171
| acrossLinkTargets.cpp:19:27:19:32 | call to source | acrossLinkTargets.cpp:12:8:12:8 | x |
163172
| clang.cpp:12:9:12:20 | sourceArray1 | clang.cpp:18:8:18:19 | sourceArray1 |
164173
| clang.cpp:12:9:12:20 | sourceArray1 | clang.cpp:23:17:23:29 | *& ... |

0 commit comments

Comments
 (0)