Skip to content

Sorting out false positives for A0-1-2 and A0-1-4 #181

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 28 commits into from
Feb 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
7462ec8
create test.cpp
jeongsoolee09 Jan 30, 2023
3bdc76d
delete test.cpp
jeongsoolee09 Jan 30, 2023
47805d8
update
jeongsoolee09 Jan 30, 2023
aca8fdc
checkpoint: A0-1-2
jeongsoolee09 Jan 31, 2023
54886ef
update test.cpp and tuple.h
jeongsoolee09 Feb 1, 2023
7b93bb8
First draft of UnusedReturnValue
jeongsoolee09 Feb 3, 2023
c59d66e
minor formatting for qldoc
jeongsoolee09 Feb 3, 2023
714e04c
revise A0-1-2 to also match VoidType
jeongsoolee09 Feb 3, 2023
b1e204d
minor comment fix
jeongsoolee09 Feb 3, 2023
a1eed75
add test.cpp for A0-1-4
jeongsoolee09 Feb 3, 2023
09ab74e
checkpoint: attempt to catch unused parameters in lambdaexpr
jeongsoolee09 Feb 3, 2023
23a1bc3
Finalize A0-1-4 except finding a good fName
jeongsoolee09 Feb 3, 2023
51ff26c
clang-format
jeongsoolee09 Feb 3, 2023
1ff8c86
format UnusedReturnValue.ql
jeongsoolee09 Feb 3, 2023
0a96640
clean up namespace, distinguish between lambda param and function params
jeongsoolee09 Feb 6, 2023
84176bc
factor out duplicate conditions and update .expected for A0-1-4
jeongsoolee09 Feb 6, 2023
8c6c581
update tuple.h
jeongsoolee09 Feb 6, 2023
d15b6db
Merge branch 'main' into jeongsoolee09/a0-1-2_and_a0-1-4
jeongsoolee09 Feb 6, 2023
a1740e7
Add bitwise-shift-left-assignment operator
jeongsoolee09 Feb 7, 2023
058ccb4
Make query for A0-1-2 more concise
jeongsoolee09 Feb 7, 2023
b89c564
Remove lambda checks on UnusedParameters.qll
jeongsoolee09 Feb 7, 2023
1212108
Remove assignment to std::ignore
jeongsoolee09 Feb 7, 2023
4cd1fc4
Merge branch 'main' into jeongsoolee09/a0-1-2_and_a0-1-4
jeongsoolee09 Feb 14, 2023
9a9c531
Incorporating suggestions (1): compressing formulas into one
jeongsoolee09 Feb 16, 2023
b14b55c
Incorporating suggestions (2): updating comments
jeongsoolee09 Feb 16, 2023
b59d5e9
Incorporating suggestions (3): delete redundant comment
jeongsoolee09 Feb 16, 2023
611056c
Merge branch 'jeongsoolee09/a0-1-2_and_a0-1-4' of github.com:github/c…
jeongsoolee09 Feb 16, 2023
349f84f
Merge branch 'main' into jeongsoolee09/a0-1-2_and_a0-1-4
jeongsoolee09 Feb 22, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions cpp/autosar/src/rules/A0-1-2/UnusedReturnValue.ql
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@

import cpp
import codingstandards.cpp.autosar
import semmle.code.cpp.dataflow.DataFlow
import codingstandards.cpp.Operator
import cpp

/*
* This query performs a simple syntactic check to ensure that the return value of the function is
Expand All @@ -39,8 +40,11 @@ where
// so the rule does not require the use of the return value
not f instanceof Operator and
// Exclude cases where the function call is generated within a macro, as the user of the macro is
// not necessarily able to address thoes results
// not necessarily able to address those results
not fc.isAffectedByMacro() and
// Rule allows disabling this rule where a static_cast<void> is applied
not fc.getExplicitlyConverted().(StaticCast).getActualType() instanceof VoidType
// Rule allows disabling this rule where a static_cast<void> or a C-style cast to void is applied
not exists(Cast cast | cast instanceof StaticCast or cast instanceof CStyleCast |
fc.getExplicitlyConverted() = cast and
cast.getActualType() instanceof VoidType
)
select fc, "Return value from call to $@ is unused.", f, f.getName()
2 changes: 1 addition & 1 deletion cpp/autosar/test/rules/A0-1-2/UnusedReturnValue.expected
Original file line number Diff line number Diff line change
@@ -1 +1 @@
| test.cpp:10:3:10:3 | call to f | Return value from call to $@ is unused. | test.cpp:1:5:1:5 | f | f |
| test.cpp:12:3:12:3 | call to f | Return value from call to $@ is unused. | test.cpp:3:5:3:5 | f | f |
12 changes: 10 additions & 2 deletions cpp/autosar/test/rules/A0-1-2/test.cpp
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#include <tuple>

int f();
void g(int x);

Expand All @@ -8,7 +10,8 @@ class A {

void test_return_val() {
f(); // NON_COMPLIANT - return value never read
static_cast<void>(f()); // COMPLIANT
static_cast<void>(f()); // COMPLIANT - explicitly ignoring the return value by
// static_cast to void.
int x = f(); // COMPLIANT - according to the rule, even though it's not in
// practice used because the unused assignment would be flagged
// by A0-1-1
Expand All @@ -17,4 +20,9 @@ void test_return_val() {
A a2;
a1 + a2; // COMPLIANT - `+` is a call to operator+, but is permitted by the
// rule
}

(void)f(); // COMPLIANT - explicitly ignoring the return value by C-style cast
// to void.
std::ignore = f(); // COMPLIANT - explicitly ignoring the return value by
// assigning to std::ignore.
}
8 changes: 4 additions & 4 deletions cpp/common/src/codingstandards/cpp/Operator.qll
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ class AnyAssignOperation extends Expr {
AnyAssignOperation() {
this instanceof AssignOperation
or
// operator op, where op is +=, -=, *=, /=, %=, ^=, &=, |=, >>=
// operator op, where op is +=, -=, *=, /=, %=, ^=, &=, |=, >>=, <<=
exists(string op |
"operator" + op = this.(FunctionCall).getTarget().getName() and
op in ["+=", "-=", "*=", "/=", "%=", "^=", "&=", "|=", ">>="]
op in ["+=", "-=", "*=", "/=", "%=", "^=", "&=", "|=", ">>=", "<<="]
)
}
}
Expand Down Expand Up @@ -121,10 +121,10 @@ class UserAssignmentOperator extends AssignmentOperator {
/** An assignment operator of any sort */
class AssignmentOperator extends MemberFunction {
AssignmentOperator() {
// operator op, where op is =, +=, -=, *=, /=, %=, ^=, &=, |=, >>=
// operator op, where op is =, +=, -=, *=, /=, %=, ^=, &=, |=, >>=, <<=
exists(string op |
"operator" + op = this.getName() and
op in ["=", "+=", "-=", "*=", "/=", "%=", "^=", "&=", "|=", ">>="]
op in ["=", "+=", "-=", "*=", "/=", "%=", "^=", "&=", "|=", ">>=", "<<="]
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,20 @@ abstract class UnusedParameterSharedQuery extends Query { }

Query getQuery() { result instanceof UnusedParameterSharedQuery }

predicate isMaybeUnusedParameter(Parameter parameter) {
parameter.getAnAttribute().toString() = "maybe_unused"
}

predicate isLambdaParameter(Parameter parameter) {
exists(LambdaExpression lambda | lambda.getLambdaFunction().getParameter(_) = parameter)
}

query predicate problems(UnusedParameter p, string message, Function f, string fName) {
not isExcluded(p, getQuery()) and
not isMaybeUnusedParameter(p) and
(if isLambdaParameter(p) then fName = "lambda expression" else fName = f.getQualifiedName()) and
f = p.getFunction() and
// Virtual functions are covered by a different rule
not f.isVirtual() and
message = "Unused parameter '" + p.getName() + "' for function $@." and
fName = f.getQualifiedName()
message = "Unused parameter '" + p.getName() + "' for function $@."
}
7 changes: 7 additions & 0 deletions cpp/common/test/includes/standard-library/tuple.h
Original file line number Diff line number Diff line change
@@ -1,4 +1,11 @@
namespace std {
template <class... Types> class tuple {};
template <class... Types> std::tuple<Types...> make_tuple(Types &&...args);
struct ignore_t {
template <typename T>
constexpr // required since C++14
void
operator=(T &&) const noexcept {}
};
inline const std::ignore_t ignore; // 'const' only until C++17
} // namespace std
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
| test.cpp:6:22:6:22 | x | Unused parameter 'x' for function $@. | test.cpp:6:6:6:16 | test_unused | test_unused |
| test.cpp:14:14:14:14 | x | Unused parameter 'x' for function $@. | test.cpp:14:8:14:8 | b | A::b |
| test.cpp:8:22:8:22 | x | Unused parameter 'x' for function $@. | test.cpp:8:6:8:16 | test_unused | test_unused |
| test.cpp:16:14:16:14 | x | Unused parameter 'x' for function $@. | test.cpp:16:8:16:8 | b | A::b |
| test.cpp:35:14:35:14 | y | Unused parameter 'y' for function $@. | test.cpp:34:9:34:9 | operator() | lambda expression |
29 changes: 28 additions & 1 deletion cpp/common/test/rules/unusedparameter/test.cpp
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
// NOTICE: SOME OF THE TEST CASES BELOW ARE ALSO INCLUDED IN THE C TEST CASE AND
// CHANGES SHOULD BE REFLECTED THERE AS WELL.

#include <tuple>

int test_used(int x) { return x; } // COMPLIANT

void test_unused(int x) {} // NON_COMPLIANT
Expand All @@ -16,4 +18,29 @@ class A {
virtual void d(int x, int y) {} // virtual, not covered by this rule
};

void test_no_def(int x); // COMPLIANT - no definition, so cannot be "unused"
void f(
int i, // COMPLIANT
int j, // COMPLIANT
int k, // COMPLIANT
[[maybe_unused]] int l // COMPLIANT: explicitly stated as [[maybe_unused]]
) {
static_cast<void>(i); // COMPLIANT: explicitly ignored by static_cast to void
(void)j; // COMPLIANT: explicitly ignored by c-style cast to void
std::ignore = k; // COMPLIANT: explicitly ignored by assignment to std::ignore
}

void test_lambda_expr() {
auto lambda =
[](int x, // COMPLIANT: used
int y, // NON_COMPLIANT: unused without explicit notice
[[maybe_unused]] int z, // COMPLIANT: stdattribute [[maybe_unused]]
int w, // COMPLIANT: static_cast to void
int u, // COMPLIANT: c-style cast to void
int) { // COMPLIANT: unnamed parameter
static_cast<void>(w);
(void)u;
return x;
};
}

void test_no_def(int x); // COMPLIANT - no definition, so cannot be "unused"