[analyzer] Ignore flex generated files

[steakhal]

Summary

Some projects [1,2,3] have flex-generated files besides bison-generated ones.
Unfortunately, the comment "/* A lexical scanner generated by flex */"
generated by the tools is not necessarily at the beginning of the file,
thus we need to quickly skim through the file for this needle string.

Luckily, StringRef can do this operation in an efficient way.

That being said, now the bison comment is not required to be at the very
beginning of the file. This allows us to detect a couple more cases
[4,5,6].

Alternatively, we could say that we only allow whitespace characters
before matching the bison/flex header comment. That would prevent the
(probably) unnecessary string search in the buffer. However, I could not
verify that these tools would actually respect this assumption.

Additionally to this, e.g. the Twin project [1] has other non-whitespace
characters (some preprocessor directives) before the flex-generated
header comment. So the heuristic in the previous paragraph won't work with that.
Thus, I would advocate the current implementation.

According to my measurement, this patch won't introduce measurable performance degradation, even though we will do 2 linear scans.

I introduce the ignore-bison-generated-files and
ignore-flex-generated-files to disable skipping these files.
Both of these options are true by default.

[1]: https://github.com/cosmos72/twin/blob/master/server/rcparse_lex.cpp#L7
[2]: https://github.com/marcauresoar/make-examples/blob/22362cdcf9dd7c597b5049ce7f176621e2e9ac7a/sandbox/count-words/lexer.c#L6
[3]: https://github.com/vladcmanea/2nd-faculty-year-Formal-Languages---Automata-assignments/blob/11abdf64629d9eb741438ba69f04636769d5a374/lab1/lex.yy.c#L6

[4]: https://github.com/KritikaChoudhary/System-Software-Lab/blob/47f5b2cfe2a2738fd54eae9f8439817f6a22034e/B_yacc/1/y1.tab.h#L2
[5]: https://github.com/VirtualMonitor/VirtualMonitor/blob/71d1bf9b1e7b392a7bd0c73dc217138dc5865651/src/VBox/Additions/x11/x11include/xorg-server-1.8.0/parser.h#L2
[6]: https://github.com/bspaulding/DrawTest/blob/3f773ceb13de14275429036b9cbc5aa19e29bab9/Framework/OpenEars.framework/Versions/A/Headers/jsgf_parser.h#L2

[xazax.hun]

I wonder if this feature should be part of CodeChecker/scan-build instead (potentially under an option).

[steakhal]

Added analyzer options disabling the skipping of bison and flex files.

[xazax.hun]

Since we already had some precedent, it looks good to me. But I still think it would be nicer to have these features outside of the analyzer in the future.

[steakhal]

In D114510#3159311, @xazax.hun wrote:

Since we already had some precedent, it looks good to me. But I still think it would be nicer to have these features outside of the analyzer in the future.

Ah, now I get it.
That would mean that I have to patch this in scan-build and CodeChecker individually. Makes sense. Let me think about it.

clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def

 ANALYZER_OPTION(
     bool, ShouldAssumeControlledEnvironment, "assume-controlled-environment",
     "Whether the analyzed application runs in a controlled environment. "
     "We will assume that environment variables exist in queries and they hold "
     "no malicious data. For instance, if this option is enabled, 'getenv()' "
     "might be modeled by the analyzer to never return NULL.",
     false)
 
+ANALYZER_OPTION(
+    bool, ShouldIgnoreBisonGeneratedFiles, "ignore-bison-generated-files",
+    "If enabled, any files containing the \"/* A Bison parser, made by\" "
+    "won't be analyzed.",
+    true)
+
+ANALYZER_OPTION(
+    bool, ShouldIgnoreFlexGeneratedFiles, "ignore-flex-generated-files",
+    "If enabled, any files containing the \"/* A lexical scanner generated by "
+    "flex\" won't be analyzed.",
+    true)
+
 //===----------------------------------------------------------------------===//
 // Unsigned analyzer options.
 //===----------------------------------------------------------------------===//
 
 ANALYZER_OPTION(unsigned, CTUImportThreshold, "ctu-import-threshold",
                 "The maximal amount of translation units that is considered "
                 "for import when inlining functions during CTU analysis. "
                 "Lowering this threshold can alleviate the memory burden of "

clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp

 #include "clang/StaticAnalyzer/Checkers/LocalCheckers.h"
 #include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugReporter.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/AnalysisManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
 #include "llvm/ADT/PostOrderIterator.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/Timer.h"
 #include "llvm/Support/raw_ostream.h"
 #include <memory>
 #include <queue>
for (const Decl *Callee : VisitedCallees)
       // Decls from CallGraph are already canonical. But Decls coming from
       // CallExprs may be not. We should canonicalize them manually.
       Visited.insert(isa<ObjCMethodDecl>(Callee) ? Callee
                                                  : Callee->getCanonicalDecl());
     VisitedAsTopLevel.insert(D);
   }
 }
 
-static bool isBisonFile(ASTContext &C) {
+static bool fileContainsString(StringRef Substring, ASTContext &C) {
   const SourceManager &SM = C.getSourceManager();
   FileID FID = SM.getMainFileID();
   StringRef Buffer = SM.getBufferOrFake(FID).getBuffer();
-  if (Buffer.startswith("/* A Bison parser, made by"))
-    return true;
-  return false;
+  return Buffer.contains(Substring);
 }
 
 void AnalysisConsumer::runAnalysisOnTranslationUnit(ASTContext &C) {
   BugReporter BR(*Mgr);
   TranslationUnitDecl *TU = C.getTranslationUnitDecl();
   if (SyntaxCheckTimer)
     SyntaxCheckTimer->startTimer();
   checkerMgr->runCheckersOnASTDecl(TU, *Mgr, BR);

 }
 
 void AnalysisConsumer::reportAnalyzerProgress(StringRef S) {
   if (Opts->AnalyzerDisplayProgress)
     llvm::errs() << S;
 }
 
 void AnalysisConsumer::HandleTranslationUnit(ASTContext &C) {
-
   // Don't run the actions if an error has occurred with parsing the file.
   DiagnosticsEngine &Diags = PP.getDiagnostics();
   if (Diags.hasErrorOccurred() || Diags.hasFatalErrorOccurred())
     return;
 
-  if (isBisonFile(C)) {
+  // Explicitly destroy the PathDiagnosticConsumer.  This will flush its output.
+  // FIXME: This should be replaced with something that doesn't rely on
+  // side-effects in PathDiagnosticConsumer's destructor. This is required when
+  // used with option -disable-free.
+  const auto DiagFlusherScopeExit =
+      llvm::make_scope_exit([this] { Mgr.reset(); });
+
+  if (Opts->ShouldIgnoreBisonGeneratedFiles &&
+      fileContainsString("/* A Bison parser, made by", C)) {
     reportAnalyzerProgress("Skipping bison-generated file\n");
-  } else if (Opts->DisableAllCheckers) {
+    return;
+  }
+
+  if (Opts->ShouldIgnoreFlexGeneratedFiles &&
+      fileContainsString("/* A lexical scanner generated by flex", C)) {
+    reportAnalyzerProgress("Skipping flex-generated file\n");
+    return;
+  }
 
-    // Don't analyze if the user explicitly asked for no checks to be performed
-    // on this file.
+  if (Opts->DisableAllCheckers) {
     reportAnalyzerProgress("All checks are disabled using a supplied option\n");
-  } else {
-    // Otherwise, just run the analysis.
-    runAnalysisOnTranslationUnit(C);
-  }
+    return;
+  }
+
 
   // Count how many basic blocks we have not covered.
   NumBlocksInAnalyzedFunctions = FunctionSummaries.getTotalNumBasicBlocks();
   NumVisitedBlocksInAnalyzedFunctions =
       FunctionSummaries.getTotalNumVisitedBasicBlocks();
   if (NumBlocksInAnalyzedFunctions > 0)
     PercentReachableBlocks =
         (FunctionSummaries.getTotalNumVisitedBasicBlocks() * 100) /
         NumBlocksInAnalyzedFunctions;
-
-  // Explicitly destroy the PathDiagnosticConsumer.  This will flush its output.
-  // FIXME: This should be replaced with something that doesn't rely on
-  // side-effects in PathDiagnosticConsumer's destructor. This is required when
-  // used with option -disable-free.
-  Mgr.reset();
 }
 
 AnalysisConsumer::AnalysisMode
 AnalysisConsumer::getModeForDecl(Decl *D, AnalysisMode Mode) {
   if (!Opts->AnalyzeSpecificFunction.empty() &&
       AnalysisDeclContext::getFunctionName(D) != Opts->AnalyzeSpecificFunction)
     return AM_None;
 

clang/test/Analysis/analyzer-config.c

 // CHECK-NEXT: display-ctu-progress = false
 // CHECK-NEXT: eagerly-assume = true
 // CHECK-NEXT: elide-constructors = true
 // CHECK-NEXT: expand-macros = false
 // CHECK-NEXT: experimental-enable-naive-ctu-analysis = false
 // CHECK-NEXT: exploration_strategy = unexplored_first_queue
 // CHECK-NEXT: faux-bodies = true
 // CHECK-NEXT: graph-trim-interval = 1000
+// CHECK-NEXT: ignore-bison-generated-files = true
+// CHECK-NEXT: ignore-flex-generated-files = true
 // CHECK-NEXT: inline-lambdas = true
 // CHECK-NEXT: ipa = dynamic-bifurcate
 // CHECK-NEXT: ipa-always-inline-size = 3
 // CHECK-NEXT: max-inlinable-size = 100
 // CHECK-NEXT: max-nodes = 225000
 // CHECK-NEXT: max-symbol-complexity = 35
 // CHECK-NEXT: max-times-inline-large = 32
 // CHECK-NEXT: min-cfg-size-treat-functions-as-large = 14

clang/test/Analysis/flexignore.c

+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify -DEXPECT_NO_DIAGNOSTICS %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify=conditional %s \
+// RUN:    -analyzer-config ignore-flex-generated-files=false
+
+#ifdef EXPECT_NO_DIAGNOSTICS
+// expected-no-diagnostics
+#endif
+
+/* A lexical scanner generated by flex */
+
+void clang_analyzer_warnIfReached();
+void foo() {
+  clang_analyzer_warnIfReached(); // conditional-warning {{REACHABLE}}
+}

clang/test/Analysis/yaccignore.c

-/* A Bison parser, made by GNU Bison 1.875.  */
-
-// RUN: rm -rf %t.plist
-// RUN: %clang_analyze_cc1 -analyzer-checker=core -analyzer-output=plist -o %t.plist -verify %s
-// RUN: FileCheck --input-file=%t.plist %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify -DEXPECT_NO_DIAGNOSTICS %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify=conditional %s \
+// RUN:    -analyzer-config ignore-bison-generated-files=false
 
+#ifdef EXPECT_NO_DIAGNOSTICS
 // expected-no-diagnostics
-int foo() {
-  int *x = 0;
-  return *x; // no-warning
-}
+#endif
 
-// CHECK:   <key>diagnostics</key>
+/* A Bison parser, made by GNU Bison 1.875.  */
+
+void clang_analyzer_warnIfReached();
+void foo() {
+  clang_analyzer_warnIfReached(); // conditional-warning {{REACHABLE}}
+}