Skip to content

Commit

Permalink
[clang][dataflow] Relax assert on existence of this pointee storage
Browse files Browse the repository at this point in the history
Support for unions is incomplete (per 99f7d55) 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.

Differential Revision: https://reviews.llvm.org/D126405
  • Loading branch information
tJener committed May 25, 2022
1 parent 67e2e6e commit 33b598a
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 3 deletions.
5 changes: 4 additions & 1 deletion clang/lib/Analysis/FlowSensitive/Transfer.cpp
Expand Up @@ -279,7 +279,10 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {

void VisitCXXThisExpr(const CXXThisExpr *S) {
auto *ThisPointeeLoc = Env.getThisPointeeStorageLocation();
assert(ThisPointeeLoc != nullptr);
if (ThisPointeeLoc == nullptr)
// Unions are not supported yet, and will not have a location for the
// `this` expression's pointee.
return;

auto &Loc = Env.createStorageLocation(*S);
Env.setStorageLocation(*S, Loc);
Expand Down
28 changes: 26 additions & 2 deletions clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
Expand Up @@ -42,10 +42,11 @@ class TransferTest : public ::testing::Test {
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);
},
Expand Down Expand Up @@ -3175,4 +3176,27 @@ TEST_F(TransferTest, LoopWithStructReferenceAssignmentConverges) {
});
}

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

0 comments on commit 33b598a

Please sign in to comment.