[PATCH] D42645: New simple Checker for mmap calls
Aleksei Sidorin via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Jan 29 09:37:41 PST 2018
a.sidorin added a comment.
Hello David,
Do you have any results of this checker on the real code? If yes, could you please share them?
There are also some inline comments regarding implementation.
================
Comment at: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:42
+
+void MmapWriteExecChecker::checkPostStmt(const CallExpr *CE,
+ CheckerContext &C) const {
----------------
For analysis of function arguments, PreCall callback is more suitable because it is called earlier.
================
Comment at: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:49
+ if (FName == "mmap") {
+ SVal ProtVal = C.getSVal(CE->getArg(2));
+ Optional<nonloc::ConcreteInt> Prot = ProtVal.getAs<nonloc::ConcreteInt>();
----------------
We need to check that the function call has at least three arguments to avoid crashing on weird redeclarations like `void mmap()`.
================
Comment at: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:51
+ Optional<nonloc::ConcreteInt> Prot = ProtVal.getAs<nonloc::ConcreteInt>();
+ int64_t prot = Prot->getValue().getSExtValue();
+
----------------
Please follow LLVM naming conventions: variable names should start with capital.
================
Comment at: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:54
+ if ((prot & (ProtWrite | ProtExec)) == (ProtWrite | ProtExec)) {
+ if (!BT) {
+ BT.reset(new BugType(this, "W^X check fail", "Write Exec prot flags set"));
----------------
Looks like this check is written in the way that allows to emit warning only once. Did you mean:
```
if (!BT)
BT.reset(new BugType(this, "W^X check fail", "Write Exec prot flags );
ExplodedNode *N = C.generateErrorNode();
if (!N)
return;
...
```
?
================
Comment at: lib/StaticAnalyzer/Checkers/MmapWriteExecChecker.cpp:55
+ if (!BT) {
+ BT.reset(new BugType(this, "W^X check fail", "Write Exec prot flags set"));
+ ExplodedNode *N = C.generateErrorNode();
----------------
Could you please give more informative warning message describing the situation and why is it harmful? Note that if user has more information, he can fix the problem faster.
Repository:
rC Clang
https://reviews.llvm.org/D42645
More information about the cfe-commits
mailing list