Skip to content

Commit 5bd7589

Browse files
committed
C++: Fix spurious flow though and accept test changes.
1 parent faf9fd6 commit 5bd7589

File tree

3 files changed

+35
-3
lines changed

3 files changed

+35
-3
lines changed

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

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,29 @@ private newtype TIRDataFlowNode =
5252
TFinalParameterNode(Parameter p, int indirectionIndex) {
5353
exists(Ssa::FinalParameterUse use |
5454
use.getParameter() = p and
55-
use.getIndirectionIndex() = indirectionIndex
55+
use.getIndirectionIndex() = indirectionIndex and
56+
parameterIsDefined(p)
5657
)
5758
} or
5859
TFinalGlobalValue(Ssa::GlobalUse globalUse) or
5960
TInitialGlobalValue(Ssa::GlobalDef globalUse)
6061

62+
/**
63+
* Holds if the value of `*p` (or `**p`, `***p`, etc.) is redefined somewhere in the body
64+
* of the enclosing function of `p`.
65+
*
66+
* Only parameters satisfying this predicate will generate a `FinalParameterNode` transferring
67+
* flow out of the function.
68+
*/
69+
private predicate parameterIsDefined(Parameter p) {
70+
exists(Ssa::Def def |
71+
def.getSourceVariable().getBaseVariable().(Ssa::BaseIRVariable).getIRVariable().getAst() = p and
72+
def.getIndirectionIndex() = 0 and
73+
def.getIndirection() > 1 and
74+
not def.getValue().asInstruction() instanceof InitializeParameterInstruction
75+
)
76+
}
77+
6178
/**
6279
* An operand that is defined by a `FieldAddressInstruction`.
6380
*/

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -918,6 +918,8 @@ class Def extends DefOrUse {
918918
Instruction getAddress() { result = this.getAddressOperand().getDef() }
919919

920920
/**
921+
* Gets the indirection index of this definition.
922+
*
921923
* This predicate ensures that joins go from `defOrUse` to the result
922924
* instead of the other way around.
923925
*/
@@ -926,6 +928,19 @@ class Def extends DefOrUse {
926928
pragma[only_bind_into](result) = pragma[only_bind_out](defOrUse).getIndirectionIndex()
927929
}
928930

931+
/**
932+
* Gets the indirection level that this definition is writing to.
933+
* For instance, `x = y` is a definition of `x` at indirection level 1 and
934+
* `*x = y` is a definition of `x` at indirection level 2.
935+
*
936+
* This predicate ensures that joins go from `defOrUse` to the result
937+
* instead of the other way around.
938+
*/
939+
pragma[inline]
940+
int getIndirection() {
941+
pragma[only_bind_into](result) = pragma[only_bind_out](defOrUse).getIndirection()
942+
}
943+
929944
Node0Impl getValue() { result = defOrUse.getValue() }
930945

931946
predicate isCertain() { defOrUse.isCertain() }

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -579,13 +579,13 @@ namespace IndirectFlowThroughGlobals {
579579
}
580580
}
581581

582-
void write_to_param(int* x) { // $ ast-def=x ir-def=*x
582+
void write_to_param(int* x) { // $ ast-def=x
583583
int s = source();
584584
x = &s;
585585
}
586586

587587
void test_write_to_param() {
588588
int x = 0;
589589
write_to_param(&x);
590-
sink(x); // $ SPURIOUS: ast,ir
590+
sink(x); // $ SPURIOUS: ast
591591
}

0 commit comments

Comments
 (0)