[clang] [StaticAnalyzer] Fix state update in VisitObjCForCollectionStmt (PR #124477)

[Balazs Benics via cfe-commits]

steakhal wrote:

I wanted to push to your branch but for some reason I could not.
Here is what I would have proposed to unblock this change:
```diff
commit ab9670b613be2bdd802342f031bd5e3d20680925
Author: Balazs Benics <benicsbalazs at gmail.com>
Date:   2025.01.29 13:02:16

    Add a unittest demonstrating that we no longer crash

diff --git a/clang/unittests/StaticAnalyzer/CMakeLists.txt b/clang/unittests/StaticAnalyzer/CMakeLists.txt
index f5da86e54560..7b574bdf7cbc 100644
--- a/clang/unittests/StaticAnalyzer/CMakeLists.txt
+++ b/clang/unittests/StaticAnalyzer/CMakeLists.txt
@@ -15,6 +15,7 @@ add_clang_unittest(StaticAnalysisTests
   IsCLibraryFunctionTest.cpp
   MemRegionDescriptiveNameTest.cpp
   NoStateChangeFuncVisitorTest.cpp
+  ObjCTest.cpp
   ParamRegionTest.cpp
   RangeSetTest.cpp
   RegisterCustomCheckersTest.cpp
diff --git a/clang/unittests/StaticAnalyzer/ObjCTest.cpp b/clang/unittests/StaticAnalyzer/ObjCTest.cpp
new file mode 100644
index 000000000000..16cab5336813
--- /dev/null
+++ b/clang/unittests/StaticAnalyzer/ObjCTest.cpp
@@ -0,0 +1,62 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "CheckerRegistration.h"
+#include "clang/StaticAnalyzer/Core/Checker.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
+#include "clang/StaticAnalyzer/Frontend/AnalysisConsumer.h"
+#include "clang/StaticAnalyzer/Frontend/CheckerRegistry.h"
+#include "gtest/gtest.h"
+
+using namespace clang;
+using namespace ento;
+
+// Some dummy trait that we can mutate back and forth to force a new State.
+REGISTER_TRAIT_WITH_PROGRAMSTATE(Flag, bool)
+
+namespace {
+class FlipFlagOnCheckLocation : public Checker<check::Location> {
+public:
+  // We make sure we alter the State every time we model a checkLocation event.
+  void checkLocation(SVal l, bool isLoad, const Stmt *S,
+                     CheckerContext &C) const {
+    ProgramStateRef State = C.getState();
+    State = State->set<Flag>(!State->get<Flag>());
+    C.addTransition(State);
+  }
+};
+
+void addFlagFlipperChecker(AnalysisASTConsumer &AnalysisConsumer,
+                           AnalyzerOptions &AnOpts) {
+  AnOpts.CheckersAndPackages = {{"test.FlipFlagOnCheckLocation", true}};
+  AnalysisConsumer.AddCheckerRegistrationFn([](CheckerRegistry &Registry) {
+    Registry.addChecker<FlipFlagOnCheckLocation>("test.FlipFlagOnCheckLocation",
+                                                 "Description", "");
+  });
+}
+
+TEST(ObjCTest, CheckLocationEventsShouldMaterializeInObjCForCollectionStmts) {
+  // Previously, the `ExprEngine::hasMoreIteration` may fired an assertion
+  // because we forgot to handle correctly the resulting nodes of the
+  // check::Location callback for the ObjCForCollectionStmts.
+  // This caused inconsistencies in the graph and triggering the assertion.
+  std::string Diags;
+  EXPECT_TRUE(runCheckerOnCodeWithArgs<addFlagFlipperChecker>(
+      R"(
+    @class NSArray, NSDictionary, NSString;
+    extern void NSLog(NSString *format, ...) __attribute__((format(__NSString__, 1, 2)));
+    void entrypoint(NSArray *bowl) {
+      for (NSString *fruit in bowl) { // no-crash
+        NSLog(@"Fruit: %@", fruit);
+      }
+    })",
+      {"-x", "objective-c"}, Diags));
+}
+
+} // namespace
```

Feel free to tweak it, especially the comment as you know the most of this context.
I checked and without your fix the test would crash, and after your fix it would work.

https://github.com/llvm/llvm-project/pull/124477