diff --git a/cpp/common/src/codingstandards/cpp/exclusions/cpp/DeadCode11.qll b/cpp/common/src/codingstandards/cpp/exclusions/cpp/DeadCode11.qll new file mode 100644 index 000000000..915759c49 --- /dev/null +++ b/cpp/common/src/codingstandards/cpp/exclusions/cpp/DeadCode11.qll @@ -0,0 +1,26 @@ +//** THIS FILE IS AUTOGENERATED, DO NOT MODIFY DIRECTLY. **/ +import cpp +import RuleMetadata +import codingstandards.cpp.exclusions.RuleMetadata + +newtype DeadCode11Query = TPotentiallyErroneousContainerUsageQuery() + +predicate isDeadCode11QueryMetadata(Query query, string queryId, string ruleId, string category) { + query = + // `Query` instance for the `potentiallyErroneousContainerUsage` query + DeadCode11Package::potentiallyErroneousContainerUsageQuery() and + queryId = + // `@id` for the `potentiallyErroneousContainerUsage` query + "cpp/misra/potentially-erroneous-container-usage" and + ruleId = "RULE-28-6-4" and + category = "required" +} + +module DeadCode11Package { + Query potentiallyErroneousContainerUsageQuery() { + //autogenerate `Query` type + result = + // `Query` type for `potentiallyErroneousContainerUsage` query + TQueryCPP(TDeadCode11PackageQuery(TPotentiallyErroneousContainerUsageQuery())) + } +} diff --git a/cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll b/cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll index 10f402990..1fea87574 100644 --- a/cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll +++ b/cpp/common/src/codingstandards/cpp/exclusions/cpp/RuleMetadata.qll @@ -17,6 +17,7 @@ import Const import Conversions import Conversions2 import DeadCode +import DeadCode11 import DeadCode3 import DeadCode4 import DeadCode6 @@ -90,6 +91,7 @@ newtype TCPPQuery = TConversionsPackageQuery(ConversionsQuery q) or TConversions2PackageQuery(Conversions2Query q) or TDeadCodePackageQuery(DeadCodeQuery q) or + TDeadCode11PackageQuery(DeadCode11Query q) or TDeadCode3PackageQuery(DeadCode3Query q) or TDeadCode4PackageQuery(DeadCode4Query q) or TDeadCode6PackageQuery(DeadCode6Query q) or @@ -163,6 +165,7 @@ predicate isQueryMetadata(Query query, string queryId, string ruleId, string cat isConversionsQueryMetadata(query, queryId, ruleId, category) or isConversions2QueryMetadata(query, queryId, ruleId, category) or isDeadCodeQueryMetadata(query, queryId, ruleId, category) or + isDeadCode11QueryMetadata(query, queryId, ruleId, category) or isDeadCode3QueryMetadata(query, queryId, ruleId, category) or isDeadCode4QueryMetadata(query, queryId, ruleId, category) or isDeadCode6QueryMetadata(query, queryId, ruleId, category) or diff --git a/cpp/common/test/includes/standard-library/algorithm b/cpp/common/test/includes/standard-library/algorithm index 43534aa5c..589efd296 100644 --- a/cpp/common/test/includes/standard-library/algorithm +++ b/cpp/common/test/includes/standard-library/algorithm @@ -10,9 +10,34 @@ template constexpr const T &min(const T &a, const T &b); template Function for_each(InputIterator first, InputIterator last, Function f); +/** std::remove */ template ForwardIterator remove(ForwardIterator first, ForwardIterator last, const T &value); +template +ForwardIterator remove(ExecutionPolicy &&policy, ForwardIterator first, + ForwardIterator last, const T &value); + +/** std::remove_if */ +template +ForwardIterator remove_if(ForwardIterator first, ForwardIterator last, + Predicate pred); +template +ForwardIterator remove_if(ExecutionPolicy &&policy, ForwardIterator first, + ForwardIterator last, Predicate pred); + +/** std::unique */ +template +ForwardIterator unique(ForwardIterator first, ForwardIterator last); +template +ForwardIterator unique(ExecutionPolicy &&policy, ForwardIterator first, + ForwardIterator last); +template +ForwardIterator unique(ForwardIterator first, ForwardIterator last, + BinaryPredicate pred); +template +ForwardIterator unique(ExecutionPolicy &&policy, ForwardIterator first, + ForwardIterator last, BinaryPredicate pred); template OutputIterator copy(InputIterator first, InputIterator last, diff --git a/cpp/common/test/includes/standard-library/array b/cpp/common/test/includes/standard-library/array index 9465ccba9..5b5ce7569 100644 --- a/cpp/common/test/includes/standard-library/array +++ b/cpp/common/test/includes/standard-library/array @@ -2,6 +2,7 @@ #define _GHLIBCPP_ARRAY #include #include +#include // Note: a few features currently unused by tests are commented out namespace std { diff --git a/cpp/common/test/includes/standard-library/deque.h b/cpp/common/test/includes/standard-library/deque.h index 00b44b704..de0ae4d93 100644 --- a/cpp/common/test/includes/standard-library/deque.h +++ b/cpp/common/test/includes/standard-library/deque.h @@ -2,6 +2,8 @@ #define _GHLIBCPP_DEQUE #include #include +#include +#include namespace std { template > class deque { @@ -11,6 +13,9 @@ template > class deque { typedef value_type &reference; typedef const value_type &const_reference; + deque() = default; + deque(std::initializer_list, const Allocator & = Allocator()); + typedef __iterator iterator; typedef __iterator const_iterator; diff --git a/cpp/common/test/includes/standard-library/empty.h b/cpp/common/test/includes/standard-library/empty.h new file mode 100644 index 000000000..fa741f88a --- /dev/null +++ b/cpp/common/test/includes/standard-library/empty.h @@ -0,0 +1,21 @@ +#ifndef _GHLIBCPP_EMPTY +#define _GHLIBCPP_EMPTY +#include +#include +/** + * Not intended to be used via #include , and not part of the public + * API. + * + * However, multiple libraries such as , , etc define std::empty, + * so this is the singular header to define it in the test suite. + */ + +namespace std { +template constexpr auto empty(const C &c) -> decltype(c.empty()); + +template constexpr bool empty(const T (&)[N]) noexcept; + +template constexpr bool empty(std::initializer_list) noexcept; +} // namespace std + +#endif // _GHLIBCPP_EMPTY \ No newline at end of file diff --git a/cpp/common/test/includes/standard-library/map b/cpp/common/test/includes/standard-library/map index 74d952f55..0a21220c5 100644 --- a/cpp/common/test/includes/standard-library/map +++ b/cpp/common/test/includes/standard-library/map @@ -1,5 +1,6 @@ #include "iterator.h" #include +#include namespace std { diff --git a/cpp/common/test/includes/standard-library/set b/cpp/common/test/includes/standard-library/set index 46a0ab1c3..a5df1ebe4 100644 --- a/cpp/common/test/includes/standard-library/set +++ b/cpp/common/test/includes/standard-library/set @@ -1,5 +1,6 @@ #include "iterator.h" #include +#include namespace std { template , diff --git a/cpp/common/test/includes/standard-library/string b/cpp/common/test/includes/standard-library/string index 3f60c1838..9542b00ff 100644 --- a/cpp/common/test/includes/standard-library/string +++ b/cpp/common/test/includes/standard-library/string @@ -6,6 +6,7 @@ #include "iosfwd.h" #include "iterator.h" #include "stddef.h" +#include namespace std { template struct char_traits { diff --git a/cpp/common/test/includes/standard-library/string_view b/cpp/common/test/includes/standard-library/string_view index 7897d2cf7..d46bc57b4 100644 --- a/cpp/common/test/includes/standard-library/string_view +++ b/cpp/common/test/includes/standard-library/string_view @@ -3,6 +3,7 @@ #define _GHLIBCPP_STRING_VIEW #include "stddef.h" +#include namespace std { diff --git a/cpp/common/test/includes/standard-library/vector.h b/cpp/common/test/includes/standard-library/vector.h index 6d0293f8f..cf9ef1228 100644 --- a/cpp/common/test/includes/standard-library/vector.h +++ b/cpp/common/test/includes/standard-library/vector.h @@ -2,6 +2,7 @@ #define _GHLIBCPP_VECTOR #include #include +#include namespace std { @@ -52,6 +53,11 @@ template > class vector { InputIterator last); iterator insert(const_iterator position, initializer_list il); + iterator erase(iterator position); + iterator erase(const_iterator position); + iterator erase(iterator first, iterator last); + iterator erase(const_iterator first, const_iterator last); + reference operator[](size_type n); const_reference operator[](size_type n) const; diff --git a/cpp/misra/src/rules/RULE-28-6-4/PotentiallyErroneousContainerUsage.ql b/cpp/misra/src/rules/RULE-28-6-4/PotentiallyErroneousContainerUsage.ql new file mode 100644 index 000000000..a1fb83c0d --- /dev/null +++ b/cpp/misra/src/rules/RULE-28-6-4/PotentiallyErroneousContainerUsage.ql @@ -0,0 +1,50 @@ +/** + * @id cpp/misra/potentially-erroneous-container-usage + * @name RULE-28-6-4: The result of std::remove, std::remove_if, std::unique and empty shall be used + * @description The `empty` method may be confused with `clear`, and the family of algorithms + * similar to `std::remove` require the resulting iterator to be used in order to + * properly handle the modifications performed. + * @kind problem + * @precision very-high + * @problem.severity error + * @tags external/misra/id/rule-28-6-4 + * scope/single-translation-unit + * correctness + * external/misra/enforcement/decidable + * external/misra/obligation/required + */ + +import cpp +import codingstandards.cpp.misra +import codingstandards.cpp.Iterators + +predicate isRemoveOrUniqueCall(FunctionCall fc) { + exists(string name | name = fc.getTarget().getName() | + name = "remove" or + name = "remove_if" or + name = "unique" + ) and + fc.getTarget().hasQualifiedName("std", _) and + fc.getAnArgument().getUnderlyingType() instanceof IteratorType +} + +predicate isEmptyCall(FunctionCall fc) { + fc.getTarget().getName() = "empty" and + ( + fc.getTarget().hasQualifiedName("std", "empty") or + fc.getTarget().(MemberFunction).getDeclaringType().hasQualifiedName("std", _) + ) +} + +from FunctionCall fc, string message +where + not isExcluded(fc, DeadCode11Package::potentiallyErroneousContainerUsageQuery()) and + exists(ExprStmt es | es.getExpr() = fc) and + ( + isRemoveOrUniqueCall(fc) and + message = "Result of call to '" + fc.getTarget().getName() + "' is not used." + or + isEmptyCall(fc) and + message = "Result of call to 'empty' is not used." + ) +select fc, message diff --git a/cpp/misra/test/rules/RULE-28-6-4/PotentiallyErroneousContainerUsage.expected b/cpp/misra/test/rules/RULE-28-6-4/PotentiallyErroneousContainerUsage.expected new file mode 100644 index 000000000..3a282d3a8 --- /dev/null +++ b/cpp/misra/test/rules/RULE-28-6-4/PotentiallyErroneousContainerUsage.expected @@ -0,0 +1,10 @@ +| test.cpp:10:3:10:13 | call to remove | Result of call to 'remove' is not used. | +| test.cpp:27:3:27:16 | call to remove_if | Result of call to 'remove_if' is not used. | +| test.cpp:49:3:49:13 | call to unique | Result of call to 'unique' is not used. | +| test.cpp:65:3:65:13 | call to unique | Result of call to 'unique' is not used. | +| test.cpp:82:6:82:10 | call to empty | Result of call to 'empty' is not used. | +| test.cpp:100:3:100:12 | call to empty | Result of call to 'empty' is not used. | +| test.cpp:117:3:117:13 | call to remove | Result of call to 'remove' is not used. | +| test.cpp:122:6:122:10 | call to empty | Result of call to 'empty' is not used. | +| test.cpp:148:21:148:31 | call to remove | Result of call to 'remove' is not used. | +| test.cpp:149:9:149:19 | call to remove | Result of call to 'remove' is not used. | diff --git a/cpp/misra/test/rules/RULE-28-6-4/PotentiallyErroneousContainerUsage.qlref b/cpp/misra/test/rules/RULE-28-6-4/PotentiallyErroneousContainerUsage.qlref new file mode 100644 index 000000000..136cb9e32 --- /dev/null +++ b/cpp/misra/test/rules/RULE-28-6-4/PotentiallyErroneousContainerUsage.qlref @@ -0,0 +1 @@ +rules/RULE-28-6-4/PotentiallyErroneousContainerUsage.ql \ No newline at end of file diff --git a/cpp/misra/test/rules/RULE-28-6-4/test.cpp b/cpp/misra/test/rules/RULE-28-6-4/test.cpp new file mode 100644 index 000000000..562e3d5a0 --- /dev/null +++ b/cpp/misra/test/rules/RULE-28-6-4/test.cpp @@ -0,0 +1,169 @@ +#include +#include +#include +#include +#include + +// Test cases for std::remove +void test_remove_result_unused() { + std::vector v1 = {1, 2, 3, 2, 4}; + std::remove(v1.begin(), v1.end(), 2); // NON_COMPLIANT +} + +void test_remove_result_used() { + std::vector v1 = {1, 2, 3, 2, 4}; + auto l1 = std::remove(v1.begin(), v1.end(), 2); // COMPLIANT + v1.erase(l1, v1.end()); +} + +void test_remove_result_used_in_erase() { + std::vector v1 = {1, 2, 3, 2, 4}; + v1.erase(std::remove(v1.begin(), v1.end(), 2), v1.end()); // COMPLIANT +} + +// Test cases for std::remove_if +void test_remove_if_result_unused() { + std::vector v1 = {1, 2, 3, 4, 5}; + std::remove_if(v1.begin(), v1.end(), + [](std::int32_t l1) { return l1 % 2 == 0; }); // NON_COMPLIANT +} + +void test_remove_if_result_used() { + std::vector v1 = {1, 2, 3, 4, 5}; + auto l1 = std::remove_if(v1.begin(), v1.end(), [](std::int32_t l2) { + return l2 % 2 == 0; + }); // COMPLIANT + v1.erase(l1, v1.end()); +} + +void test_remove_if_result_used_in_erase() { + std::vector v1 = {1, 2, 3, 4, 5}; + v1.erase(std::remove_if(v1.begin(), v1.end(), + [](std::int32_t l1) { return l1 % 2 == 0; }), + v1.end()); // COMPLIANT +} + +// Test cases for std::unique +void test_unique_result_unused() { + std::vector v1 = {0, 0, 1, 1, 2, 2, 3, 3}; + std::unique(v1.begin(), v1.end()); // NON_COMPLIANT +} + +void test_unique_result_used() { + std::vector v1 = {0, 0, 1, 1, 2, 2, 3, 3}; + auto l1 = std::unique(v1.begin(), v1.end()); // COMPLIANT + v1.erase(l1, v1.end()); +} + +void test_unique_result_used_in_erase() { + std::vector v1 = {0, 0, 1, 1, 2, 2, 3, 3}; + v1.erase(std::unique(v1.begin(), v1.end()), v1.end()); // COMPLIANT +} + +void test_unique_with_predicate_result_unused() { + std::vector v1 = {1, 2, 3, 4, 5, 6}; + std::unique(v1.begin(), v1.end(), [](std::int32_t l1, std::int32_t l2) { + return (l1 % 2) == (l2 % 2); + }); // NON_COMPLIANT +} + +void test_unique_with_predicate_result_used() { + std::vector v1 = {1, 2, 3, 4, 5, 6}; + auto l1 = + std::unique(v1.begin(), v1.end(), [](std::int32_t l2, std::int32_t l3) { + return (l2 % 2) == (l3 % 2); + }); // COMPLIANT + v1.erase(l1, v1.end()); +} + +// Test cases for empty (member function) +void test_empty_member_result_unused() { + std::vector v1 = {1, 2, 3}; + v1.empty(); // NON_COMPLIANT +} + +void test_empty_member_result_used_in_condition() { + std::vector v1 = {1, 2, 3}; + if (v1.empty()) { // COMPLIANT + v1.clear(); + } +} + +void test_empty_member_result_used_in_assignment() { + std::vector v1 = {1, 2, 3}; + bool l1 = v1.empty(); // COMPLIANT +} + +// Test cases for std::empty (non-member function) +void test_empty_nonmember_result_unused() { + std::vector v1 = {1, 2, 3}; + std::empty(v1); // NON_COMPLIANT +} + +void test_empty_nonmember_result_used_in_condition() { + std::vector v1 = {1, 2, 3}; + if (!std::empty(v1)) { // COMPLIANT + v1.clear(); + } +} + +void test_empty_nonmember_result_used_in_assignment() { + std::vector v1 = {1, 2, 3}; + bool l1 = std::empty(v1); // COMPLIANT +} + +void test_remove_with_deque() { + std::deque d1 = {1, 2, 3, 2, 4}; + std::remove(d1.begin(), d1.end(), 2); // NON_COMPLIANT +} + +void test_empty_with_string() { + std::string s1 = "hello"; + s1.empty(); // NON_COMPLIANT +} + +void test_empty_with_string_compliant() { + std::string s1 = "hello"; + if (s1.empty()) { // COMPLIANT + s1 = "default"; + } +} + +// Edge case: result stored but not used +void test_remove_result_stored_but_not_used() { + std::vector v1 = {1, 2, 3, 2, 4}; + auto l1 = std::remove(v1.begin(), v1.end(), 2); // COMPLIANT +} + +// Edge case: chaining operations +void test_remove_chained_operations() { + std::vector v1 = {1, 2, 3, 2, 4}; + std::vector v2 = {5, 6, 7}; + v1.erase(std::remove(v1.begin(), v1.end(), 2), v1.end()); // COMPLIANT + v2.erase(std::remove(v2.begin(), v2.end(), 6), v2.end()); // COMPLIANT +} + +void test_cast_to_void() { + std::vector v1 = {1, 2, 3, 2, 4}; + static_cast(std::remove(v1.begin(), v1.end(), 2)); // NON_COMPLIANT + (void)std::remove(v1.begin(), v1.end(), 2); // NON_COMPLIANT +} + +void test_unrelated_empty() { + class MyClass { + std::vector v1; + + public: + void empty() { v1.clear(); } + }; + + MyClass c1; + c1.empty(); // COMPLIANT - empty is not a member function of a standard +} + +#include +void test_cstdio_remove() { + std::remove("filename.txt"); // COMPLIANT - std::remove from + remove("filename.txt"); // COMPLIANT - std::remove from with using + // directive +} \ No newline at end of file diff --git a/rule_packages/cpp/DeadCode11.json b/rule_packages/cpp/DeadCode11.json new file mode 100644 index 000000000..174a38ff4 --- /dev/null +++ b/rule_packages/cpp/DeadCode11.json @@ -0,0 +1,25 @@ +{ + "MISRA-C++-2023": { + "RULE-28-6-4": { + "properties": { + "enforcement": "decidable", + "obligation": "required" + }, + "queries": [ + { + "description": "The `empty` method may be confused with `clear`, and the family of algorithms similar to `std::remove` require the resulting iterator to be used in order to properly handle the modifications performed.", + "kind": "problem", + "name": "The result of std::remove, std::remove_if, std::unique and empty shall be used", + "precision": "very-high", + "severity": "error", + "short_name": "PotentiallyErroneousContainerUsage", + "tags": [ + "scope/single-translation-unit", + "correctness" + ] + } + ], + "title": "The result of std::remove, std::remove_if, std::unique and empty shall be used" + } + } +} \ No newline at end of file diff --git a/rules.csv b/rules.csv index 08fa09a57..1b27b197b 100644 --- a/rules.csv +++ b/rules.csv @@ -999,6 +999,6 @@ cpp,MISRA-C++-2023,RULE-28-3-1,Yes,Required,Undecidable,System,Predicates shall cpp,MISRA-C++-2023,RULE-28-6-1,Yes,Required,Decidable,Single Translation Unit,The argument to std::move shall be a non-const lvalue,A18-9-3,Preconditions,Easy, cpp,MISRA-C++-2023,RULE-28-6-2,Yes,Required,Decidable,Single Translation Unit,Forwarding references and std::forward shall be used together,A18-9-2,ImportMisra23,Import, cpp,MISRA-C++-2023,RULE-28-6-3,Yes,Required,Decidable,Single Translation Unit,An object shall not be used while in a potentially moved-from state,A12-8-3,ImportMisra23,Import, -cpp,MISRA-C++-2023,RULE-28-6-4,Yes,Required,Decidable,Single Translation Unit,"The result of std::remove, std::remove_if, std::unique and empty shall be used",,DeadCode2,Easy, +cpp,MISRA-C++-2023,RULE-28-6-4,Yes,Required,Decidable,Single Translation Unit,"The result of std::remove, std::remove_if, std::unique and empty shall be used",,DeadCode11,Easy, cpp,MISRA-C++-2023,RULE-30-0-1,Yes,Required,Decidable,Single Translation Unit,The C Library input/output functions shall not be used,M27-0-1,ImportMisra23,Import, cpp,MISRA-C++-2023,RULE-30-0-2,Yes,Required,Undecidable,System,Reads and writes on the same file stream shall be separated by a positioning operation,A27-0-3,ImportMisra23,Import,