[clang] Revert "[clang][dataflow] Correctly handle `InitListExpr` of union type." (PR #82856)
https://github.com/martinboehme closed https://github.com/llvm/llvm-project/pull/82856 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Revert "[clang][dataflow] Correctly handle `InitListExpr` of union type." (PR #82856)
martinboehme wrote: I see you haven't merged this revert yet. Alternatively, here's a fix-forward, if you're prepared to go with that: https://github.com/llvm/llvm-project/pull/82986 Otherwise, do of course merge the revert, and I'll reland with the fix-forward incorporated. https://github.com/llvm/llvm-project/pull/82856 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Revert "[clang][dataflow] Correctly handle `InitListExpr` of union type." (PR #82856)
martinboehme wrote: Sorry about the breakage. Will re-land soon with a test for this case and a fix. https://github.com/llvm/llvm-project/pull/82856 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Revert "[clang][dataflow] Correctly handle `InitListExpr` of union type." (PR #82856)
https://github.com/ymand approved this pull request. https://github.com/llvm/llvm-project/pull/82856 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Revert "[clang][dataflow] Correctly handle `InitListExpr` of union type." (PR #82856)
https://github.com/bazuzi edited https://github.com/llvm/llvm-project/pull/82856 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Revert "[clang][dataflow] Correctly handle `InitListExpr` of union type." (PR #82856)
github-actions[bot] wrote: ⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo. Please turn off [Keep my email addresses private](https://github.com/settings/emails) setting in your account. See [LLVM Discourse](https://discourse.llvm.org/t/hidden-emails-on-github-should-we-do-something-about-it) for more information. https://github.com/llvm/llvm-project/pull/82856 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Revert "[clang][dataflow] Correctly handle `InitListExpr` of union type." (PR #82856)
bazuzi wrote: @ymand Can you merge this? https://github.com/llvm/llvm-project/pull/82856 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Revert "[clang][dataflow] Correctly handle `InitListExpr` of union type." (PR #82856)
llvmbot wrote: @llvm/pr-subscribers-clang Author: Samira Bazuzi (bazuzi) Changes Reverts llvm/llvm-project#82348, which caused crashes when analyzing empty InitListExprs for unions. --- Full diff: https://github.com/llvm/llvm-project/pull/82856.diff 5 Files Affected: - (modified) clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h (+3-6) - (modified) clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp (+4-14) - (modified) clang/lib/Analysis/FlowSensitive/Transfer.cpp (+11-14) - (modified) clang/unittests/Analysis/FlowSensitive/TestingSupport.h (-19) - (modified) clang/unittests/Analysis/FlowSensitive/TransferTest.cpp (+2-12) ``diff 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 , RecordStorageLocation *getBaseObjectLocation(const MemberExpr , const Environment ); -/// 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); +/// 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); /// Associates a new `RecordValue` with `Loc` and returns the new value. RecordValue (RecordStorageLocation , Environment ); 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 , FieldSet , if (const auto *FD = dyn_cast(VD)) Fields.insert(FD); } else if (auto *InitList = dyn_cast()) { -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 , return Env.get(*Base); } -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; - } - +std::vector 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 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 { 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 { llvm::DenseMap FieldLocs; // This only contains the direct fields for the given type. -std::vector FieldsForInit = getFieldsForInitListExpr(S); +std::vector 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
[clang] Revert "[clang][dataflow] Correctly handle `InitListExpr` of union type." (PR #82856)
llvmbot wrote: @llvm/pr-subscribers-clang-analysis Author: Samira Bazuzi (bazuzi) Changes Reverts llvm/llvm-project#82348, which caused crashes when analyzing empty InitListExprs for unions. --- Full diff: https://github.com/llvm/llvm-project/pull/82856.diff 5 Files Affected: - (modified) clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h (+3-6) - (modified) clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp (+4-14) - (modified) clang/lib/Analysis/FlowSensitive/Transfer.cpp (+11-14) - (modified) clang/unittests/Analysis/FlowSensitive/TestingSupport.h (-19) - (modified) clang/unittests/Analysis/FlowSensitive/TransferTest.cpp (+2-12) ``diff 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 , RecordStorageLocation *getBaseObjectLocation(const MemberExpr , const Environment ); -/// 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); +/// 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); /// Associates a new `RecordValue` with `Loc` and returns the new value. RecordValue (RecordStorageLocation , Environment ); 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 , FieldSet , if (const auto *FD = dyn_cast(VD)) Fields.insert(FD); } else if (auto *InitList = dyn_cast()) { -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 , return Env.get(*Base); } -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; - } - +std::vector 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 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 { 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 { llvm::DenseMap FieldLocs; // This only contains the direct fields for the given type. -std::vector FieldsForInit = getFieldsForInitListExpr(S); +std::vector 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
[clang] Revert "[clang][dataflow] Correctly handle `InitListExpr` of union type." (PR #82856)
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 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 , RecordStorageLocation *getBaseObjectLocation(const MemberExpr , const Environment ); -/// 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); +/// 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); /// Associates a new `RecordValue` with `Loc` and returns the new value. RecordValue (RecordStorageLocation , Environment ); 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 , FieldSet , if (const auto *FD = dyn_cast(VD)) Fields.insert(FD); } else if (auto *InitList = dyn_cast()) { -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 , return Env.get(*Base); } -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; - } - +std::vector 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 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 { 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 { llvm::DenseMap FieldLocs; // This only contains the direct fields for the given type. -std::vector FieldsForInit = getFieldsForInitListExpr(S); +std::vector FieldsForInit = +