https://github.com/haoNoQ updated https://github.com/llvm/llvm-project/pull/179058
>From 46f168e87055e737363014c29aaf115fcc1639b9 Mon Sep 17 00:00:00 2001 From: Artem Dergachev <[email protected]> Date: Fri, 30 Jan 2026 16:27:03 -0500 Subject: [PATCH 1/5] [clang][dataflow] Add basic modeling for compound assignments. Simularly to #178943, we need to make sure the old value does not stick around. And for correctness purposes it's better to conjure a fresh value than to leave it completely unknown. --- clang/lib/Analysis/FlowSensitive/Transfer.cpp | 25 +++++++++---------- .../Analysis/FlowSensitive/TransferTest.cpp | 6 ++--- 2 files changed, 14 insertions(+), 17 deletions(-) diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp index 51cc1f9bc26ab..a22e6284df5bc 100644 --- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp +++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp @@ -155,21 +155,18 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> { const Expr *RHS = S->getRHS(); assert(RHS != nullptr); - // Do compound assignments up-front, as there are so many of them and we - // don't want to list all of them in the switch statement below. - // To avoid generating unnecessary values, we don't create a new value but - // instead leave it to the specific analysis to do this if desired. - if (S->isCompoundAssignmentOp()) - propagateStorageLocation(*S->getLHS(), *S, Env); - - switch (S->getOpcode()) { - case BO_Assign: { + // Do assignments and compound assignments up-front, as there are + // so many of them and we don't want to list all of them in + // the switch statement below. + if (S->isAssignmentOp()) { auto *LHSLoc = Env.getStorageLocation(*LHS); if (LHSLoc == nullptr) - break; + return; - auto *RHSVal = Env.getValue(*RHS); - if (RHSVal == nullptr) + // Compound assignments involve arithmetic we don't model yet. + Value *RHSVal = + S->isCompoundAssignmentOp() ? nullptr : Env.getValue(*RHS); + if (!RHSVal) RHSVal = Env.createValue(LHS->getType()); // Assign a value to the storage location of the left-hand side. @@ -177,8 +174,10 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> { // Assign a storage location for the whole expression. Env.setStorageLocation(*S, *LHSLoc); - break; + return; } + + switch (S->getOpcode()) { case BO_LAnd: case BO_LOr: { BoolValue &LHSVal = getLogicOperatorSubExprValue(*LHS); diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp index e528ca2221ad1..c0bcb220c3d76 100644 --- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp @@ -990,8 +990,7 @@ TEST(TransferTest, BinaryOperatorAssignUnknown) { ASSERT_TRUE(isa_and_nonnull<IntegerValue>(FooAtCVal)); EXPECT_NE(FooAtAVal, FooAtBVal); - // FIXME: Should be NE too. - EXPECT_EQ(FooAtBVal, FooAtCVal); + EXPECT_NE(FooAtBVal, FooAtCVal); // Check that the storage location is correctly propagated. auto MatchResult = match(binaryOperator().bind("bo"), ASTCtx); @@ -1006,8 +1005,7 @@ TEST(TransferTest, BinaryOperatorAssignUnknown) { const Environment &EnvR = getEnvironmentAtAnnotation(Results, "r"); EXPECT_FALSE(EnvQ.proves(EnvQ.arena().makeLiteral(false))); - // FIXME: Should be FALSE too. - EXPECT_TRUE(EnvR.proves(EnvR.arena().makeLiteral(false))); + EXPECT_FALSE(EnvR.proves(EnvR.arena().makeLiteral(false))); }); } >From c6754b31a7356271cb3677b26d3bdd4b8a4b9f1a Mon Sep 17 00:00:00 2001 From: Artem Dergachev <[email protected]> Date: Sat, 31 Jan 2026 12:56:24 -0500 Subject: [PATCH 2/5] [clang][dataflow] Fixup: Preserve style. --- clang/lib/Analysis/FlowSensitive/Transfer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp index a22e6284df5bc..b2ea9a1ce02e8 100644 --- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp +++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp @@ -166,7 +166,7 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> { // Compound assignments involve arithmetic we don't model yet. Value *RHSVal = S->isCompoundAssignmentOp() ? nullptr : Env.getValue(*RHS); - if (!RHSVal) + if (RHSVal == nullptr) RHSVal = Env.createValue(LHS->getType()); // Assign a value to the storage location of the left-hand side. >From e05e6b800a1b1101c36fb13f2688277978b21440 Mon Sep 17 00:00:00 2001 From: Artem Dergachev <[email protected]> Date: Thu, 5 Feb 2026 21:12:39 -0500 Subject: [PATCH 3/5] Fixup: Unconditionally propagate storage. --- clang/lib/Analysis/FlowSensitive/Transfer.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp index 41b1e578ac6e0..b412c954b9e49 100644 --- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp +++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp @@ -163,6 +163,9 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> { if (LHSLoc == nullptr) return; + // Assign a storage location for the whole expression. + Env.setStorageLocation(*S, *LHSLoc); + // Compound assignments involve arithmetic we don't model yet. Value *RHSVal = S->isCompoundAssignmentOp() ? nullptr : Env.getValue(*RHS); @@ -179,9 +182,6 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> { // Assign a value to the storage location of the left-hand side. Env.setValue(*LHSLoc, *RHSVal); - - // Assign a storage location for the whole expression. - Env.setStorageLocation(*S, *LHSLoc); return; } >From 6defab9c5f55efff6fdc89946480f7238a7a2abc Mon Sep 17 00:00:00 2001 From: Artem Dergachev <[email protected]> Date: Thu, 5 Feb 2026 21:12:58 -0500 Subject: [PATCH 4/5] Fixup: Improve the comments. --- clang/lib/Analysis/FlowSensitive/Transfer.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp index b412c954b9e49..494e319526800 100644 --- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp +++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp @@ -167,9 +167,12 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> { Env.setStorageLocation(*S, *LHSLoc); // Compound assignments involve arithmetic we don't model yet. + // Regular assignments preserve the value so they're easy. Value *RHSVal = S->isCompoundAssignmentOp() ? nullptr : Env.getValue(*RHS); if (RHSVal == nullptr) { + // Either way, we need to conjure a value if we don't have any so that + // future lookups into that locations produced consistent results. RHSVal = Env.createValue(LHS->getType()); if (RHSVal == nullptr) { // At least make sure the old value is gone. It's unlikely to be there >From ee860f60b852bc3738f8aa1390168b58becf6209 Mon Sep 17 00:00:00 2001 From: Artem Dergachev <[email protected]> Date: Thu, 5 Feb 2026 21:17:11 -0500 Subject: [PATCH 5/5] Fixup: Unforget to unfixme the test. --- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp index bbedd8adc6358..317c6faf8d6a4 100644 --- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp @@ -1046,14 +1046,13 @@ TEST(TransferTest, BinaryOperatorAssignFloat) { // FIXME: Should be non-null. Floats aren't modeled at all. EXPECT_THAT(FooAtBVal, IsNull()); - // See if the storage location is correctly propagated. + // Check that the storage location is correctly propagated. auto MatchResult = match(binaryOperator(hasOperatorName("=")).bind("bo"), ASTCtx); const auto *BO = selectFirst<BinaryOperator>("bo", MatchResult); ASSERT_THAT(BO, NotNull()); const StorageLocation *BOLoc = EnvP.getStorageLocation(*BO); - // FIXME: Should be non-null. - EXPECT_THAT(BOLoc, IsNull()); + EXPECT_THAT(BOLoc, NotNull()); }); } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
