[clang] [clang][analyzer] Add function 'fprintf' to StreamChecker. (PR #77613)
@@ -926,6 +932,49 @@ void StreamChecker::evalFputx(const FnDescription *Desc,
const CallEvent &Call,
C.addTransition(StateFailed);
}
+void StreamChecker::evalFprintf(const FnDescription *Desc,
+const CallEvent &Call,
+CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ if (Call.getNumArgs() < 2)
+return;
+ SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
+ if (!StreamSym)
+return;
+
+ const CallExpr *CE = dyn_cast_or_null(Call.getOriginExpr());
+ if (!CE)
+return;
+
+ const StreamState *OldSS = State->get(StreamSym);
+ if (!OldSS)
+return;
+
+ assertStreamStateOpened(OldSS);
+
+ NonLoc RetVal = makeRetVal(C, CE).castAs();
+ State = State->BindExpr(CE, C.getLocationContext(), RetVal);
+ SValBuilder &SVB = C.getSValBuilder();
+ auto &ACtx = C.getASTContext();
+ auto Cond = SVB.evalBinOp(State, BO_GE, RetVal, SVB.makeZeroVal(ACtx.IntTy),
+SVB.getConditionType())
+ .getAs();
+ if (!Cond)
+return;
+ ProgramStateRef StateNotFailed, StateFailed;
+ std::tie(StateNotFailed, StateFailed) = State->assume(*Cond);
balazs-benics-sonarsource wrote:
In addition to this, we should also invalidate any buffers passed to such
calls. This also sounds like a regression compared to default eval calling
"fpintf". (Look at the format string specifier `%n`)
https://github.com/llvm/llvm-project/pull/77613
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Add function 'ungetc' to StreamChecker. (PR #77331)
@@ -916,6 +922,45 @@ void StreamChecker::evalFputx(const FnDescription *Desc,
const CallEvent &Call,
C.addTransition(StateFailed);
}
+void StreamChecker::evalUngetc(const FnDescription *Desc, const CallEvent
&Call,
+ CheckerContext &C) const {
+ ProgramStateRef State = C.getState();
+ SymbolRef StreamSym = getStreamArg(Desc, Call).getAsSymbol();
+ if (!StreamSym)
+return;
+
+ const CallExpr *CE = dyn_cast_or_null(Call.getOriginExpr());
+ if (!CE)
+return;
+
+ const StreamState *OldSS = State->get(StreamSym);
+ if (!OldSS)
+return;
+
+ assertStreamStateOpened(OldSS);
+
+ // Generate a transition for the success state.
+ std::optional PutVal = Call.getArgSVal(0).getAs();
+ if (!PutVal)
+return;
+ ProgramStateRef StateNotFailed =
+ State->BindExpr(CE, C.getLocationContext(), *PutVal);
+ StateNotFailed =
+ StateNotFailed->set(StreamSym, StreamState::getOpened(Desc));
+ C.addTransition(StateNotFailed);
+
+ // Add transition for the failed state.
+ // Failure of 'ungetc' does not result in feof or ferror state.
+ // If the PutVal has value of EofVal the function should "fail", but this is
+ // the same transition as the success state.
+ // In this case only one state transition is added by the analyzer (the two
+ // new states may be similar).
+ ProgramStateRef StateFailed = bindInt(*EofVal, State, C, CE);
+ StateFailed =
+ StateFailed->set(StreamSym, StreamState::getOpened(Desc));
+ C.addTransition(StateFailed);
balazs-benics-sonarsource wrote:
Why did we not set `ErrorFError | ErrorFEof` errors for the failure state?
https://github.com/llvm/llvm-project/pull/77331
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][analyzer] Add function 'ungetc' to StreamChecker. (PR #77331)
https://github.com/balazs-benics-sonarsource deleted https://github.com/llvm/llvm-project/pull/77331 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Retry UNDEF Z3 queries at most "crosscheck-with-z3-retries-on-timeout" times (PR #120239)
@@ -77,16 +80,32 @@ void
Z3CrosscheckVisitor::finalizeVisitor(BugReporterContext &BRC,
RefutationSolver->addConstraint(SMTConstraints);
}
- // And check for satisfiability
- llvm::TimeRecord Start = llvm::TimeRecord::getCurrentTime(/*Start=*/true);
- std::optional IsSAT = RefutationSolver->check();
- llvm::TimeRecord Diff = llvm::TimeRecord::getCurrentTime(/*Start=*/false);
- Diff -= Start;
- Result = Z3Result{
- IsSAT,
- static_cast(Diff.getWallTime() * 1000),
- RefutationSolver->getStatistics()->getUnsigned("rlimit count"),
+ auto GetUsedRLimit = [](const llvm::SMTSolverRef &Solver) {
+return Solver->getStatistics()->getUnsigned("rlimit count");
+ };
+
+ auto AttemptOnce = [&](const llvm::SMTSolverRef &Solver) -> Z3Result {
+constexpr auto getCurrentTime = llvm::TimeRecord::getCurrentTime;
+unsigned InitialRLimit = GetUsedRLimit(Solver);
+double Start = getCurrentTime(/*Start=*/true).getWallTime();
+std::optional IsSAT = Solver->check();
+double End = getCurrentTime(/*Start=*/false).getWallTime();
+return {
+IsSAT,
+static_cast((End - Start) * 1000),
+GetUsedRLimit(Solver) - InitialRLimit,
+};
};
+
+ // And check for satisfiability
+ unsigned MinQueryTimeAcrossAttempts = std::numeric_limits::max();
+ for (unsigned I = 0; I <= Opts.Z3CrosscheckMaxAttemptsPerQuery; ++I) {
balazs-benics-sonarsource wrote:
Thanks, fixed in
https://github.com/llvm/llvm-project/pull/120239/commits/930b2a8c310d1f3ae0a4671e0dfaecbf80784f83.
https://github.com/llvm/llvm-project/pull/120239
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Retry UNDEF Z3 queries at most "crosscheck-with-z3-retries-on-timeout" times (PR #120239)
https://github.com/balazs-benics-sonarsource deleted https://github.com/llvm/llvm-project/pull/120239 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Add time-trace scopes for high-level analyzer steps (PR #125508)
https://github.com/balazs-benics-sonarsource approved this pull request.
I had a look at the PR, and it looks awesome.
Could you please update the attached speedscope image?
It looks like it's out of sync with the implementation, for example if you look
at the "Loc PostStmt { ... stuff here ...}" , it appears to include the
ProgramPoint dump, and according to your implementation it should only have
"Loc ". Am I right?
https://github.com/llvm/llvm-project/pull/125508
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Add time-trace scopes for high-level analyzer steps (PR #125508)
balazs-benics-sonarsource wrote: LGTM, I'll merge this PR once the premerge checks are green. Should be ready in a couple of hours. Thanks for the PR again! https://github.com/llvm/llvm-project/pull/125508 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][scan-build] Treat --use-cc and --use-c++ as shell commands (PR #131932)
balazs-benics-sonarsource wrote: Looks good as it is right now. Thanks for putting the effort into this. I've invited the rest of the folks probably interested in this to get a second opinion. https://github.com/llvm/llvm-project/pull/131932 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][scan-build] Treat --use-cc and --use-c++ as shell commands (PR #131932)
https://github.com/balazs-benics-sonarsource edited https://github.com/llvm/llvm-project/pull/131932 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][scan-build] Treat --use-cc and --use-c++ as shell commands (PR #131932)
https://github.com/balazs-benics-sonarsource approved this pull request. Thanks for the context. It looks good to me now. @haoNoQ, maybe you know some Perl, could you have a second opinion? Otherwise, let's merge this in a week. https://github.com/llvm/llvm-project/pull/131932 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][scan-build] Treat --use-cc and --use-c++ as shell commands (PR #131932)
balazs-benics-sonarsource wrote: I'd prefer option 2, because why else would we have a default compiler if that wasn't used in some workflows. A warning could never hurt. I'm also flexible on the subject. https://github.com/llvm/llvm-project/pull/131932 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Workaround for unintended slowdown (scope increase) (PR #136720)
@@ -0,0 +1,198 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection
-verify=expected,default %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection
-analyzer-config legacy-inlining-prevention=false -verify=expected,disabled %s
+
+int getNum(void); // Get an opaque number.
+
+void clang_analyzer_numTimesReached(void);
+void clang_analyzer_dump(int arg);
+
+//-
+// Simple case: inlined function never reaches `analyzer-max-loop`.
+
+int inner_simple(void) {
+ clang_analyzer_numTimesReached(); // expected-warning {{2}}
+ return 42;
+}
+
+int outer_simple(void) {
+ int x = inner_simple();
+ int y = inner_simple();
+ return 53 / (x - y); // expected-warning {{Division by zero}}
+}
+
+//-
+// Inlined function always reaches `analyzer-max-loop`.
+
+int inner_fixed_loop_1(void) {
+ int i;
+ clang_analyzer_numTimesReached(); // expected-warning {{1}}
+ for (i = 0; i < 10; i++);
+ clang_analyzer_numTimesReached(); // no-warning
balazs-benics-sonarsource wrote:
Sounds good.
https://github.com/llvm/llvm-project/pull/136720
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Workaround for unintended slowdown (scope increase) (PR #136720)
https://github.com/balazs-benics-sonarsource edited https://github.com/llvm/llvm-project/pull/136720 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Workaround for unintended slowdown (scope increase) (PR #136720)
@@ -0,0 +1,198 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection
-verify=expected,default %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection
-analyzer-config legacy-inlining-prevention=false -verify=expected,disabled %s
+
+int getNum(void); // Get an opaque number.
+
+void clang_analyzer_numTimesReached(void);
+void clang_analyzer_dump(int arg);
+
+//-
+// Simple case: inlined function never reaches `analyzer-max-loop`.
+
+int inner_simple(void) {
+ clang_analyzer_numTimesReached(); // expected-warning {{2}}
+ return 42;
+}
+
+int outer_simple(void) {
+ int x = inner_simple();
+ int y = inner_simple();
+ return 53 / (x - y); // expected-warning {{Division by zero}}
+}
+
+//-
+// Inlined function always reaches `analyzer-max-loop`.
+
+int inner_fixed_loop_1(void) {
+ int i;
+ clang_analyzer_numTimesReached(); // expected-warning {{1}}
+ for (i = 0; i < 10; i++);
+ clang_analyzer_numTimesReached(); // no-warning
+ return 42;
+}
+
+int outer_fixed_loop_1(void) {
+ int x = inner_fixed_loop_1();
+ int y = inner_fixed_loop_1();
+ return 53 / (x - y); // no-warning
balazs-benics-sonarsource wrote:
My problem with these `no-warnings` in general in this PR that it documents
what the test currently does, but what they should document what the tests
should/could expect.
In this case in an ideal world, we should actually get a diagnostic, thus the
desired outcome is not a `no-warning`.
Consequently, a FIXME would be more appropriate I think.
https://github.com/llvm/llvm-project/pull/136720
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Workaround for unintended slowdown (scope increase) (PR #136720)
balazs-benics-sonarsource wrote: One other note. We should backport this fix to clang-20 once it lands to main. https://github.com/llvm/llvm-project/pull/136720 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Workaround for unintended slowdown (scope increase) (PR #136720)
@@ -0,0 +1,198 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection
-verify=expected,default %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection
-analyzer-config legacy-inlining-prevention=false -verify=expected,disabled %s
+
+int getNum(void); // Get an opaque number.
+
+void clang_analyzer_numTimesReached(void);
+void clang_analyzer_dump(int arg);
+
+//-
+// Simple case: inlined function never reaches `analyzer-max-loop`.
+
+int inner_simple(void) {
+ clang_analyzer_numTimesReached(); // expected-warning {{2}}
+ return 42;
+}
+
+int outer_simple(void) {
+ int x = inner_simple();
+ int y = inner_simple();
+ return 53 / (x - y); // expected-warning {{Division by zero}}
+}
+
+//-
+// Inlined function always reaches `analyzer-max-loop`.
+
+int inner_fixed_loop_1(void) {
+ int i;
+ clang_analyzer_numTimesReached(); // expected-warning {{1}}
+ for (i = 0; i < 10; i++);
+ clang_analyzer_numTimesReached(); // no-warning
+ return 42;
+}
+
+int outer_fixed_loop_1(void) {
+ int x = inner_fixed_loop_1();
+ int y = inner_fixed_loop_1();
+ return 53 / (x - y); // no-warning
+}
+
+//-
+// Inlined function always reaches `analyzer-max-loop`; inlining is prevented
+// even for different entry points.
+// This test uses `clang_analyzer_dump` and distinct `arg` values because
+// `clang_analyzer_numTimesReached` only counts the paths reaching that node
+// during the analysis of one particular entry point, so it cannot distinguish
+// "two entry points reached this, both with one path" (where the two reports
+// are unified as duplicates by the generic report postprocessing) and "one
+// entry point reached this with one path" (where naturally nothing shows that
+// the second entry point _tried_ to reach it).
+
+int inner_fixed_loop_2(int arg) {
+ // Identical copy of inner_fixed_loop_1
+ int i;
+ clang_analyzer_dump(arg); // expected-warning {{2}}
+ for (i = 0; i < 10; i++);
+ clang_analyzer_dump(arg); // no-warning
balazs-benics-sonarsource wrote:
FIXME.
https://github.com/llvm/llvm-project/pull/136720
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Workaround for unintended slowdown (scope increase) (PR #136720)
https://github.com/balazs-benics-sonarsource requested changes to this pull request. Thank you for investigating this. At Sonar, we have not yet started the upgrade to clang-20. I suppose, you already did then, thus found this regression on trunk. Maybe we should also reflect of the quality control of our submissions of core changes too, but let's leave that after the PR is discussed, and we put out the fire. My impression is that we shouldn't have a new flag for this, we should unconditionally apply this for now. Did you think about this? I left a few other comments about testing but overall I'm all for this change. Thanks again! https://github.com/llvm/llvm-project/pull/136720 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Workaround for unintended slowdown (scope increase) (PR #136720)
@@ -0,0 +1,198 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection
-verify=expected,default %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection
-analyzer-config legacy-inlining-prevention=false -verify=expected,disabled %s
+
+int getNum(void); // Get an opaque number.
+
+void clang_analyzer_numTimesReached(void);
+void clang_analyzer_dump(int arg);
+
+//-
+// Simple case: inlined function never reaches `analyzer-max-loop`.
+
+int inner_simple(void) {
+ clang_analyzer_numTimesReached(); // expected-warning {{2}}
+ return 42;
+}
+
+int outer_simple(void) {
+ int x = inner_simple();
+ int y = inner_simple();
+ return 53 / (x - y); // expected-warning {{Division by zero}}
+}
+
+//-
+// Inlined function always reaches `analyzer-max-loop`.
+
+int inner_fixed_loop_1(void) {
+ int i;
+ clang_analyzer_numTimesReached(); // expected-warning {{1}}
balazs-benics-sonarsource wrote:
You could pass a `const char*` from a string literal to differentiate which is
inlined when observing its value using a `clang_analyzer_dump()` at the
beginning of this inlined function. That should make it clear that for the
first time its inlined, but for the second time it's not.
https://github.com/llvm/llvm-project/pull/136720
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Workaround for unintended slowdown (scope increase) (PR #136720)
@@ -0,0 +1,198 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection
-verify=expected,default %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection
-analyzer-config legacy-inlining-prevention=false -verify=expected,disabled %s
+
+int getNum(void); // Get an opaque number.
+
+void clang_analyzer_numTimesReached(void);
+void clang_analyzer_dump(int arg);
+
+//-
+// Simple case: inlined function never reaches `analyzer-max-loop`.
+
+int inner_simple(void) {
+ clang_analyzer_numTimesReached(); // expected-warning {{2}}
+ return 42;
+}
+
+int outer_simple(void) {
+ int x = inner_simple();
+ int y = inner_simple();
+ return 53 / (x - y); // expected-warning {{Division by zero}}
+}
+
+//-
+// Inlined function always reaches `analyzer-max-loop`.
+
+int inner_fixed_loop_1(void) {
+ int i;
+ clang_analyzer_numTimesReached(); // expected-warning {{1}}
+ for (i = 0; i < 10; i++);
+ clang_analyzer_numTimesReached(); // no-warning
balazs-benics-sonarsource wrote:
```suggestion
clang_analyzer_numTimesReached(); // FIXME: It should be reachable
```
https://github.com/llvm/llvm-project/pull/136720
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Workaround for unintended slowdown (scope increase) (PR #136720)
@@ -2523,6 +2523,20 @@ bool ExprEngine::replayWithoutInlining(ExplodedNode *N,
return true;
}
+/// Return the innermost location context which is inlined at `Node`, unless
+/// it's the top-level (entry point) location context.
+static const LocationContext *getInlinedLocationContext(ExplodedNode *Node,
+ExplodedGraph &G) {
+ const LocationContext *CalleeLC = Node->getLocation().getLocationContext();
+ const LocationContext *RootLC =
+ (*G.roots_begin())->getLocation().getLocationContext();
+
+ if (CalleeLC->getStackFrame() == RootLC->getStackFrame())
+return nullptr;
+
+ return CalleeLC;
+}
balazs-benics-sonarsource wrote:
~~Is this basically the getStackFrame's parent's getStackFrame?~~
No, you are right. There may be some other location context above the Stack
frame context of the parent frame.
https://github.com/llvm/llvm-project/pull/136720
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Workaround for unintended slowdown (scope increase) (PR #136720)
@@ -81,10 +81,6 @@ class FunctionSummariesTy {
I->second.MayInline = 0;
}
- void markReachedMaxBlockCount(const Decl *D) {
-markShouldNotInline(D);
- }
balazs-benics-sonarsource wrote:
I'd not mind keeping this if there was more thing to do once a "Max block
count" is reached. But in this particular case I agree with you.
https://github.com/llvm/llvm-project/pull/136720
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Workaround for unintended slowdown (scope increase) (PR #136720)
@@ -0,0 +1,198 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection
-verify=expected,default %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection
-analyzer-config legacy-inlining-prevention=false -verify=expected,disabled %s
+
+int getNum(void); // Get an opaque number.
+
+void clang_analyzer_numTimesReached(void);
+void clang_analyzer_dump(int arg);
+
+//-
+// Simple case: inlined function never reaches `analyzer-max-loop`.
+
+int inner_simple(void) {
+ clang_analyzer_numTimesReached(); // expected-warning {{2}}
+ return 42;
+}
+
+int outer_simple(void) {
+ int x = inner_simple();
+ int y = inner_simple();
+ return 53 / (x - y); // expected-warning {{Division by zero}}
+}
+
+//-
+// Inlined function always reaches `analyzer-max-loop`.
+
+int inner_fixed_loop_1(void) {
+ int i;
+ clang_analyzer_numTimesReached(); // expected-warning {{1}}
+ for (i = 0; i < 10; i++);
+ clang_analyzer_numTimesReached(); // no-warning
+ return 42;
+}
+
+int outer_fixed_loop_1(void) {
+ int x = inner_fixed_loop_1();
+ int y = inner_fixed_loop_1();
balazs-benics-sonarsource wrote:
We should have comments here that first it's inlined, second it's not because
of the given heuristic.
https://github.com/llvm/llvm-project/pull/136720
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Workaround for unintended slowdown (scope increase) (PR #136720)
@@ -2856,8 +2873,29 @@ void ExprEngine::processBranch(
// conflicts with the widen-loop analysis option (which is off by
// default). If we intend to support and stabilize the loop widening,
// we must ensure that it 'plays nicely' with this logic.
- if (!SkipTrueBranch || AMgr.options.ShouldWidenLoops)
+ if (!SkipTrueBranch || AMgr.options.ShouldWidenLoops) {
Builder.generateNode(StTrue, true, PredN);
+ } else if (AMgr.options.LegacyInliningPrevention) {
+// FIXME: There is an ancient and very arbitrary heuristic in
+// `ExprEngine::processCFGBlockEntrance` which prevents all further
+// inlining of a function if it finds an execution path within that
+// function which reaches the `MaxBlockVisitOnPath` limit (a/k/a
+// `analyzer-max-loop`, by default four iterations in a loop). Adding
+// this "don't assume third iteration" logic significantly increased
+// the analysis runtime on some inputs because less functions were
+// arbitrarily excluded from being inlined, so more entrypoints used
+// up their full allocated budget. As a hacky compensation for this,
+// here we apply the "should not inline" mark in cases when the loop
+// could potentially reach the `MaxBlockVisitOnPath` limit without the
+// "don't assume third iteration" logic. This slightly overcompensates
+// (activates if the third iteration can be entered, and will not
+// recognize cases where the fourth iteration would't be completed),
but
+// should be good enough for practical purposes.
+if (const LocationContext *LC = getInlinedLocationContext(Pred, G)) {
+ Engine.FunctionSummaries->markShouldNotInline(
+ LC->getStackFrame()->getDecl());
+}
+ }
balazs-benics-sonarsource wrote:
How about if we would apply this chunk unconditionally? I don't think anyone
would want to regress 6x voluntarily.
https://github.com/llvm/llvm-project/pull/136720
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Workaround for slowdown spikes (unintended scope increase) (PR #136720)
https://github.com/balazs-benics-sonarsource edited https://github.com/llvm/llvm-project/pull/136720 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Workaround for slowdown spikes (unintended scope increase) (PR #136720)
https://github.com/balazs-benics-sonarsource commented: Looks good. There were two points unaddressed: - Finding a name for the flag without the `legacy-` prefix - Find out if we can ever have multiple root nodes in an exploded graph. https://github.com/llvm/llvm-project/pull/136720 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Workaround for slowdown spikes (unintended scope increase) (PR #136720)
@@ -2523,6 +2523,20 @@ bool ExprEngine::replayWithoutInlining(ExplodedNode *N,
return true;
}
+/// Return the innermost location context which is inlined at `Node`, unless
+/// it's the top-level (entry point) location context.
+static const LocationContext *getInlinedLocationContext(ExplodedNode *Node,
+ExplodedGraph &G) {
+ const LocationContext *CalleeLC = Node->getLocation().getLocationContext();
+ const LocationContext *RootLC =
balazs-benics-sonarsource wrote:
This wasn't addressed.
https://github.com/llvm/llvm-project/pull/136720
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Workaround for slowdown spikes (unintended scope increase) (PR #136720)
@@ -2523,6 +2523,20 @@ bool ExprEngine::replayWithoutInlining(ExplodedNode *N,
return true;
}
+/// Return the innermost location context which is inlined at `Node`, unless
+/// it's the top-level (entry point) location context.
+static const LocationContext *getInlinedLocationContext(ExplodedNode *Node,
+ExplodedGraph &G) {
+ const LocationContext *CalleeLC = Node->getLocation().getLocationContext();
+ const LocationContext *RootLC =
balazs-benics-sonarsource wrote:
Awesome, thank you for checking!
https://github.com/llvm/llvm-project/pull/136720
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Workaround for slowdown spikes (unintended scope increase) (PR #136720)
balazs-benics-sonarsource wrote: @Xazax-hun WDYT of the proposed alternative flag name? https://github.com/llvm/llvm-project/pull/136720 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Workaround for unintended slowdown (scope increase) (PR #136720)
balazs-benics-sonarsource wrote: Do you have data about the analysis times per file, and per analysis entry point? Compared against the current llvm main, and also if this workaround would restore the original running times before https://github.com/llvm/llvm-project/commit/bb27d5e5c6b194a1440b8ac4e5ace68d0ee2a849? https://github.com/llvm/llvm-project/pull/136720 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Workaround for unintended slowdown (scope increase) (PR #136720)
balazs-benics-sonarsource wrote:
> 🤔 The version with the commit under review is surprisingly fast and I don't
> exactly know why. My most plausible theory is that
> [bb27d5e](https://github.com/llvm/llvm-project/commit/bb27d5e5c6b194a1440b8ac4e5ace68d0ee2a849)
> ("Don't assume third iteration") has two effects on the analysis runtime:
>
> * it inherently speeds up the analysis of loops (because some iterations are
> skipped);
> * it slows down the analysis because it affects the inlining heuristic and
> prevents functions from being placed on the inlining blacklist.
>
> This would explain why just reverting that commit (which undoes both effects)
> produces slower analysis than applying the commit under review (which undoes
> the slowdown and keeps the speedup).
This makes sense, and I was expecting even back in the day. But I was shocked
that sometimes intuition fails, and we didn't check the RT for that patch. Now
that you did the work, it leaves me in a lot more relaxed situation. Thanks!
https://github.com/llvm/llvm-project/pull/136720
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Workaround for unintended slowdown (scope increase) (PR #136720)
balazs-benics-sonarsource wrote: I can see your points. I think they indeed moves the needle slightly for having this (or a similar) flag but barely. I'd need to think about a better flag name, but right now I'm too busy for that. https://github.com/llvm/llvm-project/pull/136720 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Workaround for slowdown spikes (unintended scope increase) (PR #136720)
https://github.com/balazs-benics-sonarsource approved this pull request. Thank you! https://github.com/llvm/llvm-project/pull/136720 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Limit Store by region-store-binding-limit (PR #127602)
https://github.com/balazs-benics-sonarsource updated
https://github.com/llvm/llvm-project/pull/127602
From f5cd6b22fb83c0bfb584717cde6899cd65fc1274 Mon Sep 17 00:00:00 2001
From: Balazs Benics
Date: Wed, 5 Feb 2025 17:13:34 +0100
Subject: [PATCH 1/7] [analyzer] Limit Store by region-store-binding-limit
In our test pool, the max entry point RT was improved by this change:
1'181 seconds (~19.7 minutes) -> 94 seconds (1.6 minutes)
BTW, the 1.6 minutes is still really bad. But a few orders of magnitude
better than it was before.
This was the most servere RT edge-case as you can see from the numbers.
There are are more known RT bottlenecks, such as:
- Large environment sizes, and `removeDead`. See more about the failed
attempt on improving it at:
https://discourse.llvm.org/t/unsuccessful-attempts-to-fix-a-slow-analysis-case-related-to-removedead-and-environment-size/84650
- Large chunk of time could be spend inside `assume`, to reach a fixed
point. This is something we want to look into a bit later if we have
time.
We have 3'075'607 entry points in our test set.
About 393'352 entry points ran longer than 1 second when measured.
To give a sense of the distribution, if we ignore the slowest 500
entry points, then the maximum entry point runs for about 14 seconds.
These 500 slow entry points are in 332 translation units.
By this patch, out of the slowest 500 entry points, 72 entry points
were improved by at least 10x after this change.
We measured no RT regression on the "usual" entry points.
CPP-6092
---
.../StaticAnalyzer/Core/AnalyzerOptions.def | 8 +
.../Core/PathSensitive/ExprEngine.h | 2 +-
.../StaticAnalyzer/Core/PathSensitive/Store.h | 10 +-
.../lib/StaticAnalyzer/Core/ProgramState.cpp | 18 +-
clang/lib/StaticAnalyzer/Core/RegionStore.cpp | 210 +++
clang/lib/StaticAnalyzer/Core/Store.cpp | 7 +-
clang/test/Analysis/analyzer-config.c | 1 +
clang/test/Analysis/region-store.cpp | 336 +-
clang/unittests/StaticAnalyzer/StoreTest.cpp | 7 +-
9 files changed, 525 insertions(+), 74 deletions(-)
diff --git a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
index a9b8d0753673b..f05c8724d583d 100644
--- a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
+++ b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
@@ -483,6 +483,14 @@ ANALYZER_OPTION(
"behavior, set the option to 0.",
5)
+ANALYZER_OPTION(
+unsigned, RegionStoreMaxBindingFanOut, "region-store-max-binding-fanout",
+"This option limits how many sub-bindings a single binding operation can "
+"scatter into. For example, binding an array would scatter into binding "
+"each individual element. Setting this to zero means unlimited, but then "
+"modelling large array initializers may take proportional time to their "
+"size.", 100)
+
//===--===//
// String analyzer options.
//===--===//
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
index 20c446e33ef9a..9fd07ce47175c 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -659,13 +659,13 @@ class ExprEngine {
SVal Loc, SVal Val,
const LocationContext *LCtx);
+public:
/// A simple wrapper when you only need to notify checkers of pointer-escape
/// of some values.
ProgramStateRef escapeValues(ProgramStateRef State, ArrayRef Vs,
PointerEscapeKind K,
const CallEvent *Call = nullptr) const;
-public:
// FIXME: 'tag' should be removed, and a LocationContext should be used
// instead.
// FIXME: Comment on the meaning of the arguments, when 'St' may not
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
index 332855a3c9c45..ebf00d49b6cc8 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/Store.h
@@ -50,6 +50,14 @@ class SymbolReaper;
using InvalidatedSymbols = llvm::DenseSet;
+struct BindResult {
+ StoreRef ResultingStore;
+
+ // If during the bind operation we exhaust the allowed binding budget, we set
+ // this to the beginning of the escaped part of the region.
+ llvm::SmallVector FailedToBindValues;
+};
+
class StoreManager {
protected:
SValBuilder &svalBuilder;
@@ -105,7 +113,7 @@ class StoreManager {
/// \return A StoreRef object that contains the same
/// bindings as \c store with the addition of having th
[clang] [analyzer] Limit Store by region-store-binding-limit (PR #127602)
@@ -176,31 +177,59 @@ class RegionBindingsRef : public
llvm::ImmutableMapRefpush_back(V);
+return *this;
+ }
+ RegionBindingsRef escapeValues(nonloc::CompoundVal::iterator Begin,
+ nonloc::CompoundVal::iterator End) const {
+for (SVal V : llvm::make_range(Begin, End))
+ escapeValue(V);
+return *this;
+ }
+
typedef llvm::ImmutableMapRef
ParentTy;
RegionBindingsRef(ClusterBindings::Factory &CBFactory,
+SmallVectorImpl *EscapedValuesDuringBind,
const RegionBindings::TreeTy *T,
-RegionBindings::TreeTy::Factory *F,
-bool IsMainAnalysis)
- : llvm::ImmutableMapRef(T, F),
-CBFactory(&CBFactory), IsMainAnalysis(IsMainAnalysis) {}
-
- RegionBindingsRef(const ParentTy &P,
-ClusterBindings::Factory &CBFactory,
-bool IsMainAnalysis)
- : llvm::ImmutableMapRef(P),
-CBFactory(&CBFactory), IsMainAnalysis(IsMainAnalysis) {}
+RegionBindings::TreeTy::Factory *F, bool IsMainAnalysis,
+unsigned BindingsLeft)
+ : RegionBindingsRef(ParentTy(T, F), CBFactory, EscapedValuesDuringBind,
+ IsMainAnalysis, BindingsLeft) {}
+
+ RegionBindingsRef(const ParentTy &P, ClusterBindings::Factory &CBFactory,
+SmallVectorImpl *EscapedValuesDuringBind,
+bool IsMainAnalysis, unsigned BindingsLeft)
+ : ParentTy(P), CBFactory(&CBFactory),
+EscapedValuesDuringBind(EscapedValuesDuringBind),
+IsMainAnalysis(IsMainAnalysis), BindingsLeft(BindingsLeft) {}
+
+ RegionBindingsRef add(key_type_ref K, data_type_ref D,
balazs-benics-sonarsource wrote:
> the real types were hidden behind the meaningless aliases key_type_ref and
> data_type_ref.)
Yes, I've desugared the param types now. Also renamed `add` and friends to
bring some meaning to their names. I hope it clarified what was missing.
https://github.com/llvm/llvm-project/pull/127602
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Limit Store by region-store-binding-limit (PR #127602)
@@ -2782,6 +2865,8 @@ RegionBindingsRef RegionStoreManager::bindStruct(RegionBindingsConstRef B, if (VI == VE) break; + if (NewB.hasExhaustedBindingLimit()) +return NewB.escapeValues(VI, VE); balazs-benics-sonarsource wrote: I've renamed the function as you proposed to `withValuesEscaped` in 2e685b966805 https://github.com/llvm/llvm-project/pull/127602 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Limit Store by region-store-binding-limit (PR #127602)
@@ -483,6 +483,14 @@ ANALYZER_OPTION( "behavior, set the option to 0.", 5) +ANALYZER_OPTION( +unsigned, RegionStoreMaxBindingFanOut, "region-store-max-binding-fanout", +"This option limits how many sub-bindings a single binding operation can " +"scatter into. For example, binding an array would scatter into binding " +"each individual element. Setting this to zero means unlimited, but then " +"modelling large array initializers may take proportional time to their " +"size.", 100) balazs-benics-sonarsource wrote: Matching with my default in 067de3d33524 https://github.com/llvm/llvm-project/pull/127602 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Limit Store by region-store-binding-limit (PR #127602)
https://github.com/balazs-benics-sonarsource edited https://github.com/llvm/llvm-project/pull/127602 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Limit Store by region-store-binding-limit (PR #127602)
https://github.com/balazs-benics-sonarsource edited https://github.com/llvm/llvm-project/pull/127602 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Limit Store by region-store-binding-limit (PR #127602)
balazs-benics-sonarsource wrote: Thanks for your long review. I'm sorry if the poor code quality hindered the comprehension. My goal was to minimize the diff for easier review, but I admit that I should have attached some considerations as to why I implemented it this way, and also how different parts work under the hood. I'll keep this in mind for next time! I'm still working through your review, but I wanted to post a quick update because I think the renamings in place now may make refining your stance easier. In short, I decided to put a strong type in place to track and enforce the bind limit. Now, we should have greater confidence of that nothing misses the checks. However, this comes at the cost of polluting the other APIs, like `BindDefaultInitial` or `BindDefaultZero` where it's highly unexpected to hit this binding limit - because all they usually do is basically add one default and maybe an additional direct binding. But now, looking at it, it led to a more consistent API, where it's harder to make mistakes, so I'm all for this change. Now, I'll get back to the rest of your comments and respond eventually. Thanks! https://github.com/llvm/llvm-project/pull/127602 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Limit Store by region-store-binding-limit (PR #127602)
https://github.com/balazs-benics-sonarsource created
https://github.com/llvm/llvm-project/pull/127602
In our test pool, the max entry point RT was improved by this change: 1'181
seconds (~19.7 minutes) -> 94 seconds (1.6 minutes)
BTW, the 1.6 minutes is still really bad. But a few orders of magnitude better
than it was before.
This was the most servere RT edge-case as you can see from the numbers. There
are are more known RT bottlenecks, such as:
- Large environment sizes, and `removeDead`. See more about the failed attempt
on improving it at:
https://discourse.llvm.org/t/unsuccessful-attempts-to-fix-a-slow-analysis-case-related-to-removedead-and-environment-size/84650
- Large chunk of time could be spend inside `assume`, to reach a fixed point.
This is something we want to look into a bit later if we have time.
We have 3'075'607 entry points in our test set.
About 393'352 entry points ran longer than 1 second when measured.
To give a sense of the distribution, if we ignore the slowest 500 entry points,
then the maximum entry point runs for about 14 seconds. These 500 slow entry
points are in 332 translation units.
By this patch, out of the slowest 500 entry points, 72 entry points were
improved by at least 10x after this change.
We measured no RT regression on the "usual" entry points.
(The dashed lines represent the maximum of their RT)
CPP-6092
From f5cd6b22fb83c0bfb584717cde6899cd65fc1274 Mon Sep 17 00:00:00 2001
From: Balazs Benics
Date: Wed, 5 Feb 2025 17:13:34 +0100
Subject: [PATCH] [analyzer] Limit Store by region-store-binding-limit
In our test pool, the max entry point RT was improved by this change:
1'181 seconds (~19.7 minutes) -> 94 seconds (1.6 minutes)
BTW, the 1.6 minutes is still really bad. But a few orders of magnitude
better than it was before.
This was the most servere RT edge-case as you can see from the numbers.
There are are more known RT bottlenecks, such as:
- Large environment sizes, and `removeDead`. See more about the failed
attempt on improving it at:
https://discourse.llvm.org/t/unsuccessful-attempts-to-fix-a-slow-analysis-case-related-to-removedead-and-environment-size/84650
- Large chunk of time could be spend inside `assume`, to reach a fixed
point. This is something we want to look into a bit later if we have
time.
We have 3'075'607 entry points in our test set.
About 393'352 entry points ran longer than 1 second when measured.
To give a sense of the distribution, if we ignore the slowest 500
entry points, then the maximum entry point runs for about 14 seconds.
These 500 slow entry points are in 332 translation units.
By this patch, out of the slowest 500 entry points, 72 entry points
were improved by at least 10x after this change.
We measured no RT regression on the "usual" entry points.
CPP-6092
---
.../StaticAnalyzer/Core/AnalyzerOptions.def | 8 +
.../Core/PathSensitive/ExprEngine.h | 2 +-
.../StaticAnalyzer/Core/PathSensitive/Store.h | 10 +-
.../lib/StaticAnalyzer/Core/ProgramState.cpp | 18 +-
clang/lib/StaticAnalyzer/Core/RegionStore.cpp | 210 +++
clang/lib/StaticAnalyzer/Core/Store.cpp | 7 +-
clang/test/Analysis/analyzer-config.c | 1 +
clang/test/Analysis/region-store.cpp | 336 +-
clang/unittests/StaticAnalyzer/StoreTest.cpp | 7 +-
9 files changed, 525 insertions(+), 74 deletions(-)
diff --git a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
index a9b8d0753673b..f05c8724d583d 100644
--- a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
+++ b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def
@@ -483,6 +483,14 @@ ANALYZER_OPTION(
"behavior, set the option to 0.",
5)
+ANALYZER_OPTION(
+unsigned, RegionStoreMaxBindingFanOut, "region-store-max-binding-fanout",
+"This option limits how many sub-bindings a single binding operation can "
+"scatter into. For example, binding an array would scatter into binding "
+"each individual element. Setting this to zero means unlimited, but then "
+"modelling large array initializers may take proportional time to their "
+"size.", 100)
+
//===--===//
// String analyzer options.
//===--===//
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
index 20c446e33ef9a..9fd07ce47175c 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
@@ -659,13 +659,13 @@ class ExprEngine {
SVal Loc, SVa
[clang] [analyzer] Limit Store by region-store-binding-limit (PR #127602)
balazs-benics-sonarsource wrote: @Flandini @necto https://github.com/llvm/llvm-project/pull/127602 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Limit Store by region-store-binding-limit (PR #127602)
balazs-benics-sonarsource wrote: > Hello @balazs-benics-sonarsource > > The following starts crashing with this patch: `clang --analyze bbi-104578.c` > It crashes with Thank you for the heads up @mikaelholmen. I'll switch to it ASAP. I'd expect the fix later today. https://github.com/llvm/llvm-project/pull/127602 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Limit Store by region-store-binding-limit (PR #127602)
balazs-benics-sonarsource wrote: Confirmed crash, https://compiler-explorer.com/z/fzoqP36xq https://github.com/llvm/llvm-project/pull/127602 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Fix RegionStore assertion failure after #127602 (PR #129224)
https://github.com/balazs-benics-sonarsource closed https://github.com/llvm/llvm-project/pull/129224 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Add time-trace scopes for high-level analyzer steps (PR #125508)
@@ -179,8 +181,41 @@ bool CoreEngine::ExecuteWorkList(const LocationContext *L,
unsigned MaxSteps,
return WList->hasWork();
}
-void CoreEngine::dispatchWorkItem(ExplodedNode* Pred, ProgramPoint Loc,
- const WorkListUnit& WU) {
+static std::string timeTraceScopeName(const ProgramPoint &Loc) {
+ if (llvm::timeTraceProfilerEnabled()) {
+return llvm::formatv("Loc {0}",
+ ProgramPoint::getProgramPointKindName(Loc.getKind()))
+.str();
+ }
+ return "";
+}
+
+static llvm::TimeTraceMetadata timeTraceMetadata(const ExplodedNode *Pred,
+ const ProgramPoint &Loc) {
+ // If time-trace profiler is not enabled, this function is never called.
+ assert(llvm::timeTraceProfilerEnabled());
+ std::string Detail = "";
+ if (const auto SP = Loc.getAs()) {
+if (const Stmt *S = SP->getStmt())
+ Detail = S->getStmtClassName();
+ }
+ auto SLoc = Loc.getSourceLocation();
+ if (!SLoc)
+return llvm::TimeTraceMetadata{Detail, ""};
+ const auto &SM = Pred->getLocationContext()
+ ->getAnalysisDeclContext()
+ ->getASTContext()
+ .getSourceManager();
+ auto Line = SM.getPresumedLineNumber(*SLoc);
+ auto Fname = SM.getFilename(*SLoc);
+ return llvm::TimeTraceMetadata{Detail, Fname.str(), static_cast(Line)};
+}
+
+void CoreEngine::dispatchWorkItem(ExplodedNode *Pred, ProgramPoint Loc,
+ const WorkListUnit &WU) {
+ llvm::TimeTraceScope tcs{timeTraceScopeName(Loc), [Loc, Pred]() {
+ return timeTraceMetadata(Pred, Loc);
+ }};
balazs-benics-sonarsource wrote:
It took me a while to correlate the name `Loc PostStmt` with this place. I
wonder if we should use `work item` as a disambiguation.
https://github.com/llvm/llvm-project/pull/125508
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Limit Store by region-store-binding-limit (PR #127602)
balazs-benics-sonarsource wrote: > Anyway, let's just merge this as it is now. > > The code is basically OK, I still don't have the brainpower to hold all the > details in my mind (kudos for the fact that _you_ managed to put this > together) and if I'll catch some divine inspiration in the future, I can > still refactor this as a follow-up commit. Thank you. I'll merge this later today. https://github.com/llvm/llvm-project/pull/127602 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Limit Store by region-store-binding-limit (PR #127602)
https://github.com/balazs-benics-sonarsource closed https://github.com/llvm/llvm-project/pull/127602 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Limit Store by region-store-binding-limit (PR #127602)
@@ -359,7 +326,80 @@ class RegionBindingsRef : public llvm::ImmutableMapRefhttps://github.com/llvm/llvm-project/pull/127602 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Limit Store by region-store-binding-limit (PR #127602)
balazs-benics-sonarsource wrote:
> As I thought a bit more about the reorganization that I suggested, I realized
> that it can be summarized as **we should synchronize adding the default
> `Unknown` binding and calling `escapeValue`** -- because they correspond to
> the two end-points of the same "_this_ value is stored at _this_ memory
> region" connection which wasn't properly recorded.
>
> Of course there is some asymmetry in that `escapeValue` must escape each
> value individually (or perhaps in a loop with `escapeValues`), but the
> default binding to `Unknown` region is (if I understand correctly) a proper
> stand-in for _all_ the connections from that side. This could be simply
> handled as "`if (`there is no default `Unknown` binding`) {` create one `}`"
> -- but if this happens to cause performance issues, then a boolean
> `didCreateDefaultUnknownBinding` can be used to cache the result.
AFAIK it's not possible (or rather would be ugly) to tie "adding the default
unknown binding" to `escapeValues`, because there may be multiple
`escapeValues` call in the recursive callstack, while popping the frames until
we leave the virtual `.*Bind.*` API.
For example, while binding a compound val like this: `{{{a,b}, {c,d}}, {e},
{f,g}}`, we may give up at `c`, which means, while leaving the method handling
the bind of `{c,d}`, will react to `hasExhaustedBindingLimit()`, and escape
only `d`. Then the parent frame of the recursive descent would conclude that it
finished binding `{c,d}`, and return. Same goes for its parent: `{{a,b},
{c,d}}`, but then it checks `hasExhaustedBindingLimit()` and escapes `{e}` and
`{f,g}`.
If we didn't add the default binding Unknown earlier, at this point it would be
difficult to recover the memory region to the specific element from which the
escapes happened, aka. the memregion of the access pattern to `c` - unless we
of course store that MemRegion in some side storage, and employ some RAII
technique to ensure that it's bound in the end.
I don't think it is worth this complexity.
I think I'm finished, I looked at everything again, and I still believe it's as
good as it gets.
I'm happy to be challenged, but I don't think I'd spend too much time on this.
Feel free to directly push to this branch to demonstrate what you would propose.
https://github.com/llvm/llvm-project/pull/127602
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Limit Store by region-store-binding-limit (PR #127602)
@@ -359,7 +326,80 @@ class RegionBindingsRef : public llvm::ImmutableMapRefhttps://github.com/llvm/llvm-project/pull/127602 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][scan-build] Treat --use-cc and --use-c++ as shell commands (PR #131932)
https://github.com/balazs-benics-sonarsource closed https://github.com/llvm/llvm-project/pull/131932 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Add metrics tracking time spent in Z3 solver (PR #133236)
https://github.com/balazs-benics-sonarsource created
https://github.com/llvm/llvm-project/pull/133236
These metrics would turn out to be useful for verifying an upgrade of Z3.
From 5fe04bcbb3eaf5682037ada6ab64fd7e021f787e Mon Sep 17 00:00:00 2001
From: Balazs Benics
Date: Thu, 27 Mar 2025 12:13:59 +0100
Subject: [PATCH] [analyzer] Add metrics tracking time spent in Z3 solver
---
clang/lib/StaticAnalyzer/Core/Z3CrosscheckVisitor.cpp| 7 +++
clang/test/Analysis/analyzer-stats/entry-point-stats.cpp | 4
2 files changed, 11 insertions(+)
diff --git a/clang/lib/StaticAnalyzer/Core/Z3CrosscheckVisitor.cpp
b/clang/lib/StaticAnalyzer/Core/Z3CrosscheckVisitor.cpp
index fca792cdf86f7..836fc375809ad 100644
--- a/clang/lib/StaticAnalyzer/Core/Z3CrosscheckVisitor.cpp
+++ b/clang/lib/StaticAnalyzer/Core/Z3CrosscheckVisitor.cpp
@@ -41,6 +41,11 @@ STAT_COUNTER(NumTimesZ3QueryRejectReport,
STAT_COUNTER(NumTimesZ3QueryRejectEQClass,
"Number of times rejecting an report equivalenece class");
+STAT_COUNTER(TimeSpentSolvingZ3Queries,
+ "Total time spent solving Z3 queries excluding retries");
+STAT_MAX(MaxTimeSpentSolvingZ3Queries,
+ "Max time spent solving a Z3 query excluding retries");
+
using namespace clang;
using namespace ento;
@@ -145,6 +150,8 @@ Z3CrosscheckOracle::Z3Decision
Z3CrosscheckOracle::interpretQueryResult(
const Z3CrosscheckVisitor::Z3Result &Query) {
++NumZ3QueriesDone;
AccumulatedZ3QueryTimeInEqClass += Query.Z3QueryTimeMilliseconds;
+ TimeSpentSolvingZ3Queries += Query.Z3QueryTimeMilliseconds;
+ MaxTimeSpentSolvingZ3Queries.updateMax(Query.Z3QueryTimeMilliseconds);
if (Query.IsSAT && Query.IsSAT.value()) {
++NumTimesZ3QueryAcceptsReport;
diff --git a/clang/test/Analysis/analyzer-stats/entry-point-stats.cpp
b/clang/test/Analysis/analyzer-stats/entry-point-stats.cpp
index bddba084ee4bf..1ff31d114ee99 100644
--- a/clang/test/Analysis/analyzer-stats/entry-point-stats.cpp
+++ b/clang/test/Analysis/analyzer-stats/entry-point-stats.cpp
@@ -31,10 +31,12 @@
// CHECK-NEXT: "NumTimesZ3SpendsTooMuchTimeOnASingleEQClass": "{{[0-9]+}}",
// CHECK-NEXT: "NumTimesZ3TimedOut": "{{[0-9]+}}",
// CHECK-NEXT: "NumZ3QueriesDone": "{{[0-9]+}}",
+// CHECK-NEXT: "TimeSpentSolvingZ3Queries": "{{[0-9]+}}",
// CHECK-NEXT: "MaxBugClassSize": "{{[0-9]+}}",
// CHECK-NEXT: "MaxCFGSize": "{{[0-9]+}}",
// CHECK-NEXT: "MaxQueueSize": "{{[0-9]+}}",
// CHECK-NEXT: "MaxReachableSize": "{{[0-9]+}}",
+// CHECK-NEXT: "MaxTimeSpentSolvingZ3Queries": "{{[0-9]+}}",
// CHECK-NEXT: "MaxValidBugClassSize": "{{[0-9]+}}",
// CHECK-NEXT: "PathRunningTime": "{{[0-9]+}}"
// CHECK-NEXT: },
@@ -64,10 +66,12 @@
// CHECK-NEXT: "NumTimesZ3SpendsTooMuchTimeOnASingleEQClass": "{{[0-9]+}}",
// CHECK-NEXT: "NumTimesZ3TimedOut": "{{[0-9]+}}",
// CHECK-NEXT: "NumZ3QueriesDone": "{{[0-9]+}}",
+// CHECK-NEXT: "TimeSpentSolvingZ3Queries": "{{[0-9]+}}",
// CHECK-NEXT: "MaxBugClassSize": "{{[0-9]+}}",
// CHECK-NEXT: "MaxCFGSize": "{{[0-9]+}}",
// CHECK-NEXT: "MaxQueueSize": "{{[0-9]+}}",
// CHECK-NEXT: "MaxReachableSize": "{{[0-9]+}}",
+// CHECK-NEXT: "MaxTimeSpentSolvingZ3Queries": "{{[0-9]+}}",
// CHECK-NEXT: "MaxValidBugClassSize": "{{[0-9]+}}",
// CHECK-NEXT: "PathRunningTime": "{{[0-9]+}}"
// CHECK-NEXT: }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Workaround for slowdown spikes (unintended scope increase) (PR #136720)
balazs-benics-sonarsource wrote: Thanks for the fix, I'll proceed with the backport if you still believe it's worthy. @NagyDonat https://github.com/llvm/llvm-project/pull/136720 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer][NFC] Move PrettyStackTraceLocationContext into dispatchWorkItem (PR #140035)
https://github.com/balazs-benics-sonarsource created
https://github.com/llvm/llvm-project/pull/140035
This change helps with ensuring that the abstract machine call stack is only
dumped exactly once no matter what checker callback we have the crash in.
Note that some checker callbacks happen outside of dispatchWorkItem, thus they
need special attention.
This was the case in the past and that is not changed in this patch. Maybe in
the future we could improve that too.
This patch is motivated by a new downstream checker callback, that is invoked
for transitioning CFG edges, thus acting on BlockEdge program points. If it
makes sense, I'd be happy to contribute that too.
Rate limit · GitHub
body {
background-color: #f6f8fa;
color: #24292e;
font-family: -apple-system,BlinkMacSystemFont,Segoe
UI,Helvetica,Arial,sans-serif,Apple Color Emoji,Segoe UI Emoji,Segoe UI Symbol;
font-size: 14px;
line-height: 1.5;
margin: 0;
}
.container { margin: 50px auto; max-width: 600px; text-align: center;
padding: 0 24px; }
a { color: #0366d6; text-decoration: none; }
a:hover { text-decoration: underline; }
h1 { line-height: 60px; font-size: 48px; font-weight: 300; margin: 0px;
text-shadow: 0 1px 0 #fff; }
p { color: rgba(0, 0, 0, 0.5); margin: 20px 0 40px; }
ul { list-style: none; margin: 25px 0; padding: 0; }
li { display: table-cell; font-weight: bold; width: 1%; }
.logo { display: inline-block; margin-top: 35px; }
.logo-img-2x { display: none; }
@media
only screen and (-webkit-min-device-pixel-ratio: 2),
only screen and ( min--moz-device-pixel-ratio: 2),
only screen and ( -o-min-device-pixel-ratio: 2/1),
only screen and (min-device-pixel-ratio: 2),
only screen and (min-resolution: 192dpi),
only screen and (min-resolution: 2dppx) {
.logo-img-1x { display: none; }
.logo-img-2x { display: inline-block; }
}
#suggestions {
margin-top: 35px;
color: #ccc;
}
#suggestions a {
color: #66;
font-weight: 200;
font-size: 14px;
margin: 0 10px;
}
Whoa there!
You have exceeded a secondary rate limit.
Please wait a few minutes before you try again;
in some cases this may take up to an hour.
https://support.github.com/contact";>Contact Support —
https://githubstatus.com";>GitHub Status —
https://twitter.com/githubstatus";>@githubstatus
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer][NFC] Move PrettyStackTraceLocationContext into dispatchWorkItem (PR #140035)
balazs-benics-sonarsource wrote: /CC @pdschbrt https://github.com/llvm/llvm-project/pull/140035 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer][NFC] Move PrettyStackTraceLocationContext into dispatchWorkItem (PR #140035)
https://github.com/balazs-benics-sonarsource edited https://github.com/llvm/llvm-project/pull/140035 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer][NFC] Move PrettyStackTraceLocationContext into dispatchWorkItem (PR #140035)
https://github.com/balazs-benics-sonarsource updated
https://github.com/llvm/llvm-project/pull/140035
From 42343959f623153dc9421e3bb569b2f0527ec119 Mon Sep 17 00:00:00 2001
From: Balazs Benics
Date: Thu, 15 May 2025 11:17:24 +0200
Subject: [PATCH 1/2] [analyzer][NFC] Move PrettyStackTraceLocationContext into
dispatchWorkItem
---
clang/lib/StaticAnalyzer/Core/CoreEngine.cpp| 2 ++
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp| 17 +++--
.../Core/ExprEngineCallAndReturn.cpp| 5 +
3 files changed, 6 insertions(+), 18 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
b/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
index 2e6631f2f620c..8cc086a12ad70 100644
--- a/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
@@ -12,6 +12,7 @@
//===--===//
#include "clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h"
+#include "PrettyStackTraceLocationContext.h"
#include "clang/AST/Expr.h"
#include "clang/AST/ExprCXX.h"
#include "clang/AST/Stmt.h"
@@ -216,6 +217,7 @@ void CoreEngine::dispatchWorkItem(ExplodedNode *Pred,
ProgramPoint Loc,
llvm::TimeTraceScope tcs{timeTraceScopeName(Loc), [Loc, Pred]() {
return timeTraceMetadata(Pred, Loc);
}};
+ PrettyStackTraceLocationContext CrashInfo(Pred->getLocationContext());
// Dispatch on the location type.
switch (Loc.getKind()) {
case ProgramPoint::BlockEdgeKind:
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index ebad83dad0c8f..1afd4b52eb354 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -968,7 +968,6 @@ void ExprEngine::processEndWorklist() {
void ExprEngine::processCFGElement(const CFGElement E, ExplodedNode *Pred,
unsigned StmtIdx, NodeBuilderContext *Ctx) {
- PrettyStackTraceLocationContext CrashInfo(Pred->getLocationContext());
currStmtIdx = StmtIdx;
currBldrCtx = Ctx;
@@ -2541,7 +2540,6 @@ static const LocationContext
*getInlinedLocationContext(ExplodedNode *Node,
void ExprEngine::processCFGBlockEntrance(const BlockEdge &L,
NodeBuilderWithSinks &nodeBuilder,
ExplodedNode *Pred) {
- PrettyStackTraceLocationContext CrashInfo(Pred->getLocationContext());
// If we reach a loop which has a known bound (and meets
// other constraints) then consider completely unrolling it.
if(AMgr.options.ShouldUnrollLoops) {
@@ -2808,8 +2806,6 @@ void ExprEngine::processBranch(
std::optional IterationsCompletedInLoop) {
assert((!Condition || !isa(Condition)) &&
"CXXBindTemporaryExprs are handled by processBindTemporary.");
- const LocationContext *LCtx = Pred->getLocationContext();
- PrettyStackTraceLocationContext StackCrashInfo(LCtx);
currBldrCtx = &BldCtx;
// Check for NULL conditions; e.g. "for(;;)"
@@ -2935,13 +2931,9 @@ void ExprEngine::processBranch(
REGISTER_TRAIT_WITH_PROGRAMSTATE(InitializedGlobalsSet,
llvm::ImmutableSet)
-void ExprEngine::processStaticInitializer(const DeclStmt *DS,
- NodeBuilderContext &BuilderCtx,
- ExplodedNode *Pred,
- ExplodedNodeSet &Dst,
- const CFGBlock *DstT,
- const CFGBlock *DstF) {
- PrettyStackTraceLocationContext CrashInfo(Pred->getLocationContext());
+void ExprEngine::processStaticInitializer(
+const DeclStmt *DS, NodeBuilderContext &BuilderCtx, ExplodedNode *Pred,
+ExplodedNodeSet &Dst, const CFGBlock *DstT, const CFGBlock *DstF) {
currBldrCtx = &BuilderCtx;
const auto *VD = cast(DS->getSingleDecl());
@@ -3064,9 +3056,6 @@ void ExprEngine::processEndOfFunction(NodeBuilderContext&
BC,
assert(areAllObjectsFullyConstructed(Pred->getState(),
Pred->getLocationContext(),
Pred->getStackFrame()->getParent()));
-
- PrettyStackTraceLocationContext CrashInfo(Pred->getLocationContext());
-
ExplodedNodeSet Dst;
if (Pred->getLocationContext()->inTopFrame()) {
// Remove dead symbols.
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
index 90625a96e9059..63bdc58030187 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -44,8 +44,6 @@ STAT_COUNTER(NumReachedInlineCountMax,
void ExprEngine::processCallEnter(NodeBuilderContext& BC, CallEnter CE,
ExplodedNode *Pred) {
// Ge
[clang] [analyzer] Introduce the check::BlockEntrance checker callback (PR #140924)
https://github.com/balazs-benics-sonarsource updated
https://github.com/llvm/llvm-project/pull/140924
From 084d821b62d5de473d32d3506da95fdd7bad1cfe Mon Sep 17 00:00:00 2001
From: Balazs Benics
Date: Thu, 15 May 2025 17:20:29 +0200
Subject: [PATCH 1/3] [analyzer] Introduce the check::BlockEntrance
Tranersing the CFG blocks of a function is a fundamental operation.
Many C++ constructs can create splits in the control-flow, such as
`if`, `for`, and similar control structures or ternary expressions,
gnu conditionals, gotos, switches and possibly more.
Checkers should be able to get notifications about entering or leaving a
CFG block of interest.
Note that in the ExplodedGraph there is always a BlockEntrance
ProgramPoint right after the BlockEdge ProgramPoint.
I considered naming this callback check::BlockEdge, but then that may
leave the observer of the graph puzzled to see BlockEdge points followed
more BlockEdge nodes describing the same CFG transition.
This confusion could also apply to Bug Report Visitors too.
Because of this, I decided to hook BlockEntrance ProgramPoints instead.
The same confusion applies here, but I find this still a better place
TBH. There would only appear only one BlockEntrance ProgramPoint in the
graph if no checkers modify the state or emit a bug report.
Otherwise they modify some GDM (aka. State) thus create a new
ExplodedNode with the same BlockEntrance ProgramPoint in the graph.
CPP-6484
---
.../clang/StaticAnalyzer/Core/Checker.h | 20 ++
.../StaticAnalyzer/Core/CheckerManager.h | 13 +
.../Core/PathSensitive/ExprEngine.h | 4 +
.../Checkers/CheckerDocumentation.cpp | 15 +-
.../StaticAnalyzer/Core/CheckerManager.cpp| 50 +++-
clang/lib/StaticAnalyzer/Core/CoreEngine.cpp | 25 +-
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp | 13 +
clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp | 6 +-
.../BlockEntranceCallbackTest.cpp | 283 ++
clang/unittests/StaticAnalyzer/CMakeLists.txt | 1 +
10 files changed, 414 insertions(+), 16 deletions(-)
create mode 100644 clang/unittests/StaticAnalyzer/BlockEntranceCallbackTest.cpp
diff --git a/clang/include/clang/StaticAnalyzer/Core/Checker.h
b/clang/include/clang/StaticAnalyzer/Core/Checker.h
index a54c5bee612f6..1b348dcce5ea7 100644
--- a/clang/include/clang/StaticAnalyzer/Core/Checker.h
+++ b/clang/include/clang/StaticAnalyzer/Core/Checker.h
@@ -221,6 +221,22 @@ class Bind {
}
};
+class BlockEntrance {
+ template
+ static void _checkBlockEntrance(void *Checker,
+ const clang::BlockEntrance &Entrance,
+ CheckerContext &C) {
+((const CHECKER *)Checker)->checkBlockEntrance(Entrance, C);
+ }
+
+public:
+ template
+ static void _register(CHECKER *checker, CheckerManager &mgr) {
+mgr._registerForBlockEntrance(CheckerManager::CheckBlockEntranceFunc(
+checker, _checkBlockEntrance));
+ }
+};
+
class EndAnalysis {
template
static void _checkEndAnalysis(void *checker, ExplodedGraph &G,
@@ -548,6 +564,8 @@ class CheckerProgramPointTag : public SimpleProgramPointTag
{
template
class Checker : public CHECK1, public CHECKs..., public CheckerBase {
public:
+ using BlockEntrance = clang::BlockEntrance;
+
template
static void _register(CHECKER *checker, CheckerManager &mgr) {
CHECK1::_register(checker, mgr);
@@ -558,6 +576,8 @@ class Checker : public CHECK1, public CHECKs..., public
CheckerBase {
template
class Checker : public CHECK1, public CheckerBase {
public:
+ using BlockEntrance = clang::BlockEntrance;
+
template
static void _register(CHECKER *checker, CheckerManager &mgr) {
CHECK1::_register(checker, mgr);
diff --git a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
index 03ffadd346d0b..b5fefdb75401d 100644
--- a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
+++ b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
@@ -376,6 +376,12 @@ class CheckerManager {
const Stmt *S, ExprEngine &Eng,
const ProgramPoint &PP);
+ /// Run checkers after taking a control flow edge.
+ void runCheckersForBlockEntrance(ExplodedNodeSet &Dst,
+ const ExplodedNodeSet &Src,
+ const BlockEntrance &Entrance,
+ ExprEngine &Eng) const;
+
/// Run checkers for end of analysis.
void runCheckersForEndAnalysis(ExplodedGraph &G, BugReporter &BR,
ExprEngine &Eng);
@@ -528,6 +534,9 @@ class CheckerManager {
using CheckBindFunc =
CheckerFn;
+ using CheckBlockEntranceFunc =
+ CheckerFn;
+
using CheckEndAnalysisFunc =
CheckerFn;
@@ -589,6 +598,8 @@ class CheckerManager {
void _registerForBind(CheckBindFunc checkfn);
+ void _registerForBlockEntrance(Che
[clang] [analyzer] Introduce the check::BlockEntrance checker callback (PR #140924)
balazs-benics-sonarsource wrote: > What is the relationship of this new callback with the `BranchCondition` > callback which is evaluated in `ProcessBranch` where the checker splits the > execution path into multiple branches? It would be nice if you could document > the difference between these two callbacks. Great question. They are for different purposes, but one should prefer `BranchCondition` over the strictly more generic `BlockEntrance` callback. State splits work as in any other checkers. If you split the state in both callback, you will end up with 4 splits for the cases that are `if`, `for`, `while`, statements with conditions and 2 paths for every other control-flow edge, like entering the first basic block from the Entry CFG node, or following a `break`, `continue` or `goto` (unconditional) CFG edge. Documented this now in c2bd657065e0edcac742941c1a535444a90b1638, and added a unit test for the case when both callbacks are defined in 910abb384af9c8d51f32d2109a73ab900682957e. > > What is the relationship of this new callback with the BranchCondition > > callback > > +1, I am also interested to learn what is the main motivation behind the new > callback. Do you have some example use cases in mind? I think users might get > confused which one to use. My motivation is to drive enabling or disabling an internal checker by entering or leaving a certain CFG block. Concretely, if a short-circuiting operator leads to the guarded CFG block (aka. the true branch for the `&&` operator and the false branch for the `||` operator), enable a checker to detect certain constructs in a guarded block. And once we are leaving the guarded block disable the checker. I hope it clarifies the motivation and demonstrates the usefulness of such a callback. I believe that there could be other similar use cases of this. https://github.com/llvm/llvm-project/pull/140924 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Add previous CFG block to BlockEntrance ProgramPoints (PR #140861)
https://github.com/balazs-benics-sonarsource closed https://github.com/llvm/llvm-project/pull/140861 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Introduce the check::BlockEntrance checker callback (PR #140924)
https://github.com/balazs-benics-sonarsource created
https://github.com/llvm/llvm-project/pull/140924
Tranersing the CFG blocks of a function is a fundamental operation. Many C++
constructs can create splits in the control-flow, such as `if`, `for`, and
similar control structures or ternary expressions, gnu conditionals, gotos,
switches and possibly more.
Checkers should be able to get notifications about entering or leaving a CFG
block of interest.
Note that in the ExplodedGraph there is always a BlockEntrance ProgramPoint
right after the BlockEdge ProgramPoint. I considered naming this callback
check::BlockEdge, but then that may leave the observer of the graph puzzled to
see BlockEdge points followed more BlockEdge nodes describing the same CFG
transition. This confusion could also apply to Bug Report Visitors too.
Because of this, I decided to hook BlockEntrance ProgramPoints instead. The
same confusion applies here, but I find this still a better place TBH. There
would only appear only one BlockEntrance ProgramPoint in the graph if no
checkers modify the state or emit a bug report. Otherwise they modify some GDM
(aka. State) thus create a new ExplodedNode with the same BlockEntrance
ProgramPoint in the graph.
CPP-6484
From 084d821b62d5de473d32d3506da95fdd7bad1cfe Mon Sep 17 00:00:00 2001
From: Balazs Benics
Date: Thu, 15 May 2025 17:20:29 +0200
Subject: [PATCH] [analyzer] Introduce the check::BlockEntrance
Tranersing the CFG blocks of a function is a fundamental operation.
Many C++ constructs can create splits in the control-flow, such as
`if`, `for`, and similar control structures or ternary expressions,
gnu conditionals, gotos, switches and possibly more.
Checkers should be able to get notifications about entering or leaving a
CFG block of interest.
Note that in the ExplodedGraph there is always a BlockEntrance
ProgramPoint right after the BlockEdge ProgramPoint.
I considered naming this callback check::BlockEdge, but then that may
leave the observer of the graph puzzled to see BlockEdge points followed
more BlockEdge nodes describing the same CFG transition.
This confusion could also apply to Bug Report Visitors too.
Because of this, I decided to hook BlockEntrance ProgramPoints instead.
The same confusion applies here, but I find this still a better place
TBH. There would only appear only one BlockEntrance ProgramPoint in the
graph if no checkers modify the state or emit a bug report.
Otherwise they modify some GDM (aka. State) thus create a new
ExplodedNode with the same BlockEntrance ProgramPoint in the graph.
CPP-6484
---
.../clang/StaticAnalyzer/Core/Checker.h | 20 ++
.../StaticAnalyzer/Core/CheckerManager.h | 13 +
.../Core/PathSensitive/ExprEngine.h | 4 +
.../Checkers/CheckerDocumentation.cpp | 15 +-
.../StaticAnalyzer/Core/CheckerManager.cpp| 50 +++-
clang/lib/StaticAnalyzer/Core/CoreEngine.cpp | 25 +-
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp | 13 +
clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp | 6 +-
.../BlockEntranceCallbackTest.cpp | 283 ++
clang/unittests/StaticAnalyzer/CMakeLists.txt | 1 +
10 files changed, 414 insertions(+), 16 deletions(-)
create mode 100644 clang/unittests/StaticAnalyzer/BlockEntranceCallbackTest.cpp
diff --git a/clang/include/clang/StaticAnalyzer/Core/Checker.h
b/clang/include/clang/StaticAnalyzer/Core/Checker.h
index a54c5bee612f6..1b348dcce5ea7 100644
--- a/clang/include/clang/StaticAnalyzer/Core/Checker.h
+++ b/clang/include/clang/StaticAnalyzer/Core/Checker.h
@@ -221,6 +221,22 @@ class Bind {
}
};
+class BlockEntrance {
+ template
+ static void _checkBlockEntrance(void *Checker,
+ const clang::BlockEntrance &Entrance,
+ CheckerContext &C) {
+((const CHECKER *)Checker)->checkBlockEntrance(Entrance, C);
+ }
+
+public:
+ template
+ static void _register(CHECKER *checker, CheckerManager &mgr) {
+mgr._registerForBlockEntrance(CheckerManager::CheckBlockEntranceFunc(
+checker, _checkBlockEntrance));
+ }
+};
+
class EndAnalysis {
template
static void _checkEndAnalysis(void *checker, ExplodedGraph &G,
@@ -548,6 +564,8 @@ class CheckerProgramPointTag : public SimpleProgramPointTag
{
template
class Checker : public CHECK1, public CHECKs..., public CheckerBase {
public:
+ using BlockEntrance = clang::BlockEntrance;
+
template
static void _register(CHECKER *checker, CheckerManager &mgr) {
CHECK1::_register(checker, mgr);
@@ -558,6 +576,8 @@ class Checker : public CHECK1, public CHECKs..., public
CheckerBase {
template
class Checker : public CHECK1, public CheckerBase {
public:
+ using BlockEntrance = clang::BlockEntrance;
+
template
static void _register(CHECKER *checker, CheckerManager &mgr) {
CHECK1::_register(checker, mgr);
diff --git a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
b/clang/include/clang/S
[clang] [analyzer][NFC] Move PrettyStackTraceLocationContext into dispatchWorkItem (PR #140035)
balazs-benics-sonarsource wrote: I've updated the PR. I noticed some mistakes in the original submission. Could you please have a look? https://github.com/llvm/llvm-project/pull/140035 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer][NFC] Move PrettyStackTraceLocationContext into dispatchWorkItem (PR #140035)
https://github.com/balazs-benics-sonarsource updated
https://github.com/llvm/llvm-project/pull/140035
From 3ef0391fdc58503f3414ac64e42370b0a6d4bddf Mon Sep 17 00:00:00 2001
From: Balazs Benics
Date: Thu, 15 May 2025 11:17:24 +0200
Subject: [PATCH] [analyzer][NFC] Move PrettyStackTraceLocationContext into
dispatchWorkItem
CPP-6476
---
clang/lib/StaticAnalyzer/Core/CoreEngine.cpp| 2 ++
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp| 17 +++--
.../Core/ExprEngineCallAndReturn.cpp| 6 +-
3 files changed, 6 insertions(+), 19 deletions(-)
diff --git a/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
b/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
index 2e6631f2f620c..8cc086a12ad70 100644
--- a/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
@@ -12,6 +12,7 @@
//===--===//
#include "clang/StaticAnalyzer/Core/PathSensitive/CoreEngine.h"
+#include "PrettyStackTraceLocationContext.h"
#include "clang/AST/Expr.h"
#include "clang/AST/ExprCXX.h"
#include "clang/AST/Stmt.h"
@@ -216,6 +217,7 @@ void CoreEngine::dispatchWorkItem(ExplodedNode *Pred,
ProgramPoint Loc,
llvm::TimeTraceScope tcs{timeTraceScopeName(Loc), [Loc, Pred]() {
return timeTraceMetadata(Pred, Loc);
}};
+ PrettyStackTraceLocationContext CrashInfo(Pred->getLocationContext());
// Dispatch on the location type.
switch (Loc.getKind()) {
case ProgramPoint::BlockEdgeKind:
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
index ebad83dad0c8f..1afd4b52eb354 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -968,7 +968,6 @@ void ExprEngine::processEndWorklist() {
void ExprEngine::processCFGElement(const CFGElement E, ExplodedNode *Pred,
unsigned StmtIdx, NodeBuilderContext *Ctx) {
- PrettyStackTraceLocationContext CrashInfo(Pred->getLocationContext());
currStmtIdx = StmtIdx;
currBldrCtx = Ctx;
@@ -2541,7 +2540,6 @@ static const LocationContext
*getInlinedLocationContext(ExplodedNode *Node,
void ExprEngine::processCFGBlockEntrance(const BlockEdge &L,
NodeBuilderWithSinks &nodeBuilder,
ExplodedNode *Pred) {
- PrettyStackTraceLocationContext CrashInfo(Pred->getLocationContext());
// If we reach a loop which has a known bound (and meets
// other constraints) then consider completely unrolling it.
if(AMgr.options.ShouldUnrollLoops) {
@@ -2808,8 +2806,6 @@ void ExprEngine::processBranch(
std::optional IterationsCompletedInLoop) {
assert((!Condition || !isa(Condition)) &&
"CXXBindTemporaryExprs are handled by processBindTemporary.");
- const LocationContext *LCtx = Pred->getLocationContext();
- PrettyStackTraceLocationContext StackCrashInfo(LCtx);
currBldrCtx = &BldCtx;
// Check for NULL conditions; e.g. "for(;;)"
@@ -2935,13 +2931,9 @@ void ExprEngine::processBranch(
REGISTER_TRAIT_WITH_PROGRAMSTATE(InitializedGlobalsSet,
llvm::ImmutableSet)
-void ExprEngine::processStaticInitializer(const DeclStmt *DS,
- NodeBuilderContext &BuilderCtx,
- ExplodedNode *Pred,
- ExplodedNodeSet &Dst,
- const CFGBlock *DstT,
- const CFGBlock *DstF) {
- PrettyStackTraceLocationContext CrashInfo(Pred->getLocationContext());
+void ExprEngine::processStaticInitializer(
+const DeclStmt *DS, NodeBuilderContext &BuilderCtx, ExplodedNode *Pred,
+ExplodedNodeSet &Dst, const CFGBlock *DstT, const CFGBlock *DstF) {
currBldrCtx = &BuilderCtx;
const auto *VD = cast(DS->getSingleDecl());
@@ -3064,9 +3056,6 @@ void ExprEngine::processEndOfFunction(NodeBuilderContext&
BC,
assert(areAllObjectsFullyConstructed(Pred->getState(),
Pred->getLocationContext(),
Pred->getStackFrame()->getParent()));
-
- PrettyStackTraceLocationContext CrashInfo(Pred->getLocationContext());
-
ExplodedNodeSet Dst;
if (Pred->getLocationContext()->inTopFrame()) {
// Remove dead symbols.
diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
index 90625a96e9059..01e5076646a2c 100644
--- a/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
+++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp
@@ -10,7 +10,6 @@
//
//===--===//
-#include "PrettyStackTraceLocationContext.h"
#include "clang/AST/CXXInheritance.
[clang] [analyzer][NFC] Move PrettyStackTraceLocationContext into dispatchWorkItem (PR #140035)
https://github.com/balazs-benics-sonarsource edited https://github.com/llvm/llvm-project/pull/140035 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer][NFC] Move PrettyStackTraceLocationContext into dispatchWorkItem (PR #140035)
https://github.com/balazs-benics-sonarsource edited https://github.com/llvm/llvm-project/pull/140035 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer][NFC] Move PrettyStackTraceLocationContext into dispatchWorkItem (PR #140035)
https://github.com/balazs-benics-sonarsource closed https://github.com/llvm/llvm-project/pull/140035 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Add previous CFG block to BlockEntrance ProgramPoints (PR #140861)
balazs-benics-sonarsource wrote: /cc @necto https://github.com/llvm/llvm-project/pull/140861 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Add previous CFG block to BlockEntrance ProgramPoints (PR #140861)
https://github.com/balazs-benics-sonarsource created
https://github.com/llvm/llvm-project/pull/140861
This helps to gain contextual information about how we entered a CFG block.
The `noexprcrash.c` test probably changed due to the fact that now
BlockEntrance ProgramPoint Profile also hashes the pointer of the previous CFG
block. I didn't investigate.
CPP-6483
From 1378271ee639bf3307cb373f07f730978373be7b Mon Sep 17 00:00:00 2001
From: Balazs Benics
Date: Thu, 15 May 2025 15:21:48 +0200
Subject: [PATCH] [analyzer] Add previous CFG block to BlockEntrance
ProgramPoints
This helps to gain contextual information about how we entered
a CFG block.
The `noexprcrash.c` test probably changed due to the fact that now
BlockEntrance ProgramPoint Profile also hashes the pointer of the
previous CFG block. I didn't investigate.
CPP-6483
---
clang/include/clang/Analysis/ProgramPoint.h| 18 --
clang/lib/StaticAnalyzer/Core/CoreEngine.cpp | 2 +-
.../Analysis/exploration_order/noexprcrash.c | 11 ++-
3 files changed, 19 insertions(+), 12 deletions(-)
diff --git a/clang/include/clang/Analysis/ProgramPoint.h
b/clang/include/clang/Analysis/ProgramPoint.h
index c40aa3d8ffb72..096ad48a42984 100644
--- a/clang/include/clang/Analysis/ProgramPoint.h
+++ b/clang/include/clang/Analysis/ProgramPoint.h
@@ -224,10 +224,14 @@ class ProgramPoint {
class BlockEntrance : public ProgramPoint {
public:
- BlockEntrance(const CFGBlock *B, const LocationContext *L,
-const ProgramPointTag *tag = nullptr)
-: ProgramPoint(B, BlockEntranceKind, L, tag) {
-assert(B && "BlockEntrance requires non-null block");
+ BlockEntrance(const CFGBlock *PrevBlock, const CFGBlock *CurrBlock,
+const LocationContext *L, const ProgramPointTag *Tag = nullptr)
+ : ProgramPoint(CurrBlock, PrevBlock, BlockEntranceKind, L, Tag) {
+assert(CurrBlock && "BlockEntrance requires non-null block");
+ }
+
+ const CFGBlock *getPreviousBlock() const {
+return reinterpret_cast(getData2());
}
const CFGBlock *getBlock() const {
@@ -760,13 +764,15 @@ template <> struct DenseMapInfo {
static inline clang::ProgramPoint getEmptyKey() {
uintptr_t x =
reinterpret_cast(DenseMapInfo::getEmptyKey()) & ~0x7;
- return clang::BlockEntrance(reinterpret_cast(x), nullptr);
+ return clang::BlockEntrance(nullptr, reinterpret_cast(x),
+ nullptr);
}
static inline clang::ProgramPoint getTombstoneKey() {
uintptr_t x =
reinterpret_cast(DenseMapInfo::getTombstoneKey()) & ~0x7;
- return clang::BlockEntrance(reinterpret_cast(x), nullptr);
+ return clang::BlockEntrance(nullptr, reinterpret_cast(x),
+ nullptr);
}
static unsigned getHashValue(const clang::ProgramPoint &Loc) {
diff --git a/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
b/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
index 8cc086a12ad70..bedb11f8b94a5 100644
--- a/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
+++ b/clang/lib/StaticAnalyzer/Core/CoreEngine.cpp
@@ -315,7 +315,7 @@ void CoreEngine::HandleBlockEdge(const BlockEdge &L,
ExplodedNode *Pred) {
// Call into the ExprEngine to process entering the CFGBlock.
ExplodedNodeSet dstNodes;
- BlockEntrance BE(Blk, Pred->getLocationContext());
+ BlockEntrance BE(L.getSrc(), L.getDst(), Pred->getLocationContext());
NodeBuilderWithSinks nodeBuilder(Pred, dstNodes, BuilderCtx, BE);
ExprEng.processCFGBlockEntrance(L, nodeBuilder, Pred);
diff --git a/clang/test/Analysis/exploration_order/noexprcrash.c
b/clang/test/Analysis/exploration_order/noexprcrash.c
index 75c2f0e6798a3..427c669783374 100644
--- a/clang/test/Analysis/exploration_order/noexprcrash.c
+++ b/clang/test/Analysis/exploration_order/noexprcrash.c
@@ -1,17 +1,18 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify
-analyzer-config exploration_strategy=unexplored_first %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify
-analyzer-config exploration_strategy=dfs %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection
-verify=common,ufirst -analyzer-config exploration_strategy=unexplored_first %s
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection
-verify=common,dfs -analyzer-config exploration_strategy=dfs %s
extern void clang_analyzer_eval(int);
typedef struct { char a; } b;
int c(b* input) {
-int x = (input->a ?: input) ? 1 : 0; // expected-warning{{pointer/integer
type mismatch}}
+int x = (input->a ?: input) ? 1 : 0; // common-warning{{pointer/integer
type mismatch}}
if (input->a) {
// FIXME: The value should actually be "TRUE",
// but is incorrect due to a bug.
- clang_analyzer_eval(x); // expected-warning{{FALSE}}
+ // dfs-warning@+1 {{FALSE}} ufirst-warning@+1 {{TRUE}}
+ clang_analyzer_eval(x);
} else {
- clang_analyzer_eval(x); // expected-warning{{TRUE}}
+
[clang] [analyzer] Introduce the check::BlockEntrance checker callback (PR #140924)
https://github.com/balazs-benics-sonarsource updated
https://github.com/llvm/llvm-project/pull/140924
From 084d821b62d5de473d32d3506da95fdd7bad1cfe Mon Sep 17 00:00:00 2001
From: Balazs Benics
Date: Thu, 15 May 2025 17:20:29 +0200
Subject: [PATCH 1/4] [analyzer] Introduce the check::BlockEntrance
Tranersing the CFG blocks of a function is a fundamental operation.
Many C++ constructs can create splits in the control-flow, such as
`if`, `for`, and similar control structures or ternary expressions,
gnu conditionals, gotos, switches and possibly more.
Checkers should be able to get notifications about entering or leaving a
CFG block of interest.
Note that in the ExplodedGraph there is always a BlockEntrance
ProgramPoint right after the BlockEdge ProgramPoint.
I considered naming this callback check::BlockEdge, but then that may
leave the observer of the graph puzzled to see BlockEdge points followed
more BlockEdge nodes describing the same CFG transition.
This confusion could also apply to Bug Report Visitors too.
Because of this, I decided to hook BlockEntrance ProgramPoints instead.
The same confusion applies here, but I find this still a better place
TBH. There would only appear only one BlockEntrance ProgramPoint in the
graph if no checkers modify the state or emit a bug report.
Otherwise they modify some GDM (aka. State) thus create a new
ExplodedNode with the same BlockEntrance ProgramPoint in the graph.
CPP-6484
---
.../clang/StaticAnalyzer/Core/Checker.h | 20 ++
.../StaticAnalyzer/Core/CheckerManager.h | 13 +
.../Core/PathSensitive/ExprEngine.h | 4 +
.../Checkers/CheckerDocumentation.cpp | 15 +-
.../StaticAnalyzer/Core/CheckerManager.cpp| 50 +++-
clang/lib/StaticAnalyzer/Core/CoreEngine.cpp | 25 +-
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp | 13 +
clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp | 6 +-
.../BlockEntranceCallbackTest.cpp | 283 ++
clang/unittests/StaticAnalyzer/CMakeLists.txt | 1 +
10 files changed, 414 insertions(+), 16 deletions(-)
create mode 100644 clang/unittests/StaticAnalyzer/BlockEntranceCallbackTest.cpp
diff --git a/clang/include/clang/StaticAnalyzer/Core/Checker.h
b/clang/include/clang/StaticAnalyzer/Core/Checker.h
index a54c5bee612f6..1b348dcce5ea7 100644
--- a/clang/include/clang/StaticAnalyzer/Core/Checker.h
+++ b/clang/include/clang/StaticAnalyzer/Core/Checker.h
@@ -221,6 +221,22 @@ class Bind {
}
};
+class BlockEntrance {
+ template
+ static void _checkBlockEntrance(void *Checker,
+ const clang::BlockEntrance &Entrance,
+ CheckerContext &C) {
+((const CHECKER *)Checker)->checkBlockEntrance(Entrance, C);
+ }
+
+public:
+ template
+ static void _register(CHECKER *checker, CheckerManager &mgr) {
+mgr._registerForBlockEntrance(CheckerManager::CheckBlockEntranceFunc(
+checker, _checkBlockEntrance));
+ }
+};
+
class EndAnalysis {
template
static void _checkEndAnalysis(void *checker, ExplodedGraph &G,
@@ -548,6 +564,8 @@ class CheckerProgramPointTag : public SimpleProgramPointTag
{
template
class Checker : public CHECK1, public CHECKs..., public CheckerBase {
public:
+ using BlockEntrance = clang::BlockEntrance;
+
template
static void _register(CHECKER *checker, CheckerManager &mgr) {
CHECK1::_register(checker, mgr);
@@ -558,6 +576,8 @@ class Checker : public CHECK1, public CHECKs..., public
CheckerBase {
template
class Checker : public CHECK1, public CheckerBase {
public:
+ using BlockEntrance = clang::BlockEntrance;
+
template
static void _register(CHECKER *checker, CheckerManager &mgr) {
CHECK1::_register(checker, mgr);
diff --git a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
index 03ffadd346d0b..b5fefdb75401d 100644
--- a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
+++ b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
@@ -376,6 +376,12 @@ class CheckerManager {
const Stmt *S, ExprEngine &Eng,
const ProgramPoint &PP);
+ /// Run checkers after taking a control flow edge.
+ void runCheckersForBlockEntrance(ExplodedNodeSet &Dst,
+ const ExplodedNodeSet &Src,
+ const BlockEntrance &Entrance,
+ ExprEngine &Eng) const;
+
/// Run checkers for end of analysis.
void runCheckersForEndAnalysis(ExplodedGraph &G, BugReporter &BR,
ExprEngine &Eng);
@@ -528,6 +534,9 @@ class CheckerManager {
using CheckBindFunc =
CheckerFn;
+ using CheckBlockEntranceFunc =
+ CheckerFn;
+
using CheckEndAnalysisFunc =
CheckerFn;
@@ -589,6 +598,8 @@ class CheckerManager {
void _registerForBind(CheckBindFunc checkfn);
+ void _registerForBlockEntrance(Che
[clang] [analyzer] Introduce the check::BlockEntrance checker callback (PR #140924)
@@ -548,6 +564,8 @@ class CheckerProgramPointTag : public SimpleProgramPointTag
{
template
class Checker : public CHECK1, public CHECKs..., public CheckerBase {
public:
+ using BlockEntrance = clang::BlockEntrance;
balazs-benics-sonarsource wrote:
Yes, the class `check::BlockEntrance` would have priority within a checker,
e.g. the DivByZero checker when spelling an unqualified name `BlockEntrance`.
This means that the function parameter of that callback would refer to the
wrong class: `checkBlockEntrance(const BlockEntrance &Entrance,...)`.
To circumvent this and provide the expected lookup rules, I need to force it to
shadow this name with the `clang::BlockEntrance`, hence this using declaration
here.
It's not pretty, but smart and has a low footprint. No users should know why it
works.
https://github.com/llvm/llvm-project/pull/140924
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Introduce the check::BlockEntrance checker callback (PR #140924)
@@ -548,6 +564,8 @@ class CheckerProgramPointTag : public SimpleProgramPointTag
{
template
class Checker : public CHECK1, public CHECKs..., public CheckerBase {
public:
+ using BlockEntrance = clang::BlockEntrance;
balazs-benics-sonarsource wrote:
> Note that I'm planning to merge my CheckerFamily PR
> https://github.com/llvm/llvm-project/pull/139256 very soon and it will
> refactor this area -- will you be able to rebase your commit to that revision?
As I was thinking about this now, I think I'd prefer landing this first.
I'd need to backport this into clang-20, and that would make it (marginally)
more complicated.
I could rebase yours if this would be the case.
But I'm also fine with merging yours first, rebase this one, then backport and
resolve conflicts for our internal fork.
My primary objective is to merge this PR tomorrow at latest.
https://github.com/llvm/llvm-project/pull/140924
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Introduce the check::BlockEntrance checker callback (PR #140924)
balazs-benics-sonarsource wrote: I know it didn't actually pass a week for a ping, but let me know if its on the horizon. https://github.com/llvm/llvm-project/pull/140924 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Introduce the check::BlockEntrance checker callback (PR #140924)
@@ -166,6 +179,23 @@ class CheckerDocumentation
/// check::Bind
void checkBind(SVal Loc, SVal Val, const Stmt *S, CheckerContext &) const {}
+ /// Called after a CFG edge is taken within a function.
+ ///
+ /// This callback can be used to obtain information about potential branching
+ /// points or any other constructs that involve traversing a CFG edge.
+ /// Note that when inlining a call, there is no CFG edge between the caller
+ /// and the callee. One will only see the edge between the entry block and
+ /// the body of the function once inlined.
balazs-benics-sonarsource wrote:
This doc should be as clear as possible. I take full blame here.
There are a couple of things when inlining a function. Each of those steps are
represented by different ProgramPoints, and they are strictly following a
specific sequence:
1) PreVisit the `CallExpr`
2) Create a `CallEnter` ProgramPoint
3) Traverse the first CFG edge from the entry (artificial) node of the callee
CFG to the first meaningful CFG block of the callee. This will be represented
by a BlockEdge. (this is the first ProgramPoint btw from starting the analysis
from top-level context)
4) Create `BlockEntrance` ProgramPoint.
5) From this patch onward, checkers may create more nodes below this
`BlockEnrtance` node with different State or split the state as they see fit.
In the past there was exactly 1 `BlockEntrance` following a `BlockEdge`.
To me, "inlining" means the first 4 of these steps, I'm only mentioning the 5th
steps because that's related to this patch.
What I wanted to describe here is that one may expect a BlockEdge from the
caller CFG basic block from where the CallExpr resides, but there won't be such
BlockEdge. But you would have this other artificial egde instead from the
imaginary entry block to the actual basic block in the callee.
To me, this was surprising so I wanted to share this.
https://github.com/llvm/llvm-project/pull/140924
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Introduce the check::BlockEntrance checker callback (PR #140924)
https://github.com/balazs-benics-sonarsource updated
https://github.com/llvm/llvm-project/pull/140924
From 084d821b62d5de473d32d3506da95fdd7bad1cfe Mon Sep 17 00:00:00 2001
From: Balazs Benics
Date: Thu, 15 May 2025 17:20:29 +0200
Subject: [PATCH 1/7] [analyzer] Introduce the check::BlockEntrance
Tranersing the CFG blocks of a function is a fundamental operation.
Many C++ constructs can create splits in the control-flow, such as
`if`, `for`, and similar control structures or ternary expressions,
gnu conditionals, gotos, switches and possibly more.
Checkers should be able to get notifications about entering or leaving a
CFG block of interest.
Note that in the ExplodedGraph there is always a BlockEntrance
ProgramPoint right after the BlockEdge ProgramPoint.
I considered naming this callback check::BlockEdge, but then that may
leave the observer of the graph puzzled to see BlockEdge points followed
more BlockEdge nodes describing the same CFG transition.
This confusion could also apply to Bug Report Visitors too.
Because of this, I decided to hook BlockEntrance ProgramPoints instead.
The same confusion applies here, but I find this still a better place
TBH. There would only appear only one BlockEntrance ProgramPoint in the
graph if no checkers modify the state or emit a bug report.
Otherwise they modify some GDM (aka. State) thus create a new
ExplodedNode with the same BlockEntrance ProgramPoint in the graph.
CPP-6484
---
.../clang/StaticAnalyzer/Core/Checker.h | 20 ++
.../StaticAnalyzer/Core/CheckerManager.h | 13 +
.../Core/PathSensitive/ExprEngine.h | 4 +
.../Checkers/CheckerDocumentation.cpp | 15 +-
.../StaticAnalyzer/Core/CheckerManager.cpp| 50 +++-
clang/lib/StaticAnalyzer/Core/CoreEngine.cpp | 25 +-
clang/lib/StaticAnalyzer/Core/ExprEngine.cpp | 13 +
clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp | 6 +-
.../BlockEntranceCallbackTest.cpp | 283 ++
clang/unittests/StaticAnalyzer/CMakeLists.txt | 1 +
10 files changed, 414 insertions(+), 16 deletions(-)
create mode 100644 clang/unittests/StaticAnalyzer/BlockEntranceCallbackTest.cpp
diff --git a/clang/include/clang/StaticAnalyzer/Core/Checker.h
b/clang/include/clang/StaticAnalyzer/Core/Checker.h
index a54c5bee612f6..1b348dcce5ea7 100644
--- a/clang/include/clang/StaticAnalyzer/Core/Checker.h
+++ b/clang/include/clang/StaticAnalyzer/Core/Checker.h
@@ -221,6 +221,22 @@ class Bind {
}
};
+class BlockEntrance {
+ template
+ static void _checkBlockEntrance(void *Checker,
+ const clang::BlockEntrance &Entrance,
+ CheckerContext &C) {
+((const CHECKER *)Checker)->checkBlockEntrance(Entrance, C);
+ }
+
+public:
+ template
+ static void _register(CHECKER *checker, CheckerManager &mgr) {
+mgr._registerForBlockEntrance(CheckerManager::CheckBlockEntranceFunc(
+checker, _checkBlockEntrance));
+ }
+};
+
class EndAnalysis {
template
static void _checkEndAnalysis(void *checker, ExplodedGraph &G,
@@ -548,6 +564,8 @@ class CheckerProgramPointTag : public SimpleProgramPointTag
{
template
class Checker : public CHECK1, public CHECKs..., public CheckerBase {
public:
+ using BlockEntrance = clang::BlockEntrance;
+
template
static void _register(CHECKER *checker, CheckerManager &mgr) {
CHECK1::_register(checker, mgr);
@@ -558,6 +576,8 @@ class Checker : public CHECK1, public CHECKs..., public
CheckerBase {
template
class Checker : public CHECK1, public CheckerBase {
public:
+ using BlockEntrance = clang::BlockEntrance;
+
template
static void _register(CHECKER *checker, CheckerManager &mgr) {
CHECK1::_register(checker, mgr);
diff --git a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
index 03ffadd346d0b..b5fefdb75401d 100644
--- a/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
+++ b/clang/include/clang/StaticAnalyzer/Core/CheckerManager.h
@@ -376,6 +376,12 @@ class CheckerManager {
const Stmt *S, ExprEngine &Eng,
const ProgramPoint &PP);
+ /// Run checkers after taking a control flow edge.
+ void runCheckersForBlockEntrance(ExplodedNodeSet &Dst,
+ const ExplodedNodeSet &Src,
+ const BlockEntrance &Entrance,
+ ExprEngine &Eng) const;
+
/// Run checkers for end of analysis.
void runCheckersForEndAnalysis(ExplodedGraph &G, BugReporter &BR,
ExprEngine &Eng);
@@ -528,6 +534,9 @@ class CheckerManager {
using CheckBindFunc =
CheckerFn;
+ using CheckBlockEntranceFunc =
+ CheckerFn;
+
using CheckEndAnalysisFunc =
CheckerFn;
@@ -589,6 +598,8 @@ class CheckerManager {
void _registerForBind(CheckBindFunc checkfn);
+ void _registerForBlockEntrance(Che
[clang] [analyzer] Introduce the check::BlockEntrance checker callback (PR #140924)
@@ -166,6 +179,23 @@ class CheckerDocumentation
/// check::Bind
void checkBind(SVal Loc, SVal Val, const Stmt *S, CheckerContext &) const {}
+ /// Called after a CFG edge is taken within a function.
+ ///
+ /// This callback can be used to obtain information about potential branching
+ /// points or any other constructs that involve traversing a CFG edge.
+ /// Note that when inlining a call, there is no CFG edge between the caller
+ /// and the callee. One will only see the edge between the entry block and
+ /// the body of the function once inlined.
balazs-benics-sonarsource wrote:
Refined the comments in 700cd9380800c9e9105787c258ff63452fd97e15
https://github.com/llvm/llvm-project/pull/140924
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Introduce the check::BlockEntrance checker callback (PR #140924)
balazs-benics-sonarsource wrote: > In the tests I felt that it'd be a bit hard to decipher the meaning of the > block identifiers like `B1` etc. -- but when I re-read the file I noticed > that you included the very nice helper function `dumpCFGAndEgraph` (IIUC) for > those who will need to debug broken cases in this test file 😄 Perhaps it > would be even nicer if you included a commented out call to that function > with "`// NOTE: Uncomment this if you want to decipher the meaning of 'B0', > 'B1', ...`" to make its existence and role more obvious. Changed the debugging interface a bit, and added comments explaining it in f339eb1dc8216da013d5a92f01332b43e0a75790 https://github.com/llvm/llvm-project/pull/140924 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Introduce the check::BlockEntrance checker callback (PR #140924)
balazs-benics-sonarsource wrote: > @steakhal Thanks for the updates, I'm completely satisfied with them. > > I don't see any connection between this commit and the buildbot failures 🤔 > ... they are probably unrelated. About 90% of the time they are unrelated. I don't usually put a confirmation to these messages. https://github.com/llvm/llvm-project/pull/140924 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [analyzer] Introduce the check::BlockEntrance checker callback (PR #140924)
https://github.com/balazs-benics-sonarsource closed https://github.com/llvm/llvm-project/pull/140924 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
