From 9873ef409c4a937c566e20e7a88af049d212ce03 Mon Sep 17 00:00:00 2001 From: Balazs Benics Date: Mon, 6 Dec 2021 10:20:17 +0100 Subject: [PATCH] [analyzer] Ignore flex generated files 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 Reviewed By: xazax.hun Differential Revision: https://reviews.llvm.org/D114510 --- .../clang/StaticAnalyzer/Core/AnalyzerOptions.def | 12 ++++++ .../StaticAnalyzer/Frontend/AnalysisConsumer.cpp | 47 +++++++++++++--------- clang/test/Analysis/analyzer-config.c | 2 + clang/test/Analysis/flexignore.c | 14 +++++++ clang/test/Analysis/yaccignore.c | 21 +++++----- 5 files changed, 67 insertions(+), 29 deletions(-) create mode 100644 clang/test/Analysis/flexignore.c diff --git a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def index aab8e12..c0cade4 100644 --- a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def +++ b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def @@ -336,6 +336,18 @@ ANALYZER_OPTION( "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. //===----------------------------------------------------------------------===// diff --git a/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp b/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp index f692c68..f6ddcb7 100644 --- a/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp +++ b/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp @@ -35,6 +35,7 @@ #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" @@ -493,13 +494,11 @@ void AnalysisConsumer::HandleDeclsCallGraph(const unsigned LocalTUDeclsSize) { } } -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) { @@ -546,38 +545,48 @@ void AnalysisConsumer::reportAnalyzerProgress(StringRef 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; + } - // Don't analyze if the user explicitly asked for no checks to be performed - // on this file. + 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; } + // Otherwise, just run the analysis. + runAnalysisOnTranslationUnit(C); + // Count how many basic blocks we have not covered. NumBlocksInAnalyzedFunctions = FunctionSummaries.getTotalNumBasicBlocks(); NumVisitedBlocksInAnalyzedFunctions = FunctionSummaries.getTotalNumVisitedBasicBlocks(); if (NumBlocksInAnalyzedFunctions > 0) PercentReachableBlocks = - (FunctionSummaries.getTotalNumVisitedBasicBlocks() * 100) / + (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 diff --git a/clang/test/Analysis/analyzer-config.c b/clang/test/Analysis/analyzer-config.c index 429229f..eae554f 100644 --- a/clang/test/Analysis/analyzer-config.c +++ b/clang/test/Analysis/analyzer-config.c @@ -83,6 +83,8 @@ // 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 diff --git a/clang/test/Analysis/flexignore.c b/clang/test/Analysis/flexignore.c new file mode 100644 index 0000000..8f2ad6d --- /dev/null +++ b/clang/test/Analysis/flexignore.c @@ -0,0 +1,14 @@ +// 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}} +} diff --git a/clang/test/Analysis/yaccignore.c b/clang/test/Analysis/yaccignore.c index c9edfad..c19356d 100644 --- a/clang/test/Analysis/yaccignore.c +++ b/clang/test/Analysis/yaccignore.c @@ -1,13 +1,14 @@ -/* 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: diagnostics +/* A Bison parser, made by GNU Bison 1.875. */ + +void clang_analyzer_warnIfReached(); +void foo() { + clang_analyzer_warnIfReached(); // conditional-warning {{REACHABLE}} +} -- 2.7.4