diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h index 0aecc749bf415..b3dc940705f87 100644 --- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h +++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h @@ -753,9 +753,12 @@ RecordStorageLocation *getImplicitObjectLocation(const CXXMemberCallExpr &MCE, RecordStorageLocation *getBaseObjectLocation(const MemberExpr &ME, const Environment &Env); -/// Returns the fields of `RD` that are initialized by an `InitListExpr`, in the -/// order in which they appear in `InitListExpr::inits()`. -std::vector getFieldsForInitListExpr(const RecordDecl *RD); +/// 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 +getFieldsForInitListExpr(const InitListExpr *InitList); /// 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 d487944ce9211..0cfc26ea952cd 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(VD)) Fields.insert(FD); } else if (auto *InitList = dyn_cast(&S)) { - if (RecordDecl *RD = InitList->getType()->getAsRecordDecl()) - for (const auto *FD : getFieldsForInitListExpr(RD)) + if (InitList->getType()->isRecordType()) + for (const auto *FD : getFieldsForInitListExpr(InitList)) Fields.insert(FD); } } @@ -1104,12 +1104,22 @@ RecordStorageLocation *getBaseObjectLocation(const MemberExpr &ME, return Env.get(*Base); } -std::vector getFieldsForInitListExpr(const RecordDecl *RD) { +std::vector +getFieldsForInitListExpr(const InitListExpr *InitList) { + const RecordDecl *RD = InitList->getType()->getAsRecordDecl(); + assert(RD != nullptr); + + std::vector Fields; + + if (InitList->getType()->isUnionType()) { + Fields.push_back(InitList->getInitializedFieldInUnion()); + return Fields; + } + // 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 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 fe13e919bddcd..cd1f04e53cff6 100644 --- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp +++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp @@ -663,14 +663,7 @@ class TransferVisitor : public ConstStmtVisitor { void VisitInitListExpr(const InitListExpr *S) { QualType Type = S->getType(); - if (Type->isUnionType()) { - // FIXME: Initialize unions properly. - if (auto *Val = Env.createValue(Type)) - Env.setValue(*S, *Val); - return; - } - - if (!Type->isStructureOrClassType()) { + if (!Type->isRecordType()) { // Until array initialization is implemented, we don't need to care about // cases where `getNumInits() > 1`. if (S->getNumInits() == 1) @@ -688,10 +681,9 @@ class TransferVisitor : public ConstStmtVisitor { llvm::DenseMap FieldLocs; // This only contains the direct fields for the given type. - std::vector FieldsForInit = - getFieldsForInitListExpr(Type->getAsRecordDecl()); + std::vector FieldsForInit = getFieldsForInitListExpr(S); - // `S->inits()` contains all the initializer epressions, including the + // `S->inits()` contains all the initializer expressions, including the // ones for direct base classes. auto Inits = S->inits(); size_t InitIdx = 0; @@ -731,6 +723,17 @@ class TransferVisitor : public ConstStmtVisitor { 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 0d36d2802897f..b7cf6cc966edb 100644 --- a/clang/unittests/Analysis/FlowSensitive/TestingSupport.h +++ b/clang/unittests/Analysis/FlowSensitive/TestingSupport.h @@ -432,6 +432,8 @@ 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: /// @@ -475,6 +477,15 @@ ValueT &getValueForDecl(ASTContext &ASTCtx, const Environment &Env, return *cast(Env.getValue(*VD)); } +/// Returns the storage location for the field called `Name` of `Loc`. +/// Optionally casts the field storage location to `T`. +template +std::enable_if_t, T &> +getFieldLoc(const RecordStorageLocation &Loc, llvm::StringRef Name, + ASTContext &ASTCtx) { + return *cast(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, @@ -487,6 +498,14 @@ 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 a65b0446ac781..e7d74581865a3 100644 --- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp @@ -2377,14 +2377,24 @@ TEST(TransferTest, InitListExprAsUnion) { } F; public: - constexpr target() : F{nullptr} {} + constexpr target() : F{nullptr} { + int *null = nullptr; + F.b; // Make sure we reference 'b' so it is modeled. + // [[p]] + } }; )cc"; runDataflow( Code, [](const llvm::StringMap> &Results, ASTContext &ASTCtx) { - // Just verify that it doesn't crash. + const Environment &Env = getEnvironmentAtAnnotation(Results, "p"); + + auto &FLoc = getFieldLoc( + *Env.getThisPointeeStorageLocation(), "F", ASTCtx); + auto *AVal = cast(getFieldValue(&FLoc, "a", ASTCtx, Env)); + ASSERT_EQ(AVal, &getValueForDecl(ASTCtx, Env, "null")); + ASSERT_EQ(getFieldValue(&FLoc, "b", ASTCtx, Env), nullptr); }); }