Skip to content

[analyzer][clang-tidy][NFC] Clean up eagerly-assume handling #112209

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Oct 16, 2024
Merged

[analyzer][clang-tidy][NFC] Clean up eagerly-assume handling #112209

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
ea6ab3f
[analyzer][clang-tidy][NFC] Clean up eagerly-assume handling
NagyDonat Oct 14, 2024
fc143af
Rewrite the documentation of eagerly-assume
NagyDonat Oct 14, 2024
f033068
Improve doc comment of eagerlyAssumeOpBifurcation
NagyDonat Oct 14, 2024
8c4ec72
s/OpBifurcation/Bifurcation/, simplify the code
NagyDonat Oct 14, 2024
5e97f6d
auto -> const auto &
NagyDonat Oct 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion clang-tools-extra/clang-tidy/ClangTidy.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,6 @@ ClangTidyASTConsumerFactory::createASTConsumer(
if (!AnalyzerOptions.CheckersAndPackages.empty()) {
setStaticAnalyzerCheckerOpts(Context.getOptions(), AnalyzerOptions);
AnalyzerOptions.AnalysisDiagOpt = PD_NONE;
AnalyzerOptions.eagerlyAssumeBinOpBifurcation = true;
Copy link
Member

@whisperity whisperity Oct 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a functionality loss from Tidy's perspective? AFAIK, there is no way to set CSA analyser engine options when invoked through Tidy.

Copy link
Contributor

@5chmidti 5chmidti Oct 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clang-Tidy can set CSA options, I recently added support for it for --verify-config in #109523, where I had checked which syntax it has, and if the syntax worked beforehand.

E.g.: clang-analyzer-optin.cplusplus.UninitializedObject:Pedantic: true

Copy link
Contributor Author

@NagyDonat NagyDonat Oct 16, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, this is not functionality loss from Tidy's perspective, because this data member is dead, its value doesn't influence anything. The eagerly-assume feature is controlled by a different data member AnalyzerOptions.ShouldEagerlyAssume (which is true by default).

std::unique_ptr<ento::AnalysisASTConsumer> AnalysisConsumer =
ento::CreateAnalysisConsumer(Compiler);
AnalysisConsumer->AddDiagnosticConsumer(
Expand Down
8 changes: 3 additions & 5 deletions clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,8 +229,6 @@ class AnalyzerOptions : public RefCountedBase<AnalyzerOptions> {
unsigned AnalyzerDisplayProgress : 1;
unsigned AnalyzerNoteAnalysisEntryPoints : 1;

unsigned eagerlyAssumeBinOpBifurcation : 1;

unsigned TrimGraph : 1;
unsigned visualizeExplodedGraphWithGraphViz : 1;
unsigned UnoptimizedCFG : 1;
Expand Down Expand Up @@ -293,9 +291,9 @@ class AnalyzerOptions : public RefCountedBase<AnalyzerOptions> {
ShowConfigOptionsList(false),
ShouldEmitErrorsOnInvalidConfigValue(false), AnalyzeAll(false),
AnalyzerDisplayProgress(false), AnalyzerNoteAnalysisEntryPoints(false),
eagerlyAssumeBinOpBifurcation(false), TrimGraph(false),
visualizeExplodedGraphWithGraphViz(false), UnoptimizedCFG(false),
PrintStats(false), NoRetryExhausted(false), AnalyzerWerror(false) {}
TrimGraph(false), visualizeExplodedGraphWithGraphViz(false),
UnoptimizedCFG(false), PrintStats(false), NoRetryExhausted(false),
AnalyzerWerror(false) {}

/// Interprets an option's string value as a boolean. The "true" string is
/// interpreted as true and the "false" string is interpreted as false.
Expand Down
12 changes: 6 additions & 6 deletions clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -583,14 +583,14 @@ class ExprEngine {
ExplodedNode *Pred,
ExplodedNodeSet &Dst);

/// evalEagerlyAssumeBinOpBifurcation - Given the nodes in 'Src', eagerly assume symbolic
/// expressions of the form 'x != 0' and generate new nodes (stored in Dst)
/// with those assumptions.
void evalEagerlyAssumeBinOpBifurcation(ExplodedNodeSet &Dst, ExplodedNodeSet &Src,
const Expr *Ex);
/// evalEagerlyAssumeOpBifurcation - Given the nodes in 'Src', eagerly assume
/// comparison operator expressions like 'x != 0' or logical negation like
/// '!foo' and generate new nodes (stored in Dst) with those assumptions.
void evalEagerlyAssumeOpBifurcation(ExplodedNodeSet &Dst,
ExplodedNodeSet &Src, const Expr *Ex);

static std::pair<const ProgramPointTag *, const ProgramPointTag *>
geteagerlyAssumeBinOpBifurcationTags();
getEagerlyAssumeOpBifurcationTags();

ProgramStateRef handleLValueBitCast(ProgramStateRef state, const Expr *Ex,
const LocationContext *LCtx, QualType T,
Expand Down
2 changes: 1 addition & 1 deletion clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2695,7 +2695,7 @@ ConditionBRVisitor::VisitNodeImpl(const ExplodedNode *N,
PathSensitiveBugReport &BR) {
ProgramPoint ProgPoint = N->getLocation();
const std::pair<const ProgramPointTag *, const ProgramPointTag *> &Tags =
ExprEngine::geteagerlyAssumeBinOpBifurcationTags();
ExprEngine::getEagerlyAssumeOpBifurcationTags();

// If an assumption was made on a branch, it should be caught
// here by looking at the state transition.
Expand Down
41 changes: 19 additions & 22 deletions clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2129,7 +2129,7 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
(B->isRelationalOp() || B->isEqualityOp())) {
ExplodedNodeSet Tmp;
VisitBinaryOperator(cast<BinaryOperator>(S), Pred, Tmp);
evalEagerlyAssumeBinOpBifurcation(Dst, Tmp, cast<Expr>(S));
evalEagerlyAssumeOpBifurcation(Dst, Tmp, cast<Expr>(S));
}
else
VisitBinaryOperator(cast<BinaryOperator>(S), Pred, Dst);
Expand Down Expand Up @@ -2402,7 +2402,7 @@ void ExprEngine::Visit(const Stmt *S, ExplodedNode *Pred,
if (AMgr.options.ShouldEagerlyAssume && (U->getOpcode() == UO_LNot)) {
ExplodedNodeSet Tmp;
VisitUnaryOperator(U, Pred, Tmp);
evalEagerlyAssumeBinOpBifurcation(Dst, Tmp, U);
evalEagerlyAssumeOpBifurcation(Dst, Tmp, U);
}
else
VisitUnaryOperator(U, Pred, Dst);
Expand Down Expand Up @@ -3742,20 +3742,18 @@ void ExprEngine::evalLocation(ExplodedNodeSet &Dst,
BldrTop.addNodes(Tmp);
}

std::pair<const ProgramPointTag *, const ProgramPointTag*>
ExprEngine::geteagerlyAssumeBinOpBifurcationTags() {
static SimpleProgramPointTag
eagerlyAssumeBinOpBifurcationTrue(TagProviderName,
"Eagerly Assume True"),
eagerlyAssumeBinOpBifurcationFalse(TagProviderName,
"Eagerly Assume False");
return std::make_pair(&eagerlyAssumeBinOpBifurcationTrue,
&eagerlyAssumeBinOpBifurcationFalse);
std::pair<const ProgramPointTag *, const ProgramPointTag *>
ExprEngine::getEagerlyAssumeOpBifurcationTags() {
static SimpleProgramPointTag eagerlyAssumeOpBifurcationTrue(
TagProviderName, "Eagerly Assume True"),
eagerlyAssumeOpBifurcationFalse(TagProviderName, "Eagerly Assume False");
return std::make_pair(&eagerlyAssumeOpBifurcationTrue,
&eagerlyAssumeOpBifurcationFalse);
}

void ExprEngine::evalEagerlyAssumeBinOpBifurcation(ExplodedNodeSet &Dst,
ExplodedNodeSet &Src,
const Expr *Ex) {
void ExprEngine::evalEagerlyAssumeOpBifurcation(ExplodedNodeSet &Dst,
ExplodedNodeSet &Src,
const Expr *Ex) {
StmtNodeBuilder Bldr(Src, Dst, *currBldrCtx);

for (const auto Pred : Src) {
Expand All @@ -3767,28 +3765,27 @@ void ExprEngine::evalEagerlyAssumeBinOpBifurcation(ExplodedNodeSet &Dst,
continue;
}

ProgramStateRef state = Pred->getState();
SVal V = state->getSVal(Ex, Pred->getLocationContext());
ProgramStateRef State = Pred->getState();
SVal V = State->getSVal(Ex, Pred->getLocationContext());
std::optional<nonloc::SymbolVal> SEV = V.getAs<nonloc::SymbolVal>();
if (SEV && SEV->isExpression()) {
const std::pair<const ProgramPointTag *, const ProgramPointTag*> &tags =
geteagerlyAssumeBinOpBifurcationTags();
const std::pair<const ProgramPointTag *, const ProgramPointTag *> &Tags =
getEagerlyAssumeOpBifurcationTags();

ProgramStateRef StateTrue, StateFalse;
std::tie(StateTrue, StateFalse) = state->assume(*SEV);
auto [StateTrue, StateFalse] = State->assume(*SEV);

// First assume that the condition is true.
if (StateTrue) {
SVal Val = svalBuilder.makeIntVal(1U, Ex->getType());
StateTrue = StateTrue->BindExpr(Ex, Pred->getLocationContext(), Val);
Bldr.generateNode(Ex, Pred, StateTrue, tags.first);
Bldr.generateNode(Ex, Pred, StateTrue, Tags.first);
}

// Next, assume that the condition is false.
if (StateFalse) {
SVal Val = svalBuilder.makeIntVal(0U, Ex->getType());
StateFalse = StateFalse->BindExpr(Ex, Pred->getLocationContext(), Val);
Bldr.generateNode(Ex, Pred, StateFalse, tags.second);
Bldr.generateNode(Ex, Pred, StateFalse, Tags.second);
}
}
}
Expand Down
Loading