[clang] Revert "[clang][dataflow] Correctly handle `InitListExpr` of union type." (PR #82856)
Samira Bazuzi via cfe-commits
cfe-commits at lists.llvm.org
Fri Feb 23 17:13:22 PST 2024
https://github.com/bazuzi created https://github.com/llvm/llvm-project/pull/82856
Reverts llvm/llvm-project#82348, which caused crashes when analyzing empty InitListExprs for unions.
>From ae17f89f38d56c2aa9feaa1ebfc7a641b41dc068 Mon Sep 17 00:00:00 2001
From: Samira Bazuzi <bazuzi at users.noreply.github.com>
Date: Fri, 23 Feb 2024 20:12:33 -0500
Subject: [PATCH] =?UTF-8?q?Revert=20"[clang][dataflow]=20Correctly=20handl?=
=?UTF-8?q?e=20`InitListExpr`=20of=20union=20type.=20(#82=E2=80=A6"?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This reverts commit 4725993f1a812c86b9ad79d229a015d0216ff550.
---
.../FlowSensitive/DataflowEnvironment.h | 9 +++----
.../FlowSensitive/DataflowEnvironment.cpp | 18 +++----------
clang/lib/Analysis/FlowSensitive/Transfer.cpp | 25 ++++++++-----------
.../Analysis/FlowSensitive/TestingSupport.h | 19 --------------
.../Analysis/FlowSensitive/TransferTest.cpp | 14 ++---------
5 files changed, 20 insertions(+), 65 deletions(-)
diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
index b3dc940705f870..0aecc749bf415c 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -753,12 +753,9 @@ RecordStorageLocation *getImplicitObjectLocation(const CXXMemberCallExpr &MCE,
RecordStorageLocation *getBaseObjectLocation(const MemberExpr &ME,
const Environment &Env);
-/// Returns the fields of a `RecordDecl` that are initialized by an
-/// `InitListExpr`, in the order in which they appear in
-/// `InitListExpr::inits()`.
-/// `Init->getType()` must be a record type.
-std::vector<const FieldDecl *>
-getFieldsForInitListExpr(const InitListExpr *InitList);
+/// Returns the fields of `RD` that are initialized by an `InitListExpr`, in the
+/// order in which they appear in `InitListExpr::inits()`.
+std::vector<FieldDecl *> getFieldsForInitListExpr(const RecordDecl *RD);
/// Associates a new `RecordValue` with `Loc` and returns the new value.
RecordValue &refreshRecordValue(RecordStorageLocation &Loc, Environment &Env);
diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 0cfc26ea952cda..d487944ce92111 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -361,8 +361,8 @@ getFieldsGlobalsAndFuncs(const Stmt &S, FieldSet &Fields,
if (const auto *FD = dyn_cast<FieldDecl>(VD))
Fields.insert(FD);
} else if (auto *InitList = dyn_cast<InitListExpr>(&S)) {
- if (InitList->getType()->isRecordType())
- for (const auto *FD : getFieldsForInitListExpr(InitList))
+ if (RecordDecl *RD = InitList->getType()->getAsRecordDecl())
+ for (const auto *FD : getFieldsForInitListExpr(RD))
Fields.insert(FD);
}
}
@@ -1104,22 +1104,12 @@ RecordStorageLocation *getBaseObjectLocation(const MemberExpr &ME,
return Env.get<RecordStorageLocation>(*Base);
}
-std::vector<const FieldDecl *>
-getFieldsForInitListExpr(const InitListExpr *InitList) {
- const RecordDecl *RD = InitList->getType()->getAsRecordDecl();
- assert(RD != nullptr);
-
- std::vector<const FieldDecl *> Fields;
-
- if (InitList->getType()->isUnionType()) {
- Fields.push_back(InitList->getInitializedFieldInUnion());
- return Fields;
- }
-
+std::vector<FieldDecl *> getFieldsForInitListExpr(const RecordDecl *RD) {
// Unnamed bitfields are only used for padding and do not appear in
// `InitListExpr`'s inits. However, those fields do appear in `RecordDecl`'s
// field list, and we thus need to remove them before mapping inits to
// fields to avoid mapping inits to the wrongs fields.
+ std::vector<FieldDecl *> Fields;
llvm::copy_if(
RD->fields(), std::back_inserter(Fields),
[](const FieldDecl *Field) { return !Field->isUnnamedBitfield(); });
diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index cd1f04e53cff68..fe13e919bddcd8 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -663,7 +663,14 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
void VisitInitListExpr(const InitListExpr *S) {
QualType Type = S->getType();
- if (!Type->isRecordType()) {
+ if (Type->isUnionType()) {
+ // FIXME: Initialize unions properly.
+ if (auto *Val = Env.createValue(Type))
+ Env.setValue(*S, *Val);
+ return;
+ }
+
+ if (!Type->isStructureOrClassType()) {
// Until array initialization is implemented, we don't need to care about
// cases where `getNumInits() > 1`.
if (S->getNumInits() == 1)
@@ -681,9 +688,10 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
llvm::DenseMap<const ValueDecl *, StorageLocation *> FieldLocs;
// This only contains the direct fields for the given type.
- std::vector<const FieldDecl *> FieldsForInit = getFieldsForInitListExpr(S);
+ std::vector<FieldDecl *> FieldsForInit =
+ getFieldsForInitListExpr(Type->getAsRecordDecl());
- // `S->inits()` contains all the initializer expressions, including the
+ // `S->inits()` contains all the initializer epressions, including the
// ones for direct base classes.
auto Inits = S->inits();
size_t InitIdx = 0;
@@ -723,17 +731,6 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
FieldLocs.insert({Field, &Loc});
}
- // In the case of a union, we don't in general have initializers for all
- // of the fields. Create storage locations for the remaining fields (but
- // don't associate them with values).
- if (Type->isUnionType()) {
- for (const FieldDecl *Field :
- Env.getDataflowAnalysisContext().getModeledFields(Type)) {
- if (auto [it, inserted] = FieldLocs.insert({Field, nullptr}); inserted)
- it->second = &Env.createStorageLocation(Field->getType());
- }
- }
-
// Check that we satisfy the invariant that a `RecordStorageLoation`
// contains exactly the set of modeled fields for that type.
// `ModeledFields` includes fields from all the bases, but only the
diff --git a/clang/unittests/Analysis/FlowSensitive/TestingSupport.h b/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
index b7cf6cc966edb0..0d36d2802897fd 100644
--- a/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ b/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -432,8 +432,6 @@ llvm::Error checkDataflowWithNoopAnalysis(
{});
/// Returns the `ValueDecl` for the given identifier.
-/// The returned pointer is guaranteed to be non-null; the function asserts if
-/// no `ValueDecl` with the given name is found.
///
/// Requirements:
///
@@ -477,15 +475,6 @@ ValueT &getValueForDecl(ASTContext &ASTCtx, const Environment &Env,
return *cast<ValueT>(Env.getValue(*VD));
}
-/// Returns the storage location for the field called `Name` of `Loc`.
-/// Optionally casts the field storage location to `T`.
-template <typename T = StorageLocation>
-std::enable_if_t<std::is_base_of_v<StorageLocation, T>, T &>
-getFieldLoc(const RecordStorageLocation &Loc, llvm::StringRef Name,
- ASTContext &ASTCtx) {
- return *cast<T>(Loc.getChild(*findValueDecl(ASTCtx, Name)));
-}
-
/// Returns the value of a `Field` on the record referenced by `Loc.`
/// Returns null if `Loc` is null.
inline Value *getFieldValue(const RecordStorageLocation *Loc,
@@ -498,14 +487,6 @@ inline Value *getFieldValue(const RecordStorageLocation *Loc,
return Env.getValue(*FieldLoc);
}
-/// Returns the value of a `Field` on the record referenced by `Loc.`
-/// Returns null if `Loc` is null.
-inline Value *getFieldValue(const RecordStorageLocation *Loc,
- llvm::StringRef Name, ASTContext &ASTCtx,
- const Environment &Env) {
- return getFieldValue(Loc, *findValueDecl(ASTCtx, Name), Env);
-}
-
/// Creates and owns constraints which are boolean values.
class ConstraintContext {
unsigned NextAtom = 0;
diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index e7d74581865a32..a65b0446ac7818 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -2377,24 +2377,14 @@ TEST(TransferTest, InitListExprAsUnion) {
} F;
public:
- constexpr target() : F{nullptr} {
- int *null = nullptr;
- F.b; // Make sure we reference 'b' so it is modeled.
- // [[p]]
- }
+ constexpr target() : F{nullptr} {}
};
)cc";
runDataflow(
Code,
[](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
ASTContext &ASTCtx) {
- const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
-
- auto &FLoc = getFieldLoc<RecordStorageLocation>(
- *Env.getThisPointeeStorageLocation(), "F", ASTCtx);
- auto *AVal = cast<PointerValue>(getFieldValue(&FLoc, "a", ASTCtx, Env));
- ASSERT_EQ(AVal, &getValueForDecl<PointerValue>(ASTCtx, Env, "null"));
- ASSERT_EQ(getFieldValue(&FLoc, "b", ASTCtx, Env), nullptr);
+ // Just verify that it doesn't crash.
});
}
More information about the cfe-commits
mailing list