[clang] cfd6b4b - [analyzer] Don't allow hidden checkers to emit diagnostics
Kirstóf Umann via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 6 06:35:40 PDT 2020
Author: Kirstóf Umann
Date: 2020-07-06T15:34:51+02:00
New Revision: cfd6b4b811aa8bb193f744264e36e83de4bb3650
URL: https://github.com/llvm/llvm-project/commit/cfd6b4b811aa8bb193f744264e36e83de4bb3650
DIFF: https://github.com/llvm/llvm-project/commit/cfd6b4b811aa8bb193f744264e36e83de4bb3650.diff
LOG: [analyzer] Don't allow hidden checkers to emit diagnostics
Hidden checkers (those marked with Hidden in Checkers.td) are meant for
development purposes only, and are only displayed under
-analyzer-checker-help-developer, so users shouldn't see reports from them.
I moved StdLibraryFunctionsArg checker to the unix package from apiModeling as
it violated this rule. I believe this change doesn't deserve a different
revision because it is in alpha, and the name is so bad anyways I don't
immediately care where it is, because we'll have to revisit it soon enough.
Differential Revision: https://reviews.llvm.org/D81750
Added:
Modified:
clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
clang/lib/StaticAnalyzer/Core/BugReporter.cpp
clang/test/Analysis/std-c-library-functions-arg-constraints.c
clang/test/Analysis/std-c-library-functions-arg-constraints.cpp
clang/test/Analysis/weak-dependencies.c
Removed:
################################################################################
diff --git a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
index c1baa52a69b6..dc1269890f93 100644
--- a/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
+++ b/clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
@@ -375,18 +375,6 @@ def TrustNonnullChecker : Checker<"TrustNonnull">,
} // end "apiModeling"
-let ParentPackage = APIModelingAlpha in {
-
-def StdCLibraryFunctionArgsChecker : Checker<"StdCLibraryFunctionArgs">,
- HelpText<"Check constraints of arguments of C standard library functions, "
- "such as whether the parameter of isalpha is in the range [0, 255] "
- "or is EOF.">,
- Dependencies<[StdCLibraryFunctionsChecker]>,
- WeakDependencies<[NonNullParamChecker]>,
- Documentation<NotDocumented>;
-
-} // end "alpha.apiModeling"
-
//===----------------------------------------------------------------------===//
// Evaluate "builtin" functions.
//===----------------------------------------------------------------------===//
@@ -545,6 +533,14 @@ def BlockInCriticalSectionChecker : Checker<"BlockInCriticalSection">,
HelpText<"Check for calls to blocking functions inside a critical section">,
Documentation<HasAlphaDocumentation>;
+def StdCLibraryFunctionArgsChecker : Checker<"StdCLibraryFunctionArgs">,
+ HelpText<"Check constraints of arguments of C standard library functions, "
+ "such as whether the parameter of isalpha is in the range [0, 255] "
+ "or is EOF.">,
+ Dependencies<[StdCLibraryFunctionsChecker]>,
+ WeakDependencies<[NonNullParamChecker]>,
+ Documentation<NotDocumented>;
+
} // end "alpha.unix"
//===----------------------------------------------------------------------===//
diff --git a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
index a832b90ddf0e..7df8dea56055 100644
--- a/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
+++ b/clang/lib/StaticAnalyzer/Core/BugReporter.cpp
@@ -2108,8 +2108,8 @@ void BuiltinBug::anchor() {}
// Methods for BugReport and subclasses.
//===----------------------------------------------------------------------===//
-static bool isDependency(const CheckerRegistryData &Registry,
- StringRef CheckerName) {
+LLVM_ATTRIBUTE_USED static bool
+isDependency(const CheckerRegistryData &Registry, StringRef CheckerName) {
for (const std::pair<StringRef, StringRef> &Pair : Registry.Dependencies) {
if (Pair.second == CheckerName)
return true;
@@ -2117,6 +2117,17 @@ static bool isDependency(const CheckerRegistryData &Registry,
return false;
}
+LLVM_ATTRIBUTE_USED static bool isHidden(const CheckerRegistryData &Registry,
+ StringRef CheckerName) {
+ for (const CheckerInfo &Checker : Registry.Checkers) {
+ if (Checker.FullName == CheckerName)
+ return Checker.IsHidden;
+ }
+ llvm_unreachable(
+ "Checker name not found in CheckerRegistry -- did you retrieve it "
+ "correctly from CheckerManager::getCurrentCheckerName?");
+}
+
PathSensitiveBugReport::PathSensitiveBugReport(
const BugType &bt, StringRef shortDesc, StringRef desc,
const ExplodedNode *errorNode, PathDiagnosticLocation LocationToUnique,
@@ -2132,6 +2143,16 @@ PathSensitiveBugReport::PathSensitiveBugReport(
"Some checkers depend on this one! We don't allow dependency "
"checkers to emit warnings, because checkers should depend on "
"*modeling*, not *diagnostics*.");
+
+ assert(
+ bt.getCheckerName().startswith("debug") ||
+ !isHidden(ErrorNode->getState()
+ ->getAnalysisManager()
+ .getCheckerManager()
+ ->getCheckerRegistryData(),
+ bt.getCheckerName()) &&
+ "Hidden checkers musn't emit diagnostics as they are by definition "
+ "non-user facing!");
}
void PathSensitiveBugReport::addVisitor(
diff --git a/clang/test/Analysis/std-c-library-functions-arg-constraints.c b/clang/test/Analysis/std-c-library-functions-arg-constraints.c
index b99248d337b3..b23700a96f38 100644
--- a/clang/test/Analysis/std-c-library-functions-arg-constraints.c
+++ b/clang/test/Analysis/std-c-library-functions-arg-constraints.c
@@ -2,7 +2,7 @@
// RUN: %clang_analyze_cc1 %s \
// RUN: -analyzer-checker=core \
// RUN: -analyzer-checker=apiModeling.StdCLibraryFunctions \
-// RUN: -analyzer-checker=alpha.apiModeling.StdCLibraryFunctionArgs \
+// RUN: -analyzer-checker=alpha.unix.StdCLibraryFunctionArgs \
// RUN: -analyzer-checker=debug.StdCLibraryFunctionsTester \
// RUN: -analyzer-checker=debug.ExprInspection \
// RUN: -triple x86_64-unknown-linux-gnu \
@@ -12,7 +12,7 @@
// RUN: %clang_analyze_cc1 %s \
// RUN: -analyzer-checker=core \
// RUN: -analyzer-checker=apiModeling.StdCLibraryFunctions \
-// RUN: -analyzer-checker=alpha.apiModeling.StdCLibraryFunctionArgs \
+// RUN: -analyzer-checker=alpha.unix.StdCLibraryFunctionArgs \
// RUN: -analyzer-checker=debug.StdCLibraryFunctionsTester \
// RUN: -analyzer-checker=debug.ExprInspection \
// RUN: -triple x86_64-unknown-linux-gnu \
diff --git a/clang/test/Analysis/std-c-library-functions-arg-constraints.cpp b/clang/test/Analysis/std-c-library-functions-arg-constraints.cpp
index ebc841a07fb7..ae26834402e6 100644
--- a/clang/test/Analysis/std-c-library-functions-arg-constraints.cpp
+++ b/clang/test/Analysis/std-c-library-functions-arg-constraints.cpp
@@ -1,7 +1,7 @@
// RUN: %clang_analyze_cc1 %s \
// RUN: -analyzer-checker=core \
// RUN: -analyzer-checker=apiModeling.StdCLibraryFunctions \
-// RUN: -analyzer-checker=alpha.apiModeling.StdCLibraryFunctionArgs \
+// RUN: -analyzer-checker=alpha.unix.StdCLibraryFunctionArgs \
// RUN: -analyzer-checker=debug.StdCLibraryFunctionsTester \
// RUN: -analyzer-checker=debug.ExprInspection \
// RUN: -analyzer-config eagerly-assume=false \
diff --git a/clang/test/Analysis/weak-dependencies.c b/clang/test/Analysis/weak-dependencies.c
index a2179ecaff57..62cb10b5b523 100644
--- a/clang/test/Analysis/weak-dependencies.c
+++ b/clang/test/Analysis/weak-dependencies.c
@@ -1,5 +1,5 @@
// RUN: %clang_analyze_cc1 %s -verify \
-// RUN: -analyzer-checker=alpha.apiModeling.StdCLibraryFunctionArgs \
+// RUN: -analyzer-checker=alpha.unix.StdCLibraryFunctionArgs \
// RUN: -analyzer-checker=core
typedef __typeof(sizeof(int)) size_t;
More information about the cfe-commits
mailing list