================
@@ -87,23 +84,22 @@ void ExprEngine::performTrivialCopy(NodeBuilder &Bldr,
ExplodedNode *Pred,
evalLocation(Tmp, CallExpr, VExpr, Pred, Pred->getState(), V,
/*isLoad=*/true);
for (ExplodedNode *N : Tmp)
- evalBind(Dst, CallExpr, N, ThisVal, V, !AlwaysReturnsLValue);
+ evalBind(DstEval, CallExpr, N, ThisVal, V, !AlwaysReturnsLValue);
} else {
// We can't copy empty classes because of empty base class optimization.
// In that case, copying the empty base class subobject would overwrite the
// object that it overlaps with - so let's not do that.
// See issue-157467.cpp for an example.
- Dst.insert(Pred);
+ DstEval.insert(Pred);
}
- PostStmt PS(CallExpr, SF);
- for (ExplodedNode *N : Dst) {
+ for (ExplodedNode *N : DstEval) {
ProgramStateRef State = N->getState();
if (AlwaysReturnsLValue)
State = State->BindExpr(CallExpr, SF, ThisVal);
else
State = bindReturnValue(Call, SF, State);
- Bldr.generateNode(PS, State, N);
+ Dst.insert(Engine.makePostStmtNode(CallExpr, State, Pred));
----------------
steakhal wrote:
Claude nailed a test for the first attempt - I hope it's not just a
hallucination:
```c++
// performTrivialCopy-predecessor-demo.cpp
//
// Demonstration probe for the semantic break that existed on the
// `eliminate-NodeBuilders-call-process` branch between commits
// `ee726d4ad9c3` (introduced) and `8b3f7dac7715` (still broken), and was
// fixed in `4f67409a61b2` "Fix use of obsolete node".
//
// HOW TO USE
// ----------
//
// 1. Apply the following patch to the LLVM source tree:
//
// --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
// +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp
// @@ -95,6 +95,15 @@ void ExprEngine::performTrivialCopy(...)
//
// for (ExplodedNode *N : DstEval) {
// + // Encodes the precondition under which the post-stmt
// + // predecessor can be either `N` or `Pred`: i.e. when they
// + // are equal. In the empty-class branch DstEval contains
// + // exactly Pred, so N == Pred and this assertion is fine.
// + // In the non-empty branch evalBind always returns fresh
// + // PostStore nodes, so N != Pred and this assertion fires.
// + // Tripping it proves that the choice between the two is
// + // observable — i.e. the two implementations differ.
// + assert(N == Pred && "performTrivialCopy: N != Pred — the "
// + "post-stmt predecessor argument is observable.");
// ProgramStateRef State = N->getState();
// if (AlwaysReturnsLValue)
// State = State->BindExpr(CallExpr, SF, ThisVal);
//
// 2. Build clang with assertions enabled.
//
// 3. Run the analyzer on this file:
//
// build/<dir>/bin/clang -cc1 -analyze -analyzer-checker=core \
// /tmp/performTrivialCopy-predecessor-demo.cpp
//
// EXPECTED RESULTS
// ----------------
//
// On HEAD (`4f67409a61b2`, fixed) **with the assertion applied**:
// The assertion fires on the `b = a` statement of `trip()`, because the
// non-empty branch of performTrivialCopy is taken and N != Pred.
// This *positively* demonstrates that the difference between
// `Bldr.generateNode(PS, State, N)` and
// `Engine.makePostStmtNode(CallExpr, State, Pred)` is observable.
//
// On any intermediate commit in `ee726d4ad9c3..8b3f7dac7715` (broken)
// **with the assertion applied**:
// The assertion fires identically. The same precondition `N == Pred`
// is violated; the difference is that the broken code propagates the
// wrong predecessor (Pred) into the analyzer graph, while the fixed
// code propagates the correct one (N).
//
// On either tree **without the assertion**:
// The analyzer runs cleanly and emits no diagnostics. The behavioral
// difference is in the predecessor edge of the resulting PostStmt
// node, which is invisible without instrumentation.
//
// EMPTY-CLASS CASE (negative control)
// -----------------------------------
//
// In `not_tripped()` below, the empty-class branch of performTrivialCopy
// runs `DstEval.insert(Pred)`, so N == Pred and the assertion does not
// fire. This confirms that empty-class trivial copies are unaffected by
// the bug, as expected.
struct NonEmpty {
int x;
int y;
};
struct Empty {};
void trip() {
NonEmpty a;
NonEmpty b;
b = a; // Implicit trivial copy assignment on a non-empty class.
// isTrivialObjectAssignment() returns true →
// defaultEvalCall calls performTrivialCopy →
// !ThisRD->isEmpty() branch is taken →
// evalLocation + evalBind populate DstEval with a
// PostStore node N (a graph descendant of Pred).
// N != Pred → assertion fires.
}
void not_tripped() {
Empty a;
Empty b;
b = a; // Empty-class branch: DstEval.insert(Pred) →
// N == Pred → assertion does not fire.
}
```
https://github.com/llvm/llvm-project/pull/203923
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
