Skip to content

Commit 549f9cd

Browse files
SzelethusSzelethus
committed
[analyzer] Evaluate all non-checker config options before analysis
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
1 parent be3f4bd commit 549f9cd

22 files changed

+381
-496
lines changed

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

Lines changed: 165 additions & 260 deletions
Large diffs are not rendered by default.

‎clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h

Lines changed: 28 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include "llvm/ADT/Optional.h"
2121
#include "llvm/ADT/StringMap.h"
2222
#include "llvm/ADT/StringRef.h"
23+
#include "llvm/ADT/StringSwitch.h"
2324
#include <string>
2425
#include <utility>
2526
#include <vector>
@@ -170,6 +171,7 @@ class AnalyzerOptions : public RefCountedBase<AnalyzerOptions> {
170171
std::vector<std::pair<std::string, bool>> CheckersControlList;
171172

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

223-
private:
224-
225-
#define ANALYZER_OPTION(TYPE, NAME, CMDFLAG, DESC, DEFAULT_VAL) \
226-
Optional<TYPE> NAME;
227225
#define ANALYZER_OPTION_DEPENDS_ON_USER_MODE(TYPE, NAME, CMDFLAG, DESC, \
228226
SHALLOW_VAL, DEEP_VAL) \
229-
Optional<TYPE> NAME;
227+
ANALYZER_OPTION(TYPE, NAME, CMDFLAG, DESC, SHALLOW_VAL)
228+
229+
#define ANALYZER_OPTION(TYPE, NAME, CMDFLAG, DESC, DEFAULT_VAL) \
230+
TYPE NAME;
231+
230232
#include "clang/StaticAnalyzer/Core/AnalyzerOptions.def"
231233
#undef ANALYZER_OPTION
232234
#undef ANALYZER_OPTION_DEPENDS_ON_USER_MODE
233235

234-
/// Query an option's string value.
235-
///
236-
/// If an option value is not provided, returns the given \p DefaultVal.
237-
/// @param [in] OptionName Name for option to retrieve.
238-
/// @param [in] DefaultVal Default value returned if no such option was
239-
/// specified.
240-
StringRef getStringOption(StringRef OptionName, StringRef DefaultVal);
241-
242-
void initOption(Optional<StringRef> &V, StringRef Name,
243-
StringRef DefaultVal);
244-
245-
void initOption(Optional<bool> &V, StringRef Name, bool DefaultVal);
246-
247-
void initOption(Optional<unsigned> &V, StringRef Name,
248-
unsigned DefaultVal);
249-
public:
250236
AnalyzerOptions()
251237
: DisableAllChecks(false), ShowCheckerHelp(false),
252238
ShowEnabledCheckerList(false), ShowConfigOptionsList(false),
@@ -308,56 +294,44 @@ class AnalyzerOptions : public RefCountedBase<AnalyzerOptions> {
308294
const ento::CheckerBase *C,
309295
bool SearchInParents = false) const;
310296

311-
#define ANALYZER_OPTION_GEN_FN(TYPE, NAME, CMDFLAG, DESC, DEFAULT_VAL, \
312-
CREATE_FN) \
313-
TYPE CREATE_FN() { \
314-
initOption(NAME, CMDFLAG, DEFAULT_VAL); \
315-
return NAME.getValue(); \
316-
}
317-
318-
#define ANALYZER_OPTION_GEN_FN_DEPENDS_ON_USER_MODE( \
319-
TYPE, NAME, CMDFLAG, DESC, SHALLOW_VAL, DEEP_VAL, CREATE_FN) \
320-
TYPE CREATE_FN() { \
321-
switch (getUserMode()) { \
322-
case UMK_Shallow: \
323-
initOption(NAME, CMDFLAG, SHALLOW_VAL); \
324-
break; \
325-
case UMK_Deep: \
326-
initOption(NAME, CMDFLAG, DEEP_VAL); \
327-
break; \
328-
} \
329-
\
330-
return NAME.getValue(); \
331-
}
332-
333-
#include "clang/StaticAnalyzer/Core/AnalyzerOptions.def"
334-
335-
#undef ANALYZER_OPTION_GEN_FN_DEPENDS_ON_USER_MODE
336-
#undef ANALYZER_OPTION_WITH_FN
337-
338297
/// Retrieves and sets the UserMode. This is a high-level option,
339298
/// which is used to set other low-level options. It is not accessible
340299
/// outside of AnalyzerOptions.
341-
UserModeKind getUserMode();
300+
UserModeKind getUserMode() const;
342301

343-
ExplorationStrategyKind getExplorationStrategy();
302+
ExplorationStrategyKind getExplorationStrategy() const;
344303

345304
/// Returns the inter-procedural analysis mode.
346-
IPAKind getIPAMode();
305+
IPAKind getIPAMode() const;
347306

348307
/// Returns the option controlling which C++ member functions will be
349308
/// considered for inlining.
350309
///
351310
/// This is controlled by the 'c++-inlining' config option.
352311
///
353312
/// \sa CXXMemberInliningMode
354-
bool mayInlineCXXMemberFunction(CXXInlineableMemberKind K);
355-
356-
StringRef getCTUDir();
313+
bool mayInlineCXXMemberFunction(CXXInlineableMemberKind K) const;
357314
};
358315

359316
using AnalyzerOptionsRef = IntrusiveRefCntPtr<AnalyzerOptions>;
360317

318+
//===----------------------------------------------------------------------===//
319+
// We'll use AnalyzerOptions in the frontend, but we can't link the frontend
320+
// with clangStaticAnalyzerCore, because clangStaticAnalyzerCore depends on
321+
// clangFrontend.
322+
//
323+
// For this reason, implement some methods in this header file.
324+
//===----------------------------------------------------------------------===//
325+
326+
inline UserModeKind AnalyzerOptions::getUserMode() const {
327+
auto K = llvm::StringSwitch<llvm::Optional<UserModeKind>>(UserMode)
328+
.Case("shallow", UMK_Shallow)
329+
.Case("deep", UMK_Deep)
330+
.Default(None);
331+
assert(K.hasValue() && "User mode is invalid.");
332+
return K.getValue();
333+
}
334+
361335
} // namespace clang
362336

363337
#endif // LLVM_CLANG_STATICANALYZER_CORE_ANALYZEROPTIONS_H

‎clang/lib/Frontend/CompilerInvocation.cpp

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,9 @@ static void addDiagnosticArgs(ArgList &Args, OptSpecifier Group,
181181
}
182182
}
183183

184+
static void parseAnalyzerConfigs(AnalyzerOptions &AnOpts,
185+
DiagnosticsEngine &Diags);
186+
184187
static void getAllNoBuiltinFuncValues(ArgList &Args,
185188
std::vector<std::string> &Funcs) {
186189
SmallVector<const char *, 8> Values;
@@ -343,6 +346,8 @@ static bool ParseAnalyzerArgs(AnalyzerOptions &Opts, ArgList &Args,
343346
}
344347
}
345348

349+
parseAnalyzerConfigs(Opts, Diags);
350+
346351
llvm::raw_string_ostream os(Opts.FullCompilerInvocation);
347352
for (unsigned i = 0; i < Args.getNumInputArgStrings(); ++i) {
348353
if (i != 0)
@@ -354,6 +359,64 @@ static bool ParseAnalyzerArgs(AnalyzerOptions &Opts, ArgList &Args,
354359
return Success;
355360
}
356361

362+
static StringRef getStringOption(AnalyzerOptions::ConfigTable &Config,
363+
StringRef OptionName, StringRef DefaultVal) {
364+
return Config.insert({OptionName, DefaultVal}).first->second;
365+
}
366+
367+
static void initOption(AnalyzerOptions::ConfigTable &Config,
368+
StringRef &OptionField, StringRef Name,
369+
StringRef DefaultVal) {
370+
OptionField = getStringOption(Config, Name, DefaultVal);
371+
}
372+
373+
static void initOption(AnalyzerOptions::ConfigTable &Config,
374+
bool &OptionField, StringRef Name, bool DefaultVal) {
375+
// FIXME: We should emit a warning here if the value is something other than
376+
// "true", "false", or the empty string (meaning the default value),
377+
// but the AnalyzerOptions doesn't have access to a diagnostic engine.
378+
OptionField = llvm::StringSwitch<bool>(getStringOption(Config, Name,
379+
(DefaultVal ? "true" : "false")))
380+
.Case("true", true)
381+
.Case("false", false)
382+
.Default(DefaultVal);
383+
}
384+
385+
static void initOption(AnalyzerOptions::ConfigTable &Config,
386+
unsigned &OptionField, StringRef Name,
387+
unsigned DefaultVal) {
388+
OptionField = DefaultVal;
389+
bool HasFailed = getStringOption(Config, Name, std::to_string(DefaultVal))
390+
.getAsInteger(10, OptionField);
391+
assert(!HasFailed && "analyzer-config option should be numeric");
392+
(void)HasFailed;
393+
}
394+
395+
static void parseAnalyzerConfigs(AnalyzerOptions &AnOpts,
396+
DiagnosticsEngine &Diags) {
397+
// TODO: Emit warnings for incorrect options.
398+
// TODO: There's no need to store the entire configtable, it'd be plenty
399+
// enough tostore checker options.
400+
401+
#define ANALYZER_OPTION(TYPE, NAME, CMDFLAG, DESC, DEFAULT_VAL) \
402+
initOption(AnOpts.Config, AnOpts.NAME, CMDFLAG, DEFAULT_VAL); \
403+
404+
#define ANALYZER_OPTION_DEPENDS_ON_USER_MODE(TYPE, NAME, CMDFLAG, DESC, \
405+
SHALLOW_VAL, DEEP_VAL) \
406+
switch (AnOpts.getUserMode()) { \
407+
case UMK_Shallow: \
408+
initOption(AnOpts.Config, AnOpts.NAME, CMDFLAG, SHALLOW_VAL); \
409+
break; \
410+
case UMK_Deep: \
411+
initOption(AnOpts.Config, AnOpts.NAME, CMDFLAG, DEEP_VAL); \
412+
break; \
413+
} \
414+
415+
#include "clang/StaticAnalyzer/Core/AnalyzerOptions.def"
416+
#undef ANALYZER_OPTION
417+
#undef ANALYZER_OPTION_DEPENDS_ON_USER_MODE
418+
}
419+
357420
static bool ParseMigratorArgs(MigratorOptions &Opts, ArgList &Args) {
358421
Opts.NoNSAllocReallocError = Args.hasArg(OPT_migrator_no_nsalloc_error);
359422
Opts.NoFinalizeRemoval = Args.hasArg(OPT_migrator_no_finalize_removal);

‎clang/lib/StaticAnalyzer/Checkers/DebugCheckers.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,9 @@ class ConfigDumper : public Checker< check::EndOfTranslationUnit > {
182182

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

187189
llvm::errs() << "[stats]\n" << "num-entries = " << Keys.size() << '\n';
188190
}

‎clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1094,7 +1094,7 @@ void MallocChecker::processNewAllocation(const CXXNewExpr *NE,
10941094

10951095
void MallocChecker::checkPostStmt(const CXXNewExpr *NE,
10961096
CheckerContext &C) const {
1097-
if (!C.getAnalysisManager().getAnalyzerOptions().mayInlineCXXAllocator())
1097+
if (!C.getAnalysisManager().getAnalyzerOptions().MayInlineCXXAllocator)
10981098
processNewAllocation(NE, C, C.getSVal(NE));
10991099
}
11001100

‎clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,16 +22,21 @@ AnalysisManager::AnalysisManager(ASTContext &ASTCtx, DiagnosticsEngine &diags,
2222
AnalyzerOptions &Options,
2323
CodeInjector *injector)
2424
: AnaCtxMgr(
25-
ASTCtx, Options.UnoptimizedCFG, Options.includeImplicitDtorsInCFG(),
26-
/*AddInitializers=*/true, Options.includeTemporaryDtorsInCFG(),
27-
Options.includeLifetimeInCFG(),
25+
ASTCtx, Options.UnoptimizedCFG,
26+
Options.ShouldIncludeImplicitDtorsInCFG,
27+
/*AddInitializers=*/true,
28+
Options.ShouldIncludeTemporaryDtorsInCFG,
29+
Options.ShouldIncludeLifetimeInCFG,
2830
// Adding LoopExit elements to the CFG is a requirement for loop
2931
// unrolling.
30-
Options.includeLoopExitInCFG() || Options.shouldUnrollLoops(),
31-
Options.includeScopesInCFG(), Options.shouldSynthesizeBodies(),
32-
Options.shouldConditionalizeStaticInitializers(),
33-
/*addCXXNewAllocator=*/true, Options.includeRichConstructorsInCFG(),
34-
Options.shouldElideConstructors(), injector),
32+
Options.ShouldIncludeLoopExitInCFG ||
33+
Options.ShouldUnrollLoops,
34+
Options.ShouldIncludeScopesInCFG,
35+
Options.ShouldSynthesizeBodies,
36+
Options.ShouldConditionalizeStaticInitializers,
37+
/*addCXXNewAllocator=*/true,
38+
Options.ShouldIncludeRichConstructorsInCFG,
39+
Options.ShouldElideConstructors, injector),
3540
Ctx(ASTCtx), Diags(diags), LangOpts(ASTCtx.getLangOpts()),
3641
PathConsumers(PDC), CreateStoreMgr(storemgr),
3742
CreateConstraintMgr(constraintmgr), CheckerMgr(checkerMgr),

0 commit comments

Comments
 (0)