• Hoist S->getOperand, rework the test file

Still looks good.

Denys, I've created a very simple patch that makes the SValBuilder to be able to look up and use a constraint for an operand of a SymbolCast. That change passes 2 of your test cases, thus I made that a parent patch.

Nice work, looks promising! Once this https://reviews.llvm.org/D125400?vs=431909&id=431924#inline-1206369 is addressed I will accept.

LGTM! Thanks! And sorry for the delay in the review, please ping me next time.

• Make reAssume friend, pin the target in the test

This change in itself reduced the run-time of the analysis to 16 seconds, on my machine. However, the repetition of States should still be addressed. I am going to upload the upper patch for a starter.

Thanks Balazs for the report.

• Compare the size of the types instead of the type pointers

• Fix Assertion `*Solver->getSort(LHS) == *Solver->getSort(RHS) && "AST's must have the same sort!"' failed.

• Add clang_analyzer_warnIfReached() to the test and remove unused debug function decls

In D126123#3531150, @steakhal wrote:

In D126123#3531112, @martong wrote:

Is it documented with getRegion? Could we decorate that with returns-nonnull https://clang.llvm.org/docs/AttributeReference.html#returns-nonnull ?

Ah, I thought one of my changes addressing this was already on review.
It turns out it was not the case. Now I uploaded it for review: D126198

Point 2) above has negligible performance impact, empirical measurements do not show any noticeable difference in the run-time.

LGTM.

We should support deprecated analyzer flags for at least one release. In this case I'm planning to drop this flag in clang-17

LGTM

Looks good, with minor revisions.

Is it documented with getRegion? Could we decorate that with returns-nonnull https://clang.llvm.org/docs/AttributeReference.html#returns-nonnull ?

Remove unused SVal::hasConjuredSymbol

The title seems to be off with the changes.

Looks, good, but it was a struggle to follow if you did the inlining right or not. TBH, someone else should also check it.

This is an initial step of removing the SimpleSValBuilder abstraction. The SValBuilder alone should be enough.

Uhh, ohh, don't forget to reflect this change in the ReleaseNotes.rst, so urers will be notified!

However, this arises a couple of burning questions.

Should the cc1 frontend still accept this flag - to keep tools/users passing this flag directly to cc1 (which is unsupported, unadvertised) working.
If we should remain backward compatible, how long?
How can we get rid of deprecated and obsolete flags at some point?

The answer might be similar to what we do in case of a checker rename (or when we move out from alpha). Such a rename have the same problems, but we have not made much efforts to combat these problem. Users had to comply.

There are Z3 refutation related assertions on open-source projects once the patch stack is applied. Interestingly it happens in fromBinOp.

clang-14: ../../git/llvm-project/clang/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConv.h:96: static const llvm::SMTExpr* clang::ento::SMTConv::fromBinOp(llvm::SMTSolverRef&, const llvm::SMTExpr* const&, clang::BinaryOperator::Opcode, const llvm::SMTExpr* const&, bool): Assertion `*Solver->getSort(LHS) == *Solver->getSort(RHS) && "AST's must have the same sort!"' failed.

In D125892#3524453, @steakhal wrote:

Now I see, the summary confused me.

This includes the refactoring of the common assumle*Dual logic into the
function template assumeDualImpl.

I felt like this is a refactoring change, but it was not. It's about fixing the behavior by using the cloneAsPosteriorlyOverconstrained().
Please reword the summary.

• Move SimpleConstraintManager's assume*Internal functions to be protected

LGTM. Are there any other getPersistentLoc function definitions in other SVals? Might they be also unused?

• Split into two patches, this first patch will be just the no-brainer copy-paste from assumeDual implementaton.

In D125892#3523296, @steakhal wrote:

Okay, it took me a while to get what's going on.
Do you have anything in mind to express it by slightly less indirection, references, templates and well, virtual functions xD

In D125871#3521967, @steakhal wrote:

Could you please give a few examples of these FPs for the record?

• Update ReleaseNotes.rst

Abandoning because by using a bool in ctuBifurcate, the CTUWorklist will not have more than one elements during the first phase. Details: https://reviews.llvm.org/D123773?id=426037#inline-1206493

• Change ctuBifurcate to use bool in GDM

LGTM

LGTM

LGTM

I'm not sure, shall I add tests?

In D125400#3516164, @balazske wrote:

@martong Do you mean that a "describe" function is needed for the return value constraint of the function and for the errno constraint type? Then a note tag can contain what the function is assumed to return on success and what is allowed (or not) to do with errno. For example: "Assuming that 'mkdir' is successful, it returns zero in this case and value of 'errno' is unspecified after the call".

In D125400#3508955, @balazske wrote:

Function mkdir is modeled incorrectly by the checker. According to the man page it can return 0 or -1 only (-1 is error) but the checker allows non-negative value at success. So the shown bug report is incorrect (it can be only -1 if not 0 and then check of errno is allowed). Anyway the note tags should be added to every function.

In D125547#3515259, @steakhal wrote:

Something is messed up with the diff. You refer to fromUnOp() but it's not defined anywhere.

• Use existing fromUnOp
• pass nullptr as FromTy

• setNewDecl -> markAsNewDecl

• Add a test case for casts related crash

• Revert "Add type parameter to evalMinus and evalComplement"