li.zhe.hua created this revision.
li.zhe.hua added a reviewer: ymandel.
Herald added subscribers: tschuett, steakhal.
Herald added a project: All.
li.zhe.hua requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Support for unions is incomplete (per 99f7d55e
<https://reviews.llvm.org/rG99f7d55eeeecd56566d23ee222e272729e05535c>) and the
`this` pointee
storage location is not set for unions. The assert in
`VisitCXXThisExpr` is then guaranteed to trigger when analyzing member
functions of a union.
This commit changes the assert to an early-return. Any expression may
be undefined, and so having a value for the `CXXThisExpr` is not a
postcondition of the transfer function.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D126405
Files:
clang/lib/Analysis/FlowSensitive/Transfer.cpp
clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===================================================================
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -42,10 +42,11 @@
template <typename Matcher>
void runDataflow(llvm::StringRef Code, Matcher Match,
LangStandard::Kind Std = LangStandard::lang_cxx17,
- bool ApplyBuiltinTransfer = true) {
+ bool ApplyBuiltinTransfer = true,
+ llvm::StringRef TargetFun = "target") {
ASSERT_THAT_ERROR(
test::checkDataflow<NoopAnalysis>(
- Code, "target",
+ Code, TargetFun,
[ApplyBuiltinTransfer](ASTContext &C, Environment &) {
return NoopAnalysis(C, ApplyBuiltinTransfer);
},
@@ -3175,4 +3176,27 @@
});
}
+TEST_F(TransferTest, DoesNotCrashOnUnionThisExpr) {
+ std::string Code = R"(
+ union Union {
+ int A;
+ float B;
+ };
+
+ void foo() {
+ Union A;
+ Union B;
+ A = B;
+ }
+ )";
+ // This is a crash regression test when calling the transfer function on a
+ // `CXXThisExpr` that refers to a union.
+ runDataflow(
+ Code,
+ [](llvm::ArrayRef<
+ std::pair<std::string, DataflowAnalysisState<NoopLattice>>>,
+ ASTContext &) {},
+ LangStandard::lang_cxx17, /*ApplyBuiltinTransfer=*/true, "operator=");
+}
+
} // namespace
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -279,7 +279,8 @@
void VisitCXXThisExpr(const CXXThisExpr *S) {
auto *ThisPointeeLoc = Env.getThisPointeeStorageLocation();
- assert(ThisPointeeLoc != nullptr);
+ if (ThisPointeeLoc == nullptr)
+ return;
auto &Loc = Env.createStorageLocation(*S);
Env.setStorageLocation(*S, Loc);
Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===================================================================
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -42,10 +42,11 @@
template <typename Matcher>
void runDataflow(llvm::StringRef Code, Matcher Match,
LangStandard::Kind Std = LangStandard::lang_cxx17,
- bool ApplyBuiltinTransfer = true) {
+ bool ApplyBuiltinTransfer = true,
+ llvm::StringRef TargetFun = "target") {
ASSERT_THAT_ERROR(
test::checkDataflow<NoopAnalysis>(
- Code, "target",
+ Code, TargetFun,
[ApplyBuiltinTransfer](ASTContext &C, Environment &) {
return NoopAnalysis(C, ApplyBuiltinTransfer);
},
@@ -3175,4 +3176,27 @@
});
}
+TEST_F(TransferTest, DoesNotCrashOnUnionThisExpr) {
+ std::string Code = R"(
+ union Union {
+ int A;
+ float B;
+ };
+
+ void foo() {
+ Union A;
+ Union B;
+ A = B;
+ }
+ )";
+ // This is a crash regression test when calling the transfer function on a
+ // `CXXThisExpr` that refers to a union.
+ runDataflow(
+ Code,
+ [](llvm::ArrayRef<
+ std::pair<std::string, DataflowAnalysisState<NoopLattice>>>,
+ ASTContext &) {},
+ LangStandard::lang_cxx17, /*ApplyBuiltinTransfer=*/true, "operator=");
+}
+
} // namespace
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -279,7 +279,8 @@
void VisitCXXThisExpr(const CXXThisExpr *S) {
auto *ThisPointeeLoc = Env.getThisPointeeStorageLocation();
- assert(ThisPointeeLoc != nullptr);
+ if (ThisPointeeLoc == nullptr)
+ return;
auto &Loc = Env.createStorageLocation(*S);
Env.setStorageLocation(*S, Loc);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
