From 2eea359f79fb03854cb2cee8d9a3f101a31df522 Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Wed, 3 Oct 2018 14:39:32 +0200 Subject: [PATCH 1/2] C++: Test for PointlessComparison with templates --- .../PointlessComparison.expected | 4 ++++ .../PointlessComparison/Templates.cpp | 17 +++++++++++++++++ 2 files changed, 21 insertions(+) create mode 100644 cpp/ql/test/query-tests/Likely Bugs/Arithmetic/PointlessComparison/Templates.cpp 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 b822d8f00006..75a10211c2d3 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 @@ -32,3 +32,7 @@ | PointlessComparison.c:129:12:129:16 | ... > ... | Comparison is always false because a <= 3. | | PointlessComparison.c:197:7:197:11 | ... < ... | Comparison is always false because x >= 0. | | RegressionTests.cpp:57:7:57:22 | ... <= ... | Comparison is always true because * ... <= 4294967295. | +| Templates.cpp:3:10:3:24 | ... <= ... | Comparison is always true because param <= 32767. | +| Templates.cpp:9:10:9:24 | ... <= ... | Comparison is always true because local <= 32767. | +| Templates.cpp:9:10:9:24 | ... <= ... | Comparison is always true because local <= 32767. | +| 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/Templates.cpp b/cpp/ql/test/query-tests/Likely Bugs/Arithmetic/PointlessComparison/Templates.cpp new file mode 100644 index 000000000000..ec0e37a55789 --- /dev/null +++ b/cpp/ql/test/query-tests/Likely Bugs/Arithmetic/PointlessComparison/Templates.cpp @@ -0,0 +1,17 @@ +template +bool sometimesPointless(T param) { + return param <= 0xFFFF; // GOOD (FALSE POSITIVE: hypothetical instantiations are okay) +} + +template +bool alwaysPointless(T param) { + short local = param; + return local <= 0xFFFF; // BAD (in all instantiations) +} + +static int caller(int i) { + return + sometimesPointless(i) || + alwaysPointless(i) || + alwaysPointless(i); +} From 364c9a69614d55b9a2150bccb65e510bf657662f Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Wed, 3 Oct 2018 14:32:44 +0200 Subject: [PATCH 2/2] C++: Suppress pointless compare in template inst. It still runs on uninstantiated templates because its underlying libraries do. It's not clear whether that leads to other false positives, but that's independent of the change I'm making here. --- cpp/ql/src/Likely Bugs/Arithmetic/PointlessComparison.ql | 4 +++- .../PointlessComparison/PointlessComparison.expected | 3 --- .../Likely Bugs/Arithmetic/PointlessComparison/Templates.cpp | 2 +- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/cpp/ql/src/Likely Bugs/Arithmetic/PointlessComparison.ql b/cpp/ql/src/Likely Bugs/Arithmetic/PointlessComparison.ql index fdde7852aa1a..439b90b68a36 100644 --- a/cpp/ql/src/Likely Bugs/Arithmetic/PointlessComparison.ql +++ b/cpp/ql/src/Likely Bugs/Arithmetic/PointlessComparison.ql @@ -22,13 +22,15 @@ import UnsignedGEZero // #define PRINTMSG(val,msg) { if (val >= PRINTLEVEL) printf(msg); } // // So to reduce the number of false positives, we do not report a result if -// the comparison is in a macro expansion. +// the comparison is in a macro expansion. Similarly for template +// instantiations. from ComparisonOperation cmp, SmallSide ss, float left, float right, boolean value, string reason where not cmp.isInMacroExpansion() and + not cmp.isFromTemplateInstantiation(_) and reachablePointlessComparison(cmp, left, right, value, ss) and // a comparison between an enum and zero is always valid because whether 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 75a10211c2d3..4ed03c756a8d 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 @@ -32,7 +32,4 @@ | PointlessComparison.c:129:12:129:16 | ... > ... | Comparison is always false because a <= 3. | | PointlessComparison.c:197:7:197:11 | ... < ... | Comparison is always false because x >= 0. | | RegressionTests.cpp:57:7:57:22 | ... <= ... | Comparison is always true because * ... <= 4294967295. | -| Templates.cpp:3:10:3:24 | ... <= ... | Comparison is always true because param <= 32767. | -| Templates.cpp:9:10:9:24 | ... <= ... | Comparison is always true because local <= 32767. | -| Templates.cpp:9:10:9:24 | ... <= ... | Comparison is always true because local <= 32767. | | 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/Templates.cpp b/cpp/ql/test/query-tests/Likely Bugs/Arithmetic/PointlessComparison/Templates.cpp index ec0e37a55789..a211b2307805 100644 --- a/cpp/ql/test/query-tests/Likely Bugs/Arithmetic/PointlessComparison/Templates.cpp +++ b/cpp/ql/test/query-tests/Likely Bugs/Arithmetic/PointlessComparison/Templates.cpp @@ -1,6 +1,6 @@ template bool sometimesPointless(T param) { - return param <= 0xFFFF; // GOOD (FALSE POSITIVE: hypothetical instantiations are okay) + return param <= 0xFFFF; // GOOD (hypothetical instantiations are okay) } template