Skip to content

C++: Produce a better toString for dataflow nodes with indirections #15107

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.

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

Merged
merged 5 commits into from
Dec 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions cpp/ql/lib/change-notes/2023-12-14-dataflow-tostring.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* Changed the output of `Node.toString` to better reflect how many indirections a given dataflow node has.
86 changes: 63 additions & 23 deletions cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll
Original file line number Diff line number Diff line change
Expand Up @@ -492,10 +492,41 @@ private string toExprString(Node n) {
result = n.asExpr(0).toString()
or
not exists(n.asExpr()) and
result = n.asIndirectExpr(0, 1).toString() + " indirection"
result = stars(n) + n.asIndirectExpr(0, 1).toString()
)
}

private module NodeStars {
private int getNumberOfIndirections(Node n) {
result = n.(RawIndirectOperand).getIndirectionIndex()
or
result = n.(RawIndirectInstruction).getIndirectionIndex()
or
result = n.(VariableNode).getIndirectionIndex()
or
result = n.(PostUpdateNodeImpl).getIndirectionIndex()
or
result = n.(FinalParameterNode).getIndirectionIndex()
}

private int maxNumberOfIndirections() { result = max(getNumberOfIndirections(_)) }

private string repeatStars(int n) {
n = 0 and result = ""
or
n = [1 .. maxNumberOfIndirections()] and
result = "*" + repeatStars(n - 1)
}

/**
* Gets the number of stars (i.e., `*`s) needed to produce the `toString`
* output for `n`.
*/
string stars(Node n) { result = repeatStars(getNumberOfIndirections(n)) }
}

private import NodeStars

/**
* A class that lifts pre-SSA dataflow nodes to regular dataflow nodes.
*/
Expand Down Expand Up @@ -786,10 +817,12 @@ class IndirectParameterNode extends Node instanceof IndirectInstruction {
override Location getLocationImpl() { result = this.getParameter().getLocation() }

override string toStringImpl() {
result = this.getParameter().toString() + " indirection"
or
not exists(this.getParameter()) and
result = "this indirection"
exists(string prefix | prefix = stars(this) |
result = prefix + this.getParameter().toString()
or
not exists(this.getParameter()) and
result = prefix + "this"
)
}
}

Expand Down Expand Up @@ -1016,7 +1049,7 @@ private module RawIndirectNodes {
}

override string toStringImpl() {
result = operandNode(this.getOperand()).toStringImpl() + " indirection"
result = stars(this) + operandNode(this.getOperand()).toStringImpl()
}
}

Expand Down Expand Up @@ -1058,7 +1091,7 @@ private module RawIndirectNodes {
}

override string toStringImpl() {
result = instructionNode(this.getInstruction()).toStringImpl() + " indirection"
result = stars(this) + instructionNode(this.getInstruction()).toStringImpl()
}
}

Expand Down Expand Up @@ -1151,9 +1184,7 @@ class FinalParameterNode extends Node, TFinalParameterNode {
result instanceof UnknownDefaultLocation
}

override string toStringImpl() {
if indirectionIndex > 1 then result = p.toString() + " indirection" else result = p.toString()
}
override string toStringImpl() { result = stars(this) + p.toString() }
}

/**
Expand Down Expand Up @@ -1787,9 +1818,7 @@ class VariableNode extends Node, TVariableNode {
result instanceof UnknownDefaultLocation
}

override string toStringImpl() {
if indirectionIndex = 1 then result = v.toString() else result = v.toString() + " indirection"
}
override string toStringImpl() { result = stars(this) + v.toString() }
}

/**
Expand Down Expand Up @@ -2249,18 +2278,33 @@ class Content extends TContent {
abstract predicate impliesClearOf(Content c);
}

private module ContentStars {
private int maxNumberOfIndirections() { result = max(any(Content c).getIndirectionIndex()) }

private string repeatStars(int n) {
n = 0 and result = ""
or
n = [1 .. maxNumberOfIndirections()] and
result = "*" + repeatStars(n - 1)
}

/**
* Gets the number of stars (i.e., `*`s) needed to produce the `toString`
* output for `c`.
*/
string contentStars(Content c) { result = repeatStars(c.getIndirectionIndex() - 1) }
}

private import ContentStars

/** A reference through a non-union instance field. */
class FieldContent extends Content, TFieldContent {
Field f;
int indirectionIndex;

FieldContent() { this = TFieldContent(f, indirectionIndex) }

override string toString() {
indirectionIndex = 1 and result = f.toString()
or
indirectionIndex > 1 and result = f.toString() + " indirection"
}
override string toString() { result = contentStars(this) + f.toString() }

Field getField() { result = f }

Expand Down Expand Up @@ -2289,11 +2333,7 @@ class UnionContent extends Content, TUnionContent {

UnionContent() { this = TUnionContent(u, bytes, indirectionIndex) }

override string toString() {
indirectionIndex = 1 and result = u.toString()
or
indirectionIndex > 1 and result = u.toString() + " indirection"
}
override string toString() { result = contentStars(this) + u.toString() }

/** Gets a field of the underlying union of this `UnionContent`, if any. */
Field getAField() { result = u.getAField() and getFieldSize(result) = bytes }
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
edges
| test.cpp:22:27:22:30 | argv indirection | test.cpp:29:13:29:20 | filePath indirection |
| test.cpp:22:27:22:30 | **argv | test.cpp:29:13:29:20 | *filePath |
nodes
| test.cpp:22:27:22:30 | argv indirection | semmle.label | argv indirection |
| test.cpp:29:13:29:20 | filePath indirection | semmle.label | filePath indirection |
| test.cpp:22:27:22:30 | **argv | semmle.label | **argv |
| test.cpp:29:13:29:20 | *filePath | semmle.label | *filePath |
subpaths
#select
| test.cpp:29:13:29:20 | filePath indirection | test.cpp:22:27:22:30 | argv indirection | test.cpp:29:13:29:20 | filePath indirection | Using user-supplied data in a `wordexp` command, without disabling command substitution, can make code vulnerable to command injection. |
| test.cpp:29:13:29:20 | *filePath | test.cpp:22:27:22:30 | **argv | test.cpp:29:13:29:20 | *filePath | Using user-supplied data in a `wordexp` command, without disabling command substitution, can make code vulnerable to command injection. |
Original file line number Diff line number Diff line change
@@ -1,87 +1,87 @@
edges
| test.cpp:4:17:4:22 | call to malloc | test.cpp:6:9:6:11 | arr |
| test.cpp:4:17:4:22 | call to malloc | test.cpp:10:9:10:11 | arr |
| test.cpp:19:9:19:16 | mk_array indirection [p] | test.cpp:28:19:28:26 | call to mk_array [p] |
| test.cpp:19:9:19:16 | mk_array indirection [p] | test.cpp:50:18:50:25 | call to mk_array [p] |
| test.cpp:21:5:21:7 | arr indirection [post update] [p] | test.cpp:22:5:22:7 | arr indirection [p] |
| test.cpp:21:5:21:24 | ... = ... | test.cpp:21:5:21:7 | arr indirection [post update] [p] |
| test.cpp:19:9:19:16 | *mk_array [p] | test.cpp:28:19:28:26 | call to mk_array [p] |
| test.cpp:19:9:19:16 | *mk_array [p] | test.cpp:50:18:50:25 | call to mk_array [p] |
| test.cpp:21:5:21:7 | *arr [post update] [p] | test.cpp:22:5:22:7 | *arr [p] |
| test.cpp:21:5:21:24 | ... = ... | test.cpp:21:5:21:7 | *arr [post update] [p] |
| test.cpp:21:13:21:18 | call to malloc | test.cpp:21:5:21:24 | ... = ... |
| test.cpp:22:5:22:7 | arr indirection [p] | test.cpp:19:9:19:16 | mk_array indirection [p] |
| test.cpp:28:19:28:26 | call to mk_array [p] | test.cpp:31:9:31:11 | arr indirection [p] |
| test.cpp:28:19:28:26 | call to mk_array [p] | test.cpp:35:9:35:11 | arr indirection [p] |
| test.cpp:31:9:31:11 | arr indirection [p] | test.cpp:31:13:31:13 | p |
| test.cpp:35:9:35:11 | arr indirection [p] | test.cpp:35:13:35:13 | p |
| test.cpp:39:27:39:29 | arr [p] | test.cpp:41:9:41:11 | arr indirection [p] |
| test.cpp:39:27:39:29 | arr [p] | test.cpp:45:9:45:11 | arr indirection [p] |
| test.cpp:41:9:41:11 | arr indirection [p] | test.cpp:41:13:41:13 | p |
| test.cpp:45:9:45:11 | arr indirection [p] | test.cpp:45:13:45:13 | p |
| test.cpp:22:5:22:7 | *arr [p] | test.cpp:19:9:19:16 | *mk_array [p] |
| test.cpp:28:19:28:26 | call to mk_array [p] | test.cpp:31:9:31:11 | *arr [p] |
| test.cpp:28:19:28:26 | call to mk_array [p] | test.cpp:35:9:35:11 | *arr [p] |
| test.cpp:31:9:31:11 | *arr [p] | test.cpp:31:13:31:13 | p |
| test.cpp:35:9:35:11 | *arr [p] | test.cpp:35:13:35:13 | p |
| test.cpp:39:27:39:29 | arr [p] | test.cpp:41:9:41:11 | *arr [p] |
| test.cpp:39:27:39:29 | arr [p] | test.cpp:45:9:45:11 | *arr [p] |
| test.cpp:41:9:41:11 | *arr [p] | test.cpp:41:13:41:13 | p |
| test.cpp:45:9:45:11 | *arr [p] | test.cpp:45:13:45:13 | p |
| test.cpp:50:18:50:25 | call to mk_array [p] | test.cpp:39:27:39:29 | arr [p] |
| test.cpp:55:5:55:7 | arr indirection [post update] [p] | test.cpp:56:5:56:7 | arr indirection [p] |
| test.cpp:55:5:55:24 | ... = ... | test.cpp:55:5:55:7 | arr indirection [post update] [p] |
| test.cpp:55:5:55:7 | *arr [post update] [p] | test.cpp:56:5:56:7 | *arr [p] |
| test.cpp:55:5:55:24 | ... = ... | test.cpp:55:5:55:7 | *arr [post update] [p] |
| test.cpp:55:13:55:18 | call to malloc | test.cpp:55:5:55:24 | ... = ... |
| test.cpp:56:5:56:7 | arr indirection [p] | test.cpp:59:9:59:11 | arr indirection [p] |
| test.cpp:56:5:56:7 | arr indirection [p] | test.cpp:63:9:63:11 | arr indirection [p] |
| test.cpp:59:9:59:11 | arr indirection [p] | test.cpp:59:13:59:13 | p |
| test.cpp:63:9:63:11 | arr indirection [p] | test.cpp:63:13:63:13 | p |
| test.cpp:67:10:67:19 | mk_array_p indirection [p] | test.cpp:76:20:76:29 | call to mk_array_p indirection [p] |
| test.cpp:67:10:67:19 | mk_array_p indirection [p] | test.cpp:98:18:98:27 | call to mk_array_p indirection [p] |
| test.cpp:69:5:69:7 | arr indirection [post update] [p] | test.cpp:70:5:70:7 | arr indirection [p] |
| test.cpp:69:5:69:25 | ... = ... | test.cpp:69:5:69:7 | arr indirection [post update] [p] |
| test.cpp:56:5:56:7 | *arr [p] | test.cpp:59:9:59:11 | *arr [p] |
| test.cpp:56:5:56:7 | *arr [p] | test.cpp:63:9:63:11 | *arr [p] |
| test.cpp:59:9:59:11 | *arr [p] | test.cpp:59:13:59:13 | p |
| test.cpp:63:9:63:11 | *arr [p] | test.cpp:63:13:63:13 | p |
| test.cpp:67:10:67:19 | **mk_array_p [p] | test.cpp:76:20:76:29 | *call to mk_array_p [p] |
| test.cpp:67:10:67:19 | **mk_array_p [p] | test.cpp:98:18:98:27 | *call to mk_array_p [p] |
| test.cpp:69:5:69:7 | *arr [post update] [p] | test.cpp:70:5:70:7 | *arr [p] |
| test.cpp:69:5:69:25 | ... = ... | test.cpp:69:5:69:7 | *arr [post update] [p] |
| test.cpp:69:14:69:19 | call to malloc | test.cpp:69:5:69:25 | ... = ... |
| test.cpp:70:5:70:7 | arr indirection [p] | test.cpp:67:10:67:19 | mk_array_p indirection [p] |
| test.cpp:76:20:76:29 | call to mk_array_p indirection [p] | test.cpp:79:9:79:11 | arr indirection [p] |
| test.cpp:76:20:76:29 | call to mk_array_p indirection [p] | test.cpp:83:9:83:11 | arr indirection [p] |
| test.cpp:79:9:79:11 | arr indirection [p] | test.cpp:79:14:79:14 | p |
| test.cpp:83:9:83:11 | arr indirection [p] | test.cpp:83:14:83:14 | p |
| test.cpp:87:28:87:30 | arr indirection [p] | test.cpp:89:9:89:11 | arr indirection [p] |
| test.cpp:87:28:87:30 | arr indirection [p] | test.cpp:93:9:93:11 | arr indirection [p] |
| test.cpp:89:9:89:11 | arr indirection [p] | test.cpp:89:14:89:14 | p |
| test.cpp:93:9:93:11 | arr indirection [p] | test.cpp:93:14:93:14 | p |
| test.cpp:98:18:98:27 | call to mk_array_p indirection [p] | test.cpp:87:28:87:30 | arr indirection [p] |
| test.cpp:70:5:70:7 | *arr [p] | test.cpp:67:10:67:19 | **mk_array_p [p] |
| test.cpp:76:20:76:29 | *call to mk_array_p [p] | test.cpp:79:9:79:11 | *arr [p] |
| test.cpp:76:20:76:29 | *call to mk_array_p [p] | test.cpp:83:9:83:11 | *arr [p] |
| test.cpp:79:9:79:11 | *arr [p] | test.cpp:79:14:79:14 | p |
| test.cpp:83:9:83:11 | *arr [p] | test.cpp:83:14:83:14 | p |
| test.cpp:87:28:87:30 | *arr [p] | test.cpp:89:9:89:11 | *arr [p] |
| test.cpp:87:28:87:30 | *arr [p] | test.cpp:93:9:93:11 | *arr [p] |
| test.cpp:89:9:89:11 | *arr [p] | test.cpp:89:14:89:14 | p |
| test.cpp:93:9:93:11 | *arr [p] | test.cpp:93:14:93:14 | p |
| test.cpp:98:18:98:27 | *call to mk_array_p [p] | test.cpp:87:28:87:30 | *arr [p] |
nodes
| test.cpp:4:17:4:22 | call to malloc | semmle.label | call to malloc |
| test.cpp:6:9:6:11 | arr | semmle.label | arr |
| test.cpp:10:9:10:11 | arr | semmle.label | arr |
| test.cpp:19:9:19:16 | mk_array indirection [p] | semmle.label | mk_array indirection [p] |
| test.cpp:21:5:21:7 | arr indirection [post update] [p] | semmle.label | arr indirection [post update] [p] |
| test.cpp:19:9:19:16 | *mk_array [p] | semmle.label | *mk_array [p] |
| test.cpp:21:5:21:7 | *arr [post update] [p] | semmle.label | *arr [post update] [p] |
| test.cpp:21:5:21:24 | ... = ... | semmle.label | ... = ... |
| test.cpp:21:13:21:18 | call to malloc | semmle.label | call to malloc |
| test.cpp:22:5:22:7 | arr indirection [p] | semmle.label | arr indirection [p] |
| test.cpp:22:5:22:7 | *arr [p] | semmle.label | *arr [p] |
| test.cpp:28:19:28:26 | call to mk_array [p] | semmle.label | call to mk_array [p] |
| test.cpp:31:9:31:11 | arr indirection [p] | semmle.label | arr indirection [p] |
| test.cpp:31:9:31:11 | *arr [p] | semmle.label | *arr [p] |
| test.cpp:31:13:31:13 | p | semmle.label | p |
| test.cpp:35:9:35:11 | arr indirection [p] | semmle.label | arr indirection [p] |
| test.cpp:35:9:35:11 | *arr [p] | semmle.label | *arr [p] |
| test.cpp:35:13:35:13 | p | semmle.label | p |
| test.cpp:39:27:39:29 | arr [p] | semmle.label | arr [p] |
| test.cpp:41:9:41:11 | arr indirection [p] | semmle.label | arr indirection [p] |
| test.cpp:41:9:41:11 | *arr [p] | semmle.label | *arr [p] |
| test.cpp:41:13:41:13 | p | semmle.label | p |
| test.cpp:45:9:45:11 | arr indirection [p] | semmle.label | arr indirection [p] |
| test.cpp:45:9:45:11 | *arr [p] | semmle.label | *arr [p] |
| test.cpp:45:13:45:13 | p | semmle.label | p |
| test.cpp:50:18:50:25 | call to mk_array [p] | semmle.label | call to mk_array [p] |
| test.cpp:55:5:55:7 | arr indirection [post update] [p] | semmle.label | arr indirection [post update] [p] |
| test.cpp:55:5:55:7 | *arr [post update] [p] | semmle.label | *arr [post update] [p] |
| test.cpp:55:5:55:24 | ... = ... | semmle.label | ... = ... |
| test.cpp:55:13:55:18 | call to malloc | semmle.label | call to malloc |
| test.cpp:56:5:56:7 | arr indirection [p] | semmle.label | arr indirection [p] |
| test.cpp:59:9:59:11 | arr indirection [p] | semmle.label | arr indirection [p] |
| test.cpp:56:5:56:7 | *arr [p] | semmle.label | *arr [p] |
| test.cpp:59:9:59:11 | *arr [p] | semmle.label | *arr [p] |
| test.cpp:59:13:59:13 | p | semmle.label | p |
| test.cpp:63:9:63:11 | arr indirection [p] | semmle.label | arr indirection [p] |
| test.cpp:63:9:63:11 | *arr [p] | semmle.label | *arr [p] |
| test.cpp:63:13:63:13 | p | semmle.label | p |
| test.cpp:67:10:67:19 | mk_array_p indirection [p] | semmle.label | mk_array_p indirection [p] |
| test.cpp:69:5:69:7 | arr indirection [post update] [p] | semmle.label | arr indirection [post update] [p] |
| test.cpp:67:10:67:19 | **mk_array_p [p] | semmle.label | **mk_array_p [p] |
| test.cpp:69:5:69:7 | *arr [post update] [p] | semmle.label | *arr [post update] [p] |
| test.cpp:69:5:69:25 | ... = ... | semmle.label | ... = ... |
| test.cpp:69:14:69:19 | call to malloc | semmle.label | call to malloc |
| test.cpp:70:5:70:7 | arr indirection [p] | semmle.label | arr indirection [p] |
| test.cpp:76:20:76:29 | call to mk_array_p indirection [p] | semmle.label | call to mk_array_p indirection [p] |
| test.cpp:79:9:79:11 | arr indirection [p] | semmle.label | arr indirection [p] |
| test.cpp:70:5:70:7 | *arr [p] | semmle.label | *arr [p] |
| test.cpp:76:20:76:29 | *call to mk_array_p [p] | semmle.label | *call to mk_array_p [p] |
| test.cpp:79:9:79:11 | *arr [p] | semmle.label | *arr [p] |
| test.cpp:79:14:79:14 | p | semmle.label | p |
| test.cpp:83:9:83:11 | arr indirection [p] | semmle.label | arr indirection [p] |
| test.cpp:83:9:83:11 | *arr [p] | semmle.label | *arr [p] |
| test.cpp:83:14:83:14 | p | semmle.label | p |
| test.cpp:87:28:87:30 | arr indirection [p] | semmle.label | arr indirection [p] |
| test.cpp:89:9:89:11 | arr indirection [p] | semmle.label | arr indirection [p] |
| test.cpp:87:28:87:30 | *arr [p] | semmle.label | *arr [p] |
| test.cpp:89:9:89:11 | *arr [p] | semmle.label | *arr [p] |
| test.cpp:89:14:89:14 | p | semmle.label | p |
| test.cpp:93:9:93:11 | arr indirection [p] | semmle.label | arr indirection [p] |
| test.cpp:93:9:93:11 | *arr [p] | semmle.label | *arr [p] |
| test.cpp:93:14:93:14 | p | semmle.label | p |
| test.cpp:98:18:98:27 | call to mk_array_p indirection [p] | semmle.label | call to mk_array_p indirection [p] |
| test.cpp:98:18:98:27 | *call to mk_array_p [p] | semmle.label | *call to mk_array_p [p] |
subpaths
#select
| test.cpp:10:9:10:11 | arr | test.cpp:4:17:4:22 | call to malloc | test.cpp:10:9:10:11 | arr | Off-by one error allocated at $@ bounded by $@. | test.cpp:4:17:4:22 | call to malloc | call to malloc | test.cpp:4:24:4:27 | size | size |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,10 @@ edges
| test.cpp:136:9:136:16 | ... += ... | test.cpp:138:13:138:15 | arr |
| test.cpp:143:18:143:21 | asdf | test.cpp:134:25:134:27 | arr |
| test.cpp:143:18:143:21 | asdf | test.cpp:143:18:143:21 | asdf |
| test.cpp:146:26:146:26 | p indirection | test.cpp:147:4:147:9 | -- ... |
| test.cpp:146:26:146:26 | *p | test.cpp:147:4:147:9 | -- ... |
| test.cpp:156:12:156:14 | buf | test.cpp:156:12:156:18 | ... + ... |
| test.cpp:156:12:156:18 | ... + ... | test.cpp:158:17:158:18 | & ... indirection |
| test.cpp:158:17:158:18 | & ... indirection | test.cpp:146:26:146:26 | p indirection |
| test.cpp:156:12:156:18 | ... + ... | test.cpp:158:17:158:18 | *& ... |
| test.cpp:158:17:158:18 | *& ... | test.cpp:146:26:146:26 | *p |
| test.cpp:218:23:218:28 | buffer | test.cpp:220:5:220:11 | access to array |
| test.cpp:218:23:218:28 | buffer | test.cpp:221:5:221:11 | access to array |
| test.cpp:229:25:229:29 | array | test.cpp:231:5:231:10 | access to array |
Expand Down Expand Up @@ -121,11 +121,11 @@ nodes
| test.cpp:138:13:138:15 | arr | semmle.label | arr |
| test.cpp:143:18:143:21 | asdf | semmle.label | asdf |
| test.cpp:143:18:143:21 | asdf | semmle.label | asdf |
| test.cpp:146:26:146:26 | p indirection | semmle.label | p indirection |
| test.cpp:146:26:146:26 | *p | semmle.label | *p |
| test.cpp:147:4:147:9 | -- ... | semmle.label | -- ... |
| test.cpp:156:12:156:14 | buf | semmle.label | buf |
| test.cpp:156:12:156:18 | ... + ... | semmle.label | ... + ... |
| test.cpp:158:17:158:18 | & ... indirection | semmle.label | & ... indirection |
| test.cpp:158:17:158:18 | *& ... | semmle.label | *& ... |
| test.cpp:218:23:218:28 | buffer | semmle.label | buffer |
| test.cpp:220:5:220:11 | access to array | semmle.label | access to array |
| test.cpp:221:5:221:11 | access to array | semmle.label | access to array |
Expand Down
Loading