[analyzer] Don't allow hidden checkers to emit diagnostics

[Szelethus]

Summary

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.

[gamesh411]

LGTM if nobody is against moving the checker. I will accept nevertheless.

clang/include/clang/StaticAnalyzer/Checkers/Checkers.td

 
 def TrustNonnullChecker : Checker<"TrustNonnull">,
   HelpText<"Trust that returns from framework methods annotated with _Nonnull "
            "are not null">,
   Documentation<NotDocumented>;
 
 } // 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.
 //===----------------------------------------------------------------------===//
 
 let ParentPackage = CoreBuiltin in {
 
 def NoReturnFunctionChecker : Checker<"NoReturnFunctions">,
   HelpText<"Evaluate \"panic\" functions that are known to not return to the "

 def SimpleStreamChecker : Checker<"SimpleStream">,
   HelpText<"Check for misuses of stream APIs">,
   Documentation<HasAlphaDocumentation>;
 
 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"
 
 //===----------------------------------------------------------------------===//
 // C++ checkers.
 //===----------------------------------------------------------------------===//
 
 let ParentPackage = Cplusplus in {
 

clang/lib/StaticAnalyzer/Core/BugReporter.cpp

 void BugType::anchor() {}
 
 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;
   }
   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,
     const Decl *DeclToUnique)
     : BugReport(Kind::PathSensitive, bt, shortDesc, desc), ErrorNode(errorNode),
       ErrorNodeRange(getStmt() ? getStmt()->getSourceRange() : SourceRange()),
       UniqueingLocation(LocationToUnique), UniqueingDecl(DeclToUnique) {
   assert(!isDependency(ErrorNode->getState()
                            ->getAnalysisManager()
                            .getCheckerManager()
                            ->getCheckerRegistryData(),
                        bt.getCheckerName()) &&
          "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(
     std::unique_ptr<BugReporterVisitor> visitor) {
   if (!visitor)
     return;
 
   llvm::FoldingSetNodeID ID;

clang/test/Analysis/std-c-library-functions-arg-constraints.c

 // Check the basic reporting/warning and the application of constraints.
 // 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 \
 // RUN:   -verify=report
 
 // Check the bugpath related to the reports.
 // 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 \
 // RUN:   -analyzer-output=text \
 // RUN:   -verify=bugpath
 
 void clang_analyzer_eval(int);
 

clang/test/Analysis/std-c-library-functions-arg-constraints.cpp

 // 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 \
 // RUN:   -triple i686-unknown-linux \
 // RUN:   -verify
 
 void clang_analyzer_eval(int);
 
 int __defaultparam(void *, int y = 3);
 
 void test_arg_constraint_on_fun_with_default_param() {
   __defaultparam(nullptr); // \
   // expected-warning{{Function argument constraint is not satisfied}}
 }

clang/test/Analysis/weak-dependencies.c

 // 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;
 
 struct FILE;
 typedef struct FILE FILE;
 
 size_t fread(void *restrict, size_t, size_t, FILE *restrict) __attribute__((nonnull(1)));
 
 void f(FILE *F) {
   int *p = 0;
   fread(p, sizeof(int), 5, F); // expected-warning{{Null pointer passed to 1st parameter expecting 'nonnull' [core.NonNullParamChecker]}}
 }