diff --git a/cpp/ql/src/Critical/GlobalUseBeforeInit.ql b/cpp/ql/src/Critical/GlobalUseBeforeInit.ql index e9a637bd7d73..946fc755a9fe 100644 --- a/cpp/ql/src/Critical/GlobalUseBeforeInit.ql +++ b/cpp/ql/src/Critical/GlobalUseBeforeInit.ql @@ -21,13 +21,29 @@ predicate initFunc(GlobalVariable v, Function f) { ) } +/** Holds if `v` has an initializer in function `f` that dominates `node`. */ +predicate dominatingInitInFunc(GlobalVariable v, Function f, ControlFlowNode node) { + exists(VariableAccess initAccess | + v.getAnAccess() = initAccess and + initAccess.isUsedAsLValue() and + initAccess.getEnclosingFunction() = f and + dominates(initAccess, node) + ) +} + +predicate safeAccess(VariableAccess access) { + // it is safe if the variable access is part of a `sizeof` expression + exists(SizeofExprOperator e | e.getAChild*() = access) +} + predicate useFunc(GlobalVariable v, Function f) { exists(VariableAccess access | v.getAnAccess() = access and access.isRValue() and - access.getEnclosingFunction() = f - ) and - not initFunc(v, f) + access.getEnclosingFunction() = f and + not safeAccess(access) and + not dominatingInitInFunc(v, f, access) + ) } predicate uninitialisedBefore(GlobalVariable v, Function f) { @@ -38,12 +54,14 @@ predicate uninitialisedBefore(GlobalVariable v, Function f) { exists(Call call, Function g | uninitialisedBefore(v, g) and call.getEnclosingFunction() = g and - (not functionInitialises(f, v) or locallyUninitialisedAt(v, call)) and + (not functionInitialises(g, v) or locallyUninitialisedAt(v, call)) and resolvedCall(call, f) ) } predicate functionInitialises(Function f, GlobalVariable v) { + initFunc(v, f) + or exists(Call call | call.getEnclosingFunction() = f and initialisedBy(v, call) @@ -60,7 +78,8 @@ predicate locallyUninitialisedAt(GlobalVariable v, Call call) { exists(Call mid | locallyUninitialisedAt(v, mid) and not initialisedBy(v, mid) and callPair(mid, call) ) - ) + ) and + not dominatingInitInFunc(v, call.getEnclosingFunction(), call) } predicate initialisedBy(GlobalVariable v, Call call) { diff --git a/cpp/ql/src/change-notes/2025-07-01-global-vars-ubi-query-fixes.md.md b/cpp/ql/src/change-notes/2025-07-01-global-vars-ubi-query-fixes.md.md new file mode 100644 index 000000000000..b5ab2362bf43 --- /dev/null +++ b/cpp/ql/src/change-notes/2025-07-01-global-vars-ubi-query-fixes.md.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* Fixed a number of false positives and false negatives in `cpp/global-use-before-init`. Note that this query is not part of any of the default query suites. diff --git a/cpp/ql/test/query-tests/Critical/GlobalUseBeforeInit/GlobalUseBeforeInit.expected b/cpp/ql/test/query-tests/Critical/GlobalUseBeforeInit/GlobalUseBeforeInit.expected index c7c2d1ffad49..8298707a6b0a 100644 --- a/cpp/ql/test/query-tests/Critical/GlobalUseBeforeInit/GlobalUseBeforeInit.expected +++ b/cpp/ql/test/query-tests/Critical/GlobalUseBeforeInit/GlobalUseBeforeInit.expected @@ -1 +1,2 @@ -| test.cpp:27:5:27:6 | f1 | The variable $@ is used in this function but may not be initialized when it is called. | test.cpp:14:5:14:5 | b | b | +| test.cpp:28:5:28:6 | f1 | The variable $@ is used in this function but may not be initialized when it is called. | test.cpp:14:5:14:5 | b | b | +| test.cpp:39:5:39:8 | main | The variable $@ is used in this function but may not be initialized when it is called. | test.cpp:14:5:14:5 | b | b | diff --git a/cpp/ql/test/query-tests/Critical/GlobalUseBeforeInit/test.cpp b/cpp/ql/test/query-tests/Critical/GlobalUseBeforeInit/test.cpp index fcecf6c5c44a..81883a1a8a16 100644 --- a/cpp/ql/test/query-tests/Critical/GlobalUseBeforeInit/test.cpp +++ b/cpp/ql/test/query-tests/Critical/GlobalUseBeforeInit/test.cpp @@ -12,6 +12,7 @@ int vfprintf (FILE *, const char *, va_list); int a = 1; int b; +int *c; int my_printf(const char * fmt, ...) { @@ -31,8 +32,15 @@ int f1() return 0; } +void f2() { + my_printf("%d\n", b); // GOOD +} + int main() { - int b = f1(); + unsigned size = sizeof(*c); // GOOD + my_printf("%d\n", b); // BAD + b = f1(); + f2(); return 0; -} \ No newline at end of file +}