diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h index 9ac2cb90ccc4d..33e9f0fba02bc 100644 --- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h +++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h @@ -260,11 +260,7 @@ class Environment { /// if `D` isn't assigned a storage location in the environment. StorageLocation *getStorageLocation(const ValueDecl &D) const; - /// Removes the location assigned to `D` in the environment. - /// - /// Requirements: - /// - /// `D` must have a storage location assigned in the environment. + /// Removes the location assigned to `D` in the environment (if any). void removeDecl(const ValueDecl &D); /// Assigns `Loc` as the storage location of the glvalue `E` in the diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp index 01c6cc69e2b9f..00e56ce51c530 100644 --- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp +++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp @@ -35,20 +35,6 @@ namespace dataflow { static constexpr int MaxCompositeValueDepth = 3; static constexpr int MaxCompositeValueSize = 1000; -/// Returns whether all declarations that `DeclToLoc1` and `DeclToLoc2` have in -/// common map to the same storage location in both maps. -bool declToLocConsistent( - const llvm::DenseMap &DeclToLoc1, - const llvm::DenseMap &DeclToLoc2) { - for (auto &Entry : DeclToLoc1) { - auto It = DeclToLoc2.find(Entry.first); - if (It != DeclToLoc2.end() && Entry.second != It->second) - return false; - } - - return true; -} - /// Returns a map consisting of key-value entries that are present in both maps. template llvm::DenseMap intersectDenseMaps(const llvm::DenseMap &Map1, @@ -662,7 +648,6 @@ Environment Environment::join(const Environment &EnvA, const Environment &EnvB, else JoinedEnv.ReturnLoc = nullptr; - assert(declToLocConsistent(EnvA.DeclToLoc, EnvB.DeclToLoc)); JoinedEnv.DeclToLoc = intersectDenseMaps(EnvA.DeclToLoc, EnvB.DeclToLoc); JoinedEnv.ExprToLoc = intersectDenseMaps(EnvA.ExprToLoc, EnvB.ExprToLoc); @@ -714,10 +699,7 @@ StorageLocation *Environment::getStorageLocation(const ValueDecl &D) const { return Loc; } -void Environment::removeDecl(const ValueDecl &D) { - assert(DeclToLoc.contains(&D)); - DeclToLoc.erase(&D); -} +void Environment::removeDecl(const ValueDecl &D) { DeclToLoc.erase(&D); } void Environment::setStorageLocation(const Expr &E, StorageLocation &Loc) { // `DeclRefExpr`s to builtin function types aren't glvalues, for some reason, diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp index f5d9c785b6397..0c2106777560e 100644 --- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp @@ -6236,4 +6236,48 @@ TEST(TransferTest, LambdaCaptureThis) { }); } +TEST(TransferTest, DifferentReferenceLocInJoin) { + // This test triggers a case where the storage location for a reference-type + // variable is different for two states being joined. We used to believe this + // could not happen and therefore had an assertion disallowing this; this test + // exists to demonstrate that we can handle this condition without a failing + // assertion. See also the discussion here: + // https://discourse.llvm.org/t/70086/6 + std::string Code = R"( + namespace std { + template struct initializer_list { + const T* begin(); + const T* end(); + }; + } + + void target(char* p, char* end) { + while (p != end) { + if (*p == ' ') { + p++; + continue; + } + + auto && range = {1, 2}; + for (auto b = range.begin(), e = range.end(); b != e; ++b) { + } + (void)0; + // [[p]] + } + } + )"; + runDataflow( + Code, + [](const llvm::StringMap> &Results, + ASTContext &ASTCtx) { + const Environment &Env = getEnvironmentAtAnnotation(Results, "p"); + + // Joining environments with different storage locations for the same + // declaration results in the declaration being removed from the joined + // environment. + const ValueDecl *VD = findValueDecl(ASTCtx, "range"); + ASSERT_EQ(Env.getStorageLocation(*VD), nullptr); + }); +} + } // namespace