Szelethus created this revision.
Szelethus added reviewers: NoQ, george.karpenkov, xazax.hun, MTC.
Herald added subscribers: cfe-commits, gamesh411, dkrupp, donat.nagy, jfb,
mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, whisperity.
The actual implementation of `unix.API` features a dual-checker: two checkers
in one, even though they don't even interact at all. Split them up, as this is
a problem for establishing dependencies.
Since the plist files change (and that's a benefit!) this patch isn't NFC.
Repository:
rC Clang
https://reviews.llvm.org/D55425
Files:
lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
test/Analysis/Inputs/expected-plists/unix-fns.c.plist
Index: test/Analysis/Inputs/expected-plists/unix-fns.c.plist
===================================================================
--- test/Analysis/Inputs/expected-plists/unix-fns.c.plist
+++ test/Analysis/Inputs/expected-plists/unix-fns.c.plist
@@ -3,7 +3,7 @@
<plist version="1.0">
<dict>
<key>clang_version</key>
-<string>clang version 8.0.0 </string>
+<string>clang version 8.0.0</string>
<key>diagnostics</key>
<array>
<dict>
@@ -744,9 +744,9 @@
<key>description</key><string>Call to 'malloc' has an allocation size of 0 bytes</string>
<key>category</key><string>Unix API</string>
<key>type</key><string>Undefined allocation of 0 bytes (CERT MEM04-C; CWE-131)</string>
- <key>check_name</key><string>unix.API</string>
+ <key>check_name</key><string>optin.portability.UnixAPI</string>
<!-- This hash is experimental and going to change! -->
- <key>issue_hash_content_of_line_in_context</key><string>0e841458f0cb7cf161d35f9db5862dcf</string>
+ <key>issue_hash_content_of_line_in_context</key><string>4ddbefeb3fa802a0636dc63d679bdc89</string>
<key>issue_context_kind</key><string>function</string>
<key>issue_context</key><string>pr2899</string>
<key>issue_hash_function_offset</key><string>1</string>
@@ -835,9 +835,9 @@
<key>description</key><string>Call to 'calloc' has an allocation size of 0 bytes</string>
<key>category</key><string>Unix API</string>
<key>type</key><string>Undefined allocation of 0 bytes (CERT MEM04-C; CWE-131)</string>
- <key>check_name</key><string>unix.API</string>
+ <key>check_name</key><string>optin.portability.UnixAPI</string>
<!-- This hash is experimental and going to change! -->
- <key>issue_hash_content_of_line_in_context</key><string>a267ff573c7e8b959a3f886677893eb0</string>
+ <key>issue_hash_content_of_line_in_context</key><string>9f12ad2f0a645cb7e4485fed526f536e</string>
<key>issue_context_kind</key><string>function</string>
<key>issue_context</key><string>test_calloc</string>
<key>issue_hash_function_offset</key><string>1</string>
@@ -926,9 +926,9 @@
<key>description</key><string>Call to 'calloc' has an allocation size of 0 bytes</string>
<key>category</key><string>Unix API</string>
<key>type</key><string>Undefined allocation of 0 bytes (CERT MEM04-C; CWE-131)</string>
- <key>check_name</key><string>unix.API</string>
+ <key>check_name</key><string>optin.portability.UnixAPI</string>
<!-- This hash is experimental and going to change! -->
- <key>issue_hash_content_of_line_in_context</key><string>14eb72957baab3c63bac610a10e6f48b</string>
+ <key>issue_hash_content_of_line_in_context</key><string>835b2375daee5b05ac48f24ac578de4c</string>
<key>issue_context_kind</key><string>function</string>
<key>issue_context</key><string>test_calloc2</string>
<key>issue_hash_function_offset</key><string>1</string>
@@ -1017,9 +1017,9 @@
<key>description</key><string>Call to 'realloc' has an allocation size of 0 bytes</string>
<key>category</key><string>Unix API</string>
<key>type</key><string>Undefined allocation of 0 bytes (CERT MEM04-C; CWE-131)</string>
- <key>check_name</key><string>unix.API</string>
+ <key>check_name</key><string>optin.portability.UnixAPI</string>
<!-- This hash is experimental and going to change! -->
- <key>issue_hash_content_of_line_in_context</key><string>7f6f67ebe3d481aed7750005bea7e371</string>
+ <key>issue_hash_content_of_line_in_context</key><string>bbdabcb6c5a3783012ae34bfea2a10fb</string>
<key>issue_context_kind</key><string>function</string>
<key>issue_context</key><string>test_realloc</string>
<key>issue_hash_function_offset</key><string>1</string>
@@ -1108,9 +1108,9 @@
<key>description</key><string>Call to 'reallocf' has an allocation size of 0 bytes</string>
<key>category</key><string>Unix API</string>
<key>type</key><string>Undefined allocation of 0 bytes (CERT MEM04-C; CWE-131)</string>
- <key>check_name</key><string>unix.API</string>
+ <key>check_name</key><string>optin.portability.UnixAPI</string>
<!-- This hash is experimental and going to change! -->
- <key>issue_hash_content_of_line_in_context</key><string>4941698efbd81601653dff10ef9c645b</string>
+ <key>issue_hash_content_of_line_in_context</key><string>5d222055bbf58b08ec345f0ebfd7b9d1</string>
<key>issue_context_kind</key><string>function</string>
<key>issue_context</key><string>test_reallocf</string>
<key>issue_hash_function_offset</key><string>1</string>
@@ -1199,9 +1199,9 @@
<key>description</key><string>Call to 'alloca' has an allocation size of 0 bytes</string>
<key>category</key><string>Unix API</string>
<key>type</key><string>Undefined allocation of 0 bytes (CERT MEM04-C; CWE-131)</string>
- <key>check_name</key><string>unix.API</string>
+ <key>check_name</key><string>optin.portability.UnixAPI</string>
<!-- This hash is experimental and going to change! -->
- <key>issue_hash_content_of_line_in_context</key><string>b7ca3488e81d9d9d4b8dc545258ce97c</string>
+ <key>issue_hash_content_of_line_in_context</key><string>f7bdefde93c0a58ec236918fb0c3a54e</string>
<key>issue_context_kind</key><string>function</string>
<key>issue_context</key><string>test_alloca</string>
<key>issue_hash_function_offset</key><string>1</string>
@@ -1290,9 +1290,9 @@
<key>description</key><string>Call to 'alloca' has an allocation size of 0 bytes</string>
<key>category</key><string>Unix API</string>
<key>type</key><string>Undefined allocation of 0 bytes (CERT MEM04-C; CWE-131)</string>
- <key>check_name</key><string>unix.API</string>
+ <key>check_name</key><string>optin.portability.UnixAPI</string>
<!-- This hash is experimental and going to change! -->
- <key>issue_hash_content_of_line_in_context</key><string>1ec52551362b070237f47f6bb6c3847d</string>
+ <key>issue_hash_content_of_line_in_context</key><string>4247526f8da82479508f2d364c2992d5</string>
<key>issue_context_kind</key><string>function</string>
<key>issue_context</key><string>test_builtin_alloca</string>
<key>issue_hash_function_offset</key><string>1</string>
@@ -1381,9 +1381,9 @@
<key>description</key><string>Call to 'valloc' has an allocation size of 0 bytes</string>
<key>category</key><string>Unix API</string>
<key>type</key><string>Undefined allocation of 0 bytes (CERT MEM04-C; CWE-131)</string>
- <key>check_name</key><string>unix.API</string>
+ <key>check_name</key><string>optin.portability.UnixAPI</string>
<!-- This hash is experimental and going to change! -->
- <key>issue_hash_content_of_line_in_context</key><string>675741e04c8d0071d280324e23f41d35</string>
+ <key>issue_hash_content_of_line_in_context</key><string>e16dfa9598fd2fafe6dc5563990c1dd3</string>
<key>issue_context_kind</key><string>function</string>
<key>issue_context</key><string>test_valloc</string>
<key>issue_hash_function_offset</key><string>1</string>
@@ -3015,7 +3015,7 @@
</array>
<key>files</key>
<array>
- <string>/clang/test/Analysis/unix-fns.c</string>
+ <string>/home/szelethus/Documents/analyzer_opts/clang/test/Analysis/unix-fns.c</string>
</array>
</dict>
</plist>
Index: lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
@@ -21,10 +21,7 @@
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallString.h"
-#include "llvm/ADT/StringExtras.h"
-#include "llvm/ADT/StringSwitch.h"
#include "llvm/Support/raw_ostream.h"
-#include <fcntl.h>
using namespace clang;
using namespace ento;
@@ -40,8 +37,9 @@
};
namespace {
-class UnixAPIChecker : public Checker< check::PreStmt<CallExpr> > {
- mutable std::unique_ptr<BugType> BT_open, BT_pthreadOnce, BT_mallocZero;
+
+class UnixAPIMisuseChecker : public Checker< check::PreStmt<CallExpr> > {
+ mutable std::unique_ptr<BugType> BT_open, BT_pthreadOnce;
mutable Optional<uint64_t> Val_O_CREAT;
public:
@@ -51,8 +49,25 @@
void CheckOpen(CheckerContext &C, const CallExpr *CE) const;
void CheckOpenAt(CheckerContext &C, const CallExpr *CE) const;
-
void CheckPthreadOnce(CheckerContext &C, const CallExpr *CE) const;
+
+ void CheckOpenVariant(CheckerContext &C,
+ const CallExpr *CE, OpenVariant Variant) const;
+
+ void ReportOpenBug(CheckerContext &C,
+ ProgramStateRef State,
+ const char *Msg,
+ SourceRange SR) const;
+
+};
+
+class UnixAPIPortabilityChecker : public Checker< check::PreStmt<CallExpr> > {
+public:
+ void checkPreStmt(const CallExpr *CE, CheckerContext &C) const;
+
+private:
+ mutable std::unique_ptr<BugType> BT_mallocZero;
+
void CheckCallocZero(CheckerContext &C, const CallExpr *CE) const;
void CheckMallocZero(CheckerContext &C, const CallExpr *CE) const;
void CheckReallocZero(CheckerContext &C, const CallExpr *CE) const;
@@ -61,13 +76,6 @@
void CheckAllocaWithAlignZero(CheckerContext &C, const CallExpr *CE) const;
void CheckVallocZero(CheckerContext &C, const CallExpr *CE) const;
- typedef void (UnixAPIChecker::*SubChecker)(CheckerContext &,
- const CallExpr *) const;
-private:
-
- void CheckOpenVariant(CheckerContext &C,
- const CallExpr *CE, OpenVariant Variant) const;
-
bool ReportZeroByteAllocation(CheckerContext &C,
ProgramStateRef falseState,
const Expr *arg,
@@ -77,48 +85,75 @@
const unsigned numArgs,
const unsigned sizeArg,
const char *fn) const;
- void LazyInitialize(std::unique_ptr<BugType> &BT, const char *name) const {
- if (BT)
- return;
- BT.reset(new BugType(this, name, categories::UnixAPI));
- }
- void ReportOpenBug(CheckerContext &C,
- ProgramStateRef State,
- const char *Msg,
- SourceRange SR) const;
};
+
} //end anonymous namespace
+static void LazyInitialize(const CheckerBase *Checker,
+ std::unique_ptr<BugType> &BT,
+ const char *name) {
+ if (BT)
+ return;
+ BT.reset(new BugType(Checker, name, categories::UnixAPI));
+}
+
//===----------------------------------------------------------------------===//
// "open" (man 2 open)
-//===----------------------------------------------------------------------===//
+//===----------------------------------------------------------------------===/
+
+void UnixAPIMisuseChecker::checkPreStmt(const CallExpr *CE,
+ CheckerContext &C) const {
+ const FunctionDecl *FD = C.getCalleeDecl(CE);
+ if (!FD || FD->getKind() != Decl::Function)
+ return;
+
+ // Don't treat functions in namespaces with the same name a Unix function
+ // as a call to the Unix function.
+ const DeclContext *NamespaceCtx = FD->getEnclosingNamespaceContext();
+ if (NamespaceCtx && isa<NamespaceDecl>(NamespaceCtx))
+ return;
-void UnixAPIChecker::ReportOpenBug(CheckerContext &C,
- ProgramStateRef State,
- const char *Msg,
- SourceRange SR) const {
+ StringRef FName = C.getCalleeName(FD);
+ if (FName.empty())
+ return;
+
+ if (FName == "open")
+ CheckOpen(C, CE);
+
+ else if (FName == "openat")
+ CheckOpenAt(C, CE);
+
+ else if (FName == "pthread_once")
+ CheckPthreadOnce(C, CE);
+}
+void UnixAPIMisuseChecker::ReportOpenBug(CheckerContext &C,
+ ProgramStateRef State,
+ const char *Msg,
+ SourceRange SR) const {
ExplodedNode *N = C.generateErrorNode(State);
if (!N)
return;
- LazyInitialize(BT_open, "Improper use of 'open'");
+ LazyInitialize(this, BT_open, "Improper use of 'open'");
auto Report = llvm::make_unique<BugReport>(*BT_open, Msg, N);
Report->addRange(SR);
C.emitReport(std::move(Report));
}
-void UnixAPIChecker::CheckOpen(CheckerContext &C, const CallExpr *CE) const {
+void UnixAPIMisuseChecker::CheckOpen(CheckerContext &C,
+ const CallExpr *CE) const {
CheckOpenVariant(C, CE, OpenVariant::Open);
}
-void UnixAPIChecker::CheckOpenAt(CheckerContext &C, const CallExpr *CE) const {
+void UnixAPIMisuseChecker::CheckOpenAt(CheckerContext &C,
+ const CallExpr *CE) const {
CheckOpenVariant(C, CE, OpenVariant::OpenAt);
}
-void UnixAPIChecker::CheckOpenVariant(CheckerContext &C,
- const CallExpr *CE,
- OpenVariant Variant) const {
+void UnixAPIMisuseChecker::CheckOpenVariant(CheckerContext &C,
+ const CallExpr *CE,
+ OpenVariant Variant) const {
// The index of the argument taking the flags open flags (O_RDONLY,
// O_WRONLY, O_CREAT, etc.),
unsigned int FlagsArgIndex;
@@ -236,7 +271,7 @@
// pthread_once
//===----------------------------------------------------------------------===//
-void UnixAPIChecker::CheckPthreadOnce(CheckerContext &C,
+void UnixAPIMisuseChecker::CheckPthreadOnce(CheckerContext &C,
const CallExpr *CE) const {
// This is similar to 'CheckDispatchOnce' in the MacOSXAPIChecker.
@@ -268,7 +303,7 @@
if (isa<VarRegion>(R) && isa<StackLocalsSpaceRegion>(R->getMemorySpace()))
os << " Perhaps you intended to declare the variable as 'static'?";
- LazyInitialize(BT_pthreadOnce, "Improper use of 'pthread_once'");
+ LazyInitialize(this, BT_pthreadOnce, "Improper use of 'pthread_once'");
auto report = llvm::make_unique<BugReport>(*BT_pthreadOnce, os.str(), N);
report->addRange(CE->getArg(0)->getSourceRange());
@@ -279,15 +314,16 @@
// "calloc", "malloc", "realloc", "reallocf", "alloca" and "valloc"
// with allocation size 0
//===----------------------------------------------------------------------===//
+
// FIXME: Eventually these should be rolled into the MallocChecker, but right now
// they're more basic and valuable for widespread use.
// Returns true if we try to do a zero byte allocation, false otherwise.
// Fills in trueState and falseState.
static bool IsZeroByteAllocation(ProgramStateRef state,
- const SVal argVal,
- ProgramStateRef *trueState,
- ProgramStateRef *falseState) {
+ const SVal argVal,
+ ProgramStateRef *trueState,
+ ProgramStateRef *falseState) {
std::tie(*trueState, *falseState) =
state->assume(argVal.castAs<DefinedSVal>());
@@ -297,15 +333,16 @@
// Generates an error report, indicating that the function whose name is given
// will perform a zero byte allocation.
// Returns false if an error occurred, true otherwise.
-bool UnixAPIChecker::ReportZeroByteAllocation(CheckerContext &C,
- ProgramStateRef falseState,
- const Expr *arg,
- const char *fn_name) const {
+bool UnixAPIPortabilityChecker::ReportZeroByteAllocation(
+ CheckerContext &C,
+ ProgramStateRef falseState,
+ const Expr *arg,
+ const char *fn_name) const {
ExplodedNode *N = C.generateErrorNode(falseState);
if (!N)
return false;
- LazyInitialize(BT_mallocZero,
+ LazyInitialize(this, BT_mallocZero,
"Undefined allocation of 0 bytes (CERT MEM04-C; CWE-131)");
SmallString<256> S;
@@ -322,11 +359,11 @@
// Does a basic check for 0-sized allocations suitable for most of the below
// functions (modulo "calloc")
-void UnixAPIChecker::BasicAllocationCheck(CheckerContext &C,
- const CallExpr *CE,
- const unsigned numArgs,
- const unsigned sizeArg,
- const char *fn) const {
+void UnixAPIPortabilityChecker::BasicAllocationCheck(CheckerContext &C,
+ const CallExpr *CE,
+ const unsigned numArgs,
+ const unsigned sizeArg,
+ const char *fn) const {
// Sanity check for the correct number of arguments
if (CE->getNumArgs() != numArgs)
return;
@@ -351,8 +388,8 @@
C.addTransition(trueState);
}
-void UnixAPIChecker::CheckCallocZero(CheckerContext &C,
- const CallExpr *CE) const {
+void UnixAPIPortabilityChecker::CheckCallocZero(CheckerContext &C,
+ const CallExpr *CE) const {
unsigned int nArgs = CE->getNumArgs();
if (nArgs != 2)
return;
@@ -387,43 +424,39 @@
C.addTransition(trueState);
}
-void UnixAPIChecker::CheckMallocZero(CheckerContext &C,
- const CallExpr *CE) const {
+void UnixAPIPortabilityChecker::CheckMallocZero(CheckerContext &C,
+ const CallExpr *CE) const {
BasicAllocationCheck(C, CE, 1, 0, "malloc");
}
-void UnixAPIChecker::CheckReallocZero(CheckerContext &C,
- const CallExpr *CE) const {
+void UnixAPIPortabilityChecker::CheckReallocZero(CheckerContext &C,
+ const CallExpr *CE) const {
BasicAllocationCheck(C, CE, 2, 1, "realloc");
}
-void UnixAPIChecker::CheckReallocfZero(CheckerContext &C,
- const CallExpr *CE) const {
+void UnixAPIPortabilityChecker::CheckReallocfZero(CheckerContext &C,
+ const CallExpr *CE) const {
BasicAllocationCheck(C, CE, 2, 1, "reallocf");
}
-void UnixAPIChecker::CheckAllocaZero(CheckerContext &C,
- const CallExpr *CE) const {
+void UnixAPIPortabilityChecker::CheckAllocaZero(CheckerContext &C,
+ const CallExpr *CE) const {
BasicAllocationCheck(C, CE, 1, 0, "alloca");
}
-void UnixAPIChecker::CheckAllocaWithAlignZero(CheckerContext &C,
- const CallExpr *CE) const {
+void UnixAPIPortabilityChecker::CheckAllocaWithAlignZero(
+ CheckerContext &C,
+ const CallExpr *CE) const {
BasicAllocationCheck(C, CE, 2, 0, "__builtin_alloca_with_align");
}
-void UnixAPIChecker::CheckVallocZero(CheckerContext &C,
- const CallExpr *CE) const {
+void UnixAPIPortabilityChecker::CheckVallocZero(CheckerContext &C,
+ const CallExpr *CE) const {
BasicAllocationCheck(C, CE, 1, 0, "valloc");
}
-
-//===----------------------------------------------------------------------===//
-// Central dispatch function.
-//===----------------------------------------------------------------------===//
-
-void UnixAPIChecker::checkPreStmt(const CallExpr *CE,
- CheckerContext &C) const {
+void UnixAPIPortabilityChecker::checkPreStmt(const CallExpr *CE,
+ CheckerContext &C) const {
const FunctionDecl *FD = C.getCalleeDecl(CE);
if (!FD || FD->getKind() != Decl::Function)
return;
@@ -438,46 +471,40 @@
if (FName.empty())
return;
- if (CheckMisuse) {
- if (SubChecker SC =
- llvm::StringSwitch<SubChecker>(FName)
- .Case("open", &UnixAPIChecker::CheckOpen)
- .Case("openat", &UnixAPIChecker::CheckOpenAt)
- .Case("pthread_once", &UnixAPIChecker::CheckPthreadOnce)
- .Default(nullptr)) {
- (this->*SC)(C, CE);
- }
- }
- if (CheckPortability) {
- if (SubChecker SC =
- llvm::StringSwitch<SubChecker>(FName)
- .Case("calloc", &UnixAPIChecker::CheckCallocZero)
- .Case("malloc", &UnixAPIChecker::CheckMallocZero)
- .Case("realloc", &UnixAPIChecker::CheckReallocZero)
- .Case("reallocf", &UnixAPIChecker::CheckReallocfZero)
- .Cases("alloca", "__builtin_alloca",
- &UnixAPIChecker::CheckAllocaZero)
- .Case("__builtin_alloca_with_align",
- &UnixAPIChecker::CheckAllocaWithAlignZero)
- .Case("valloc", &UnixAPIChecker::CheckVallocZero)
- .Default(nullptr)) {
- (this->*SC)(C, CE);
- }
- }
+ if (FName == "calloc")
+ CheckCallocZero(C, CE);
+
+ else if (FName == "malloc")
+ CheckMallocZero(C, CE);
+
+ else if (FName == "realloc")
+ CheckReallocZero(C, CE);
+
+ else if (FName == "reallocf")
+ CheckReallocfZero(C, CE);
+
+ else if (FName == "alloca" || FName == "__builtin_alloca")
+ CheckAllocaZero(C, CE);
+
+ else if (FName == "__builtin_alloca_with_align")
+ CheckAllocaWithAlignZero(C, CE);
+
+ else if (FName == "valloc")
+ CheckVallocZero(C, CE);
}
//===----------------------------------------------------------------------===//
// Registration.
//===----------------------------------------------------------------------===//
-#define REGISTER_CHECKER(Name) \
- void ento::registerUnixAPI##Name##Checker(CheckerManager &mgr) { \
- mgr.registerChecker<UnixAPIChecker>()->Check##Name = true; \
+#define REGISTER_CHECKER(CHECKERNAME) \
+ void ento::register##CHECKERNAME(CheckerManager &mgr) { \
+ mgr.registerChecker<CHECKERNAME>(); \
} \
\
- bool ento::shouldRegisterUnixAPI##Name##Checker(const LangOptions &LO) { \
+ bool ento::shouldRegister##CHECKERNAME(const LangOptions &LO) { \
return true; \
}
-REGISTER_CHECKER(Misuse)
-REGISTER_CHECKER(Portability)
+REGISTER_CHECKER(UnixAPIMisuseChecker)
+REGISTER_CHECKER(UnixAPIPortabilityChecker)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
