From: Kristof Umann <dkszelethus@gmail.com>
Date: Fri, 2 Nov 2018 15:48:10 +0000 (+0000)
Subject: [analyzer][NFC] Fix some incorrect uses of -analyzer-config options
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=73bb3f29cb2fb72daa363ca49b226fd41a789da1;p=clang

[analyzer][NFC] Fix some incorrect uses of -analyzer-config options

I'm in the process of refactoring AnalyzerOptions. The main motivation behind
here is to emit warnings if an invalid -analyzer-config option is given from
the command line, and be able to list them all.

In this patch, I found some flags that should've been used as checker options,
or have absolutely no mention of in AnalyzerOptions, or are nonexistent.

- NonLocalizedStringChecker now uses its "AggressiveReport" flag as a checker
    option
- lib/StaticAnalyzer/Frontend/ModelInjector.cpp now accesses the "model-path"
    option through a getter in AnalyzerOptions
- -analyzer-config path-diagnostics-alternate=false is not a thing, I removed it,
- lib/StaticAnalyzer/Checkers/AllocationDiagnostics.cpp and
    lib/StaticAnalyzer/Checkers/AllocationDiagnostics.h are weird, they actually
    only contain an option getter. I deleted them, and fixed RetainCountChecker
    to get it's "leak-diagnostics-reference-allocation" option as a checker option,
- "region-store-small-struct-limit" has a proper getter now.

Differential Revision: https://reviews.llvm.org/D53276


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@345985 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h b/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
index f9ccd5f679..66c30fac22 100644
--- a/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
+++ b/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
@@ -328,6 +328,11 @@ private:
   /// \sa shouldElideConstructors
   Optional<bool> ElideConstructors;
 
+  /// \sa getModelPath
+  Optional<StringRef> ModelPath;
+
+  /// \sa getRegionStoreSmallStructLimit
+  Optional<unsigned> RegionStoreSmallStructLimit;
 
   /// A helper function that retrieves option for a given full-qualified
   /// checker name.
@@ -745,6 +750,21 @@ public:
   /// Starting with C++17 some elisions become mandatory, and in these cases
   /// the option will be ignored.
   bool shouldElideConstructors();
+
+  /// The largest number of fields a struct can have and still be
+  /// considered "small".
+  ///
+  /// This is currently used to decide whether or not it is worth "forcing" a
+  /// LazyCompoundVal on bind.
+  ///
+  /// This is controlled by 'region-store-small-struct-limit' option.
+  /// To disable all small-struct-dependent behavior, set the option to "0".
+  unsigned getRegionStoreSmallStructLimit();
+
+  /// The analyzer can inline an alternative implementation written in C at the
+  /// call site if the called function's body is not available. This is a path
+  /// where to look for those alternative implementations (called models)
+  StringRef getModelPath();
 };
 
 using AnalyzerOptionsRef = IntrusiveRefCntPtr<AnalyzerOptions>;
diff --git a/lib/StaticAnalyzer/Checkers/AllocationDiagnostics.cpp b/lib/StaticAnalyzer/Checkers/AllocationDiagnostics.cpp
deleted file mode 100644
index 3dec8a58c9..0000000000
--- a/lib/StaticAnalyzer/Checkers/AllocationDiagnostics.cpp
+++ /dev/null
@@ -1,24 +0,0 @@
-//=- AllocationDiagnostics.cpp - Config options for allocation diags *- C++ -*-//
-//
-//                     The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-//
-// Declares the configuration functions for leaks/allocation diagnostics.
-//
-//===--------------------------
-
-#include "AllocationDiagnostics.h"
-
-namespace clang {
-namespace ento {
-
-bool shouldIncludeAllocationSiteInLeakDiagnostics(AnalyzerOptions &AOpts) {
-  return AOpts.getBooleanOption("leak-diagnostics-reference-allocation",
-                                false);
-}
-
-}}
diff --git a/lib/StaticAnalyzer/Checkers/AllocationDiagnostics.h b/lib/StaticAnalyzer/Checkers/AllocationDiagnostics.h
deleted file mode 100644
index 62b7fab073..0000000000
--- a/lib/StaticAnalyzer/Checkers/AllocationDiagnostics.h
+++ /dev/null
@@ -1,31 +0,0 @@
-//=--- AllocationDiagnostics.h - Config options for allocation diags *- C++ -*-//
-//
-//                     The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===----------------------------------------------------------------------===//
-//
-// Declares the configuration functions for leaks/allocation diagnostics.
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_ALLOCATIONDIAGNOSTICS_H
-#define LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_ALLOCATIONDIAGNOSTICS_H
-
-#include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
-
-namespace clang { namespace ento {
-
-/// Returns true if leak diagnostics should directly reference
-/// the allocatin site (where possible).
-///
-/// The default is false.
-///
-bool shouldIncludeAllocationSiteInLeakDiagnostics(AnalyzerOptions &AOpts);
-
-}}
-
-#endif
-
diff --git a/lib/StaticAnalyzer/Checkers/CMakeLists.txt b/lib/StaticAnalyzer/Checkers/CMakeLists.txt
index 306813b880..ec52cad71b 100644
--- a/lib/StaticAnalyzer/Checkers/CMakeLists.txt
+++ b/lib/StaticAnalyzer/Checkers/CMakeLists.txt
@@ -3,7 +3,6 @@ set(LLVM_LINK_COMPONENTS
   )
 
 add_clang_library(clangStaticAnalyzerCheckers
-  AllocationDiagnostics.cpp
   AnalysisOrderChecker.cpp
   AnalyzerStatsChecker.cpp
   ArrayBoundChecker.cpp
diff --git a/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp b/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp
index 0e51cf1184..ec05c737ac 100644
--- a/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/LocalizationChecker.cpp
@@ -1398,7 +1398,8 @@ void ento::registerNonLocalizedStringChecker(CheckerManager &mgr) {
   NonLocalizedStringChecker *checker =
       mgr.registerChecker<NonLocalizedStringChecker>();
   checker->IsAggressive =
-      mgr.getAnalyzerOptions().getBooleanOption("AggressiveReport", false);
+      mgr.getAnalyzerOptions().getBooleanOption("AggressiveReport", false,
+                                                checker);
 }
 
 void ento::registerEmptyLocalizationContextChecker(CheckerManager &mgr) {
diff --git a/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp b/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
index febeffda1b..89d3795b1b 100644
--- a/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
+++ b/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
@@ -1393,5 +1393,12 @@ void RetainCountChecker::printState(raw_ostream &Out, ProgramStateRef State,
 //===----------------------------------------------------------------------===//
 
 void ento::registerRetainCountChecker(CheckerManager &Mgr) {
-  Mgr.registerChecker<RetainCountChecker>(Mgr.getAnalyzerOptions());
+  auto *Chk = Mgr.registerChecker<RetainCountChecker>();
+
+  AnalyzerOptions &Options = Mgr.getAnalyzerOptions();
+
+  Chk->IncludeAllocationLine = Options.getBooleanOption(
+                           "leak-diagnostics-reference-allocation", false, Chk);
+  Chk->ShouldCheckOSObjectRetainCount = Options.getBooleanOption(
+                                                   "CheckOSObject", true, Chk);
 }
diff --git a/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h b/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h
index 388c2eaaf0..3c5076391a 100644
--- a/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h
+++ b/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h
@@ -16,7 +16,6 @@
 #define LLVM_CLANG_LIB_STATICANALYZER_CHECKERS_RETAINCOUNTCHECKER_H
 
 #include "../ClangSACheckers.h"
-#include "../AllocationDiagnostics.h"
 #include "RetainCountDiagnostics.h"
 #include "clang/AST/Attr.h"
 #include "clang/AST/DeclCXX.h"
@@ -266,25 +265,18 @@ class RetainCountChecker
   mutable std::unique_ptr<RetainSummaryManager> Summaries;
   mutable SummaryLogTy SummaryLog;
 
-  AnalyzerOptions &Options;
   mutable bool ShouldResetSummaryLog;
 
+public:
   /// Optional setting to indicate if leak reports should include
   /// the allocation line.
-  mutable bool IncludeAllocationLine;
+  bool IncludeAllocationLine;
+  bool ShouldCheckOSObjectRetainCount;
 
-public:
-  RetainCountChecker(AnalyzerOptions &Options)
-      : Options(Options), ShouldResetSummaryLog(false),
-        IncludeAllocationLine(
-            shouldIncludeAllocationSiteInLeakDiagnostics(Options)) {}
+  RetainCountChecker() : ShouldResetSummaryLog(false) {}
 
   ~RetainCountChecker() override { DeleteContainerSeconds(DeadSymbolTags); }
 
-  bool shouldCheckOSObjectRetainCount() const {
-    return Options.getBooleanOption("CheckOSObject", true, this);
-  }
-
   void checkEndAnalysis(ExplodedGraph &G, BugReporter &BR,
                         ExprEngine &Eng) const {
     // FIXME: This is a hack to make sure the summary log gets cleared between
@@ -339,7 +331,7 @@ public:
     bool ARCEnabled = (bool)Ctx.getLangOpts().ObjCAutoRefCount;
     if (!Summaries) {
       Summaries.reset(new RetainSummaryManager(
-          Ctx, ARCEnabled, shouldCheckOSObjectRetainCount()));
+          Ctx, ARCEnabled, ShouldCheckOSObjectRetainCount));
     } else {
       assert(Summaries->isARCEnabled() == ARCEnabled);
     }
diff --git a/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp b/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
index cde2f4c259..55ace6b412 100644
--- a/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
+++ b/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
@@ -503,8 +503,21 @@ bool AnalyzerOptions::naiveCTUEnabled() {
   return NaiveCTU.getValue();
 }
 
+unsigned AnalyzerOptions::getRegionStoreSmallStructLimit() {
+  if (!RegionStoreSmallStructLimit.hasValue())
+    RegionStoreSmallStructLimit =
+                       getOptionAsInteger("region-store-small-struct-limit", 2);
+  return RegionStoreSmallStructLimit.getValue();
+}
+
 StringRef AnalyzerOptions::getCTUIndexName() {
   if (!CTUIndexName.hasValue())
     CTUIndexName = getOptionAsString("ctu-index-name", "externalFnMap.txt");
   return CTUIndexName.getValue();
 }
+
+StringRef AnalyzerOptions::getModelPath() {
+  if (!ModelPath.hasValue())
+    ModelPath = getOptionAsString("model-path", "");
+  return ModelPath.getValue();
+}
diff --git a/lib/StaticAnalyzer/Core/RegionStore.cpp b/lib/StaticAnalyzer/Core/RegionStore.cpp
index 98d11fa39c..012058886c 100644
--- a/lib/StaticAnalyzer/Core/RegionStore.cpp
+++ b/lib/StaticAnalyzer/Core/RegionStore.cpp
@@ -350,7 +350,7 @@ public:
     if (SubEngine *Eng = StateMgr.getOwningEngine()) {
       AnalyzerOptions &Options = Eng->getAnalysisManager().options;
       SmallStructLimit =
-        Options.getOptionAsInteger("region-store-small-struct-limit", 2);
+        Options.getRegionStoreSmallStructLimit();
     }
   }
 
diff --git a/lib/StaticAnalyzer/Frontend/ModelInjector.cpp b/lib/StaticAnalyzer/Frontend/ModelInjector.cpp
index c43d30440c..c0cf395e28 100644
--- a/lib/StaticAnalyzer/Frontend/ModelInjector.cpp
+++ b/lib/StaticAnalyzer/Frontend/ModelInjector.cpp
@@ -48,7 +48,7 @@ void ModelInjector::onBodySynthesis(const NamedDecl *D) {
   FileID mainFileID = SM.getMainFileID();
 
   AnalyzerOptionsRef analyzerOpts = CI.getAnalyzerOpts();
-  llvm::StringRef modelPath = analyzerOpts->Config["model-path"];
+  llvm::StringRef modelPath = analyzerOpts->getModelPath();
 
   llvm::SmallString<128> fileName;
 
diff --git a/test/Analysis/analyzer-config.c b/test/Analysis/analyzer-config.c
index c932fe12ae..20e2fe5a61 100644
--- a/test/Analysis/analyzer-config.c
+++ b/test/Analysis/analyzer-config.c
@@ -26,7 +26,6 @@ void foo() {
 // CHECK-NEXT: inline-lambdas = true
 // CHECK-NEXT: ipa = dynamic-bifurcate
 // CHECK-NEXT: ipa-always-inline-size = 3
-// CHECK-NEXT: leak-diagnostics-reference-allocation = false
 // CHECK-NEXT: max-inlinable-size = 100
 // CHECK-NEXT: max-nodes = 225000
 // CHECK-NEXT: max-times-inline-large = 32
@@ -37,4 +36,4 @@ void foo() {
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: widen-loops = false
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 25
+// CHECK-NEXT: num-entries = 24
diff --git a/test/Analysis/analyzer-config.cpp b/test/Analysis/analyzer-config.cpp
index 5bbe71d8c0..2c8098e173 100644
--- a/test/Analysis/analyzer-config.cpp
+++ b/test/Analysis/analyzer-config.cpp
@@ -41,7 +41,6 @@ public:
 // CHECK-NEXT: inline-lambdas = true
 // CHECK-NEXT: ipa = dynamic-bifurcate
 // CHECK-NEXT: ipa-always-inline-size = 3
-// CHECK-NEXT: leak-diagnostics-reference-allocation = false
 // CHECK-NEXT: max-inlinable-size = 100
 // CHECK-NEXT: max-nodes = 225000
 // CHECK-NEXT: max-times-inline-large = 32
@@ -52,4 +51,4 @@ public:
 // CHECK-NEXT: unroll-loops = false
 // CHECK-NEXT: widen-loops = false
 // CHECK-NEXT: [stats]
-// CHECK-NEXT: num-entries = 32
+// CHECK-NEXT: num-entries = 31
diff --git a/test/Analysis/cstring-plist.c b/test/Analysis/cstring-plist.c
index 395937f340..c527564d49 100644
--- a/test/Analysis/cstring-plist.c
+++ b/test/Analysis/cstring-plist.c
@@ -1,5 +1,8 @@
 // RUN: rm -f %t
-// RUN: %clang_analyze_cc1 -fblocks -analyzer-checker=core,unix.Malloc,unix.cstring.NullArg -analyzer-disable-checker=alpha.unix.cstring.OutOfBounds -analyzer-output=plist -analyzer-config path-diagnostics-alternate=false -o %t %s
+// RUN: %clang_analyze_cc1 -fblocks \
+// RUN:   -analyzer-checker=core,unix.Malloc,unix.cstring.NullArg \
+// RUN:   -analyzer-disable-checker=alpha.unix.cstring.OutOfBounds \
+// RUN:   -analyzer-output=plist -o %t %s
 // RUN: FileCheck -input-file %t %s
 
 typedef __typeof(sizeof(int)) size_t;
diff --git a/test/Analysis/localization-aggressive.m b/test/Analysis/localization-aggressive.m
index ea5e0b1529..2e273e0c48 100644
--- a/test/Analysis/localization-aggressive.m
+++ b/test/Analysis/localization-aggressive.m
@@ -1,6 +1,10 @@
 // RUN: %clang_cc1 -fblocks -x objective-c-header -emit-pch -o %t.pch %S/Inputs/localization-pch.h
 
-// RUN: %clang_analyze_cc1 -fblocks -analyzer-store=region  -analyzer-checker=optin.osx.cocoa.localizability.NonLocalizedStringChecker -analyzer-checker=optin.osx.cocoa.localizability.EmptyLocalizationContextChecker -include-pch %t.pch -verify  -analyzer-config AggressiveReport=true %s
+// RUN: %clang_analyze_cc1 -fblocks -analyzer-store=region \
+// RUN:   -analyzer-config optin.osx.cocoa.localizability.NonLocalizedStringChecker:AggressiveReport=true \
+// RUN:   -analyzer-checker=optin.osx.cocoa.localizability.NonLocalizedStringChecker \
+// RUN:   -analyzer-checker=optin.osx.cocoa.localizability.EmptyLocalizationContextChecker \
+// RUN:   -include-pch %t.pch -verify  %s
 
 // These declarations were reduced using Delta-Debugging from Foundation.h
 // on Mac OS X.