Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang][dataflow] Remove declToLocConsistent() assertion. #69819

Merged
merged 1 commit into from
Oct 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
20 changes: 1 addition & 19 deletions clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<const ValueDecl *, StorageLocation *> &DeclToLoc1,
const llvm::DenseMap<const ValueDecl *, StorageLocation *> &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 <typename K, typename V>
llvm::DenseMap<K, V> intersectDenseMaps(const llvm::DenseMap<K, V> &Map1,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand Down
44 changes: 44 additions & 0 deletions clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 <class T> 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<DataflowAnalysisState<NoopLattice>> &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