Skip to content

C++: exclude uninitialized uses inside pure expression statements #13647

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 3 commits into from
Jul 11, 2023

Conversation

szsam
Copy link
Contributor

@szsam szsam commented Jul 3, 2023

This eliminates FPs such as casting a variable explicitly to void type. E.g.

   (void) x;

Developers use this cast to supress compiler warnings on unused variables.

@szsam szsam requested a review from a team as a code owner July 3, 2023 06:24
@szsam szsam force-pushed the uninitialized-local branch from 890f8b9 to 5b1f118 Compare July 3, 2023 17:57
@jketema
Copy link
Contributor

jketema commented Jul 5, 2023

This will still need some tests to show that only (void) x is picked up, and not things like x and x + y. The tests can be added to https://github.com/github/codeql/tree/main/cpp/ql/test/query-tests/Security/CWE/CWE-457/semmle/tests

@@ -0,0 +1,4 @@
---
category: majorAnalysis
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
category: majorAnalysis
category: minorAnalysis

I don't think this warrants a majorAnalysis tag. A majorAnalysis change bumps the minor version-number, which I think is a bit too much for this change.

This eliminates FPs caused by casting a variable explicitly to
void type. Developers use this cast to suppress compiler warnings
on unused variables, e.g.
   (void) x;
@szsam
Copy link
Contributor Author

szsam commented Jul 6, 2023

@jketema
Variables are not considered actually used in the toplevel expression of an expression statement. For example, the x in x; is not considered used. The existing uninitialized-local query does not report them.

not e = any(ExprInVoidContext eivc | e = eivc.getConversion*())

/**
* An expression that occurs in a void context, i.e. either as the toplevel expression of
* an expression statement or on the left hand side of the comma operator.
*
* Expressions that are explicitly cast to void are not considered to be in void context.
*/
class ExprInVoidContext extends Expr {
ExprInVoidContext() { exprInVoidContext(this) }
}

However, as stated in the above comment,

Expressions that are explicitly cast to void are not considered to be in void context.

Thus the existing query will treat (void)x; as an uninitialized use.

@szsam szsam force-pushed the uninitialized-local branch from 5b1f118 to 4b4c0cd Compare July 7, 2023 00:11
Co-authored-by: Jeroen Ketema <93738568+jketema@users.noreply.github.com>
Copy link
Contributor

@jketema jketema left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for your contribution!

@jketema jketema merged commit d217e1e into github:main Jul 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants