Skip to content

Commit b83d4e0

Browse files
MathiasVPMathiasVP
authored
Merge pull request #21611 from MathiasVP/nsdmi-dataflow-3
C++: Add dataflow through NSDMI
2 parents 095a9cb + 5db069e commit b83d4e0

File tree

11 files changed

+78
-72
lines changed

11 files changed

+78
-72
lines changed

‎cpp/ql/lib/change-notes/2026-03-30-nsdmi-dataflow.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Added dataflow through members initialized via non-static data member initialization (NSDMI).

‎cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowNodes.qll‎

Lines changed: 32 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -850,11 +850,6 @@ module Public {
850850
{
851851
ThisParameterInstructionNode() { instr.getIRVariable() instanceof IRThisVariable }
852852

853-
override predicate isSourceParameterOf(Function f, ParameterPosition pos) {
854-
pos.(DirectPosition).getArgumentIndex() = -1 and
855-
instr.getEnclosingFunction() = f
856-
}
857-
858853
override string toStringImpl() { result = "this" }
859854
}
860855

@@ -1129,7 +1124,7 @@ class IndirectArgumentOutNode extends PostUpdateNodeImpl {
11291124
/**
11301125
* Gets the `Function` that the call targets, if this is statically known.
11311126
*/
1132-
Function getStaticCallTarget() { result = this.getCallInstruction().getStaticCallTarget() }
1127+
Declaration getStaticCallTarget() { result = this.getCallInstruction().getStaticCallTarget() }
11331128

11341129
override string toStringImpl() {
11351130
exists(string prefix | if indirectionIndex > 0 then prefix = "" else prefix = "pointer to " |
@@ -1633,7 +1628,7 @@ abstract private class AbstractParameterNode extends Node {
16331628
* implicit `this` parameter is considered to have position `-1`, and
16341629
* pointer-indirection parameters are at further negative positions.
16351630
*/
1636-
predicate isSourceParameterOf(Function f, ParameterPosition pos) { none() }
1631+
predicate isSourceParameterOf(Declaration f, ParameterPosition pos) { none() }
16371632

16381633
/**
16391634
* Holds if this node is the parameter of `sc` at the specified position. The
@@ -1659,6 +1654,11 @@ abstract private class AbstractParameterNode extends Node {
16591654

16601655
/** Gets the `Parameter` associated with this node, if it exists. */
16611656
Parameter getParameter() { none() } // overridden by subclasses
1657+
1658+
/**
1659+
* Holds if this node represents an implicit `this` parameter, if it exists.
1660+
*/
1661+
predicate isThis() { none() } // overridden by subclasses
16621662
}
16631663

16641664
abstract private class AbstractIndirectParameterNode extends AbstractParameterNode {
@@ -1687,7 +1687,9 @@ private class IndirectInstructionParameterNode extends AbstractIndirectParameter
16871687
InitializeParameterInstruction init;
16881688

16891689
IndirectInstructionParameterNode() {
1690-
IndirectInstruction.super.hasInstructionAndIndirectionIndex(init, _)
1690+
IndirectInstruction.super.hasInstructionAndIndirectionIndex(init, _) and
1691+
// We don't model catch parameters as parameter nodes
1692+
not exists(init.getParameter().getCatchBlock())
16911693
}
16921694

16931695
int getArgumentIndex() { init.hasIndex(result) }
@@ -1701,16 +1703,17 @@ private class IndirectInstructionParameterNode extends AbstractIndirectParameter
17011703
)
17021704
}
17031705

1704-
/** Gets the parameter whose indirection is initialized. */
17051706
override Parameter getParameter() { result = init.getParameter() }
17061707

1708+
override predicate isThis() { init.hasIndex(-1) }
1709+
17071710
override DataFlowCallable getEnclosingCallable() {
17081711
result.asSourceCallable() = this.getFunction()
17091712
}
17101713

17111714
override Declaration getFunction() { result = init.getEnclosingFunction() }
17121715

1713-
override predicate isSourceParameterOf(Function f, ParameterPosition pos) {
1716+
override predicate isSourceParameterOf(Declaration f, ParameterPosition pos) {
17141717
this.getFunction() = f and
17151718
exists(int argumentIndex, int indirectionIndex |
17161719
indirectPositionHasArgumentIndexAndIndex(pos, argumentIndex, indirectionIndex) and
@@ -1738,6 +1741,18 @@ abstract class InstructionDirectParameterNode extends InstructionNode, AbstractD
17381741
* Gets the `IRVariable` that this parameter references.
17391742
*/
17401743
final IRVariable getIRVariable() { result = instr.getIRVariable() }
1744+
1745+
override predicate isThis() { instr.hasIndex(-1) }
1746+
1747+
override Parameter getParameter() { result = instr.getParameter() }
1748+
1749+
override predicate isSourceParameterOf(Declaration f, ParameterPosition pos) {
1750+
this.getFunction() = f and
1751+
exists(int argumentIndex |
1752+
pos.(DirectPosition).getArgumentIndex() = argumentIndex and
1753+
instr.hasIndex(argumentIndex)
1754+
)
1755+
}
17411756
}
17421757

17431758
abstract private class AbstractExplicitParameterNode extends AbstractDirectParameterNode { }
@@ -1746,15 +1761,12 @@ abstract private class AbstractExplicitParameterNode extends AbstractDirectParam
17461761
private class ExplicitParameterInstructionNode extends AbstractExplicitParameterNode,
17471762
InstructionDirectParameterNode
17481763
{
1749-
ExplicitParameterInstructionNode() { exists(instr.getParameter()) }
1750-
1751-
override predicate isSourceParameterOf(Function f, ParameterPosition pos) {
1752-
f.getParameter(pos.(DirectPosition).getArgumentIndex()) = instr.getParameter()
1764+
ExplicitParameterInstructionNode() {
1765+
// We don't model catch parameters as parameter nodes.
1766+
exists(instr.getParameter().getFunction())
17531767
}
17541768

17551769
override string toStringImpl() { result = instr.getParameter().toString() }
1756-
1757-
override Parameter getParameter() { result = instr.getParameter() }
17581770
}
17591771

17601772
/**
@@ -1782,9 +1794,9 @@ private class DirectBodyLessParameterNode extends AbstractExplicitParameterNode,
17821794
{
17831795
DirectBodyLessParameterNode() { indirectionIndex = 0 }
17841796

1785-
override predicate isSourceParameterOf(Function f, ParameterPosition pos) {
1797+
override predicate isSourceParameterOf(Declaration f, ParameterPosition pos) {
17861798
this.getFunction() = f and
1787-
f.getParameter(pos.(DirectPosition).getArgumentIndex()) = p
1799+
f.(Function).getParameter(pos.(DirectPosition).getArgumentIndex()) = p
17881800
}
17891801

17901802
override Parameter getParameter() { result = p }
@@ -1795,10 +1807,10 @@ private class IndirectBodyLessParameterNode extends AbstractIndirectParameterNod
17951807
{
17961808
IndirectBodyLessParameterNode() { not this instanceof DirectBodyLessParameterNode }
17971809

1798-
override predicate isSourceParameterOf(Function f, ParameterPosition pos) {
1810+
override predicate isSourceParameterOf(Declaration f, ParameterPosition pos) {
17991811
exists(int argumentPosition |
18001812
this.getFunction() = f and
1801-
f.getParameter(argumentPosition) = p and
1813+
f.(Function).getParameter(argumentPosition) = p and
18021814
indirectPositionHasArgumentIndexAndIndex(pos, argumentPosition, indirectionIndex)
18031815
)
18041816
}

‎cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1170,7 +1170,7 @@ class DataFlowCall extends TDataFlowCall {
11701170
/**
11711171
* Gets the `Function` that the call targets, if this is statically known.
11721172
*/
1173-
Function getStaticCallSourceTarget() { none() }
1173+
Declaration getStaticCallSourceTarget() { none() }
11741174

11751175
/**
11761176
* Gets the target of this call. We use the following strategy for deciding
@@ -1182,7 +1182,7 @@ class DataFlowCall extends TDataFlowCall {
11821182
* whether is it manual or generated.
11831183
*/
11841184
final DataFlowCallable getStaticCallTarget() {
1185-
exists(Function target | target = this.getStaticCallSourceTarget() |
1185+
exists(Declaration target | target = this.getStaticCallSourceTarget() |
11861186
// Don't use the source callable if there is a manual model for the
11871187
// target
11881188
not exists(SummarizedCallable sc |
@@ -1242,7 +1242,7 @@ private class NormalCall extends DataFlowCall, TNormalCall {
12421242

12431243
override CallTargetOperand getCallTargetOperand() { result = call.getCallTargetOperand() }
12441244

1245-
override Function getStaticCallSourceTarget() { result = call.getStaticCallTarget() }
1245+
override Declaration getStaticCallSourceTarget() { result = call.getStaticCallTarget() }
12461246

12471247
override ArgumentOperand getArgumentOperand(int index) { result = call.getArgumentOperand(index) }
12481248

‎cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/SsaImplCommon.qll‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,18 @@ private import TypeFlow
1111
private import semmle.code.cpp.ir.ValueNumbering
1212

1313
/**
14-
* Gets the C++ type of `this` in the member function `f`.
14+
* Gets the C++ type of `this` in an `IRFunction` generated from `f`.
1515
* The result is a glvalue if `isGLValue` is true, and
1616
* a prvalue if `isGLValue` is false.
1717
*/
1818
bindingset[isGLValue]
19-
private CppType getThisType(Cpp::MemberFunction f, boolean isGLValue) {
20-
result.hasType(f.getTypeOfThis(), isGLValue)
19+
private CppType getThisType(Cpp::Declaration f, boolean isGLValue) {
20+
result.hasType(f.(Cpp::MemberFunction).getTypeOfThis(), isGLValue)
21+
or
22+
exists(Cpp::PointerType pt |
23+
pt.getBaseType() = f.(Cpp::Field).getDeclaringType() and
24+
result.hasType(pt, isGLValue)
25+
)
2126
}
2227

2328
/**

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -391,6 +391,7 @@ irFlow
391391
| test.cpp:1308:7:1308:12 | call to source | test.cpp:1309:8:1309:16 | ... ++ |
392392
| test.cpp:1312:7:1312:12 | call to source | test.cpp:1313:8:1313:24 | ... ? ... : ... |
393393
| test.cpp:1312:7:1312:12 | call to source | test.cpp:1314:8:1314:8 | x |
394+
| test.cpp:1318:13:1318:18 | call to source | test.cpp:1327:10:1327:10 | i |
394395
| test.cpp:1329:11:1329:16 | call to source | test.cpp:1330:10:1330:10 | i |
395396
| true_upon_entry.cpp:9:11:9:16 | call to source | true_upon_entry.cpp:13:8:13:8 | x |
396397
| true_upon_entry.cpp:17:11:17:16 | call to source | true_upon_entry.cpp:21:8:21:8 | x |

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1324,7 +1324,7 @@ struct nsdmi {
13241324

13251325
void nsdmi_test() {
13261326
nsdmi x;
1327-
sink(x.i); // $ MISSING: ir ast
1327+
sink(x.i); // $ ir MISSING: ast
13281328

13291329
nsdmi y(source());
13301330
sink(y.i); // $ ir ast

‎cpp/ql/test/library-tests/dataflow/dataflow-tests/type-bugs.expected‎

Lines changed: 0 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,47 +1,5 @@
11
astTypeBugs
22
irTypeBugs
3-
| ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | [summary param] *0 in iterator |
4-
| ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | [summary param] this in iterator |
5-
| ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | [summary] read: Argument[*0].Element in iterator |
6-
| ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | [summary] read: Argument[*0].Element[****] in iterator |
7-
| ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | [summary] read: Argument[*0].Element[***] in iterator |
8-
| ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | [summary] read: Argument[*0].Element[**] in iterator |
9-
| ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | [summary] read: Argument[*0].Element[*] in iterator |
10-
| ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | [summary] to write: Argument[this] in iterator |
11-
| ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | [summary] to write: Argument[this].Element in iterator |
12-
| ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | [summary] to write: Argument[this].Element[****] in iterator |
13-
| ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | [summary] to write: Argument[this].Element[***] in iterator |
14-
| ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | [summary] to write: Argument[this].Element[**] in iterator |
15-
| ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | ../../../include/iterator.h:21:3:21:10 | [summary] to write: Argument[this].Element[*] in iterator |
16-
| ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | [summary param] *0 in iterator |
17-
| ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | [summary param] this in iterator |
18-
| ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | [summary] read: Argument[*0].Element in iterator |
19-
| ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | [summary] read: Argument[*0].Element[****] in iterator |
20-
| ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | [summary] read: Argument[*0].Element[***] in iterator |
21-
| ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | [summary] read: Argument[*0].Element[**] in iterator |
22-
| ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | [summary] read: Argument[*0].Element[*] in iterator |
23-
| ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | [summary] to write: Argument[this] in iterator |
24-
| ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | [summary] to write: Argument[this].Element in iterator |
25-
| ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | [summary] to write: Argument[this].Element[****] in iterator |
26-
| ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | [summary] to write: Argument[this].Element[***] in iterator |
27-
| ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | [summary] to write: Argument[this].Element[**] in iterator |
28-
| ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | ../../../include/iterator.h:22:3:22:10 | [summary] to write: Argument[this].Element[*] in iterator |
29-
| ../../../include/iterator.h:30:18:30:26 | ../../../include/iterator.h:30:18:30:26 | ../../../include/iterator.h:30:18:30:26 | [summary param] this in operator* |
30-
| ../../../include/iterator.h:30:18:30:26 | ../../../include/iterator.h:30:18:30:26 | ../../../include/iterator.h:30:18:30:26 | [summary] read: Argument[this].Element in operator* |
31-
| ../../../include/iterator.h:30:18:30:26 | ../../../include/iterator.h:30:18:30:26 | ../../../include/iterator.h:30:18:30:26 | [summary] read: Argument[this].Element[*] in operator* |
32-
| ../../../include/iterator.h:30:18:30:26 | ../../../include/iterator.h:30:18:30:26 | ../../../include/iterator.h:30:18:30:26 | [summary] to write: ReturnValue[**] in operator* |
33-
| ../../../include/iterator.h:30:18:30:26 | ../../../include/iterator.h:30:18:30:26 | ../../../include/iterator.h:30:18:30:26 | [summary] to write: ReturnValue[*] in operator* |
34-
| ../../../include/iterator.h:31:16:31:25 | ../../../include/iterator.h:31:16:31:25 | ../../../include/iterator.h:31:16:31:25 | [summary param] this in operator-> |
35-
| ../../../include/iterator.h:31:16:31:25 | ../../../include/iterator.h:31:16:31:25 | ../../../include/iterator.h:31:16:31:25 | [summary] read: Argument[this].Element in operator-> |
36-
| ../../../include/iterator.h:31:16:31:25 | ../../../include/iterator.h:31:16:31:25 | ../../../include/iterator.h:31:16:31:25 | [summary] read: Argument[this].Element[*] in operator-> |
37-
| ../../../include/iterator.h:31:16:31:25 | ../../../include/iterator.h:31:16:31:25 | ../../../include/iterator.h:31:16:31:25 | [summary] to write: ReturnValue[**] in operator-> |
38-
| ../../../include/iterator.h:31:16:31:25 | ../../../include/iterator.h:31:16:31:25 | ../../../include/iterator.h:31:16:31:25 | [summary] to write: ReturnValue[*] in operator-> |
39-
| test.cpp:356:7:356:11 | test.cpp:356:7:356:11 | test.cpp:356:7:356:11 | field |
40-
| test.cpp:356:7:356:11 | test.cpp:356:7:356:11 | test.cpp:356:7:356:11 | field |
41-
| test.cpp:356:7:356:11 | test.cpp:356:7:356:11 | test.cpp:356:7:356:11 | field |
42-
| test.cpp:1318:9:1318:9 | test.cpp:1318:9:1318:9 | test.cpp:1318:9:1318:9 | i |
43-
| test.cpp:1318:9:1318:9 | test.cpp:1318:9:1318:9 | test.cpp:1318:9:1318:9 | i |
44-
| test.cpp:1318:9:1318:9 | test.cpp:1318:9:1318:9 | test.cpp:1318:9:1318:9 | i |
453
incorrectBaseType
464
| clang.cpp:22:8:22:20 | *& ... | Expected 'Node.getType()' to be int, but it was int * |
475
| clang.cpp:23:17:23:29 | *& ... | Expected 'Node.getType()' to be int, but it was int * |

‎cpp/ql/test/library-tests/dataflow/dataflow-tests/type-bugs.ql‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,13 @@ import AstTest
1717

1818
module IrTest {
1919
private import semmle.code.cpp.ir.dataflow.internal.DataFlowUtil
20+
private import semmle.code.cpp.ir.dataflow.internal.DataFlowNodes
2021

2122
query predicate irTypeBugs(Location location, Node node) {
2223
exists(int n |
24+
// Flow summary nodes don't have a type since we don't necessarily have
25+
// the source code in the database.
26+
not node instanceof FlowSummaryNode and
2327
n = count(node.getType()) and
2428
location = node.getLocation() and
2529
n != 1

‎cpp/ql/test/library-tests/dataflow/fields/C.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ class C
2727
void func()
2828
{
2929
sink(s1); // $ ast,ir
30-
sink(s2); // $ MISSING: ast,ir
30+
sink(s2); // $ ir MISSING: ast
3131
sink(s3); // $ ast,ir
3232
sink(s4); // $ MISSING: ast,ir
3333
}

0 commit comments

Comments
 (0)