hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-30 03:05:15 +02:00
Code Issues Packages Projects Releases Wiki Activity

C++: Fix reverse read dataflow consistency failure and accept tests

Browse Source
This commit is contained in:
Mathias Vorreiter Pedersen
2020-04-06 15:50:08 +02:00
parent 3aa293210d
commit c577541850
8 changed files with 68 additions and 32 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll
View File

@@ -148,10 +148,10 @@ predicate storeStep(Node node1, Content f, PostUpdateNode node2) {
*/
predicate readStep(Node node1, Content f, Node node2) {
exists(FieldAddressInstruction fa, LoadInstruction load |
fa.getField() = f.(FieldContent).getField() and
node1.asInstruction() = load and
load.getSourceAddress() = fa and
node2.asInstruction().getAnOperand().getAnyDef() = load
node1.asInstruction() = load.getSourceValueOperand().getAnyDef() and
fa.getField() = f.(FieldContent).getField() and
load = node2.asInstruction()
)
}

22
cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll
View File

@@ -257,9 +257,29 @@ private class ExplicitFieldStoreQualifierNode extends PartialDefinitionNode {
)
}
// There might be multiple `ChiInstructions` that has a particular instruction as
// the total operand - so this definition give consistency errors in
// DataFlowImplConsistency::Consistency. However, it's not clear what (if any) implications
// this consistency failure has.
override Node getPreUpdateNode() { result.asInstruction() = instr.getTotal() }
}
private class ExplicitSingleFieldStoreQualifierNode extends PartialDefinitionNode {
override StoreInstruction instr;
FieldAddressInstruction field;
ExplicitSingleFieldStoreQualifierNode() {
field = instr.getDestinationAddress() and
not exists(ChiInstruction chi | chi.getPartial() = instr)
}
// Since there is no Chi instruction with a total operand for us to use we let the pre update node
// be the address of the object containing the field.
// Note that, unlike in the case where a struct has multiple fields (and thus has a `Chi`
// instruction), the pre update node will be an instruction with a register result.
override Node getPreUpdateNode() { result.asInstruction() = field.getObjectAddress() }
}
/**
* A node that represents the value of a variable after a function call that
* may have changed the variable because it's passed by reference.
@@ -281,6 +301,8 @@ class DefinitionByReferenceNode extends PartialDefinitionNode {
call = write.getPrimaryInstruction()
}
// See the comment on ExplicitFieldStoreQualifierNode::getPreUpdateNode for comments on why
// this causes failures in DataFlowImplConsistency::Consistency.
override Node getPreUpdateNode() { result.asInstruction() = instr.getTotal() }
/** Gets the argument corresponding to this node. */

3
cpp/ql/test/library-tests/dataflow/dataflow-tests/dataflow-ir-consistency.expected
View File

@@ -30,6 +30,9 @@ localCallNodes
postIsNotPre
postHasUniquePre
uniquePostUpdate
| ref.cpp:83:5:83:17 | Chi | Node has multiple PostUpdateNodes. |
| ref.cpp:100:34:100:36 | InitializeIndirection | Node has multiple PostUpdateNodes. |
| ref.cpp:109:5:109:22 | Chi | Node has multiple PostUpdateNodes. |
postIsInSameCallable
reverseRead
storeIsPostUpdate

8
cpp/ql/test/library-tests/dataflow/dataflow-tests/test_diff.expected
View File

@@ -2,6 +2,7 @@
| BarrierGuard.cpp:60:11:60:16 | BarrierGuard.cpp:62:14:62:14 | AST only |
| clang.cpp:12:9:12:20 | clang.cpp:22:8:22:20 | AST only |
| clang.cpp:28:27:28:32 | clang.cpp:30:27:30:34 | AST only |
| clang.cpp:28:27:28:32 | clang.cpp:31:27:31:28 | IR only |
| clang.cpp:39:42:39:47 | clang.cpp:41:18:41:19 | IR only |
| dispatch.cpp:16:37:16:42 | dispatch.cpp:32:16:32:24 | IR only |
| dispatch.cpp:16:37:16:42 | dispatch.cpp:40:15:40:23 | IR only |
@@ -31,6 +32,10 @@
| ref.cpp:53:17:53:18 | ref.cpp:62:10:62:11 | AST only |
| ref.cpp:53:21:53:22 | ref.cpp:65:10:65:11 | AST only |
| ref.cpp:55:23:55:28 | ref.cpp:56:10:56:11 | AST only |
| ref.cpp:94:15:94:20 | ref.cpp:129:13:129:15 | AST only |
| ref.cpp:109:15:109:20 | ref.cpp:132:13:132:15 | AST only |
| ref.cpp:122:23:122:28 | ref.cpp:123:13:123:15 | AST only |
| ref.cpp:125:19:125:24 | ref.cpp:126:13:126:15 | AST only |
| test.cpp:75:7:75:8 | test.cpp:76:8:76:9 | AST only |
| test.cpp:83:7:83:8 | test.cpp:84:8:84:18 | AST only |
| test.cpp:83:7:83:8 | test.cpp:86:8:86:9 | AST only |
@@ -41,9 +46,6 @@
| test.cpp:359:13:359:18 | test.cpp:365:10:365:14 | AST only |
| test.cpp:373:13:373:18 | test.cpp:369:10:369:14 | AST only |
| test.cpp:373:13:373:18 | test.cpp:375:10:375:14 | AST only |
| test.cpp:382:48:382:54 | test.cpp:385:8:385:10 | AST only |
| test.cpp:388:53:388:59 | test.cpp:392:8:392:10 | AST only |
| test.cpp:388:53:388:59 | test.cpp:394:10:394:12 | AST only |
| test.cpp:399:7:399:9 | test.cpp:401:8:401:10 | AST only |
| test.cpp:405:7:405:9 | test.cpp:408:8:408:10 | AST only |
| test.cpp:416:7:416:11 | test.cpp:418:8:418:12 | AST only |

8
cpp/ql/test/library-tests/dataflow/dataflow-tests/test_ir.expected
View File

@@ -12,6 +12,7 @@
| clang.cpp:18:8:18:19 | (const int *)... | clang.cpp:12:9:12:20 | sourceArray1 |
| clang.cpp:18:8:18:19 | sourceArray1 | clang.cpp:12:9:12:20 | sourceArray1 |
| clang.cpp:29:27:29:28 | m1 | clang.cpp:28:27:28:32 | call to source |
| clang.cpp:31:27:31:28 | m2 | clang.cpp:28:27:28:32 | call to source |
| clang.cpp:37:10:37:11 | m2 | clang.cpp:34:32:34:37 | call to source |
| clang.cpp:41:18:41:19 | m2 | clang.cpp:39:42:39:47 | call to source |
| clang.cpp:45:17:45:18 | m2 | clang.cpp:43:35:43:40 | call to source |
@@ -39,10 +40,6 @@
| globals.cpp:12:10:12:24 | flowTestGlobal1 | globals.cpp:13:23:13:28 | call to source |
| globals.cpp:19:10:19:24 | flowTestGlobal2 | globals.cpp:23:23:23:28 | call to source |
| lambdas.cpp:35:8:35:8 | a | lambdas.cpp:8:10:8:15 | call to source |
| ref.cpp:123:13:123:15 | val | ref.cpp:122:23:122:28 | call to source |
| ref.cpp:126:13:126:15 | val | ref.cpp:125:19:125:24 | call to source |
| ref.cpp:129:13:129:15 | val | ref.cpp:94:15:94:20 | call to source |
| ref.cpp:132:13:132:15 | val | ref.cpp:109:15:109:20 | call to source |
| test.cpp:7:8:7:9 | t1 | test.cpp:6:12:6:17 | call to source |
| test.cpp:9:8:9:9 | t1 | test.cpp:6:12:6:17 | call to source |
| test.cpp:10:8:10:9 | t2 | test.cpp:6:12:6:17 | call to source |
@@ -65,6 +62,9 @@
| test.cpp:266:12:266:12 | x | test.cpp:265:22:265:27 | call to source |
| test.cpp:289:14:289:14 | x | test.cpp:305:17:305:22 | call to source |
| test.cpp:318:7:318:7 | x | test.cpp:314:4:314:9 | call to source |
| test.cpp:385:8:385:10 | tmp | test.cpp:382:48:382:54 | source1 |
| test.cpp:392:8:392:10 | tmp | test.cpp:388:53:388:59 | source1 |
| test.cpp:394:10:394:12 | tmp | test.cpp:388:53:388:59 | source1 |
| test.cpp:450:9:450:22 | (statement expression) | test.cpp:449:26:449:32 | source1 |
| test.cpp:461:8:461:12 | local | test.cpp:449:26:449:32 | source1 |
| true_upon_entry.cpp:13:8:13:8 | x | true_upon_entry.cpp:9:11:9:16 | call to source |

21
cpp/ql/test/library-tests/dataflow/fields/ir-flow.expected
View File

@@ -1,24 +1,45 @@
edges
| aliasing.cpp:37:3:37:24 | Store : void | aliasing.cpp:38:11:38:12 | m1 |
| aliasing.cpp:37:13:37:22 | call to user_input : void | aliasing.cpp:37:3:37:24 | Store : void |
| aliasing.cpp:37:13:37:22 | call to user_input : void | aliasing.cpp:38:11:38:12 | m1 |
| aliasing.cpp:42:3:42:22 | Store : void | aliasing.cpp:43:13:43:14 | m1 |
| aliasing.cpp:42:11:42:20 | call to user_input : void | aliasing.cpp:42:3:42:22 | Store : void |
| aliasing.cpp:42:11:42:20 | call to user_input : void | aliasing.cpp:43:13:43:14 | m1 |
| aliasing.cpp:79:3:79:22 | Store : void | aliasing.cpp:80:12:80:13 | m1 |
| aliasing.cpp:79:11:79:20 | call to user_input : void | aliasing.cpp:79:3:79:22 | Store : void |
| aliasing.cpp:79:11:79:20 | call to user_input : void | aliasing.cpp:80:12:80:13 | m1 |
| aliasing.cpp:86:3:86:21 | Store : void | aliasing.cpp:87:12:87:13 | m1 |
| aliasing.cpp:86:10:86:19 | call to user_input : void | aliasing.cpp:86:3:86:21 | Store : void |
| aliasing.cpp:86:10:86:19 | call to user_input : void | aliasing.cpp:87:12:87:13 | m1 |
| aliasing.cpp:92:3:92:23 | Store : void | aliasing.cpp:93:12:93:13 | m1 |
| aliasing.cpp:92:12:92:21 | call to user_input : void | aliasing.cpp:92:3:92:23 | Store : void |
| aliasing.cpp:92:12:92:21 | call to user_input : void | aliasing.cpp:93:12:93:13 | m1 |
| struct_init.c:20:20:20:29 | Store : void | struct_init.c:22:11:22:11 | a |
| struct_init.c:20:20:20:29 | call to user_input : void | struct_init.c:20:20:20:29 | Store : void |
| struct_init.c:20:20:20:29 | call to user_input : void | struct_init.c:22:11:22:11 | a |
| struct_init.c:27:7:27:16 | Store : void | struct_init.c:31:23:31:23 | a |
| struct_init.c:27:7:27:16 | call to user_input : void | struct_init.c:27:7:27:16 | Store : void |
| struct_init.c:27:7:27:16 | call to user_input : void | struct_init.c:31:23:31:23 | a |
nodes
| aliasing.cpp:37:3:37:24 | Store : void | semmle.label | Store : void |
| aliasing.cpp:37:13:37:22 | call to user_input : void | semmle.label | call to user_input : void |
| aliasing.cpp:38:11:38:12 | m1 | semmle.label | m1 |
| aliasing.cpp:42:3:42:22 | Store : void | semmle.label | Store : void |
| aliasing.cpp:42:11:42:20 | call to user_input : void | semmle.label | call to user_input : void |
| aliasing.cpp:43:13:43:14 | m1 | semmle.label | m1 |
| aliasing.cpp:79:3:79:22 | Store : void | semmle.label | Store : void |
| aliasing.cpp:79:11:79:20 | call to user_input : void | semmle.label | call to user_input : void |
| aliasing.cpp:80:12:80:13 | m1 | semmle.label | m1 |
| aliasing.cpp:86:3:86:21 | Store : void | semmle.label | Store : void |
| aliasing.cpp:86:10:86:19 | call to user_input : void | semmle.label | call to user_input : void |
| aliasing.cpp:87:12:87:13 | m1 | semmle.label | m1 |
| aliasing.cpp:92:3:92:23 | Store : void | semmle.label | Store : void |
| aliasing.cpp:92:12:92:21 | call to user_input : void | semmle.label | call to user_input : void |
| aliasing.cpp:93:12:93:13 | m1 | semmle.label | m1 |
| struct_init.c:20:20:20:29 | Store : void | semmle.label | Store : void |
| struct_init.c:20:20:20:29 | call to user_input : void | semmle.label | call to user_input : void |
| struct_init.c:22:11:22:11 | a | semmle.label | a |
| struct_init.c:27:7:27:16 | Store : void | semmle.label | Store : void |
| struct_init.c:27:7:27:16 | call to user_input : void | semmle.label | call to user_input : void |
| struct_init.c:31:23:31:23 | a | semmle.label | a |
#select

16
cpp/ql/test/library-tests/dataflow/taint-tests/test_diff.expected
View File

@@ -11,25 +11,21 @@
| taint.cpp:41:7:41:13 | taint.cpp:35:12:35:17 | AST only |
| taint.cpp:42:7:42:13 | taint.cpp:35:12:35:17 | AST only |
| taint.cpp:43:7:43:13 | taint.cpp:37:22:37:27 | AST only |
| taint.cpp:88:11:88:11 | taint.cpp:77:7:77:12 | IR only |
| taint.cpp:89:11:89:11 | taint.cpp:71:22:71:27 | AST only |
| taint.cpp:89:11:89:11 | taint.cpp:77:7:77:12 | IR only |
| taint.cpp:90:11:90:11 | taint.cpp:72:7:72:12 | AST only |
| taint.cpp:90:11:90:11 | taint.cpp:77:7:77:12 | IR only |
| taint.cpp:92:11:92:11 | taint.cpp:71:22:71:27 | IR only |
| taint.cpp:92:11:92:11 | taint.cpp:72:7:72:12 | IR only |
| taint.cpp:93:11:93:11 | taint.cpp:72:7:72:12 | IR only |
| taint.cpp:94:11:94:11 | taint.cpp:71:22:71:27 | IR only |
| taint.cpp:95:11:95:11 | taint.cpp:71:22:71:27 | IR only |
| taint.cpp:95:11:95:11 | taint.cpp:72:7:72:12 | IR only |
| taint.cpp:91:11:91:11 | taint.cpp:77:7:77:12 | AST only |
| taint.cpp:93:11:93:11 | taint.cpp:71:22:71:27 | AST only |
| taint.cpp:94:11:94:11 | taint.cpp:72:7:72:12 | AST only |
| taint.cpp:109:7:109:13 | taint.cpp:105:12:105:17 | IR only |
| taint.cpp:110:7:110:13 | taint.cpp:105:12:105:17 | IR only |
| taint.cpp:111:7:111:13 | taint.cpp:106:12:106:17 | IR only |
| taint.cpp:112:7:112:13 | taint.cpp:106:12:106:17 | IR only |
| taint.cpp:130:7:130:9 | taint.cpp:127:8:127:13 | IR only |
| taint.cpp:137:7:137:9 | taint.cpp:120:11:120:16 | AST only |
| taint.cpp:173:8:173:13 | taint.cpp:164:19:164:24 | AST only |
| taint.cpp:181:8:181:9 | taint.cpp:185:11:185:16 | AST only |
| taint.cpp:195:7:195:7 | taint.cpp:192:23:192:28 | AST only |
| taint.cpp:195:7:195:7 | taint.cpp:193:6:193:6 | AST only |
| taint.cpp:216:7:216:7 | taint.cpp:207:6:207:11 | AST only |
| taint.cpp:229:3:229:6 | taint.cpp:223:10:223:15 | AST only |
| taint.cpp:233:8:233:8 | taint.cpp:223:10:223:15 | AST only |
| taint.cpp:236:3:236:6 | taint.cpp:223:10:223:15 | AST only |

16
cpp/ql/test/library-tests/dataflow/taint-tests/test_ir.expected
View File

@@ -1,19 +1,10 @@
| taint.cpp:8:8:8:13 | clean1 | taint.cpp:4:27:4:33 | source1 |
| taint.cpp:16:8:16:14 | source1 | taint.cpp:12:22:12:27 | call to source |
| taint.cpp:17:8:17:16 | ++ ... | taint.cpp:12:22:12:27 | call to source |
| taint.cpp:88:11:88:11 | a | taint.cpp:77:7:77:12 | call to source |
| taint.cpp:89:11:89:11 | b | taint.cpp:77:7:77:12 | call to source |
| taint.cpp:90:11:90:11 | c | taint.cpp:77:7:77:12 | call to source |
| taint.cpp:91:11:91:11 | d | taint.cpp:77:7:77:12 | call to source |
| taint.cpp:92:11:92:11 | a | taint.cpp:71:22:71:27 | call to source |
| taint.cpp:92:11:92:11 | a | taint.cpp:72:7:72:12 | call to source |
| taint.cpp:93:11:93:11 | b | taint.cpp:71:22:71:27 | call to source |
| taint.cpp:93:11:93:11 | b | taint.cpp:72:7:72:12 | call to source |
| taint.cpp:94:11:94:11 | c | taint.cpp:71:22:71:27 | call to source |
| taint.cpp:94:11:94:11 | c | taint.cpp:72:7:72:12 | call to source |
| taint.cpp:95:11:95:11 | d | taint.cpp:71:22:71:27 | call to source |
| taint.cpp:95:11:95:11 | d | taint.cpp:72:7:72:12 | call to source |
| taint.cpp:109:7:109:13 | access to array | taint.cpp:105:12:105:17 | call to source |
| taint.cpp:110:7:110:13 | access to array | taint.cpp:105:12:105:17 | call to source |
| taint.cpp:111:7:111:13 | access to array | taint.cpp:106:12:106:17 | call to source |
| taint.cpp:112:7:112:13 | access to array | taint.cpp:106:12:106:17 | call to source |
| taint.cpp:129:7:129:9 | * ... | taint.cpp:120:11:120:16 | call to source |
| taint.cpp:130:7:130:9 | * ... | taint.cpp:127:8:127:13 | call to source |
| taint.cpp:134:7:134:9 | * ... | taint.cpp:120:11:120:16 | call to source |
@@ -22,6 +13,7 @@
| taint.cpp:168:8:168:14 | tainted | taint.cpp:164:19:164:24 | call to source |
| taint.cpp:210:7:210:7 | x | taint.cpp:207:6:207:11 | call to source |
| taint.cpp:215:7:215:7 | x | taint.cpp:207:6:207:11 | call to source |
| taint.cpp:216:7:216:7 | y | taint.cpp:207:6:207:11 | call to source |
| taint.cpp:250:8:250:8 | a | taint.cpp:223:10:223:15 | call to source |
| taint.cpp:280:7:280:7 | t | taint.cpp:275:6:275:11 | call to source |
| taint.cpp:289:7:289:7 | t | taint.cpp:275:6:275:11 | call to source |