Author: Martin Braenne Date: 2023-06-29T07:12:55Z New Revision: 1d7f9ce61f6e689ca63df2e36808885c873cf80b
URL: https://github.com/llvm/llvm-project/commit/1d7f9ce61f6e689ca63df2e36808885c873cf80b DIFF: https://github.com/llvm/llvm-project/commit/1d7f9ce61f6e689ca63df2e36808885c873cf80b.diff LOG: [clang][dataflow] Don't crash when creating pointers to members. The newly added tests crash without the other changes in this patch. Reviewed By: sammccall, xazax.hun, gribozavr2 Differential Revision: https://reviews.llvm.org/D153960 Added: Modified: clang/lib/Analysis/FlowSensitive/Transfer.cpp clang/unittests/Analysis/FlowSensitive/TransferTest.cpp Removed: ################################################################################ diff --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp b/clang/lib/Analysis/FlowSensitive/Transfer.cpp index 651930f0dd22b..5ad176dc1cdbe 100644 --- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp +++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp @@ -231,6 +231,15 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> { void VisitDeclRefExpr(const DeclRefExpr *S) { const ValueDecl *VD = S->getDecl(); assert(VD != nullptr); + + // `DeclRefExpr`s to fields and non-static methods aren't glvalues, and + // there's also no sensible `Value` we can assign to them, so skip them. + if (isa<FieldDecl>(VD)) + return; + if (auto *Method = dyn_cast<CXXMethodDecl>(VD); + Method && !Method->isStatic()) + return; + auto *DeclLoc = Env.getStorageLocation(*VD); if (DeclLoc == nullptr) return; @@ -397,8 +406,7 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> { propagateValueOrStorageLocation(*SubExpr, *S, Env); break; } - case CK_NullToPointer: - case CK_NullToMemberPointer: { + case CK_NullToPointer: { auto &Loc = Env.createStorageLocation(S->getType()); Env.setStorageLocation(*S, Loc); @@ -407,6 +415,10 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> { Env.setValue(Loc, NullPointerVal); break; } + case CK_NullToMemberPointer: + // FIXME: Implement pointers to members. For now, don't associate a value + // with this expression. + break; case CK_FunctionToPointerDecay: case CK_BuiltinFnToFnPtr: { StorageLocation *PointeeLoc = @@ -440,14 +452,12 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> { break; } case UO_AddrOf: { - // Do not form a pointer to a reference. If `SubExpr` is assigned a - // `ReferenceValue` then form a value that points to the location of its - // pointee. - StorageLocation *PointeeLoc = Env.getStorageLocationStrict(*SubExpr); - if (PointeeLoc == nullptr) + // FIXME: Model pointers to members. + if (S->getType()->isMemberPointerType()) break; - Env.setValueStrict(*S, Env.create<PointerValue>(*PointeeLoc)); + if (StorageLocation *PointeeLoc = Env.getStorageLocationStrict(*SubExpr)) + Env.setValueStrict(*S, Env.create<PointerValue>(*PointeeLoc)); break; } case UO_LNot: { diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp index 5d2a82b581f3b..210b85f7ae8d6 100644 --- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp @@ -2530,10 +2530,56 @@ TEST(TransferTest, NullToPointerCast) { }); } +TEST(TransferTest, PointerToMemberVariable) { + std::string Code = R"( + struct S { + int i; + }; + void target() { + int S::*MemberPointer = &S::i; + // [[p]] + } + )"; + runDataflow( + Code, + [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results, + ASTContext &ASTCtx) { + const Environment &Env = getEnvironmentAtAnnotation(Results, "p"); + + const ValueDecl *MemberPointerDecl = + findValueDecl(ASTCtx, "MemberPointer"); + ASSERT_THAT(MemberPointerDecl, NotNull()); + ASSERT_THAT(Env.getValue(*MemberPointerDecl), IsNull()); + }); +} + +TEST(TransferTest, PointerToMemberFunction) { + std::string Code = R"( + struct S { + void Method(); + }; + void target() { + void (S::*MemberPointer)() = &S::Method; + // [[p]] + } + )"; + runDataflow( + Code, + [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results, + ASTContext &ASTCtx) { + const Environment &Env = getEnvironmentAtAnnotation(Results, "p"); + + const ValueDecl *MemberPointerDecl = + findValueDecl(ASTCtx, "MemberPointer"); + ASSERT_THAT(MemberPointerDecl, NotNull()); + ASSERT_THAT(Env.getValue(*MemberPointerDecl), IsNull()); + }); +} + TEST(TransferTest, NullToMemberPointerCast) { std::string Code = R"( struct Foo {}; - void target(Foo *Foo) { + void target() { int Foo::*MemberPointer = nullptr; // [[p]] } @@ -2548,12 +2594,7 @@ TEST(TransferTest, NullToMemberPointerCast) { const ValueDecl *MemberPointerDecl = findValueDecl(ASTCtx, "MemberPointer"); ASSERT_THAT(MemberPointerDecl, NotNull()); - - const auto *MemberPointerVal = - cast<PointerValue>(Env.getValue(*MemberPointerDecl)); - - const StorageLocation &MemberLoc = MemberPointerVal->getPointeeLoc(); - EXPECT_THAT(Env.getValue(MemberLoc), IsNull()); + ASSERT_THAT(Env.getValue(*MemberPointerDecl), IsNull()); }); } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits