From c599b02e98e5c8625d606dfe0f02f6e1480eb007 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Tue, 20 Sep 2022 15:23:13 +0100 Subject: [PATCH 1/7] C++: Add test case. --- .../UnusedStaticFunctions.expected | 2 ++ .../UnusedStaticFunctions/extraction_error.c | 15 +++++++++++++++ 2 files changed, 17 insertions(+) create mode 100644 cpp/ql/test/query-tests/Best Practices/Unused Entities/UnusedStaticFunctions/extraction_error.c diff --git a/cpp/ql/test/query-tests/Best Practices/Unused Entities/UnusedStaticFunctions/UnusedStaticFunctions.expected b/cpp/ql/test/query-tests/Best Practices/Unused Entities/UnusedStaticFunctions/UnusedStaticFunctions.expected index 46a5698c8c6..6409e4adece 100644 --- a/cpp/ql/test/query-tests/Best Practices/Unused Entities/UnusedStaticFunctions/UnusedStaticFunctions.expected +++ b/cpp/ql/test/query-tests/Best Practices/Unused Entities/UnusedStaticFunctions/UnusedStaticFunctions.expected @@ -1,3 +1,5 @@ +| extraction_error.c:4:13:4:43 | my_function2_called_after_error | Static function my_function2_called_after_error is unreachable | extraction_error.c:4:13:4:43 | my_function2_called_after_error | my_function2_called_after_error | +| extraction_error.c:5:13:5:35 | my_function3_not_called | Static function my_function3_not_called is unreachable | extraction_error.c:5:13:5:35 | my_function3_not_called | my_function3_not_called | | unused_functions.c:16:13:16:27 | unused_function | Static function unused_function is unreachable | unused_functions.c:16:13:16:27 | unused_function | unused_function | | unused_functions.c:20:13:20:28 | unused_function2 | Static function unused_function2 is unreachable ($@ must be removed at the same time) | unused_functions.c:24:13:24:28 | unused_function3 | unused_function3 | | unused_functions.c:24:13:24:28 | unused_function3 | Static function unused_function3 is unreachable | unused_functions.c:24:13:24:28 | unused_function3 | unused_function3 | diff --git a/cpp/ql/test/query-tests/Best Practices/Unused Entities/UnusedStaticFunctions/extraction_error.c b/cpp/ql/test/query-tests/Best Practices/Unused Entities/UnusedStaticFunctions/extraction_error.c new file mode 100644 index 00000000000..a5ef25a6921 --- /dev/null +++ b/cpp/ql/test/query-tests/Best Practices/Unused Entities/UnusedStaticFunctions/extraction_error.c @@ -0,0 +1,15 @@ +// semmle-extractor-options: --expect_errors + +static void my_function1_called() {} // GOOD +static void my_function2_called_after_error() {} // GOOD [FALSE POSITIVE] +static void my_function3_not_called() {} // BAD + +int main(void) { + my_function1_called(); + +--- compilation stops here because this line is not valid C code --- + + my_function2_called_after_error(); + + return 0; +} From 2756c0e7afc496079d1051d9a7c3a5d1b31139da Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Tue, 20 Sep 2022 16:24:28 +0100 Subject: [PATCH 2/7] C++: Don't report results in files with compilation errors. --- .../Unused Entities/UnusedStaticFunctions.ql | 29 ++++++++++++++----- .../UnusedStaticFunctions.expected | 2 -- .../UnusedStaticFunctions/extraction_error.c | 4 +-- 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/cpp/ql/src/Best Practices/Unused Entities/UnusedStaticFunctions.ql b/cpp/ql/src/Best Practices/Unused Entities/UnusedStaticFunctions.ql index 418250d15ac..cfca5bad544 100644 --- a/cpp/ql/src/Best Practices/Unused Entities/UnusedStaticFunctions.ql +++ b/cpp/ql/src/Best Practices/Unused Entities/UnusedStaticFunctions.ql @@ -14,15 +14,28 @@ import cpp predicate immediatelyReachableFunction(Function f) { - not f.isStatic() or - exists(BlockExpr be | be.getFunction() = f) or - f instanceof MemberFunction or - f instanceof TemplateFunction or - f.getFile() instanceof HeaderFile or - f.getAnAttribute().hasName("constructor") or - f.getAnAttribute().hasName("destructor") or - f.getAnAttribute().hasName("used") or + not f.isStatic() + or + exists(BlockExpr be | be.getFunction() = f) + or + f instanceof MemberFunction + or + f instanceof TemplateFunction + or + f.getFile() instanceof HeaderFile + or + f.getAnAttribute().hasName("constructor") + or + f.getAnAttribute().hasName("destructor") + or + f.getAnAttribute().hasName("used") + or f.getAnAttribute().hasName("unused") + or + // a compiler error in the same file suggests we may be missing data + exists(Diagnostic d | d.getFile() = f.getFile() and d.getSeverity() >= 3) + or + exists(ErrorExpr ee | ee.getFile() = f.getFile()) } predicate immediatelyReachableVariable(Variable v) { diff --git a/cpp/ql/test/query-tests/Best Practices/Unused Entities/UnusedStaticFunctions/UnusedStaticFunctions.expected b/cpp/ql/test/query-tests/Best Practices/Unused Entities/UnusedStaticFunctions/UnusedStaticFunctions.expected index 6409e4adece..46a5698c8c6 100644 --- a/cpp/ql/test/query-tests/Best Practices/Unused Entities/UnusedStaticFunctions/UnusedStaticFunctions.expected +++ b/cpp/ql/test/query-tests/Best Practices/Unused Entities/UnusedStaticFunctions/UnusedStaticFunctions.expected @@ -1,5 +1,3 @@ -| extraction_error.c:4:13:4:43 | my_function2_called_after_error | Static function my_function2_called_after_error is unreachable | extraction_error.c:4:13:4:43 | my_function2_called_after_error | my_function2_called_after_error | -| extraction_error.c:5:13:5:35 | my_function3_not_called | Static function my_function3_not_called is unreachable | extraction_error.c:5:13:5:35 | my_function3_not_called | my_function3_not_called | | unused_functions.c:16:13:16:27 | unused_function | Static function unused_function is unreachable | unused_functions.c:16:13:16:27 | unused_function | unused_function | | unused_functions.c:20:13:20:28 | unused_function2 | Static function unused_function2 is unreachable ($@ must be removed at the same time) | unused_functions.c:24:13:24:28 | unused_function3 | unused_function3 | | unused_functions.c:24:13:24:28 | unused_function3 | Static function unused_function3 is unreachable | unused_functions.c:24:13:24:28 | unused_function3 | unused_function3 | diff --git a/cpp/ql/test/query-tests/Best Practices/Unused Entities/UnusedStaticFunctions/extraction_error.c b/cpp/ql/test/query-tests/Best Practices/Unused Entities/UnusedStaticFunctions/extraction_error.c index a5ef25a6921..66eedf743fb 100644 --- a/cpp/ql/test/query-tests/Best Practices/Unused Entities/UnusedStaticFunctions/extraction_error.c +++ b/cpp/ql/test/query-tests/Best Practices/Unused Entities/UnusedStaticFunctions/extraction_error.c @@ -1,8 +1,8 @@ // semmle-extractor-options: --expect_errors static void my_function1_called() {} // GOOD -static void my_function2_called_after_error() {} // GOOD [FALSE POSITIVE] -static void my_function3_not_called() {} // BAD +static void my_function2_called_after_error() {} // GOOD +static void my_function3_not_called() {} // BAD [NOT DETECTED] int main(void) { my_function1_called(); From e319c1773e48304610c1aa330121a4e45afc39a1 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Wed, 21 Sep 2022 10:34:36 +0100 Subject: [PATCH 3/7] C++: Change note. --- cpp/ql/src/change-notes/2022-09-21-unused-static-function.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 cpp/ql/src/change-notes/2022-09-21-unused-static-function.md diff --git a/cpp/ql/src/change-notes/2022-09-21-unused-static-function.md b/cpp/ql/src/change-notes/2022-09-21-unused-static-function.md new file mode 100644 index 00000000000..80bd25b7179 --- /dev/null +++ b/cpp/ql/src/change-notes/2022-09-21-unused-static-function.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* Fixed false positives from the "Unused static function" (`cpp/unused-static-function`) query in files that had errors during compilation. From 1cdaaf7882a563ae7dc2f5986d4a2e7b5a195652 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Wed, 21 Sep 2022 11:11:11 +0100 Subject: [PATCH 4/7] C++: Performance fix. --- .../Unused Entities/UnusedStaticFunctions.ql | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/cpp/ql/src/Best Practices/Unused Entities/UnusedStaticFunctions.ql b/cpp/ql/src/Best Practices/Unused Entities/UnusedStaticFunctions.ql index cfca5bad544..242b0cd0683 100644 --- a/cpp/ql/src/Best Practices/Unused Entities/UnusedStaticFunctions.ql +++ b/cpp/ql/src/Best Practices/Unused Entities/UnusedStaticFunctions.ql @@ -13,6 +13,12 @@ import cpp +predicate possiblyIncompleteFile(File f) { + exists(Diagnostic d | d.getFile() = f and d.getSeverity() >= 3) + or + exists(ErrorExpr ee | ee.getFile() = f) +} + predicate immediatelyReachableFunction(Function f) { not f.isStatic() or @@ -33,9 +39,7 @@ predicate immediatelyReachableFunction(Function f) { f.getAnAttribute().hasName("unused") or // a compiler error in the same file suggests we may be missing data - exists(Diagnostic d | d.getFile() = f.getFile() and d.getSeverity() >= 3) - or - exists(ErrorExpr ee | ee.getFile() = f.getFile()) + possiblyIncompleteFile(f.getFile()) } predicate immediatelyReachableVariable(Variable v) { From 0584191b6c349763e8a79513e43c1be71cdb65c7 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Wed, 21 Sep 2022 11:49:28 +0100 Subject: [PATCH 5/7] C++: Add pragma[noinline]. --- .../src/Best Practices/Unused Entities/UnusedStaticFunctions.ql | 1 + 1 file changed, 1 insertion(+) diff --git a/cpp/ql/src/Best Practices/Unused Entities/UnusedStaticFunctions.ql b/cpp/ql/src/Best Practices/Unused Entities/UnusedStaticFunctions.ql index 242b0cd0683..8d5c969f7b7 100644 --- a/cpp/ql/src/Best Practices/Unused Entities/UnusedStaticFunctions.ql +++ b/cpp/ql/src/Best Practices/Unused Entities/UnusedStaticFunctions.ql @@ -13,6 +13,7 @@ import cpp +pragma[noinline] predicate possiblyIncompleteFile(File f) { exists(Diagnostic d | d.getFile() = f and d.getSeverity() >= 3) or From 518b45bc8e8c16516f3fb15cf8019f25992911c5 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Wed, 21 Sep 2022 15:41:27 +0100 Subject: [PATCH 6/7] C++: Add two more test cases. --- .../unused_static_functions.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/cpp/ql/test/query-tests/Best Practices/Unused Entities/UnusedStaticFunctions/unused_static_functions.cpp b/cpp/ql/test/query-tests/Best Practices/Unused Entities/UnusedStaticFunctions/unused_static_functions.cpp index 2984d8f0b1a..c0d83b52a57 100644 --- a/cpp/ql/test/query-tests/Best Practices/Unused Entities/UnusedStaticFunctions/unused_static_functions.cpp +++ b/cpp/ql/test/query-tests/Best Practices/Unused Entities/UnusedStaticFunctions/unused_static_functions.cpp @@ -33,3 +33,16 @@ static void f6(void); static void f5(void) { f6(); } static void f6(void) { f5(); } +// f7 and f8 are reachable from `function_caller` +static int f7() { return 1; } // GOOD +static void f8() { } // GOOD + +void function_caller() +{ + auto my_lambda = []() { + return f7(); + }(); + + f8(); +} + From d60a829569d4d7f6ac893c4a4cddad69978ef4c5 Mon Sep 17 00:00:00 2001 From: Geoffrey White <40627776+geoffw0@users.noreply.github.com> Date: Fri, 23 Sep 2022 12:17:09 +0100 Subject: [PATCH 7/7] C++: Remove ErrorExpr case. --- .../src/Best Practices/Unused Entities/UnusedStaticFunctions.ql | 2 -- 1 file changed, 2 deletions(-) diff --git a/cpp/ql/src/Best Practices/Unused Entities/UnusedStaticFunctions.ql b/cpp/ql/src/Best Practices/Unused Entities/UnusedStaticFunctions.ql index 8d5c969f7b7..514cfac9a81 100644 --- a/cpp/ql/src/Best Practices/Unused Entities/UnusedStaticFunctions.ql +++ b/cpp/ql/src/Best Practices/Unused Entities/UnusedStaticFunctions.ql @@ -16,8 +16,6 @@ import cpp pragma[noinline] predicate possiblyIncompleteFile(File f) { exists(Diagnostic d | d.getFile() = f and d.getSeverity() >= 3) - or - exists(ErrorExpr ee | ee.getFile() = f) } predicate immediatelyReachableFunction(Function f) {