Skip to content

Commit

Permalink
[analyzer] Evaluate all non-checker config options before analysis
Browse files Browse the repository at this point in the history
In earlier patches regarding AnalyzerOptions, a lot of effort went into
gathering all config options, and changing the interface so that potential
misuse can be eliminited.

Up until this point, AnalyzerOptions only evaluated an option when it was
querried. For example, if we had a "-no-false-positives" flag, AnalyzerOptions
would store an Optional field for it that would be None up until somewhere in
the code until the flag's getter function is called.

However, now that we're confident that we've gathered all configs, we can
evaluate off of them before analysis, so we can emit a error on invalid input
even if that prticular flag will not matter in that particular run of the
analyzer. Another very big benefit of this is that debug.ConfigDumper will now
show the value of all configs every single time.

Also, almost all options related class have a similar interface, so uniformity
is also a benefit.

The implementation for errors on invalid input will be commited shorty.

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

llvm-svn: 348031
  • Loading branch information
Szelethus committed Nov 30, 2018
1 parent be3f4bd commit 549f9cd
Show file tree
Hide file tree
Showing 22 changed files with 381 additions and 496 deletions.
425 changes: 165 additions & 260 deletions clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def

Large diffs are not rendered by default.

82 changes: 28 additions & 54 deletions clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/StringSwitch.h"
#include <string>
#include <utility>
#include <vector>
Expand Down Expand Up @@ -170,6 +171,7 @@ class AnalyzerOptions : public RefCountedBase<AnalyzerOptions> {
std::vector<std::pair<std::string, bool>> CheckersControlList;

/// A key-value table of use-specified configuration values.
// TODO: This shouldn't be public.
ConfigTable Config;
AnalysisStores AnalysisStoreOpt = RegionStoreModel;
AnalysisConstraints AnalysisConstraintsOpt = RangeConstraintsModel;
Expand Down Expand Up @@ -220,33 +222,17 @@ class AnalyzerOptions : public RefCountedBase<AnalyzerOptions> {
/// The mode of function selection used during inlining.
AnalysisInliningMode InliningMode = NoRedundancy;

private:

#define ANALYZER_OPTION(TYPE, NAME, CMDFLAG, DESC, DEFAULT_VAL) \
Optional<TYPE> NAME;
#define ANALYZER_OPTION_DEPENDS_ON_USER_MODE(TYPE, NAME, CMDFLAG, DESC, \
SHALLOW_VAL, DEEP_VAL) \
Optional<TYPE> NAME;
ANALYZER_OPTION(TYPE, NAME, CMDFLAG, DESC, SHALLOW_VAL)

#define ANALYZER_OPTION(TYPE, NAME, CMDFLAG, DESC, DEFAULT_VAL) \
TYPE NAME;

#include "clang/StaticAnalyzer/Core/AnalyzerOptions.def"
#undef ANALYZER_OPTION
#undef ANALYZER_OPTION_DEPENDS_ON_USER_MODE

/// Query an option's string value.
///
/// If an option value is not provided, returns the given \p DefaultVal.
/// @param [in] OptionName Name for option to retrieve.
/// @param [in] DefaultVal Default value returned if no such option was
/// specified.
StringRef getStringOption(StringRef OptionName, StringRef DefaultVal);

void initOption(Optional<StringRef> &V, StringRef Name,
StringRef DefaultVal);

void initOption(Optional<bool> &V, StringRef Name, bool DefaultVal);

void initOption(Optional<unsigned> &V, StringRef Name,
unsigned DefaultVal);
public:
AnalyzerOptions()
: DisableAllChecks(false), ShowCheckerHelp(false),
ShowEnabledCheckerList(false), ShowConfigOptionsList(false),
Expand Down Expand Up @@ -308,56 +294,44 @@ class AnalyzerOptions : public RefCountedBase<AnalyzerOptions> {
const ento::CheckerBase *C,
bool SearchInParents = false) const;

#define ANALYZER_OPTION_GEN_FN(TYPE, NAME, CMDFLAG, DESC, DEFAULT_VAL, \
CREATE_FN) \
TYPE CREATE_FN() { \
initOption(NAME, CMDFLAG, DEFAULT_VAL); \
return NAME.getValue(); \
}

#define ANALYZER_OPTION_GEN_FN_DEPENDS_ON_USER_MODE( \
TYPE, NAME, CMDFLAG, DESC, SHALLOW_VAL, DEEP_VAL, CREATE_FN) \
TYPE CREATE_FN() { \
switch (getUserMode()) { \
case UMK_Shallow: \
initOption(NAME, CMDFLAG, SHALLOW_VAL); \
break; \
case UMK_Deep: \
initOption(NAME, CMDFLAG, DEEP_VAL); \
break; \
} \
\
return NAME.getValue(); \
}

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

#undef ANALYZER_OPTION_GEN_FN_DEPENDS_ON_USER_MODE
#undef ANALYZER_OPTION_WITH_FN

/// Retrieves and sets the UserMode. This is a high-level option,
/// which is used to set other low-level options. It is not accessible
/// outside of AnalyzerOptions.
UserModeKind getUserMode();
UserModeKind getUserMode() const;

ExplorationStrategyKind getExplorationStrategy();
ExplorationStrategyKind getExplorationStrategy() const;

/// Returns the inter-procedural analysis mode.
IPAKind getIPAMode();
IPAKind getIPAMode() const;

/// Returns the option controlling which C++ member functions will be
/// considered for inlining.
///
/// This is controlled by the 'c++-inlining' config option.
///
/// \sa CXXMemberInliningMode
bool mayInlineCXXMemberFunction(CXXInlineableMemberKind K);

StringRef getCTUDir();
bool mayInlineCXXMemberFunction(CXXInlineableMemberKind K) const;
};

using AnalyzerOptionsRef = IntrusiveRefCntPtr<AnalyzerOptions>;

//===----------------------------------------------------------------------===//
// We'll use AnalyzerOptions in the frontend, but we can't link the frontend
// with clangStaticAnalyzerCore, because clangStaticAnalyzerCore depends on
// clangFrontend.
//
// For this reason, implement some methods in this header file.
//===----------------------------------------------------------------------===//

inline UserModeKind AnalyzerOptions::getUserMode() const {
auto K = llvm::StringSwitch<llvm::Optional<UserModeKind>>(UserMode)
.Case("shallow", UMK_Shallow)
.Case("deep", UMK_Deep)
.Default(None);
assert(K.hasValue() && "User mode is invalid.");
return K.getValue();
}

} // namespace clang

#endif // LLVM_CLANG_STATICANALYZER_CORE_ANALYZEROPTIONS_H
63 changes: 63 additions & 0 deletions clang/lib/Frontend/CompilerInvocation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,9 @@ static void addDiagnosticArgs(ArgList &Args, OptSpecifier Group,
}
}

static void parseAnalyzerConfigs(AnalyzerOptions &AnOpts,
DiagnosticsEngine &Diags);

static void getAllNoBuiltinFuncValues(ArgList &Args,
std::vector<std::string> &Funcs) {
SmallVector<const char *, 8> Values;
Expand Down Expand Up @@ -343,6 +346,8 @@ static bool ParseAnalyzerArgs(AnalyzerOptions &Opts, ArgList &Args,
}
}

parseAnalyzerConfigs(Opts, Diags);

llvm::raw_string_ostream os(Opts.FullCompilerInvocation);
for (unsigned i = 0; i < Args.getNumInputArgStrings(); ++i) {
if (i != 0)
Expand All @@ -354,6 +359,64 @@ static bool ParseAnalyzerArgs(AnalyzerOptions &Opts, ArgList &Args,
return Success;
}

static StringRef getStringOption(AnalyzerOptions::ConfigTable &Config,
StringRef OptionName, StringRef DefaultVal) {
return Config.insert({OptionName, DefaultVal}).first->second;
}

static void initOption(AnalyzerOptions::ConfigTable &Config,
StringRef &OptionField, StringRef Name,
StringRef DefaultVal) {
OptionField = getStringOption(Config, Name, DefaultVal);
}

static void initOption(AnalyzerOptions::ConfigTable &Config,
bool &OptionField, StringRef Name, bool DefaultVal) {
// FIXME: We should emit a warning here if the value is something other than
// "true", "false", or the empty string (meaning the default value),
// but the AnalyzerOptions doesn't have access to a diagnostic engine.
OptionField = llvm::StringSwitch<bool>(getStringOption(Config, Name,
(DefaultVal ? "true" : "false")))
.Case("true", true)
.Case("false", false)
.Default(DefaultVal);
}

static void initOption(AnalyzerOptions::ConfigTable &Config,
unsigned &OptionField, StringRef Name,
unsigned DefaultVal) {
OptionField = DefaultVal;
bool HasFailed = getStringOption(Config, Name, std::to_string(DefaultVal))
.getAsInteger(10, OptionField);
assert(!HasFailed && "analyzer-config option should be numeric");
(void)HasFailed;
}

static void parseAnalyzerConfigs(AnalyzerOptions &AnOpts,
DiagnosticsEngine &Diags) {
// TODO: Emit warnings for incorrect options.
// TODO: There's no need to store the entire configtable, it'd be plenty
// enough tostore checker options.

#define ANALYZER_OPTION(TYPE, NAME, CMDFLAG, DESC, DEFAULT_VAL) \
initOption(AnOpts.Config, AnOpts.NAME, CMDFLAG, DEFAULT_VAL); \

#define ANALYZER_OPTION_DEPENDS_ON_USER_MODE(TYPE, NAME, CMDFLAG, DESC, \
SHALLOW_VAL, DEEP_VAL) \
switch (AnOpts.getUserMode()) { \
case UMK_Shallow: \
initOption(AnOpts.Config, AnOpts.NAME, CMDFLAG, SHALLOW_VAL); \
break; \
case UMK_Deep: \
initOption(AnOpts.Config, AnOpts.NAME, CMDFLAG, DEEP_VAL); \
break; \
} \

#include "clang/StaticAnalyzer/Core/AnalyzerOptions.def"
#undef ANALYZER_OPTION
#undef ANALYZER_OPTION_DEPENDS_ON_USER_MODE
}

static bool ParseMigratorArgs(MigratorOptions &Opts, ArgList &Args) {
Opts.NoNSAllocReallocError = Args.hasArg(OPT_migrator_no_nsalloc_error);
Opts.NoFinalizeRemoval = Args.hasArg(OPT_migrator_no_finalize_removal);
Expand Down
4 changes: 3 additions & 1 deletion clang/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,9 @@ class ConfigDumper : public Checker< check::EndOfTranslationUnit > {

llvm::errs() << "[config]\n";
for (unsigned I = 0, E = Keys.size(); I != E; ++I)
llvm::errs() << Keys[I]->getKey() << " = " << Keys[I]->second << '\n';
llvm::errs() << Keys[I]->getKey() << " = "
<< (Keys[I]->second.empty() ? "\"\"" : Keys[I]->second)
<< '\n';

llvm::errs() << "[stats]\n" << "num-entries = " << Keys.size() << '\n';
}
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1094,7 +1094,7 @@ void MallocChecker::processNewAllocation(const CXXNewExpr *NE,

void MallocChecker::checkPostStmt(const CXXNewExpr *NE,
CheckerContext &C) const {
if (!C.getAnalysisManager().getAnalyzerOptions().mayInlineCXXAllocator())
if (!C.getAnalysisManager().getAnalyzerOptions().MayInlineCXXAllocator)
processNewAllocation(NE, C, C.getSVal(NE));
}

Expand Down
21 changes: 13 additions & 8 deletions clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,21 @@ AnalysisManager::AnalysisManager(ASTContext &ASTCtx, DiagnosticsEngine &diags,
AnalyzerOptions &Options,
CodeInjector *injector)
: AnaCtxMgr(
ASTCtx, Options.UnoptimizedCFG, Options.includeImplicitDtorsInCFG(),
/*AddInitializers=*/true, Options.includeTemporaryDtorsInCFG(),
Options.includeLifetimeInCFG(),
ASTCtx, Options.UnoptimizedCFG,
Options.ShouldIncludeImplicitDtorsInCFG,
/*AddInitializers=*/true,
Options.ShouldIncludeTemporaryDtorsInCFG,
Options.ShouldIncludeLifetimeInCFG,
// Adding LoopExit elements to the CFG is a requirement for loop
// unrolling.
Options.includeLoopExitInCFG() || Options.shouldUnrollLoops(),
Options.includeScopesInCFG(), Options.shouldSynthesizeBodies(),
Options.shouldConditionalizeStaticInitializers(),
/*addCXXNewAllocator=*/true, Options.includeRichConstructorsInCFG(),
Options.shouldElideConstructors(), injector),
Options.ShouldIncludeLoopExitInCFG ||
Options.ShouldUnrollLoops,
Options.ShouldIncludeScopesInCFG,
Options.ShouldSynthesizeBodies,
Options.ShouldConditionalizeStaticInitializers,
/*addCXXNewAllocator=*/true,
Options.ShouldIncludeRichConstructorsInCFG,
Options.ShouldElideConstructors, injector),
Ctx(ASTCtx), Diags(diags), LangOpts(ASTCtx.getLangOpts()),
PathConsumers(PDC), CreateStoreMgr(storemgr),
CreateConstraintMgr(constraintmgr), CheckerMgr(checkerMgr),
Expand Down
Loading

0 comments on commit 549f9cd

Please sign in to comment.