Skip to content

C++: Fix missing global variable flow #20126

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

C++: Fix missing global variable flow #20126

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d5d1498
C++: Add tests with missing flow through globals.
MathiasVP Jul 22, 2025
d7ae9f0
C++: Fix missing flow by also generating final global uses for functi…
MathiasVP Jul 22, 2025
849454c
C++: Accept test changes.
MathiasVP Jul 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,13 @@ private module IndirectInstructions {

import IndirectInstructions

predicate isPostUpdateNodeImpl(Operand operand, int indirectionIndex) {
operand = any(FieldAddress fa).getObjectAddressOperand() and
indirectionIndex = [0 .. Ssa::countIndirectionsForCppType(Ssa::getLanguageType(operand))]
or
Ssa::isModifiableByCall(operand, indirectionIndex)
}

/** Gets the callable in which this node occurs. */
DataFlowCallable nodeGetEnclosingCallable(Node n) { result = n.getEnclosingCallable() }

Expand Down
5 changes: 1 addition & 4 deletions cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,7 @@ private newtype TIRDataFlowNode =
[getMinIndirectionsForType(var.getUnspecifiedType()) .. Ssa::getMaxIndirectionsForType(var.getUnspecifiedType())]
} or
TPostUpdateNodeImpl(Operand operand, int indirectionIndex) {
operand = any(FieldAddress fa).getObjectAddressOperand() and
indirectionIndex = [0 .. Ssa::countIndirectionsForCppType(Ssa::getLanguageType(operand))]
or
Ssa::isModifiableByCall(operand, indirectionIndex)
isPostUpdateNodeImpl(operand, indirectionIndex)
} or
TSsaSynthNode(Ssa::SynthNode n) or
TSsaIteratorNode(IteratorFlow::IteratorFlowNode n) or
Expand Down
9 changes: 8 additions & 1 deletion cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,14 @@ private predicate isGlobalUse(
min(int cand, VariableAddressInstruction vai |
vai.getEnclosingIRFunction() = f and
vai.getAstVariable() = v and
isDef(_, _, _, vai, cand, indirectionIndex)
(
isDef(_, _, _, vai, cand, indirectionIndex)
or
exists(Operand operand |
isUse(_, operand, vai, cand, indirectionIndex) and
isPostUpdateNodeImpl(operand, indirectionIndex)
)
)
|
cand
)
Expand Down
8 changes: 8 additions & 0 deletions cpp/ql/test/library-tests/dataflow/dataflow-tests/dataflow-consistency.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ argHasPostUpdate
| test.cpp:813:19:813:35 | * ... | ArgumentNode is missing PostUpdateNode. |
| test.cpp:848:23:848:25 | rpx | ArgumentNode is missing PostUpdateNode. |
| test.cpp:1093:19:1093:21 | * ... | ArgumentNode is missing PostUpdateNode. |
| test.cpp:1206:19:1206:37 | * ... | ArgumentNode is missing PostUpdateNode. |
| test.cpp:1207:10:1207:28 | * ... | ArgumentNode is missing PostUpdateNode. |
| test.cpp:1224:19:1224:37 | * ... | ArgumentNode is missing PostUpdateNode. |
| test.cpp:1225:10:1225:28 | * ... | ArgumentNode is missing PostUpdateNode. |
postWithInFlow
| BarrierGuard.cpp:49:6:49:6 | x [post update] | PostUpdateNode should not be the target of local flow. |
| BarrierGuard.cpp:60:7:60:7 | x [post update] | PostUpdateNode should not be the target of local flow. |
Expand Down Expand Up @@ -193,6 +197,10 @@ postWithInFlow
| test.cpp:1139:4:1139:7 | data [inner post update] | PostUpdateNode should not be the target of local flow. |
| test.cpp:1153:5:1153:6 | * ... [post update] | PostUpdateNode should not be the target of local flow. |
| test.cpp:1153:6:1153:6 | p [inner post update] | PostUpdateNode should not be the target of local flow. |
| test.cpp:1165:5:1165:6 | * ... [post update] | PostUpdateNode should not be the target of local flow. |
| test.cpp:1165:6:1165:6 | p [inner post update] | PostUpdateNode should not be the target of local flow. |
| test.cpp:1195:5:1195:6 | * ... [post update] | PostUpdateNode should not be the target of local flow. |
| test.cpp:1195:6:1195:6 | p [inner post update] | PostUpdateNode should not be the target of local flow. |
viableImplInCallContextTooLarge
uniqueParameterNodeAtPosition
uniqueParameterNodePosition
Expand Down
19 changes: 19 additions & 0 deletions cpp/ql/test/library-tests/dataflow/dataflow-tests/test-source-sink.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,11 @@ astFlow
| test.cpp:1086:12:1086:12 | a | test.cpp:1088:8:1088:9 | & ... |
| test.cpp:1137:7:1137:10 | data | test.cpp:1140:8:1140:18 | * ... |
| test.cpp:1138:17:1138:22 | call to source | test.cpp:1140:8:1140:18 | * ... |
| test.cpp:1165:10:1165:15 | call to source | test.cpp:1170:19:1170:32 | global_int_ptr |
| test.cpp:1165:10:1165:15 | call to source | test.cpp:1234:19:1234:34 | global_int_array |
| test.cpp:1165:10:1165:15 | call to source | test.cpp:1239:10:1239:26 | * ... |
| test.cpp:1195:10:1195:24 | call to indirect_source | test.cpp:1200:19:1200:36 | global_int_ptr_ptr |
| test.cpp:1195:10:1195:24 | call to indirect_source | test.cpp:1201:10:1201:27 | global_int_ptr_ptr |
| true_upon_entry.cpp:17:11:17:16 | call to source | true_upon_entry.cpp:21:8:21:8 | x |
| true_upon_entry.cpp:27:9:27:14 | call to source | true_upon_entry.cpp:29:8:29:8 | x |
| true_upon_entry.cpp:33:11:33:16 | call to source | true_upon_entry.cpp:39:8:39:8 | x |
Expand Down Expand Up @@ -327,6 +332,20 @@ irFlow
| test.cpp:1117:27:1117:34 | call to source | test.cpp:1117:27:1117:34 | call to source |
| test.cpp:1132:11:1132:16 | call to source | test.cpp:1121:8:1121:8 | x |
| test.cpp:1138:17:1138:22 | call to source | test.cpp:1140:8:1140:18 | * ... |
| test.cpp:1165:10:1165:15 | call to source | test.cpp:1170:19:1170:32 | *global_int_ptr |
| test.cpp:1165:10:1165:15 | call to source | test.cpp:1175:10:1175:24 | * ... |
| test.cpp:1165:10:1165:15 | call to source | test.cpp:1184:19:1184:32 | *global_int_ptr |
| test.cpp:1165:10:1165:15 | call to source | test.cpp:1189:10:1189:24 | * ... |
| test.cpp:1165:10:1165:15 | call to source | test.cpp:1234:19:1234:34 | *global_int_array |
| test.cpp:1165:10:1165:15 | call to source | test.cpp:1239:10:1239:26 | * ... |
| test.cpp:1165:10:1165:15 | call to source | test.cpp:1248:19:1248:34 | *global_int_array |
| test.cpp:1165:10:1165:15 | call to source | test.cpp:1253:10:1253:26 | * ... |
| test.cpp:1195:10:1195:24 | *call to indirect_source | test.cpp:1200:19:1200:36 | **global_int_ptr_ptr |
| test.cpp:1195:10:1195:24 | *call to indirect_source | test.cpp:1206:19:1206:37 | ** ... |
| test.cpp:1195:10:1195:24 | *call to indirect_source | test.cpp:1209:10:1209:29 | * ... |
| test.cpp:1195:10:1195:24 | *call to indirect_source | test.cpp:1218:19:1218:36 | **global_int_ptr_ptr |
| test.cpp:1195:10:1195:24 | *call to indirect_source | test.cpp:1224:19:1224:37 | ** ... |
| test.cpp:1195:10:1195:24 | *call to indirect_source | test.cpp:1227:10:1227:29 | * ... |
| true_upon_entry.cpp:9:11:9:16 | call to source | true_upon_entry.cpp:13:8:13:8 | x |
| true_upon_entry.cpp:17:11:17:16 | call to source | true_upon_entry.cpp:21:8:21:8 | x |
| true_upon_entry.cpp:27:9:27:14 | call to source | true_upon_entry.cpp:29:8:29:8 | x |
Expand Down
99 changes: 98 additions & 1 deletion cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1155,4 +1155,101 @@ namespace conflation_regression {
}
}

int recursion = (sink(recursion), source()); // clean
int recursion = (sink(recursion), source()); // clean


namespace globals_without_explicit_def {
int* global_int_ptr;

void set(int* p) { // $ ast-def=p ir-def=*p
*p = source();
}

void test1() {
set(global_int_ptr);
indirect_sink(global_int_ptr); // $ ir,ast
}

void test2() {
set(global_int_ptr);
sink(*global_int_ptr); // $ ir MISSING: ast
}

void calls_set() {
set(global_int_ptr);
}

void test3() {
calls_set();
indirect_sink(global_int_ptr); // $ ir MISSING: ast
}

void test4() {
calls_set();
sink(*global_int_ptr); // $ ir MISSING: ast
}

int** global_int_ptr_ptr;

void set_indirect(int** p) { // $ ast-def=p ir-def=*p ir-def=**p
*p = indirect_source();
}

void test5() {
set_indirect(global_int_ptr_ptr);
indirect_sink(global_int_ptr_ptr); // $ ir,ast
sink(global_int_ptr_ptr); // $ SPURIOUS: ast
}

void test6() {
set_indirect(global_int_ptr_ptr);
indirect_sink(*global_int_ptr_ptr); // $ ir MISSING: ast
sink(*global_int_ptr_ptr);
indirect_sink(**global_int_ptr_ptr);
sink(**global_int_ptr_ptr); // $ ir
}

void calls_set_indirect() {
set_indirect(global_int_ptr_ptr);
}

void test7() {
calls_set_indirect();
indirect_sink(global_int_ptr_ptr); // $ ir MISSING: ast
sink(global_int_ptr_ptr); // $ MISSING: ast
}

void test8() {
calls_set_indirect();
indirect_sink(*global_int_ptr_ptr); // $ ir MISSING: ast
sink(*global_int_ptr_ptr);
indirect_sink(**global_int_ptr_ptr);
sink(**global_int_ptr_ptr); // $ ir MISSING: ast
}

int global_int_array[10];

void test9() {
set(global_int_array);
indirect_sink(global_int_array); // $ ir,ast
}

void test10() {
set(global_int_array);
sink(*global_int_array); // $ ir,ast
}

void calls_set_array() {
set(global_int_array);
}

void test11() {
calls_set_array();
indirect_sink(global_int_array); // $ ir MISSING: ast
}

void test12() {
calls_set_array();
sink(*global_int_array); // $ ir MISSING: ast
}
}
61 changes: 61 additions & 0 deletions cpp/ql/test/library-tests/dataflow/fields/aliasing.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,4 +204,65 @@ void deep_member_field_arrow(S2 *ps2) {
void deep_member_field_arrow_different_fields(S2 *ps2) {
taint_a_ptr(&ps2->s.m1);
sink(ps2->s.m2);
}


namespace GlobalFieldFlow {
S global_s;
S2 global_s2;

void set_field() {
global_s.m1 = user_input();
}

void read_field() {
sink(global_s.m1); // $ ir MISSING: ast
}

void set_nested_field() {
global_s2.s.m1 = user_input();
}

void read_nested_field() {
sink(global_s2.s.m1); // $ ir MISSING: ast
}

S* global_s_ptr;
S2* global_s2_ptr;

void set_field_ptr() {
global_s_ptr->m1 = user_input();
}

void read_field_ptr() {
sink(global_s_ptr->m1); // $ ir MISSING: ast
}

void set_nested_field_ptr() {
global_s2_ptr->s.m1 = user_input();
}

void read_nested_field_ptr() {
sink(global_s2_ptr->s.m1); // $ ir MISSING: ast
}

S_with_pointer global_s_with_pointer;

void set_field_indirect() {
*global_s_with_pointer.data = user_input();
}

void read_field_indirect() {
sink(*global_s_with_pointer.data); // $ ir MISSING: ast
}

S_with_array global_s_with_array;

void set_field_array() {
*global_s_with_array.data = user_input();
}

void read_field_array() {
sink(*global_s_with_array.data); // $ ir MISSING: ast
}
}
8 changes: 8 additions & 0 deletions cpp/ql/test/library-tests/dataflow/fields/dataflow-consistency.expected
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,14 @@ postWithInFlow
| aliasing.cpp:194:21:194:22 | m1 [inner post update] | PostUpdateNode should not be the target of local flow. |
| aliasing.cpp:200:23:200:24 | m1 [inner post update] | PostUpdateNode should not be the target of local flow. |
| aliasing.cpp:205:23:205:24 | m1 [inner post update] | PostUpdateNode should not be the target of local flow. |
| aliasing.cpp:215:14:215:15 | m1 [post update] | PostUpdateNode should not be the target of local flow. |
| aliasing.cpp:223:17:223:18 | m1 [post update] | PostUpdateNode should not be the target of local flow. |
| aliasing.cpp:234:19:234:20 | m1 [post update] | PostUpdateNode should not be the target of local flow. |
| aliasing.cpp:242:22:242:23 | m1 [post update] | PostUpdateNode should not be the target of local flow. |
| aliasing.cpp:252:5:252:31 | * ... [post update] | PostUpdateNode should not be the target of local flow. |
| aliasing.cpp:252:28:252:31 | data [inner post update] | PostUpdateNode should not be the target of local flow. |
| aliasing.cpp:262:5:262:29 | * ... [post update] | PostUpdateNode should not be the target of local flow. |
| aliasing.cpp:262:26:262:29 | data [inner post update] | PostUpdateNode should not be the target of local flow. |
| arrays.cpp:6:3:6:5 | arr [inner post update] | PostUpdateNode should not be the target of local flow. |
| arrays.cpp:6:3:6:8 | access to array [post update] | PostUpdateNode should not be the target of local flow. |
| arrays.cpp:15:3:15:10 | * ... [post update] | PostUpdateNode should not be the target of local flow. |
Expand Down
Loading