https://github.com/martinboehme created https://github.com/llvm/llvm-project/pull/82986
This fixes a crash introduced by https://github.com/llvm/llvm-project/pull/82348 but also adds additional handling to make sure that we treat empty initializer lists for both unions and structs/classes correctly (see tests added in this patch). >From a48dc2e9a481ba5ed5d801a59c1dbc23fe0092d1 Mon Sep 17 00:00:00 2001 From: Martin Braenne <mboe...@google.com> Date: Mon, 26 Feb 2024 11:41:08 +0000 Subject: [PATCH] [clang][dataflow] Correctly treat empty initializer lists for unions. This fixes a crash introduced by https://github.com/llvm/llvm-project/pull/82348 but also adds additional handling to make sure that we treat empty initializer lists for both unions and structs/classes correctly (see tests added in this patch). --- .../FlowSensitive/DataflowEnvironment.cpp | 7 +- clang/lib/Analysis/FlowSensitive/Transfer.cpp | 15 +++- .../Analysis/FlowSensitive/TransferTest.cpp | 68 ++++++++++++++++++- 3 files changed, 86 insertions(+), 4 deletions(-) diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp index 0cfc26ea952cda..fd7b06efcc7861 100644 --- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp +++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp @@ -983,7 +983,7 @@ StorageLocation &Environment::createObjectInternal(const ValueDecl *D, } Value *Val = nullptr; - if (InitExpr) + if (InitExpr) { // In the (few) cases where an expression is intentionally // "uninterpreted", `InitExpr` is not associated with a value. There are // two ways to handle this situation: propagate the status, so that @@ -998,6 +998,11 @@ StorageLocation &Environment::createObjectInternal(const ValueDecl *D, // default value (assuming we don't update the environment API to return // references). Val = getValue(*InitExpr); + + if (!Val && isa<ImplicitValueInitExpr>(InitExpr) && + InitExpr->getType()->isPointerType()) + Val = &getOrCreateNullPointerValue(InitExpr->getType()->getPointeeType()); + } if (!Val) Val = createValue(Ty); diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp index cd1f04e53cff68..d73965806a533f 100644 --- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp +++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp @@ -685,9 +685,22 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> { // `S->inits()` contains all the initializer expressions, including the // ones for direct base classes. - auto Inits = S->inits(); + ArrayRef<Expr *> Inits = S->inits(); size_t InitIdx = 0; + // Unions initialized with an empty initializer list need special treatment. + // For structs/classes initialized with an empty initializer list, Clang + // puts `ImplicitValueInitExpr`s in `InitListExpr::inits()`, but for unions, + // it doesn't do this -- so we create an `ImplicitValueInitExpr` ourselves. + std::optional<ImplicitValueInitExpr> ImplicitValueInitForUnion; + SmallVector<Expr *> InitsForUnion; + if (S->getType()->isUnionType() && Inits.empty()) { + assert(FieldsForInit.size() == 1); + ImplicitValueInitForUnion.emplace(FieldsForInit.front()->getType()); + InitsForUnion.push_back(&*ImplicitValueInitForUnion); + Inits = InitsForUnion; + } + // Initialize base classes. if (auto* R = S->getType()->getAsCXXRecordDecl()) { assert(FieldsForInit.size() + R->getNumBases() == Inits.size()); diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp index e7d74581865a32..2eb9a37f289426 100644 --- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp @@ -2393,8 +2393,72 @@ TEST(TransferTest, InitListExprAsUnion) { 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); + EXPECT_EQ(AVal, &getValueForDecl<PointerValue>(ASTCtx, Env, "null")); + EXPECT_EQ(getFieldValue(&FLoc, "b", ASTCtx, Env), nullptr); + }); +} + +TEST(TransferTest, EmptyInitListExprForUnion) { + // This is a crash repro. + std::string Code = R"cc( + class target { + union { + int *a; + bool *b; + } F; + + public: + constexpr target() : F{} { + int *null = nullptr; + F.b; // Make sure we reference 'b' so it is modeled. + // [[p]] + } + }; + )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)); + EXPECT_EQ(AVal, &getValueForDecl<PointerValue>(ASTCtx, Env, "null")); + EXPECT_EQ(getFieldValue(&FLoc, "b", ASTCtx, Env), nullptr); + }); +} + +TEST(TransferTest, EmptyInitListExprForStruct) { + std::string Code = R"cc( + class target { + struct { + int *a; + bool *b; + } F; + + public: + constexpr target() : F{} { + int *NullIntPtr = nullptr; + bool *NullBoolPtr = nullptr; + // [[p]] + } + }; + )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, "NullIntPtr")); + auto *BVal = cast<PointerValue>(getFieldValue(&FLoc, "b", ASTCtx, Env)); + ASSERT_EQ(BVal, + &getValueForDecl<PointerValue>(ASTCtx, Env, "NullBoolPtr")); }); } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
