NoQ created this revision.
Checkers that find implementation-defined behavior seem to better be off by
default - or, at least, there should be a way to turn them off - because we're
not sure if our users are developing cross-platform code or target a specific
platform. If the behavior is well-defined on any particular target platform,
then the user may say "this code works correctly, the behavior is documented, i
personally don't care about portability, so the analyzer shouldn't warn".
I'm introducing an `optin.portability` package with this patch. The UNIX
zero-size-malloc check is moved here, because the behavior is
implementation-defined according even to the C standard, and man pages of
various platforms clearly document which behavior is implemented. Of course,
that behavior is different on linux vs. bsd/mac though, which is the whole
point of the checker.
Suggestions/complains are very welcome. I'm thinking of enabling portability
checks by default when we're cross-compiling, or maybe on per-platform basis,
eg. linux/bsd developers care about portability much more often (and in this
case we shouldn't probably add the `optin` prefix to the package).
https://reviews.llvm.org/D34102
Files:
include/clang/StaticAnalyzer/Checkers/Checkers.td
lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
test/Analysis/malloc-overflow2.c
test/Analysis/unix-fns.c
Index: test/Analysis/unix-fns.c
===================================================================
--- test/Analysis/unix-fns.c
+++ test/Analysis/unix-fns.c
@@ -1,7 +1,7 @@
-// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,unix.API,osx.API %s -analyzer-store=region -analyzer-output=plist -analyzer-eagerly-assume -analyzer-config faux-bodies=true -analyzer-config path-diagnostics-alternate=false -fblocks -verify -o %t.plist
+// RUN: %clang_analyze_cc1 -triple x86_64-apple-darwin10 -analyzer-checker=core,unix.API,osx.API,optin.portability.unix.API %s -analyzer-store=region -analyzer-output=plist -analyzer-eagerly-assume -analyzer-config faux-bodies=true -analyzer-config path-diagnostics-alternate=false -fblocks -verify -o %t.plist
// RUN: FileCheck --input-file=%t.plist %s
// RUN: mkdir -p %t.dir
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.API,osx.API -analyzer-output=html -analyzer-config faux-bodies=true -fblocks -o %t.dir %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,unix.API,osx.API,optin.portability.unix.API -analyzer-output=html -analyzer-config faux-bodies=true -fblocks -o %t.dir %s
// RUN: rm -fR %t.dir
struct _opaque_pthread_once_t {
long __sig;
Index: test/Analysis/malloc-overflow2.c
===================================================================
--- test/Analysis/malloc-overflow2.c
+++ test/Analysis/malloc-overflow2.c
@@ -1,4 +1,5 @@
// RUN: %clang_analyze_cc1 -triple x86_64-unknown-unknown -analyzer-checker=alpha.security.MallocOverflow,unix -verify %s
+// RUN: %clang_analyze_cc1 -triple x86_64-unknown-unknown -analyzer-checker=alpha.security.MallocOverflow,unix,optin.portability.unix -DPORTABILITY -verify %s
typedef __typeof__(sizeof(int)) size_t;
extern void *malloc(size_t);
@@ -32,5 +33,8 @@
}
void *f(int n) {
- return malloc(n * 0 * sizeof(int)); // expected-warning {{Call to 'malloc' has an allocation size of 0 bytes}}
+ return malloc(n * 0 * sizeof(int));
+#ifdef PORTABILITY
+ // expected-warning@-2{{Call to 'malloc' has an allocation size of 0 bytes}}
+#endif
}
Index: lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
===================================================================
--- lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UnixAPIChecker.cpp
@@ -45,6 +45,8 @@
mutable Optional<uint64_t> Val_O_CREAT;
public:
+ DefaultBool CheckMisuse, CheckPortability;
+
void checkPreStmt(const CallExpr *CE, CheckerContext &C) const;
void CheckOpen(CheckerContext &C, const CallExpr *CE) const;
@@ -437,29 +439,42 @@
if (FName.empty())
return;
- SubChecker SC =
- llvm::StringSwitch<SubChecker>(FName)
- .Case("open", &UnixAPIChecker::CheckOpen)
- .Case("openat", &UnixAPIChecker::CheckOpenAt)
- .Case("pthread_once", &UnixAPIChecker::CheckPthreadOnce)
- .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);
-
- if (SC)
- (this->*SC)(C, CE);
+ 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);
+ }
+ }
}
//===----------------------------------------------------------------------===//
// Registration.
//===----------------------------------------------------------------------===//
-void ento::registerUnixAPIChecker(CheckerManager &mgr) {
- mgr.registerChecker<UnixAPIChecker>();
-}
+#define REGISTER_CHECKER(Name) \
+ void ento::registerUnixAPI##Name##Checker(CheckerManager &mgr) { \
+ mgr.registerChecker<UnixAPIChecker>()->Check##Name = true; \
+ }
+
+REGISTER_CHECKER(Misuse)
+REGISTER_CHECKER(Portability)
Index: include/clang/StaticAnalyzer/Checkers/Checkers.td
===================================================================
--- include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -38,6 +38,11 @@
// default. Such checkers belong in the alpha package.
def OptIn : Package<"optin">;
+// In the Portability package reside checkers for finding code that relies on
+// implementation-defined behavior. Such checks are wanted for cross-platform
+// development, but unwanted for developers who target only a single platform.
+def PortabilityOptIn : Package<"portability">, InPackage<OptIn>;
+
def Nullability : Package<"nullability">;
def Cplusplus : Package<"cplusplus">;
@@ -57,6 +62,7 @@
def Taint : Package<"taint">, InPackage<SecurityAlpha>, Hidden;
def Unix : Package<"unix">;
+def UnixPortabilityOptIn : Package<"unix">, InPackage<PortabilityOptIn>;
def UnixAlpha : Package<"unix">, InPackage<Alpha>, Hidden;
def CString : Package<"cstring">, InPackage<Unix>, Hidden;
def CStringAlpha : Package<"cstring">, InPackage<UnixAlpha>, Hidden;
@@ -416,7 +422,7 @@
let ParentPackage = Unix in {
-def UnixAPIChecker : Checker<"API">,
+def UnixAPIMisuseChecker : Checker<"API">,
HelpText<"Check calls to various UNIX/Posix functions">,
DescFile<"UnixAPIChecker.cpp">;
@@ -442,6 +448,14 @@
} // end "unix"
+let ParentPackage = UnixPortabilityOptIn in {
+
+def UnixAPIPortabilityChecker : Checker<"API">,
+ HelpText<"Finds implementation-defined behavior in UNIX/Posix functions">,
+ DescFile<"UnixAPIChecker.cpp">;
+
+} // end optin.portability.unix
+
let ParentPackage = UnixAlpha in {
def ChrootChecker : Checker<"Chroot">,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
