-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[clang][analyzer] Add StoreToImmutable checker #150417
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
Changes from 1 commit
0630d81
fa3f84f
b022182
5190ee0
856a865
d8f3456
01d0521
2aacf92
6e8a332
d65aa88
7e94b10
4db2804
7e73177
cac94fe
373679b
7cbabf8
e377b19
cfedf88
fa0a379
4e6c988
17a0e9c
89389a5
4d883f1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -11,6 +11,7 @@ | |||||||||||||||||
| // | ||||||||||||||||||
| //===----------------------------------------------------------------------===// | ||||||||||||||||||
|
|
||||||||||||||||||
| #include "clang/AST/ParentMap.h" | ||||||||||||||||||
| #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h" | ||||||||||||||||||
| #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h" | ||||||||||||||||||
| #include "clang/StaticAnalyzer/Core/Checker.h" | ||||||||||||||||||
|
|
@@ -29,11 +30,59 @@ class StoreToImmutableChecker : public Checker<check::Bind> { | |||||||||||||||||
| void checkBind(SVal Loc, SVal Val, const Stmt *S, CheckerContext &C) const; | ||||||||||||||||||
|
|
||||||||||||||||||
| private: | ||||||||||||||||||
| bool isInitializationContext(const Stmt *S, CheckerContext &C) const; | ||||||||||||||||||
| bool isEffectivelyConstRegion(const MemRegion *MR, CheckerContext &C) const; | ||||||||||||||||||
| bool isConstQualifiedType(const MemRegion *MR, CheckerContext &C) const; | ||||||||||||||||||
| }; | ||||||||||||||||||
| } // end anonymous namespace | ||||||||||||||||||
|
|
||||||||||||||||||
| bool StoreToImmutableChecker::isInitializationContext(const Stmt *S, | ||||||||||||||||||
| CheckerContext &C) const { | ||||||||||||||||||
| // Check if this is a DeclStmt (variable declaration) | ||||||||||||||||||
| if (isa<DeclStmt>(S)) | ||||||||||||||||||
| return true; | ||||||||||||||||||
|
|
||||||||||||||||||
| // This part is specific for initialization of const lambdas pre-C++17. | ||||||||||||||||||
| // Lets look at the AST of the statement: | ||||||||||||||||||
| // ``` | ||||||||||||||||||
| // const auto lambda = [](){}; | ||||||||||||||||||
| // ``` | ||||||||||||||||||
| // | ||||||||||||||||||
| // The relevant part of the AST for this case prior to C++17 is: | ||||||||||||||||||
| // ... | ||||||||||||||||||
| // `-DeclStmt | ||||||||||||||||||
| // `-VarDecl | ||||||||||||||||||
| // `-ExprWithCleanups | ||||||||||||||||||
| // `-CXXConstructExpr | ||||||||||||||||||
| // ... | ||||||||||||||||||
| // In C++17 and later, the AST is different: | ||||||||||||||||||
| // ... | ||||||||||||||||||
| // `-DeclStmt | ||||||||||||||||||
| // `-VarDecl | ||||||||||||||||||
| // `-ImplicitCastExpr | ||||||||||||||||||
| // `-LambdaExpr | ||||||||||||||||||
| // |-CXXRecordDecl | ||||||||||||||||||
| // `-CXXConstructExpr | ||||||||||||||||||
| // ... | ||||||||||||||||||
| // And even beside this, the statement `S` that is given to the checkBind | ||||||||||||||||||
| // callback is the VarDecl in C++17 and later, and the CXXConstructExpr in | ||||||||||||||||||
| // C++14 and before. So in order to support the C++14 we need the following | ||||||||||||||||||
| // ugly hack to detect whether this construction is used to initialize a | ||||||||||||||||||
| // variable. | ||||||||||||||||||
| // | ||||||||||||||||||
| // FIXME: This should be eliminated once the API of checkBind would allow to | ||||||||||||||||||
| // distinguish between initialization and assignment, because this information | ||||||||||||||||||
| // is already available in the engine, it is just not passed to the checker | ||||||||||||||||||
| // API. | ||||||||||||||||||
|
||||||||||||||||||
| // FIXME: This should be eliminated once the API of checkBind would allow to | |
| // distinguish between initialization and assignment, because this information | |
| // is already available in the engine, it is just not passed to the checker | |
| // API. | |
| // FIXME: This should be eliminated by improving the API of checkBind to | |
| // ensure that it consistently passes the `VarDecl` (instead of the | |
| // `CXXConstructExpr`) when the constructor call denotes the initialization | |
| // of a variable with a lambda. |
As we discussed in person, the most probable solution to this corner case is slightly different from what you originally wrote here. I hope that this will become irrelevant soon (if you can restore consistency in the engine), but it is still slightly better to document this more accurately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added this with a sidenote.
Uh oh!
There was an error while loading. Please reload this page.