From e68352bcdee2464d4ad355a0af6474e1f8131d94 Mon Sep 17 00:00:00 2001 From: Cornelius Riemenschneider Date: Tue, 24 Nov 2020 12:14:46 +0100 Subject: [PATCH 1/5] C++: Add testcase for false positive. --- .../PointlessComparison/PointlessComparison.expected | 1 + .../Arithmetic/PointlessComparison/RegressionTests.cpp | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/cpp/ql/test/query-tests/Likely Bugs/Arithmetic/PointlessComparison/PointlessComparison.expected b/cpp/ql/test/query-tests/Likely Bugs/Arithmetic/PointlessComparison/PointlessComparison.expected index 6c273b985ee..71fe2c12a39 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Arithmetic/PointlessComparison/PointlessComparison.expected +++ b/cpp/ql/test/query-tests/Likely Bugs/Arithmetic/PointlessComparison/PointlessComparison.expected @@ -50,4 +50,5 @@ | PointlessComparison.cpp:43:6:43:29 | ... >= ... | Comparison is always true because ... >> ... >= 140737488355327.5. | | PointlessComparison.cpp:44:6:44:28 | ... >= ... | Comparison is always true because ... >> ... >= 140737488355327.5. | | RegressionTests.cpp:57:7:57:22 | ... <= ... | Comparison is always true because * ... <= 4294967295. | +| RegressionTests.cpp:125:7:125:11 | ... > ... | Comparison is always false because x <= 0. | | Templates.cpp:9:10:9:24 | ... <= ... | Comparison is always true because local <= 32767. | diff --git a/cpp/ql/test/query-tests/Likely Bugs/Arithmetic/PointlessComparison/RegressionTests.cpp b/cpp/ql/test/query-tests/Likely Bugs/Arithmetic/PointlessComparison/RegressionTests.cpp index 28d211a129e..ebd01cf087a 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Arithmetic/PointlessComparison/RegressionTests.cpp +++ b/cpp/ql/test/query-tests/Likely Bugs/Arithmetic/PointlessComparison/RegressionTests.cpp @@ -116,3 +116,11 @@ void negativeZero4(int val) { if (val == 0) // GOOD [NO LONGER REPORTED] ; } + +void f(int *const &ref_to_ptr); + +void testTempObject() { + int x = 0; + f(&x); + if (x > 0) {} // BAD [FALSE POSITIVE] +} From 8d024c7ff1d8b68815d054eb6abf286cd15c6e86 Mon Sep 17 00:00:00 2001 From: Cornelius Riemenschneider Date: Tue, 24 Nov 2020 12:27:39 +0100 Subject: [PATCH 2/5] C++: Add tests around references to pointers with temporary objects. --- .../test/library-tests/defuse/definition.expected | 2 ++ .../defuse/definitionUsePair.expected | 4 ++++ .../library-tests/defuse/exprDefinition.expected | 2 ++ .../defuse/isAddressOfAccess.expected | 4 ++++ cpp/ql/test/library-tests/defuse/pass_by_ref.cpp | 14 ++++++++++++++ cpp/ql/test/library-tests/defuse/useOfVar.expected | 4 ++++ .../library-tests/defuse/useOfVarActual.expected | 2 ++ .../test/library-tests/defuse/useUsePair.expected | 2 ++ 8 files changed, 34 insertions(+) diff --git a/cpp/ql/test/library-tests/defuse/definition.expected b/cpp/ql/test/library-tests/defuse/definition.expected index cf7ff1942e2..05a0742bd7e 100644 --- a/cpp/ql/test/library-tests/defuse/definition.expected +++ b/cpp/ql/test/library-tests/defuse/definition.expected @@ -77,6 +77,8 @@ | pass_by_ref.cpp:46:7:46:7 | i | pass_by_ref.cpp:46:11:46:11 | n | | pass_by_ref.cpp:46:7:46:7 | i | pass_by_ref.cpp:49:5:49:7 | ... ++ | | pass_by_ref.cpp:46:7:46:7 | i | pass_by_ref.cpp:53:22:53:22 | i | +| pass_by_ref.cpp:60:7:60:7 | x | pass_by_ref.cpp:60:10:60:11 | 2 | +| pass_by_ref.cpp:66:8:66:8 | p | pass_by_ref.cpp:66:12:66:18 | 0 | | test.cpp:4:7:4:7 | a | test.cpp:5:3:5:8 | ... = ... | | test.cpp:4:7:4:7 | a | test.cpp:13:3:13:7 | ... = ... | | test.cpp:4:7:4:7 | a | test.cpp:19:5:19:9 | ... = ... | diff --git a/cpp/ql/test/library-tests/defuse/definitionUsePair.expected b/cpp/ql/test/library-tests/defuse/definitionUsePair.expected index 96389527265..2d11e5df31f 100644 --- a/cpp/ql/test/library-tests/defuse/definitionUsePair.expected +++ b/cpp/ql/test/library-tests/defuse/definitionUsePair.expected @@ -75,6 +75,10 @@ | pass_by_ref.cpp:46:7:46:7 | i | pass_by_ref.cpp:46:11:46:11 | n | pass_by_ref.cpp:53:22:53:22 | i | | pass_by_ref.cpp:46:7:46:7 | i | pass_by_ref.cpp:49:5:49:7 | ... ++ | pass_by_ref.cpp:53:22:53:22 | i | | pass_by_ref.cpp:46:7:46:7 | i | pass_by_ref.cpp:53:22:53:22 | i | pass_by_ref.cpp:54:10:54:10 | i | +| pass_by_ref.cpp:60:7:60:7 | x | pass_by_ref.cpp:60:10:60:11 | 2 | pass_by_ref.cpp:61:35:61:35 | x | +| pass_by_ref.cpp:60:7:60:7 | x | pass_by_ref.cpp:60:10:60:11 | 2 | pass_by_ref.cpp:62:10:62:10 | x | +| pass_by_ref.cpp:66:8:66:8 | p | pass_by_ref.cpp:66:12:66:18 | 0 | pass_by_ref.cpp:67:34:67:34 | p | +| pass_by_ref.cpp:66:8:66:8 | p | pass_by_ref.cpp:66:12:66:18 | 0 | pass_by_ref.cpp:68:10:68:10 | p | | test.cpp:4:7:4:7 | a | test.cpp:5:3:5:8 | ... = ... | test.cpp:9:7:9:7 | a | | test.cpp:4:7:4:7 | a | test.cpp:13:3:13:7 | ... = ... | test.cpp:14:7:14:7 | a | | test.cpp:4:7:4:7 | a | test.cpp:13:3:13:7 | ... = ... | test.cpp:18:7:18:7 | a | diff --git a/cpp/ql/test/library-tests/defuse/exprDefinition.expected b/cpp/ql/test/library-tests/defuse/exprDefinition.expected index c2431284bfa..61655578476 100644 --- a/cpp/ql/test/library-tests/defuse/exprDefinition.expected +++ b/cpp/ql/test/library-tests/defuse/exprDefinition.expected @@ -17,6 +17,8 @@ | pass_by_ref.cpp:31:7:31:9 | arr | pass_by_ref.cpp:31:15:31:18 | {...} | pass_by_ref.cpp:31:15:31:18 | {...} | | pass_by_ref.cpp:37:7:37:9 | arr | pass_by_ref.cpp:37:15:37:18 | {...} | pass_by_ref.cpp:37:15:37:18 | {...} | | pass_by_ref.cpp:46:7:46:7 | i | pass_by_ref.cpp:46:11:46:11 | n | pass_by_ref.cpp:46:11:46:11 | n | +| pass_by_ref.cpp:60:7:60:7 | x | pass_by_ref.cpp:60:10:60:11 | 2 | pass_by_ref.cpp:60:10:60:11 | 2 | +| pass_by_ref.cpp:66:8:66:8 | p | pass_by_ref.cpp:66:12:66:18 | 0 | pass_by_ref.cpp:66:12:66:18 | 0 | | test.cpp:4:7:4:7 | a | test.cpp:5:3:5:8 | ... = ... | test.cpp:5:7:5:8 | a0 | | test.cpp:4:7:4:7 | a | test.cpp:13:3:13:7 | ... = ... | test.cpp:13:7:13:7 | b | | test.cpp:4:7:4:7 | a | test.cpp:19:5:19:9 | ... = ... | test.cpp:19:9:19:9 | 1 | diff --git a/cpp/ql/test/library-tests/defuse/isAddressOfAccess.expected b/cpp/ql/test/library-tests/defuse/isAddressOfAccess.expected index c7a5adde333..18f0187da4f 100644 --- a/cpp/ql/test/library-tests/defuse/isAddressOfAccess.expected +++ b/cpp/ql/test/library-tests/defuse/isAddressOfAccess.expected @@ -151,6 +151,10 @@ | pass_by_ref.cpp:49:5:49:5 | i | | | pass_by_ref.cpp:53:22:53:22 | i | non-const address | | pass_by_ref.cpp:54:10:54:10 | i | | +| pass_by_ref.cpp:61:35:61:35 | x | const address | +| pass_by_ref.cpp:62:10:62:10 | x | | +| pass_by_ref.cpp:67:34:67:34 | p | const address | +| pass_by_ref.cpp:68:10:68:10 | p | | | test.cpp:5:3:5:3 | a | | | test.cpp:5:7:5:8 | a0 | | | test.cpp:6:3:6:3 | b | | diff --git a/cpp/ql/test/library-tests/defuse/pass_by_ref.cpp b/cpp/ql/test/library-tests/defuse/pass_by_ref.cpp index 2633786d243..baf030c6a40 100644 --- a/cpp/ql/test/library-tests/defuse/pass_by_ref.cpp +++ b/cpp/ql/test/library-tests/defuse/pass_by_ref.cpp @@ -53,3 +53,17 @@ int afterIf(int n) { referenceParameter(i); return i; } + +void constPointerReferenceParameter(int * const & pRef); + +int temporaryObject() { + int x = 2; + constPointerReferenceParameter(&x); + return x; +} + +int * noTemporaryObject() { + int *p = nullptr; + constPointerReferenceParameter(p); + return p; +} diff --git a/cpp/ql/test/library-tests/defuse/useOfVar.expected b/cpp/ql/test/library-tests/defuse/useOfVar.expected index f1b2a0e6951..2d6742a9128 100644 --- a/cpp/ql/test/library-tests/defuse/useOfVar.expected +++ b/cpp/ql/test/library-tests/defuse/useOfVar.expected @@ -118,6 +118,10 @@ | pass_by_ref.cpp:46:7:46:7 | i | pass_by_ref.cpp:49:5:49:5 | i | | pass_by_ref.cpp:46:7:46:7 | i | pass_by_ref.cpp:53:22:53:22 | i | | pass_by_ref.cpp:46:7:46:7 | i | pass_by_ref.cpp:54:10:54:10 | i | +| pass_by_ref.cpp:60:7:60:7 | x | pass_by_ref.cpp:61:35:61:35 | x | +| pass_by_ref.cpp:60:7:60:7 | x | pass_by_ref.cpp:62:10:62:10 | x | +| pass_by_ref.cpp:66:8:66:8 | p | pass_by_ref.cpp:67:34:67:34 | p | +| pass_by_ref.cpp:66:8:66:8 | p | pass_by_ref.cpp:68:10:68:10 | p | | test.cpp:3:16:3:17 | a0 | test.cpp:5:7:5:8 | a0 | | test.cpp:3:24:3:25 | b0 | test.cpp:6:7:6:8 | b0 | | test.cpp:3:32:3:33 | c0 | test.cpp:7:7:7:8 | c0 | diff --git a/cpp/ql/test/library-tests/defuse/useOfVarActual.expected b/cpp/ql/test/library-tests/defuse/useOfVarActual.expected index c17bbfd6203..133a59386eb 100644 --- a/cpp/ql/test/library-tests/defuse/useOfVarActual.expected +++ b/cpp/ql/test/library-tests/defuse/useOfVarActual.expected @@ -56,6 +56,8 @@ | pass_by_ref.cpp:45:17:45:17 | n | pass_by_ref.cpp:48:7:48:7 | n | | pass_by_ref.cpp:46:7:46:7 | i | pass_by_ref.cpp:49:5:49:5 | i | | pass_by_ref.cpp:46:7:46:7 | i | pass_by_ref.cpp:54:10:54:10 | i | +| pass_by_ref.cpp:60:7:60:7 | x | pass_by_ref.cpp:62:10:62:10 | x | +| pass_by_ref.cpp:66:8:66:8 | p | pass_by_ref.cpp:68:10:68:10 | p | | test.cpp:3:16:3:17 | a0 | test.cpp:5:7:5:8 | a0 | | test.cpp:3:24:3:25 | b0 | test.cpp:6:7:6:8 | b0 | | test.cpp:3:32:3:33 | c0 | test.cpp:7:7:7:8 | c0 | diff --git a/cpp/ql/test/library-tests/defuse/useUsePair.expected b/cpp/ql/test/library-tests/defuse/useUsePair.expected index f58ff997eff..4569437390c 100644 --- a/cpp/ql/test/library-tests/defuse/useUsePair.expected +++ b/cpp/ql/test/library-tests/defuse/useUsePair.expected @@ -25,6 +25,8 @@ | pass_by_ref.cpp:21:7:21:8 | i2 | pass_by_ref.cpp:24:26:24:27 | i2 | pass_by_ref.cpp:27:39:27:40 | i2 | | pass_by_ref.cpp:21:7:21:8 | i2 | pass_by_ref.cpp:25:27:25:28 | i2 | pass_by_ref.cpp:27:39:27:40 | i2 | | pass_by_ref.cpp:45:17:45:17 | n | pass_by_ref.cpp:46:11:46:11 | n | pass_by_ref.cpp:48:7:48:7 | n | +| pass_by_ref.cpp:60:7:60:7 | x | pass_by_ref.cpp:61:35:61:35 | x | pass_by_ref.cpp:62:10:62:10 | x | +| pass_by_ref.cpp:66:8:66:8 | p | pass_by_ref.cpp:67:34:67:34 | p | pass_by_ref.cpp:68:10:68:10 | p | | test.cpp:4:7:4:7 | a | test.cpp:14:7:14:7 | a | test.cpp:18:7:18:7 | a | | test.cpp:4:7:4:7 | a | test.cpp:14:7:14:7 | a | test.cpp:24:7:24:7 | a | | test.cpp:4:7:4:7 | a | test.cpp:14:7:14:7 | a | test.cpp:28:11:28:11 | a | From 7f13d4c356602abebf7632ddb2347c34b3d6366c Mon Sep 17 00:00:00 2001 From: Cornelius Riemenschneider Date: Tue, 24 Nov 2020 12:33:33 +0100 Subject: [PATCH 3/5] C++: Improve EscapesTree analysis in the presence of temporary objects. --- cpp/ql/src/semmle/code/cpp/dataflow/EscapesTree.qll | 10 +++++++++- cpp/ql/test/library-tests/defuse/definition.expected | 2 ++ .../library-tests/defuse/definitionUsePair.expected | 2 +- .../library-tests/defuse/isAddressOfAccess.expected | 4 ++-- cpp/ql/test/library-tests/defuse/useUsePair.expected | 1 - .../PointlessComparison/PointlessComparison.expected | 1 - .../Arithmetic/PointlessComparison/RegressionTests.cpp | 2 +- 7 files changed, 15 insertions(+), 7 deletions(-) diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/EscapesTree.qll b/cpp/ql/src/semmle/code/cpp/dataflow/EscapesTree.qll index 183ae598775..7fc8049245a 100644 --- a/cpp/ql/src/semmle/code/cpp/dataflow/EscapesTree.qll +++ b/cpp/ql/src/semmle/code/cpp/dataflow/EscapesTree.qll @@ -222,6 +222,14 @@ private predicate addressMayEscapeMutablyAt(Expr e) { // If the address has been cast to an integral type, conservatively assume that it may eventually be cast back to a // pointer to non-const type. t instanceof IntegralType + or + // If we go through a temporary object step, we can take a reference to a temporary const pointer + // object, where the pointer doesn't point to a const value + exists(TemporaryObjectExpr temp, PointerType pt | + temp.getConversion() = e and pt = temp.getType().stripTopLevelSpecifiers() + | + not pt.getBaseType().isConst() + ) ) } @@ -249,7 +257,7 @@ private predicate addressFromVariableAccess(VariableAccess va, Expr e) { // `e` could be a pointer that is converted to a reference as the final step, // meaning that we pass a value that is two dereferences away from referring // to `va`. This happens, for example, with `void std::vector::push_back(T&& - // value);` when called as `v.push_back(&x)`, for a static variable `x`. It + // value);` when called as `v.push_back(&x)`, for a variable `x`. It // can also happen when taking a reference to a const pointer to a // (potentially non-const) value. exists(Expr pointerValue | diff --git a/cpp/ql/test/library-tests/defuse/definition.expected b/cpp/ql/test/library-tests/defuse/definition.expected index 05a0742bd7e..865779c9639 100644 --- a/cpp/ql/test/library-tests/defuse/definition.expected +++ b/cpp/ql/test/library-tests/defuse/definition.expected @@ -1,5 +1,6 @@ | addressOf.cpp:23:9:23:9 | i | addressOf.cpp:24:22:24:30 | call to move | | addressOf.cpp:23:9:23:9 | i | addressOf.cpp:25:25:25:26 | & ... | +| addressOf.cpp:23:9:23:9 | i | addressOf.cpp:26:31:26:32 | & ... | | addressOf.cpp:31:23:31:23 | i | addressOf.cpp:32:18:32:19 | & ... | | addressOf.cpp:31:23:31:23 | i | addressOf.cpp:34:18:34:33 | ... ? ... : ... | | addressOf.cpp:31:23:31:23 | i | addressOf.cpp:35:18:35:23 | & ... | @@ -78,6 +79,7 @@ | pass_by_ref.cpp:46:7:46:7 | i | pass_by_ref.cpp:49:5:49:7 | ... ++ | | pass_by_ref.cpp:46:7:46:7 | i | pass_by_ref.cpp:53:22:53:22 | i | | pass_by_ref.cpp:60:7:60:7 | x | pass_by_ref.cpp:60:10:60:11 | 2 | +| pass_by_ref.cpp:60:7:60:7 | x | pass_by_ref.cpp:61:34:61:35 | & ... | | pass_by_ref.cpp:66:8:66:8 | p | pass_by_ref.cpp:66:12:66:18 | 0 | | test.cpp:4:7:4:7 | a | test.cpp:5:3:5:8 | ... = ... | | test.cpp:4:7:4:7 | a | test.cpp:13:3:13:7 | ... = ... | diff --git a/cpp/ql/test/library-tests/defuse/definitionUsePair.expected b/cpp/ql/test/library-tests/defuse/definitionUsePair.expected index 2d11e5df31f..cf375abfad1 100644 --- a/cpp/ql/test/library-tests/defuse/definitionUsePair.expected +++ b/cpp/ql/test/library-tests/defuse/definitionUsePair.expected @@ -76,7 +76,7 @@ | pass_by_ref.cpp:46:7:46:7 | i | pass_by_ref.cpp:49:5:49:7 | ... ++ | pass_by_ref.cpp:53:22:53:22 | i | | pass_by_ref.cpp:46:7:46:7 | i | pass_by_ref.cpp:53:22:53:22 | i | pass_by_ref.cpp:54:10:54:10 | i | | pass_by_ref.cpp:60:7:60:7 | x | pass_by_ref.cpp:60:10:60:11 | 2 | pass_by_ref.cpp:61:35:61:35 | x | -| pass_by_ref.cpp:60:7:60:7 | x | pass_by_ref.cpp:60:10:60:11 | 2 | pass_by_ref.cpp:62:10:62:10 | x | +| pass_by_ref.cpp:60:7:60:7 | x | pass_by_ref.cpp:61:34:61:35 | & ... | pass_by_ref.cpp:62:10:62:10 | x | | pass_by_ref.cpp:66:8:66:8 | p | pass_by_ref.cpp:66:12:66:18 | 0 | pass_by_ref.cpp:67:34:67:34 | p | | pass_by_ref.cpp:66:8:66:8 | p | pass_by_ref.cpp:66:12:66:18 | 0 | pass_by_ref.cpp:68:10:68:10 | p | | test.cpp:4:7:4:7 | a | test.cpp:5:3:5:8 | ... = ... | test.cpp:9:7:9:7 | a | diff --git a/cpp/ql/test/library-tests/defuse/isAddressOfAccess.expected b/cpp/ql/test/library-tests/defuse/isAddressOfAccess.expected index 18f0187da4f..770e3eb3f45 100644 --- a/cpp/ql/test/library-tests/defuse/isAddressOfAccess.expected +++ b/cpp/ql/test/library-tests/defuse/isAddressOfAccess.expected @@ -1,7 +1,7 @@ | addressOf.cpp:14:28:14:29 | d_ | non-const address | | addressOf.cpp:24:32:24:32 | i | non-const address | | addressOf.cpp:25:26:25:26 | i | non-const address | -| addressOf.cpp:26:32:26:32 | i | const address | +| addressOf.cpp:26:32:26:32 | i | non-const address | | addressOf.cpp:32:19:32:19 | i | non-const address | | addressOf.cpp:34:18:34:18 | i | | | addressOf.cpp:34:23:34:23 | i | non-const address | @@ -151,7 +151,7 @@ | pass_by_ref.cpp:49:5:49:5 | i | | | pass_by_ref.cpp:53:22:53:22 | i | non-const address | | pass_by_ref.cpp:54:10:54:10 | i | | -| pass_by_ref.cpp:61:35:61:35 | x | const address | +| pass_by_ref.cpp:61:35:61:35 | x | non-const address | | pass_by_ref.cpp:62:10:62:10 | x | | | pass_by_ref.cpp:67:34:67:34 | p | const address | | pass_by_ref.cpp:68:10:68:10 | p | | diff --git a/cpp/ql/test/library-tests/defuse/useUsePair.expected b/cpp/ql/test/library-tests/defuse/useUsePair.expected index 4569437390c..cefe1025844 100644 --- a/cpp/ql/test/library-tests/defuse/useUsePair.expected +++ b/cpp/ql/test/library-tests/defuse/useUsePair.expected @@ -25,7 +25,6 @@ | pass_by_ref.cpp:21:7:21:8 | i2 | pass_by_ref.cpp:24:26:24:27 | i2 | pass_by_ref.cpp:27:39:27:40 | i2 | | pass_by_ref.cpp:21:7:21:8 | i2 | pass_by_ref.cpp:25:27:25:28 | i2 | pass_by_ref.cpp:27:39:27:40 | i2 | | pass_by_ref.cpp:45:17:45:17 | n | pass_by_ref.cpp:46:11:46:11 | n | pass_by_ref.cpp:48:7:48:7 | n | -| pass_by_ref.cpp:60:7:60:7 | x | pass_by_ref.cpp:61:35:61:35 | x | pass_by_ref.cpp:62:10:62:10 | x | | pass_by_ref.cpp:66:8:66:8 | p | pass_by_ref.cpp:67:34:67:34 | p | pass_by_ref.cpp:68:10:68:10 | p | | test.cpp:4:7:4:7 | a | test.cpp:14:7:14:7 | a | test.cpp:18:7:18:7 | a | | test.cpp:4:7:4:7 | a | test.cpp:14:7:14:7 | a | test.cpp:24:7:24:7 | a | diff --git a/cpp/ql/test/query-tests/Likely Bugs/Arithmetic/PointlessComparison/PointlessComparison.expected b/cpp/ql/test/query-tests/Likely Bugs/Arithmetic/PointlessComparison/PointlessComparison.expected index 71fe2c12a39..6c273b985ee 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Arithmetic/PointlessComparison/PointlessComparison.expected +++ b/cpp/ql/test/query-tests/Likely Bugs/Arithmetic/PointlessComparison/PointlessComparison.expected @@ -50,5 +50,4 @@ | PointlessComparison.cpp:43:6:43:29 | ... >= ... | Comparison is always true because ... >> ... >= 140737488355327.5. | | PointlessComparison.cpp:44:6:44:28 | ... >= ... | Comparison is always true because ... >> ... >= 140737488355327.5. | | RegressionTests.cpp:57:7:57:22 | ... <= ... | Comparison is always true because * ... <= 4294967295. | -| RegressionTests.cpp:125:7:125:11 | ... > ... | Comparison is always false because x <= 0. | | Templates.cpp:9:10:9:24 | ... <= ... | Comparison is always true because local <= 32767. | diff --git a/cpp/ql/test/query-tests/Likely Bugs/Arithmetic/PointlessComparison/RegressionTests.cpp b/cpp/ql/test/query-tests/Likely Bugs/Arithmetic/PointlessComparison/RegressionTests.cpp index ebd01cf087a..9813a07e29d 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Arithmetic/PointlessComparison/RegressionTests.cpp +++ b/cpp/ql/test/query-tests/Likely Bugs/Arithmetic/PointlessComparison/RegressionTests.cpp @@ -122,5 +122,5 @@ void f(int *const &ref_to_ptr); void testTempObject() { int x = 0; f(&x); - if (x > 0) {} // BAD [FALSE POSITIVE] + if (x > 0) {} // GOOD [NO LONGER REPORTED] } From b4e45ad6cbe1fdb9101592dbef69e83e9c01248f Mon Sep 17 00:00:00 2001 From: Cornelius Riemenschneider Date: Wed, 25 Nov 2020 16:24:25 +0100 Subject: [PATCH 4/5] C++: Address review. --- cpp/ql/src/semmle/code/cpp/dataflow/EscapesTree.qll | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cpp/ql/src/semmle/code/cpp/dataflow/EscapesTree.qll b/cpp/ql/src/semmle/code/cpp/dataflow/EscapesTree.qll index 7fc8049245a..798c62d14c3 100644 --- a/cpp/ql/src/semmle/code/cpp/dataflow/EscapesTree.qll +++ b/cpp/ql/src/semmle/code/cpp/dataflow/EscapesTree.qll @@ -226,7 +226,8 @@ private predicate addressMayEscapeMutablyAt(Expr e) { // If we go through a temporary object step, we can take a reference to a temporary const pointer // object, where the pointer doesn't point to a const value exists(TemporaryObjectExpr temp, PointerType pt | - temp.getConversion() = e and pt = temp.getType().stripTopLevelSpecifiers() + temp.getConversion() = e.(ReferenceToExpr) and + pt = temp.getType().stripTopLevelSpecifiers() | not pt.getBaseType().isConst() ) From 0b8403fc052c7072164dd06382128bda6e64994b Mon Sep 17 00:00:00 2001 From: Cornelius Riemenschneider Date: Wed, 25 Nov 2020 16:24:55 +0100 Subject: [PATCH 5/5] C++: Add one more test. --- cpp/ql/test/library-tests/defuse/definition.expected | 2 ++ .../test/library-tests/defuse/definitionUsePair.expected | 2 ++ cpp/ql/test/library-tests/defuse/exprDefinition.expected | 1 + .../test/library-tests/defuse/isAddressOfAccess.expected | 2 ++ cpp/ql/test/library-tests/defuse/pass_by_ref.cpp | 8 ++++++++ cpp/ql/test/library-tests/defuse/useOfVar.expected | 2 ++ cpp/ql/test/library-tests/defuse/useOfVarActual.expected | 1 + 7 files changed, 18 insertions(+) diff --git a/cpp/ql/test/library-tests/defuse/definition.expected b/cpp/ql/test/library-tests/defuse/definition.expected index 865779c9639..865f4472496 100644 --- a/cpp/ql/test/library-tests/defuse/definition.expected +++ b/cpp/ql/test/library-tests/defuse/definition.expected @@ -81,6 +81,8 @@ | pass_by_ref.cpp:60:7:60:7 | x | pass_by_ref.cpp:60:10:60:11 | 2 | | pass_by_ref.cpp:60:7:60:7 | x | pass_by_ref.cpp:61:34:61:35 | & ... | | pass_by_ref.cpp:66:8:66:8 | p | pass_by_ref.cpp:66:12:66:18 | 0 | +| pass_by_ref.cpp:74:7:74:7 | x | pass_by_ref.cpp:74:10:74:11 | 2 | +| pass_by_ref.cpp:74:7:74:7 | x | pass_by_ref.cpp:75:35:75:36 | & ... | | test.cpp:4:7:4:7 | a | test.cpp:5:3:5:8 | ... = ... | | test.cpp:4:7:4:7 | a | test.cpp:13:3:13:7 | ... = ... | | test.cpp:4:7:4:7 | a | test.cpp:19:5:19:9 | ... = ... | diff --git a/cpp/ql/test/library-tests/defuse/definitionUsePair.expected b/cpp/ql/test/library-tests/defuse/definitionUsePair.expected index cf375abfad1..701e2166713 100644 --- a/cpp/ql/test/library-tests/defuse/definitionUsePair.expected +++ b/cpp/ql/test/library-tests/defuse/definitionUsePair.expected @@ -79,6 +79,8 @@ | pass_by_ref.cpp:60:7:60:7 | x | pass_by_ref.cpp:61:34:61:35 | & ... | pass_by_ref.cpp:62:10:62:10 | x | | pass_by_ref.cpp:66:8:66:8 | p | pass_by_ref.cpp:66:12:66:18 | 0 | pass_by_ref.cpp:67:34:67:34 | p | | pass_by_ref.cpp:66:8:66:8 | p | pass_by_ref.cpp:66:12:66:18 | 0 | pass_by_ref.cpp:68:10:68:10 | p | +| pass_by_ref.cpp:74:7:74:7 | x | pass_by_ref.cpp:74:10:74:11 | 2 | pass_by_ref.cpp:75:36:75:36 | x | +| pass_by_ref.cpp:74:7:74:7 | x | pass_by_ref.cpp:75:35:75:36 | & ... | pass_by_ref.cpp:76:10:76:10 | x | | test.cpp:4:7:4:7 | a | test.cpp:5:3:5:8 | ... = ... | test.cpp:9:7:9:7 | a | | test.cpp:4:7:4:7 | a | test.cpp:13:3:13:7 | ... = ... | test.cpp:14:7:14:7 | a | | test.cpp:4:7:4:7 | a | test.cpp:13:3:13:7 | ... = ... | test.cpp:18:7:18:7 | a | diff --git a/cpp/ql/test/library-tests/defuse/exprDefinition.expected b/cpp/ql/test/library-tests/defuse/exprDefinition.expected index 61655578476..4b99a56a438 100644 --- a/cpp/ql/test/library-tests/defuse/exprDefinition.expected +++ b/cpp/ql/test/library-tests/defuse/exprDefinition.expected @@ -19,6 +19,7 @@ | pass_by_ref.cpp:46:7:46:7 | i | pass_by_ref.cpp:46:11:46:11 | n | pass_by_ref.cpp:46:11:46:11 | n | | pass_by_ref.cpp:60:7:60:7 | x | pass_by_ref.cpp:60:10:60:11 | 2 | pass_by_ref.cpp:60:10:60:11 | 2 | | pass_by_ref.cpp:66:8:66:8 | p | pass_by_ref.cpp:66:12:66:18 | 0 | pass_by_ref.cpp:66:12:66:18 | 0 | +| pass_by_ref.cpp:74:7:74:7 | x | pass_by_ref.cpp:74:10:74:11 | 2 | pass_by_ref.cpp:74:10:74:11 | 2 | | test.cpp:4:7:4:7 | a | test.cpp:5:3:5:8 | ... = ... | test.cpp:5:7:5:8 | a0 | | test.cpp:4:7:4:7 | a | test.cpp:13:3:13:7 | ... = ... | test.cpp:13:7:13:7 | b | | test.cpp:4:7:4:7 | a | test.cpp:19:5:19:9 | ... = ... | test.cpp:19:9:19:9 | 1 | diff --git a/cpp/ql/test/library-tests/defuse/isAddressOfAccess.expected b/cpp/ql/test/library-tests/defuse/isAddressOfAccess.expected index 770e3eb3f45..e47d9faa980 100644 --- a/cpp/ql/test/library-tests/defuse/isAddressOfAccess.expected +++ b/cpp/ql/test/library-tests/defuse/isAddressOfAccess.expected @@ -155,6 +155,8 @@ | pass_by_ref.cpp:62:10:62:10 | x | | | pass_by_ref.cpp:67:34:67:34 | p | const address | | pass_by_ref.cpp:68:10:68:10 | p | | +| pass_by_ref.cpp:75:36:75:36 | x | non-const address | +| pass_by_ref.cpp:76:10:76:10 | x | | | test.cpp:5:3:5:3 | a | | | test.cpp:5:7:5:8 | a0 | | | test.cpp:6:3:6:3 | b | | diff --git a/cpp/ql/test/library-tests/defuse/pass_by_ref.cpp b/cpp/ql/test/library-tests/defuse/pass_by_ref.cpp index baf030c6a40..93d93edc9e3 100644 --- a/cpp/ql/test/library-tests/defuse/pass_by_ref.cpp +++ b/cpp/ql/test/library-tests/defuse/pass_by_ref.cpp @@ -67,3 +67,11 @@ int * noTemporaryObject() { constPointerReferenceParameter(p); return p; } + +void pointerRvalueReferenceParameter(int * && pRef); + +int temporaryObject2() { + int x = 2; + pointerRvalueReferenceParameter(&x); + return x; +} diff --git a/cpp/ql/test/library-tests/defuse/useOfVar.expected b/cpp/ql/test/library-tests/defuse/useOfVar.expected index 2d6742a9128..404509c7326 100644 --- a/cpp/ql/test/library-tests/defuse/useOfVar.expected +++ b/cpp/ql/test/library-tests/defuse/useOfVar.expected @@ -122,6 +122,8 @@ | pass_by_ref.cpp:60:7:60:7 | x | pass_by_ref.cpp:62:10:62:10 | x | | pass_by_ref.cpp:66:8:66:8 | p | pass_by_ref.cpp:67:34:67:34 | p | | pass_by_ref.cpp:66:8:66:8 | p | pass_by_ref.cpp:68:10:68:10 | p | +| pass_by_ref.cpp:74:7:74:7 | x | pass_by_ref.cpp:75:36:75:36 | x | +| pass_by_ref.cpp:74:7:74:7 | x | pass_by_ref.cpp:76:10:76:10 | x | | test.cpp:3:16:3:17 | a0 | test.cpp:5:7:5:8 | a0 | | test.cpp:3:24:3:25 | b0 | test.cpp:6:7:6:8 | b0 | | test.cpp:3:32:3:33 | c0 | test.cpp:7:7:7:8 | c0 | diff --git a/cpp/ql/test/library-tests/defuse/useOfVarActual.expected b/cpp/ql/test/library-tests/defuse/useOfVarActual.expected index 133a59386eb..0b704dcc820 100644 --- a/cpp/ql/test/library-tests/defuse/useOfVarActual.expected +++ b/cpp/ql/test/library-tests/defuse/useOfVarActual.expected @@ -58,6 +58,7 @@ | pass_by_ref.cpp:46:7:46:7 | i | pass_by_ref.cpp:54:10:54:10 | i | | pass_by_ref.cpp:60:7:60:7 | x | pass_by_ref.cpp:62:10:62:10 | x | | pass_by_ref.cpp:66:8:66:8 | p | pass_by_ref.cpp:68:10:68:10 | p | +| pass_by_ref.cpp:74:7:74:7 | x | pass_by_ref.cpp:76:10:76:10 | x | | test.cpp:3:16:3:17 | a0 | test.cpp:5:7:5:8 | a0 | | test.cpp:3:24:3:25 | b0 | test.cpp:6:7:6:8 | b0 | | test.cpp:3:32:3:33 | c0 | test.cpp:7:7:7:8 | c0 |