[clang] [clang][analyzer] Add function 'fprintf' to StreamChecker. (PR #77613)

[Balázs Benics via cfe-commits]

@@ -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)

[Balázs Benics via cfe-commits]

@@ -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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

@@ -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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

@@ -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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

@@ -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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

@@ -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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

@@ -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)

[Balázs Benics via cfe-commits]

@@ -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)

[Balázs Benics via cfe-commits]

@@ -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)

[Balázs Benics via cfe-commits]

@@ -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)

[Balázs Benics via cfe-commits]

@@ -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)

[Balázs Benics via cfe-commits]

@@ -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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

@@ -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)

[Balázs Benics via cfe-commits]

@@ -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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

@@ -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)

[Balázs Benics via cfe-commits]

@@ -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)

[Balázs Benics via cfe-commits]

@@ -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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

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.

![slow-entrypoints-before-and-after-bind-limit](https://github.com/user-attachments/assets/44425a76-f1cb-449c-bc3e-f44beb8c5dc7)
(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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

@@ -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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

@@ -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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

@@ -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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

@@ -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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

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

[clang] [analyzer] Introduce the check::BlockEntrance checker callback (PR #140924)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

@@ -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)

[Balázs Benics via cfe-commits]

@@ -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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

@@ -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][NFC] Move PrettyStackTraceLocationContext into dispatchWorkItem (PR #140035)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

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)

[Balázs Benics via cfe-commits]

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] Enforce not making overly complicated symbols (PR #144327)

[Balázs Benics via cfe-commits]

@@ -46,7 +50,7 @@ class SymbolRegionValue : public SymbolData {
 
   friend class SymExprAllocator;
   SymbolRegionValue(SymbolID sym, const TypedValueRegion *r)
-  : SymbolData(SymbolRegionValueKind, sym), R(r) {
+  : SymbolData(ClassKind, sym), R(r) {

balazs-benics-sonarsource wrote:

I do depend on this, although it could be technically split from this PR.
I need to compile time check if a `T` is a subclass of `SymbolData`, thus I 
need a constexpr type-trait (`classof`).
If we have a `SymbolData` then the `acquire` can never hit max symbol 
complexity, thus we always return the type that was requested. Otherwise, we 
"complicate" a symbol which may return a substitute symbol instead of type `T`.

I'll try to split it from this PR.

https://github.com/llvm/llvm-project/pull/144327
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Enforce not making overly complicated symbols (PR #144327)

[Balázs Benics via cfe-commits]

@@ -134,6 +137,101 @@ class SymbolConjured : public SymbolData {
   static constexpr bool classof(Kind K) { return K == ClassKind; }
 };
 
+/// A symbol representing the result of an expression that became too
+/// complicated. In other words, its complexity would have surpassed the
+/// MaxSymbolComplexity threshold.
+/// TODO: When the MaxSymbolComplexity is reached, we should propagate the 
taint
+/// info to it.
+class SymbolTooComplex final : public SymbolData {
+  // Pointer to either "SymExpr" or "APSInt".
+  const void *LHS;
+  const void *RHS = nullptr; // optional
+  QualType Ty;
+  using OpKindType = std::make_unsigned_t<
+  std::common_type_t>;
+  OpKindType Op = 0;

balazs-benics-sonarsource wrote:

I explored the direction of creating the overly complicated symbol, and then 
just wrapping that in bb82e63b88d1bd233a7895129d25d2cc3b5bc5e4. I think this is 
what you asked.

Note that this approach has the slight disadvantage of actually creating overly 
complicated symbols to wrap.
Sometimes this new symbol will have a complexity equal to max complexity, and 
other times well over that complexity in case we combined two max complexity 
symbols by a binop like `+`.
Remember, complexity is calculated by `complexity(LHS) + complexity(RHS)`.

This simplifies the code quite a bit though. Besides aesthetics, it should not 
change the overall behavior too much.

https://github.com/llvm/llvm-project/pull/144327
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer][NFC] Add xrefs to a test case that has poor git blame (PR #145501)

[Balázs Benics via cfe-commits]

https://github.com/balazs-benics-sonarsource closed 
https://github.com/llvm/llvm-project/pull/145501
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Enforce not making overly complicated symbols (PR #144327)

[Balázs Benics via cfe-commits]

@@ -134,6 +137,101 @@ class SymbolConjured : public SymbolData {
   static constexpr bool classof(Kind K) { return K == ClassKind; }
 };
 
+/// A symbol representing the result of an expression that became too
+/// complicated. In other words, its complexity would have surpassed the
+/// MaxSymbolComplexity threshold.
+/// TODO: When the MaxSymbolComplexity is reached, we should propagate the 
taint
+/// info to it.

balazs-benics-sonarsource wrote:

In my opinion, the llvm-project static analyzer issue tracker is cursed. Once 
an issue gets there, will remain there for ever. FYI I just move this TODO to a 
more appropriate place. I didn't actually add it here.

https://github.com/llvm/llvm-project/pull/144327
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Enforce not making overly complicated symbols (PR #144327)

[Balázs Benics via cfe-commits]

@@ -671,17 +779,33 @@ class SymbolVisitor {
   virtual bool VisitMemRegion(const MemRegion *) { return true; }
 };
 
+// Returns a pointer to T if T is a SymbolData, otherwise SymExpr.

balazs-benics-sonarsource wrote:

Reworked the code, and dropped the `auto` type spec. This should be more 
readable now.

https://github.com/llvm/llvm-project/pull/144327
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Enforce not making overly complicated symbols (PR #144327)

[Balázs Benics via cfe-commits]

balazs-benics-sonarsource wrote:

> > Should I measure the perf of this change?
> 
> I think so, because the patch is substantially different. For example, the 
> overly complex symbol tree is now preserved, even if it is apparently not 
> traversed in its entirety any longer. That makes it not obvious if the 
> initial performance gains are still there.

Agreed.



> Thanks for your patience in this review! I'm happy to see the additional 
> simplifications coming from just wrapping the over-complicated symbol instead 
> of storing its parts separately. (I see that it's a bit inelegant to 
> construct the very thing that we're trying to avoid, but this is the 
> pragmatic choice...)
> I don't expect any performance issues (the "frozen" complex symbols shouldn't 
> be problematic if they aren't traversed), but performance testing is never 
> completely useless.

I'll do a usual regression test, but not anything as detailed as the evaluation 
in the PR summary presents.
The test case sufficiently demonstrates that this enforcement works as 
intended. I also had a look at the time trace jsons of the test with the new 
way of this enforcement and the originally proposed variant. They look 
identical to the naked eye.
I also ran the test with the two versions and they run the same way according 
to `hyperfine`.

Nevertheless, I'll do a usual regression test.

https://github.com/llvm/llvm-project/pull/144327
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Enforce not making overly complicated symbols (PR #144327)

[Balázs Benics via cfe-commits]

https://github.com/balazs-benics-sonarsource updated 
https://github.com/llvm/llvm-project/pull/144327

From a5ae39c1b8a5abc39f8948a724b354839e708e4b Mon Sep 17 00:00:00 2001
From: Balazs Benics 
Date: Mon, 16 Jun 2025 12:14:06 +0200
Subject: [PATCH 1/3] [analyzer] Enforce not making overly complicated symbols

Out of the worst 500 entry points, 45 were improved by at least 10%.
Out of these 45, 5 were improved by more than 50%.
Out of these 45, 2 were improved by more than 80%.

For example, for the
`DelugeFirmware/src/OSLikeStuff/fault_handler/fault_handler.c` TU:
- `printPointers` entry point was improved from 31.1 seconds to 1.1 second 
(28x).
- `handle_cpu_fault` entry point was improved from 15.5 seconds to 3 seconds 
(5x).

We had in total 3'037'085 entry points in the test pool.
Out of these 390'156 were measured to run over a second.

TODO: Add the plot here about RT.

CPP-6182
---
 .../Core/PathSensitive/SValBuilder.h  |  24 +--
 .../Core/PathSensitive/SymExpr.h  |  25 +--
 .../Core/PathSensitive/SymbolManager.h| 166 --
 clang/lib/StaticAnalyzer/Checkers/Taint.cpp   |   2 +-
 .../Checkers/TrustNonnullChecker.cpp  |   2 +-
 clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp |   2 +-
 .../Core/RangeConstraintManager.cpp   |  38 ++--
 .../Core/RangedConstraintManager.cpp  |  15 +-
 clang/lib/StaticAnalyzer/Core/SValBuilder.cpp |  89 +-
 .../StaticAnalyzer/Core/SimpleSValBuilder.cpp |  29 +--
 .../lib/StaticAnalyzer/Core/SymbolManager.cpp |   6 +
 .../ensure-capped-symbol-complexity.cpp   |  53 ++
 12 files changed, 298 insertions(+), 153 deletions(-)
 create mode 100644 clang/test/Analysis/ensure-capped-symbol-complexity.cpp

diff --git 
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
index 2911554de9d97..0458a6125db9a 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
@@ -57,6 +57,8 @@ class SValBuilder {
 protected:
   ASTContext &Context;
 
+  const AnalyzerOptions &AnOpts;
+
   /// Manager of APSInt values.
   BasicValueFactory BasicVals;
 
@@ -68,8 +70,6 @@ class SValBuilder {
 
   ProgramStateManager &StateMgr;
 
-  const AnalyzerOptions &AnOpts;
-
   /// The scalar type to use for array indices.
   const QualType ArrayIndexTy;
 
@@ -317,21 +317,21 @@ class SValBuilder {
 return nonloc::LocAsInteger(BasicVals.getPersistentSValWithData(loc, 
bits));
   }
 
-  nonloc::SymbolVal makeNonLoc(const SymExpr *lhs, BinaryOperator::Opcode op,
-   APSIntPtr rhs, QualType type);
+  DefinedOrUnknownSVal makeNonLoc(const SymExpr *lhs, BinaryOperator::Opcode 
op,
+  APSIntPtr rhs, QualType type);
 
-  nonloc::SymbolVal makeNonLoc(APSIntPtr rhs, BinaryOperator::Opcode op,
-   const SymExpr *lhs, QualType type);
+  DefinedOrUnknownSVal makeNonLoc(APSIntPtr rhs, BinaryOperator::Opcode op,
+  const SymExpr *lhs, QualType type);
 
-  nonloc::SymbolVal makeNonLoc(const SymExpr *lhs, BinaryOperator::Opcode op,
-   const SymExpr *rhs, QualType type);
+  DefinedOrUnknownSVal makeNonLoc(const SymExpr *lhs, BinaryOperator::Opcode 
op,
+  const SymExpr *rhs, QualType type);
 
-  NonLoc makeNonLoc(const SymExpr *operand, UnaryOperator::Opcode op,
-QualType type);
+  DefinedOrUnknownSVal makeNonLoc(const SymExpr *operand,
+  UnaryOperator::Opcode op, QualType type);
 
   /// Create a NonLoc value for cast.
-  nonloc::SymbolVal makeNonLoc(const SymExpr *operand, QualType fromTy,
-   QualType toTy);
+  DefinedOrUnknownSVal makeNonLoc(const SymExpr *operand, QualType fromTy,
+  QualType toTy);
 
   nonloc::ConcreteInt makeTruthVal(bool b, QualType type) {
 return nonloc::ConcreteInt(BasicVals.getTruthValue(b, type));
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h
index aca14cf813c4b..11d0a22a31c46 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h
@@ -51,9 +51,11 @@ class SymExpr : public llvm::FoldingSetNode {
   /// Note, however, that it can't be used in Profile because SymbolManager
   /// needs to compute Profile before allocating SymExpr.
   const SymbolID Sym;
+  const unsigned Complexity;
 
 protected:
-  SymExpr(Kind k, SymbolID Sym) : K(k), Sym(Sym) {}
+  SymExpr(Kind k, SymbolID Sym, unsigned Complexity)
+  : K(k), Sym(Sym), Complexity(Complexity) {}
 
   static bool isValidTypeForSymbol(QualType T) {
 // FIXME: Depending on whether we choose to depre

[clang] [analyzer][NFC] Make SymExpr::classof methods constexpr (PR #145526)

[Balázs Benics via cfe-commits]

https://github.com/balazs-benics-sonarsource created 
https://github.com/llvm/llvm-project/pull/145526

This should enable more powerful type metaprograms.

Split from #144327

From 2241ead76477da9eee6b8206a06abe841168b666 Mon Sep 17 00:00:00 2001
From: Balazs Benics 
Date: Tue, 24 Jun 2025 16:56:16 +0200
Subject: [PATCH] [analyzer][NFC] Make SymExpr::classof methods constexpr

This should enable more powerful type metaprograms.
---
 .../Core/PathSensitive/SymExpr.h  |   6 +-
 .../Core/PathSensitive/SymbolManager.h| 100 +-
 2 files changed, 54 insertions(+), 52 deletions(-)

diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h
index aca14cf813c4b..6233a22d2ca2b 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h
@@ -152,9 +152,9 @@ class SymbolData : public SymExpr {
   };
 
   // Implement isa support.
-  static inline bool classof(const SymExpr *SE) {
-Kind k = SE->getKind();
-return k >= BEGIN_SYMBOLS && k <= END_SYMBOLS;
+  static bool classof(const SymExpr *SE) { return classof(SE->getKind()); }
+  static constexpr bool classof(Kind K) {
+return K >= BEGIN_SYMBOLS && K <= END_SYMBOLS;
   }
 };
 
diff --git 
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
index 86774ad5043dd..7af86cd721e8e 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
@@ -46,7 +46,7 @@ class SymbolRegionValue : public SymbolData {
 
   friend class SymExprAllocator;
   SymbolRegionValue(SymbolID sym, const TypedValueRegion *r)
-  : SymbolData(SymbolRegionValueKind, sym), R(r) {
+  : SymbolData(ClassKind, sym), R(r) {
 assert(r);
 assert(isValidTypeForSymbol(r->getValueType()));
   }
@@ -56,7 +56,7 @@ class SymbolRegionValue : public SymbolData {
   const TypedValueRegion *getRegion() const { return R; }
 
   static void Profile(llvm::FoldingSetNodeID& profile, const TypedValueRegion* 
R) {
-profile.AddInteger((unsigned) SymbolRegionValueKind);
+profile.AddInteger((unsigned)ClassKind);
 profile.AddPointer(R);
   }
 
@@ -72,9 +72,9 @@ class SymbolRegionValue : public SymbolData {
   QualType getType() const override;
 
   // Implement isa support.
-  static bool classof(const SymExpr *SE) {
-return SE->getKind() == SymbolRegionValueKind;
-  }
+  static constexpr Kind ClassKind = SymbolRegionValueKind;
+  static bool classof(const SymExpr *SE) { return classof(SE->getKind()); }
+  static constexpr bool classof(Kind K) { return K == ClassKind; }
 };
 
 /// A symbol representing the result of an expression in the case when we do
@@ -90,8 +90,8 @@ class SymbolConjured : public SymbolData {
   SymbolConjured(SymbolID sym, ConstCFGElementRef elem,
  const LocationContext *lctx, QualType t, unsigned count,
  const void *symbolTag)
-  : SymbolData(SymbolConjuredKind, sym), Elem(elem), T(t), Count(count),
-LCtx(lctx), SymbolTag(symbolTag) {
+  : SymbolData(ClassKind, sym), Elem(elem), T(t), Count(count), LCtx(lctx),
+SymbolTag(symbolTag) {
 assert(lctx);
 assert(isValidTypeForSymbol(t));
   }
@@ -115,7 +115,7 @@ class SymbolConjured : public SymbolData {
   static void Profile(llvm::FoldingSetNodeID &profile, ConstCFGElementRef Elem,
   const LocationContext *LCtx, QualType T, unsigned Count,
   const void *SymbolTag) {
-profile.AddInteger((unsigned)SymbolConjuredKind);
+profile.AddInteger((unsigned)ClassKind);
 profile.Add(Elem);
 profile.AddPointer(LCtx);
 profile.Add(T);
@@ -128,9 +128,9 @@ class SymbolConjured : public SymbolData {
   }
 
   // Implement isa support.
-  static bool classof(const SymExpr *SE) {
-return SE->getKind() == SymbolConjuredKind;
-  }
+  static constexpr Kind ClassKind = SymbolConjuredKind;
+  static bool classof(const SymExpr *SE) { return classof(SE->getKind()); }
+  static constexpr bool classof(Kind K) { return K == ClassKind; }
 };
 
 /// A symbol representing the value of a MemRegion whose parent region has
@@ -141,7 +141,7 @@ class SymbolDerived : public SymbolData {
 
   friend class SymExprAllocator;
   SymbolDerived(SymbolID sym, SymbolRef parent, const TypedValueRegion *r)
-  : SymbolData(SymbolDerivedKind, sym), parentSymbol(parent), R(r) {
+  : SymbolData(ClassKind, sym), parentSymbol(parent), R(r) {
 assert(parent);
 assert(r);
 assert(isValidTypeForSymbol(r->getValueType()));
@@ -162,7 +162,7 @@ class SymbolDerived : public SymbolData {
 
   static void Profile(llvm::FoldingSetNodeID& profile, SymbolRef parent,
   const TypedValueRegion *r) {
-profile.AddInteger((uns

[clang] [analyzer][NFC] Add xrefs to a test case that has poor git blame (PR #145501)

[Balázs Benics via cfe-commits]

balazs-benics-sonarsource wrote:

I wanted to look at recently of the history of this test file. I managed to dig 
it up, but since the git blame didn't tell me much I think it's worth to spell 
out these refs explicitly.

https://github.com/llvm/llvm-project/pull/145501
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Enforce not making overly complicated symbols (PR #144327)

[Balázs Benics via cfe-commits]

balazs-benics-sonarsource wrote:

Just rebased the PR to exclude the refactor change of `classof`. I'll schedule 
a measurement now.

https://github.com/llvm/llvm-project/pull/144327
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Enforce not making overly complicated symbols (PR #144327)

[Balázs Benics via cfe-commits]

https://github.com/balazs-benics-sonarsource updated 
https://github.com/llvm/llvm-project/pull/144327

From 2c32b9f6dfa9a236cea5679b6613e567cd8dfc2b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bal=C3=A1zs=20Benics?=
 <108414871+balazs-benics-sonarsou...@users.noreply.github.com>
Date: Tue, 24 Jun 2025 18:57:25 +0200
Subject: [PATCH 1/2] [analyzer][NFC] Make SymExpr::classof methods constexpr
 (#145526)

This should enable more powerful type metaprograms.

Split from #144327
---
 .../Core/PathSensitive/SymExpr.h  |   6 +-
 .../Core/PathSensitive/SymbolManager.h| 100 +-
 2 files changed, 54 insertions(+), 52 deletions(-)

diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h
index aca14cf813c4b..6233a22d2ca2b 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h
@@ -152,9 +152,9 @@ class SymbolData : public SymExpr {
   };
 
   // Implement isa support.
-  static inline bool classof(const SymExpr *SE) {
-Kind k = SE->getKind();
-return k >= BEGIN_SYMBOLS && k <= END_SYMBOLS;
+  static bool classof(const SymExpr *SE) { return classof(SE->getKind()); }
+  static constexpr bool classof(Kind K) {
+return K >= BEGIN_SYMBOLS && K <= END_SYMBOLS;
   }
 };
 
diff --git 
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
index 86774ad5043dd..7af86cd721e8e 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
@@ -46,7 +46,7 @@ class SymbolRegionValue : public SymbolData {
 
   friend class SymExprAllocator;
   SymbolRegionValue(SymbolID sym, const TypedValueRegion *r)
-  : SymbolData(SymbolRegionValueKind, sym), R(r) {
+  : SymbolData(ClassKind, sym), R(r) {
 assert(r);
 assert(isValidTypeForSymbol(r->getValueType()));
   }
@@ -56,7 +56,7 @@ class SymbolRegionValue : public SymbolData {
   const TypedValueRegion *getRegion() const { return R; }
 
   static void Profile(llvm::FoldingSetNodeID& profile, const TypedValueRegion* 
R) {
-profile.AddInteger((unsigned) SymbolRegionValueKind);
+profile.AddInteger((unsigned)ClassKind);
 profile.AddPointer(R);
   }
 
@@ -72,9 +72,9 @@ class SymbolRegionValue : public SymbolData {
   QualType getType() const override;
 
   // Implement isa support.
-  static bool classof(const SymExpr *SE) {
-return SE->getKind() == SymbolRegionValueKind;
-  }
+  static constexpr Kind ClassKind = SymbolRegionValueKind;
+  static bool classof(const SymExpr *SE) { return classof(SE->getKind()); }
+  static constexpr bool classof(Kind K) { return K == ClassKind; }
 };
 
 /// A symbol representing the result of an expression in the case when we do
@@ -90,8 +90,8 @@ class SymbolConjured : public SymbolData {
   SymbolConjured(SymbolID sym, ConstCFGElementRef elem,
  const LocationContext *lctx, QualType t, unsigned count,
  const void *symbolTag)
-  : SymbolData(SymbolConjuredKind, sym), Elem(elem), T(t), Count(count),
-LCtx(lctx), SymbolTag(symbolTag) {
+  : SymbolData(ClassKind, sym), Elem(elem), T(t), Count(count), LCtx(lctx),
+SymbolTag(symbolTag) {
 assert(lctx);
 assert(isValidTypeForSymbol(t));
   }
@@ -115,7 +115,7 @@ class SymbolConjured : public SymbolData {
   static void Profile(llvm::FoldingSetNodeID &profile, ConstCFGElementRef Elem,
   const LocationContext *LCtx, QualType T, unsigned Count,
   const void *SymbolTag) {
-profile.AddInteger((unsigned)SymbolConjuredKind);
+profile.AddInteger((unsigned)ClassKind);
 profile.Add(Elem);
 profile.AddPointer(LCtx);
 profile.Add(T);
@@ -128,9 +128,9 @@ class SymbolConjured : public SymbolData {
   }
 
   // Implement isa support.
-  static bool classof(const SymExpr *SE) {
-return SE->getKind() == SymbolConjuredKind;
-  }
+  static constexpr Kind ClassKind = SymbolConjuredKind;
+  static bool classof(const SymExpr *SE) { return classof(SE->getKind()); }
+  static constexpr bool classof(Kind K) { return K == ClassKind; }
 };
 
 /// A symbol representing the value of a MemRegion whose parent region has
@@ -141,7 +141,7 @@ class SymbolDerived : public SymbolData {
 
   friend class SymExprAllocator;
   SymbolDerived(SymbolID sym, SymbolRef parent, const TypedValueRegion *r)
-  : SymbolData(SymbolDerivedKind, sym), parentSymbol(parent), R(r) {
+  : SymbolData(ClassKind, sym), parentSymbol(parent), R(r) {
 assert(parent);
 assert(r);
 assert(isValidTypeForSymbol(r->getValueType()));
@@ -162,7 +162,7 @@ class SymbolDerived : public SymbolData {
 
   static void Profile(llvm::FoldingSetNodeID& profile, SymbolRef parent,
   const TypedVa

[clang] [analyzer] Enforce not making overly complicated symbols (PR #144327)

[Balázs Benics via cfe-commits]

balazs-benics-sonarsource wrote:

I was thinking about possible ways to unblock this change.

If the additional code complexity needs justification, by measuring the average 
impact (to ensure no regression happens in the common cases), and by repeating 
the measurement of the handful of edge-cases where it should bring noticeable 
(10%+) improvement. My estimate of these efforts would be a couple of days of 
work. I probably can't afford to spend this time.

It's hard to say, but there are two other options I'm contemplating:
 - ask for second opinions on Discuss (or here by CC-ing the other 
maintainers), or
 - abandon the patch (for now, possibly indefinitely).

But I don't really like either of these options.

I'm open for suggestions about how to proceed with this patch.

https://github.com/llvm/llvm-project/pull/144327
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Enforce not making overly complicated symbols (PR #144327)

[Balázs Benics via cfe-commits]

@@ -45,6 +45,7 @@ SYMBOL(SymbolCast, SymExpr)
 
 ABSTRACT_SYMBOL(SymbolData, SymExpr)
   SYMBOL(SymbolConjured, SymbolData)
+  SYMBOL(SymbolTooComplex, SymbolData)

balazs-benics-sonarsource wrote:

Renamed in b9edbc75bc87e4e263cb629f088ae9977a9cac03

https://github.com/llvm/llvm-project/pull/144327
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer][docs] Mention perfetto for visualizing trace JSONs (PR #145500)

[Balázs Benics via cfe-commits]

https://github.com/balazs-benics-sonarsource closed 
https://github.com/llvm/llvm-project/pull/145500
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Enforce not making overly complicated symbols (PR #144327)

[Balázs Benics via cfe-commits]

https://github.com/balazs-benics-sonarsource updated 
https://github.com/llvm/llvm-project/pull/144327

From bc7dfc2b4f143c2caa1189db096bf9e4ea76f053 Mon Sep 17 00:00:00 2001
From: Balazs Benics 
Date: Mon, 16 Jun 2025 12:14:06 +0200
Subject: [PATCH] [analyzer] Enforce not making overly complicated symbols

CPP-6182
---
 .../Core/PathSensitive/SValBuilder.h  |   8 +-
 .../Core/PathSensitive/SymExpr.h  |  19 +--
 .../Core/PathSensitive/SymbolManager.h| 127 +-
 .../Core/PathSensitive/Symbols.def|   1 +
 clang/lib/StaticAnalyzer/Checkers/Taint.cpp   |   2 +-
 .../Checkers/TrustNonnullChecker.cpp  |   2 +-
 clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp |   2 +-
 .../Core/RangeConstraintManager.cpp   |  11 +-
 clang/lib/StaticAnalyzer/Core/SValBuilder.cpp |  43 +++---
 .../StaticAnalyzer/Core/SimpleSValBuilder.cpp |   6 +-
 .../lib/StaticAnalyzer/Core/SymbolManager.cpp |  13 ++
 .../ensure-capped-symbol-complexity.cpp   |  58 
 12 files changed, 211 insertions(+), 81 deletions(-)
 create mode 100644 clang/test/Analysis/ensure-capped-symbol-complexity.cpp

diff --git 
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
index 2911554de9d97..fae78c564e0ab 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
@@ -57,6 +57,8 @@ class SValBuilder {
 protected:
   ASTContext &Context;
 
+  const AnalyzerOptions &AnOpts;
+
   /// Manager of APSInt values.
   BasicValueFactory BasicVals;
 
@@ -68,8 +70,6 @@ class SValBuilder {
 
   ProgramStateManager &StateMgr;
 
-  const AnalyzerOptions &AnOpts;
-
   /// The scalar type to use for array indices.
   const QualType ArrayIndexTy;
 
@@ -326,8 +326,8 @@ class SValBuilder {
   nonloc::SymbolVal makeNonLoc(const SymExpr *lhs, BinaryOperator::Opcode op,
const SymExpr *rhs, QualType type);
 
-  NonLoc makeNonLoc(const SymExpr *operand, UnaryOperator::Opcode op,
-QualType type);
+  nonloc::SymbolVal makeNonLoc(const SymExpr *operand, UnaryOperator::Opcode 
op,
+   QualType type);
 
   /// Create a NonLoc value for cast.
   nonloc::SymbolVal makeNonLoc(const SymExpr *operand, QualType fromTy,
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h
index 6233a22d2ca2b..11d0a22a31c46 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h
@@ -51,9 +51,11 @@ class SymExpr : public llvm::FoldingSetNode {
   /// Note, however, that it can't be used in Profile because SymbolManager
   /// needs to compute Profile before allocating SymExpr.
   const SymbolID Sym;
+  const unsigned Complexity;
 
 protected:
-  SymExpr(Kind k, SymbolID Sym) : K(k), Sym(Sym) {}
+  SymExpr(Kind k, SymbolID Sym, unsigned Complexity)
+  : K(k), Sym(Sym), Complexity(Complexity) {}
 
   static bool isValidTypeForSymbol(QualType T) {
 // FIXME: Depending on whether we choose to deprecate structural symbols,
@@ -61,8 +63,6 @@ class SymExpr : public llvm::FoldingSetNode {
 return !T.isNull() && !T->isVoidType();
   }
 
-  mutable unsigned Complexity = 0;
-
 public:
   virtual ~SymExpr() = default;
 
@@ -108,7 +108,7 @@ class SymExpr : public llvm::FoldingSetNode {
 return llvm::make_range(symbol_iterator(this), symbol_iterator());
   }
 
-  virtual unsigned computeComplexity() const = 0;
+  unsigned complexity() const { return Complexity; }
 
   /// Find the region from which this symbol originates.
   ///
@@ -136,10 +136,15 @@ using SymbolRefSmallVectorTy = SmallVector;
 /// A symbol representing data which can be stored in a memory location
 /// (region).
 class SymbolData : public SymExpr {
+  friend class SymbolManager;
   void anchor() override;
 
 protected:
-  SymbolData(Kind k, SymbolID sym) : SymExpr(k, sym) { assert(classof(this)); }
+  SymbolData(Kind k, SymbolID sym) : SymExpr(k, sym, computeComplexity()) {
+assert(classof(this));
+  }
+
+  static unsigned computeComplexity(...) { return 1; }
 
 public:
   ~SymbolData() override = default;
@@ -147,10 +152,6 @@ class SymbolData : public SymExpr {
   /// Get a string representation of the kind of the region.
   virtual StringRef getKindStr() const = 0;
 
-  unsigned computeComplexity() const override {
-return 1;
-  };
-
   // Implement isa support.
   static bool classof(const SymExpr *SE) { return classof(SE->getKind()); }
   static constexpr bool classof(Kind K) {
diff --git 
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
index 7af86cd721e8e..9c27669a61ba9 100644
--- a/clang/include/clang/StaticAnalyzer/Cor

[clang] [analyzer][NFC] Add xrefs to a test case that has poor git blame (PR #145501)

[Balázs Benics via cfe-commits]

https://github.com/balazs-benics-sonarsource created 
https://github.com/llvm/llvm-project/pull/145501

None

From b67c2c5bfadc80121899690723ba938b398c4e65 Mon Sep 17 00:00:00 2001
From: Balazs Benics 
Date: Tue, 24 Jun 2025 13:14:31 +0200
Subject: [PATCH] [analyzer][NFC] Add xrefs to a test case that has poor git
 blame

---
 clang/test/Analysis/PR38208.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/clang/test/Analysis/PR38208.c b/clang/test/Analysis/PR38208.c
index fb0a5a15eac6b..750cbc50cc749 100644
--- a/clang/test/Analysis/PR38208.c
+++ b/clang/test/Analysis/PR38208.c
@@ -2,6 +2,11 @@
 
 // expected-no-diagnostics
 
+// This test case used to demonstrate a huge slowdown regression.
+// Reported in https://bugs.llvm.org/show_bug.cgi?id=38208
+// Caused by 2bbccca9f75b6bce08d77cf19abfb206d0c3bc2e aka. 
"aggressive-binary-operation-simplification"
+// Fixed by dcde8acc32f1355f37d3bc2814c528fdc2ca5f94
+
 int foo(int a, int b) {
   a += b; b -= a;
   a += b; b -= a;

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer][docs] Mention perfetto for visualizing trace JSONs (PR #145500)

[Balázs Benics via cfe-commits]

https://github.com/balazs-benics-sonarsource created 
https://github.com/llvm/llvm-project/pull/145500

None

From 274a38f83b2c5c81968de865403cf114fc9550c0 Mon Sep 17 00:00:00 2001
From: Balazs Benics 
Date: Tue, 24 Jun 2025 13:42:42 +0200
Subject: [PATCH] [analyzer][docs] Mention perfetto for visualizing trace JSONs

---
 clang/docs/analyzer/developer-docs/PerformanceInvestigation.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/docs/analyzer/developer-docs/PerformanceInvestigation.rst 
b/clang/docs/analyzer/developer-docs/PerformanceInvestigation.rst
index ca3a56828209b..5d662cfb65be2 100644
--- a/clang/docs/analyzer/developer-docs/PerformanceInvestigation.rst
+++ b/clang/docs/analyzer/developer-docs/PerformanceInvestigation.rst
@@ -10,7 +10,7 @@ Performance analysis using ``-ftime-trace``
 
 You can add the ``-ftime-trace=file.json`` option to break down the analysis 
time into individual entry points and steps within each entry point.
 You can explore the generated JSON file in a Chromium browser using the 
``chrome://tracing`` URL,
-or using `speedscope `_.
+or using `perfetto `_ or `speedscope 
`_.
 Once you narrow down to specific analysis steps you are interested in, you can 
more effectively employ heavier profilers,
 such as `Perf `_ and `Callgrind 
`_.
 

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Enforce not making overly complicated symbols (PR #144327)

[Balázs Benics via cfe-commits]

https://github.com/balazs-benics-sonarsource created 
https://github.com/llvm/llvm-project/pull/144327

Out of the worst 500 entry points, 45 were improved by at least 10%. Out of 
these 45, 5 were improved by more than 50%. Out of these 45, 2 were improved by 
more than 80%.

For example, for the
`DelugeFirmware/src/OSLikeStuff/fault_handler/fault_handler.c` TU:
- `printPointers` entry point was improved from 31.1 seconds to 1.1 second 
(28x).
- `handle_cpu_fault` entry point was improved from 15.5 seconds to 3 seconds 
(5x).

We had in total 3'037'085 entry points in the test pool. Out of these 390'156 
were measured to run over a second.

![asset](https://github.com/user-attachments/assets/7e5df195-debd-4a15-ba5d-361708dfc2b9)

CPP-6182

From a5ae39c1b8a5abc39f8948a724b354839e708e4b Mon Sep 17 00:00:00 2001
From: Balazs Benics 
Date: Mon, 16 Jun 2025 12:14:06 +0200
Subject: [PATCH] [analyzer] Enforce not making overly complicated symbols

Out of the worst 500 entry points, 45 were improved by at least 10%.
Out of these 45, 5 were improved by more than 50%.
Out of these 45, 2 were improved by more than 80%.

For example, for the
`DelugeFirmware/src/OSLikeStuff/fault_handler/fault_handler.c` TU:
- `printPointers` entry point was improved from 31.1 seconds to 1.1 second 
(28x).
- `handle_cpu_fault` entry point was improved from 15.5 seconds to 3 seconds 
(5x).

We had in total 3'037'085 entry points in the test pool.
Out of these 390'156 were measured to run over a second.

TODO: Add the plot here about RT.

CPP-6182
---
 .../Core/PathSensitive/SValBuilder.h  |  24 +--
 .../Core/PathSensitive/SymExpr.h  |  25 +--
 .../Core/PathSensitive/SymbolManager.h| 166 --
 clang/lib/StaticAnalyzer/Checkers/Taint.cpp   |   2 +-
 .../Checkers/TrustNonnullChecker.cpp  |   2 +-
 clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp |   2 +-
 .../Core/RangeConstraintManager.cpp   |  38 ++--
 .../Core/RangedConstraintManager.cpp  |  15 +-
 clang/lib/StaticAnalyzer/Core/SValBuilder.cpp |  89 +-
 .../StaticAnalyzer/Core/SimpleSValBuilder.cpp |  29 +--
 .../lib/StaticAnalyzer/Core/SymbolManager.cpp |   6 +
 .../ensure-capped-symbol-complexity.cpp   |  53 ++
 12 files changed, 298 insertions(+), 153 deletions(-)
 create mode 100644 clang/test/Analysis/ensure-capped-symbol-complexity.cpp

diff --git 
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
index 2911554de9d97..0458a6125db9a 100644
--- a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
+++ b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
@@ -57,6 +57,8 @@ class SValBuilder {
 protected:
   ASTContext &Context;
 
+  const AnalyzerOptions &AnOpts;
+
   /// Manager of APSInt values.
   BasicValueFactory BasicVals;
 
@@ -68,8 +70,6 @@ class SValBuilder {
 
   ProgramStateManager &StateMgr;
 
-  const AnalyzerOptions &AnOpts;
-
   /// The scalar type to use for array indices.
   const QualType ArrayIndexTy;
 
@@ -317,21 +317,21 @@ class SValBuilder {
 return nonloc::LocAsInteger(BasicVals.getPersistentSValWithData(loc, 
bits));
   }
 
-  nonloc::SymbolVal makeNonLoc(const SymExpr *lhs, BinaryOperator::Opcode op,
-   APSIntPtr rhs, QualType type);
+  DefinedOrUnknownSVal makeNonLoc(const SymExpr *lhs, BinaryOperator::Opcode 
op,
+  APSIntPtr rhs, QualType type);
 
-  nonloc::SymbolVal makeNonLoc(APSIntPtr rhs, BinaryOperator::Opcode op,
-   const SymExpr *lhs, QualType type);
+  DefinedOrUnknownSVal makeNonLoc(APSIntPtr rhs, BinaryOperator::Opcode op,
+  const SymExpr *lhs, QualType type);
 
-  nonloc::SymbolVal makeNonLoc(const SymExpr *lhs, BinaryOperator::Opcode op,
-   const SymExpr *rhs, QualType type);
+  DefinedOrUnknownSVal makeNonLoc(const SymExpr *lhs, BinaryOperator::Opcode 
op,
+  const SymExpr *rhs, QualType type);
 
-  NonLoc makeNonLoc(const SymExpr *operand, UnaryOperator::Opcode op,
-QualType type);
+  DefinedOrUnknownSVal makeNonLoc(const SymExpr *operand,
+  UnaryOperator::Opcode op, QualType type);
 
   /// Create a NonLoc value for cast.
-  nonloc::SymbolVal makeNonLoc(const SymExpr *operand, QualType fromTy,
-   QualType toTy);
+  DefinedOrUnknownSVal makeNonLoc(const SymExpr *operand, QualType fromTy,
+  QualType toTy);
 
   nonloc::ConcreteInt makeTruthVal(bool b, QualType type) {
 return nonloc::ConcreteInt(BasicVals.getTruthValue(b, type));
diff --git a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h
index aca14cf813c4b..11d0a22a31c46 100644
--- a/clang/include/clang/Sta

[clang] [analyzer] Enforce not making overly complicated symbols (PR #144327)

[Balázs Benics via cfe-commits]

@@ -67,7 +67,7 @@ void ExprEngine::VisitBinaryOperator(const BinaryOperator* B,
   if (RightV.isUnknown()) {
 unsigned Count = currBldrCtx->blockCount();
 RightV = svalBuilder.conjureSymbolVal(nullptr, getCFGElementRef(), 
LCtx,
-  Count);
+  RHS->getType(), Count);

balazs-benics-sonarsource wrote:

Interestingly in #137355 @fangyi-zhou changed the behavior of this line, thus 
needed a tiny bit of adjustment to make the new test pass while I was uplifting 
this downstream patch to current llvm main.
I didn't investigate the case beyond that this was the line that conjured a 
symbol of a wrong type after #137355, probably because in the past we directly 
passed a QualType here but after that change we rely on deducing the type from 
`getCFGElementRef()` - which is apparently wrong. To see the behavior, revert 
this hunk and see the broken test. There could be more places where this type 
mismatch on conjure could cause issues, but I didn't audit the code further.

https://github.com/llvm/llvm-project/pull/144327
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Fix a false memory leak reports involving placement new (PR #144341)

[Balázs Benics via cfe-commits]

@@ -63,6 +64,42 @@ void testGlobalNoThrowPlacementExprNewBeforeOverload() {
   int *p = new(std::nothrow) int;
 } // leak-warning{{Potential leak of memory pointed to by 'p'}}
 
+//- Standard pointer placement operators
+void testGlobalPointerPlacementNew() {
+  int i;
+  void *p1 = operator new(0, &i); // no warn
+
+  void *p2 = operator new[](0, &i); // no warn
+
+  int *p3 = new(&i) int; // no warn
+
+  int *p4 = new(&i) int[0]; // no warn

balazs-benics-sonarsource wrote:

```suggestion
  void *p1 = operator new(0, &i); // no leak: placement new never allocates
  void *p2 = operator new[](0, &i); // no leak
  int *p3 = new(&i) int; // no leak
  int *p4 = new(&i) int[0]; // no leak
```

https://github.com/llvm/llvm-project/pull/144341
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Enforce not making overly complicated symbols (PR #144327)

[Balázs Benics via cfe-commits]

balazs-benics-sonarsource wrote:

> Unfortunately I'm not convinced that this is the right direction for 
> improving the analyzer runtime.
> 
> On the "risks" side I think that adding the corner case that "this may also 
> return `UnknownVal` in rare situations" into many functions complicates the 
> logic, burdens the code with early return branches and I fear that it will 
> act as a footgun.
>
> On the "benefits" side I fear that your statistics don't prove enough:
> 
> 1. You found that _"Out of the worst 500 entry points, 45 were improved by at 
> least 10%. Out of these 45, 5 were improved by more than 50%. Out of these 
> 45, 2 were improved by more than 80%."_ but this only covers 9% of the worst 
> 500 entry points. Eyeballing the graph suggests that there are some cases 
> where the runtime actually got _worse_ -- so please check that the overall 
> effect of the change is also positive (e.g. the total runtime is reduced 
> meaningfully).
> 2. Moreover, if "worst 500 entry points" means "worst 500 in the first run", 
> then it is _a biased sample_: if you pick the worst outliers (i.e. the entry 
> points where the sum _expected runtime + luck factor_ is largest), then you 
> are expected to get entry points with worse than average luck (because among 
> two similar entry points, the one with bad luck ends up in the worst 500 
> while the one with good luck avoids it), so if you redo the measurement, then 
> [regression toward the 
> mean](https://en.wikipedia.org/wiki/Regression_toward_the_mean) will produce 
> better results -- even if you do both measurements with the same setup! As a 
> sanity check, please redo the statistics on the the entry points that 
> produced the worst 500 runtimes _in the second run_ -- I fear that on that 
> sample (which is biased in the opposite direction) you will see that the new 
> revision is _worse_ than the baseline.
> 3. I'm also interested in comparing the statistical results with a second 
> independent measurement -- is the set of "worst 500 entry points" stable 
> between runs, or are these random unlucky functions that are hit with 
> environmental issues?
> 
> If you can share the raw data, I can help with statistical calculations.

Could you paraphrase your concerns?

How I read this you have mainly 2 concerns:
 1) The use of this strong-type makes it tedious the existing APIs to use 
because one needs to unwrap the value and frequently make an early-return to 
explicitly handle the case when a symbol-creation failed?
 2) The measurements weren't conclusive. There was no evidence provided that on 
the usual cases (all entry points) the RT would not regress. It was also not 
fair to look at only the longest 500 entry points to evaluate the effectiveness 
of limiting the max symbol complexity (in other words, honoring the max symbol 
complexity limit).
 
 > [...] [regression toward the 
 > mean](https://en.wikipedia.org/wiki/Regression_toward_the_mean) [...]

I formulated the test case in the tests by inspecting a long-running test case. 
It is consistently low-performing. You can also check it on godbolt, that it 
would not finish, because the symbol simplifier would need to do so much work 
due walking the overly complicated symbols. I've also inspected about 10 (a 
couple of months ago) other radically improved cases, and all showed a large 
number of binary op manipulations like hashing, or the test case I supply in 
this patch. This doesn't seem to be a coincidence to me.
From my experience, our pool is large enough to be consistent and roughly 
reproducible for long-ish entry points. According to our data, usually an entry 
point should finish in about 1 second if not less. Above that suggests 
something to look at.

To me, encountering symbols with complexity over the dedicated max symbol 
complexity threshold is a bug.
Can you think of other ways to ensure we never create overly complicated 
symbols?

https://github.com/llvm/llvm-project/pull/144327
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [analyzer] Enforce not making overly complicated symbols (PR #144327)

[Balázs Benics via cfe-commits]

balazs-benics-sonarsource wrote:

> > I was thinking about possible ways to unblock this change.
> > If the additional code complexity needs justification, by measuring the 
> > average impact (to ensure no regression happens in the common cases), and 
> > by repeating the measurement of the handful of edge-cases where it should 
> > bring noticeable (10%+) improvement. My estimate of these efforts would be 
> > a couple of days of work. I probably can't afford to spend this time.
> 
> I don't think that these additional measurements would be worth the effort. 
> The measurement quality was my chronologically first concern, but it's not 
> the crux of the matter.
> 
> * If I understand the situation correctly you don't claim that this patch 
> would provide any benefit that can be detected by users who analyze a 
> complete real-world software project (and not just individual outlier TUs).
> * I find it plausible that this PR indeed improves the runtime of a handful 
> of edge-cases and I don't suspect that it would increase the overall runtime, 
> but I don't think that the benefits which are only visible on isolated 
> outlier entry points (or artificial test cases) justify the additional code 
> complexity.

I'd say yes to both, while pointing out again that the handful of cases where 
the improvement can be observed are also real-world code, but rare code.

> My personal opinion is that the best decision would be abandoning this PR 
> (costs outweigh the benefits), but this is an inherently subjective 
> judgement, so if you are convinced that merging this PR would be better, then 
> asking other contributors is the right way forward (and I won't block the 
> decision if others agree with it).

Thank you for your flexibility. I still believe that obeying max symbol 
complexity on its own brings value even without knowing that sometimes it also 
eliminates long running edge cases. I believe it justifies for the added 
complexity, but I'm very open about refining the patch in other directions that 
would also ensure max symbol complexity.

/cc @Xazax-hun @haoNoQ for second opinion.

https://github.com/llvm/llvm-project/pull/144327
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits