diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll index fa54c9c736a3..e3f30b07dad0 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll @@ -632,20 +632,20 @@ predicate jumpStep(Node n1, Node n2) { v = globalUse.getVariable() and n1.(FinalGlobalValue).getGlobalUse() = globalUse | - globalUse.getIndirectionIndex() = 1 and + globalUse.getIndirection() = 1 and v = n2.asVariable() or - v = n2.asIndirectVariable(globalUse.getIndirectionIndex()) + v = n2.asIndirectVariable(globalUse.getIndirection()) ) or exists(Ssa::GlobalDef globalDef | v = globalDef.getVariable() and n2.(InitialGlobalValue).getGlobalDef() = globalDef | - globalDef.getIndirectionIndex() = 1 and + globalDef.getIndirection() = 1 and v = n1.asVariable() or - v = n1.asIndirectVariable(globalDef.getIndirectionIndex()) + v = n1.asIndirectVariable(globalDef.getIndirection()) ) ) } diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll index b6afadfe0e1b..76741dfc5cc8 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaInternals.qll @@ -113,22 +113,12 @@ private newtype TDefOrUseImpl = TGlobalUse(GlobalLikeVariable v, IRFunction f, int indirectionIndex) { // Represents a final "use" of a global variable to ensure that // the assignment to a global variable isn't ruled out as dead. - exists(VariableAddressInstruction vai, int defIndex | - vai.getEnclosingIRFunction() = f and - vai.getAstVariable() = v and - isDef(_, _, _, vai, _, defIndex) and - indirectionIndex = [0 .. defIndex] + 1 - ) + isGlobalUse(v, f, _, indirectionIndex) } or TGlobalDefImpl(GlobalLikeVariable v, IRFunction f, int indirectionIndex) { // Represents the initial "definition" of a global variable when entering // a function body. - exists(VariableAddressInstruction vai | - vai.getEnclosingIRFunction() = f and - vai.getAstVariable() = v and - isUse(_, _, vai, _, indirectionIndex) and - not isDef(_, _, vai.getAUse(), _, _, _) - ) + isGlobalDefImpl(v, f, _, indirectionIndex) } or TIteratorDef( Operand iteratorDerefAddress, BaseSourceVariableInstruction container, int indirectionIndex @@ -150,6 +140,27 @@ private newtype TDefOrUseImpl = ) } +private predicate isGlobalUse( + GlobalLikeVariable v, IRFunction f, int indirection, int indirectionIndex +) { + exists(VariableAddressInstruction vai | + vai.getEnclosingIRFunction() = f and + vai.getAstVariable() = v and + isDef(_, _, _, vai, indirection, indirectionIndex) + ) +} + +private predicate isGlobalDefImpl( + GlobalLikeVariable v, IRFunction f, int indirection, int indirectionIndex +) { + exists(VariableAddressInstruction vai | + vai.getEnclosingIRFunction() = f and + vai.getAstVariable() = v and + isUse(_, _, vai, indirection, indirectionIndex) and + not isDef(_, _, _, vai, _, indirectionIndex) + ) +} + private predicate unspecifiedTypeIsModifiableAt(Type unspecified, int indirectionIndex) { indirectionIndex = [1 .. getIndirectionForUnspecifiedType(unspecified).getNumberOfIndirections()] and exists(CppType cppType | @@ -438,7 +449,7 @@ class GlobalUse extends UseImpl, TGlobalUse { override FinalGlobalValue getNode() { result.getGlobalUse() = this } - override int getIndirection() { result = ind + 1 } + override int getIndirection() { isGlobalUse(global, f, result, ind) } /** Gets the global variable associated with this use. */ GlobalLikeVariable getVariable() { result = global } @@ -460,7 +471,9 @@ class GlobalUse extends UseImpl, TGlobalUse { ) } - override SourceVariable getSourceVariable() { sourceVariableIsGlobal(result, global, f, ind) } + override SourceVariable getSourceVariable() { + sourceVariableIsGlobal(result, global, f, this.getIndirection()) + } final override Cpp::Location getLocation() { result = f.getLocation() } @@ -501,16 +514,18 @@ class GlobalDefImpl extends DefOrUseImpl, TGlobalDefImpl { /** Gets the global variable associated with this definition. */ override SourceVariable getSourceVariable() { - sourceVariableIsGlobal(result, global, f, indirectionIndex) + sourceVariableIsGlobal(result, global, f, this.getIndirection()) } + int getIndirection() { result = indirectionIndex } + /** * Gets the type of this use after specifiers have been deeply stripped * and typedefs have been resolved. */ Type getUnspecifiedType() { result = global.getUnspecifiedType() } - override string toString() { result = "GlobalDef" } + override string toString() { result = "Def of " + this.getSourceVariable() } override Location getLocation() { result = f.getLocation() } @@ -980,7 +995,7 @@ class GlobalDef extends TGlobalDef, SsaDefOrUse { final override Location getLocation() { result = global.getLocation() } /** Gets a textual representation of this definition. */ - override string toString() { result = "GlobalDef" } + override string toString() { result = global.toString() } /** * Holds if this definition has index `index` in block `block`, and @@ -990,6 +1005,9 @@ class GlobalDef extends TGlobalDef, SsaDefOrUse { global.hasIndexInBlock(block, index, sv) } + /** Gets the indirection index of this definition. */ + int getIndirection() { result = global.getIndirection() } + /** Gets the indirection index of this definition. */ int getIndirectionIndex() { result = global.getIndirectionIndex() } diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/dataflow-consistency.expected b/cpp/ql/test/library-tests/dataflow/dataflow-tests/dataflow-consistency.expected index 01fc86f1a383..da59987d7421 100644 --- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/dataflow-consistency.expected +++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/dataflow-consistency.expected @@ -23,6 +23,7 @@ argHasPostUpdate | lambdas.cpp:38:2:38:2 | d | ArgumentNode is missing PostUpdateNode. | | lambdas.cpp:45:2:45:2 | e | ArgumentNode is missing PostUpdateNode. | | test.cpp:67:29:67:35 | source1 | ArgumentNode is missing PostUpdateNode. | +| test.cpp:813:19:813:35 | * ... | 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. | @@ -136,6 +137,9 @@ postWithInFlow | test.cpp:728:3:728:4 | * ... [post update] | PostUpdateNode should not be the target of local flow. | | test.cpp:728:4:728:4 | p [inner post update] | PostUpdateNode should not be the target of local flow. | | test.cpp:734:41:734:41 | x [inner post update] | PostUpdateNode should not be the target of local flow. | +| test.cpp:808:5:808:21 | * ... [post update] | PostUpdateNode should not be the target of local flow. | +| test.cpp:808:6:808:21 | global_indirect1 [inner post update] | PostUpdateNode should not be the target of local flow. | +| test.cpp:832:5:832:17 | global_direct [post update] | PostUpdateNode should not be the target of local flow. | viableImplInCallContextTooLarge uniqueParameterNodeAtPosition uniqueParameterNodePosition diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp index c5f7ffcf1603..73c9fd28b933 100644 --- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp +++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp @@ -796,4 +796,44 @@ void test() { MyStruct a; intPointerSource(a.content, a.content); indirect_sink(a.content); // $ ast ir +} + +namespace MoreGlobalTests { + int **global_indirect1; + int **global_indirect2; + int **global_direct; + + void set_indirect1() + { + *global_indirect1 = indirect_source(); + } + + void read_indirect1() { + sink(global_indirect1); // clean + indirect_sink(*global_indirect1); // $ ir MISSING: ast + } + + void set_indirect2() + { + **global_indirect2 = source(); + } + + void read_indirect2() { + sink(global_indirect2); // clean + sink(**global_indirect2); // $ ir MISSING: ast + } + + // overload source with a boolean parameter so + // that we can define a variant that return an int**. + int** source(bool); + + void set_direct() + { + global_direct = source(true); + } + + void read_direct() { + sink(global_direct); // $ ir MISSING: ast + indirect_sink(global_direct); // clean + } } \ No newline at end of file diff --git a/cpp/ql/test/library-tests/syntax-zoo/dataflow-ir-consistency.expected b/cpp/ql/test/library-tests/syntax-zoo/dataflow-ir-consistency.expected index 0065fe2648fc..4bb6a9e27363 100644 --- a/cpp/ql/test/library-tests/syntax-zoo/dataflow-ir-consistency.expected +++ b/cpp/ql/test/library-tests/syntax-zoo/dataflow-ir-consistency.expected @@ -4,7 +4,12 @@ uniqueType uniqueNodeLocation missingLocation uniqueNodeToString -| cpp11.cpp:50:15:50:16 | (no string representation) | Node should have one toString but has 0. | +| builtin.c:5:5:5:11 | (no string representation) | Node should have one toString but has 0. | +| misc.c:227:7:227:28 | (no string representation) | Node should have one toString but has 0. | +| static_init_templates.cpp:80:18:80:23 | (no string representation) | Node should have one toString but has 0. | +| static_init_templates.cpp:80:18:80:23 | (no string representation) | Node should have one toString but has 0. | +| static_init_templates.cpp:89:18:89:23 | (no string representation) | Node should have one toString but has 0. | +| static_init_templates.cpp:89:18:89:23 | (no string representation) | Node should have one toString but has 0. | parameterCallable localFlowIsLocal readStepIsLocal