diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h index 62e7af7ac219b..7f4a8f0341c9b 100644 --- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h +++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h @@ -436,6 +436,11 @@ class Environment { return createObjectInternal(&D, D.getType(), InitExpr); } + /// Initializes the fields (including synthetic fields) of `Loc` with values, + /// unless values of the field type are not supported or we hit one of the + /// limits at which we stop producing values. + void initializeFieldsWithValues(RecordStorageLocation &Loc); + /// Assigns `Val` as the value of `Loc` in the environment. void setValue(const StorageLocation &Loc, Value &Val); diff --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp index fd7b06efcc786..e359f037ea3a7 100644 --- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp +++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp @@ -414,8 +414,15 @@ void Environment::initialize() { } } else if (MethodDecl->isImplicitObjectMemberFunction()) { QualType ThisPointeeType = MethodDecl->getFunctionObjectParameterType(); - setThisPointeeStorageLocation( - cast(createObject(ThisPointeeType))); + auto &ThisLoc = + cast(createStorageLocation(ThisPointeeType)); + setThisPointeeStorageLocation(ThisLoc); + refreshRecordValue(ThisLoc, *this); + // Initialize fields of `*this` with values, but only if we're not + // analyzing a constructor; after all, it's the constructor's job to do + // this (and we want to be able to test that). + if (!isa(MethodDecl)) + initializeFieldsWithValues(ThisLoc); } } } @@ -799,6 +806,16 @@ PointerValue &Environment::getOrCreateNullPointerValue(QualType PointeeType) { return DACtx->getOrCreateNullPointerValue(PointeeType); } +void Environment::initializeFieldsWithValues(RecordStorageLocation &Loc) { + llvm::DenseSet Visited; + int CreatedValuesCount = 0; + initializeFieldsWithValues(Loc, Visited, 0, CreatedValuesCount); + if (CreatedValuesCount > MaxCompositeValueSize) { + llvm::errs() << "Attempting to initialize a huge value of type: " + << Loc.getType() << '\n'; + } +} + void Environment::setValue(const StorageLocation &Loc, Value &Val) { assert(!isa(&Val) || &cast(&Val)->getLoc() == &Loc); diff --git a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp index 4c88c46142d64..4bc6b19f2553f 100644 --- a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp +++ b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp @@ -388,7 +388,6 @@ builtinTransferInitializer(const CFGInitializer &Elt, } } assert(Member != nullptr); - assert(MemberLoc != nullptr); // FIXME: Instead of these case distinctions, we would ideally want to be able // to simply use `Environment::createObject()` here, the same way that we do @@ -404,6 +403,7 @@ builtinTransferInitializer(const CFGInitializer &Elt, ParentLoc->setChild(*Member, InitExprLoc); } else if (auto *InitExprVal = Env.getValue(*InitExpr)) { + assert(MemberLoc != nullptr); if (Member->getType()->isRecordType()) { auto *InitValStruct = cast(InitExprVal); // FIXME: Rather than performing a copy here, we should really be diff --git a/clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp b/clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp index 09f5524e152c9..5c4d42c6ccdcf 100644 --- a/clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp @@ -179,7 +179,10 @@ llvm::Error test::checkDataflowWithNoopAnalysis( // -fnodelayed-template-parsing is the default everywhere but on Windows. // Set it explicitly so that tests behave the same on Windows as on other // platforms. - "-fsyntax-only", "-fno-delayed-template-parsing", + // Set -Wno-unused-value because it's often desirable in tests to write + // expressions with unused value, and we don't want the output to be + // cluttered with warnings about them. + "-fsyntax-only", "-fno-delayed-template-parsing", "-Wno-unused-value", "-std=" + std::string(LangStandard::getLangStandardForKind(Std).getName())}; AnalysisInputs AI( diff --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp index f534ccb125470..9fde4179db1c4 100644 --- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp +++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp @@ -1476,6 +1476,69 @@ TEST(TransferTest, BaseClassInitializer) { llvm::Succeeded()); } +TEST(TransferTest, FieldsDontHaveValuesInConstructor) { + // In a constructor, unlike in regular member functions, we don't want fields + // to be pre-initialized with values, because doing so is the job of the + // constructor. + std::string Code = R"( + struct target { + target() { + 0; + // [[p]] + // Mention the field so it is modeled; + Val; + } + + int Val; + }; + )"; + runDataflow( + Code, + [](const llvm::StringMap> &Results, + ASTContext &ASTCtx) { + const Environment &Env = getEnvironmentAtAnnotation(Results, "p"); + EXPECT_EQ(getFieldValue(Env.getThisPointeeStorageLocation(), "Val", + ASTCtx, Env), + nullptr); + }); +} + +TEST(TransferTest, FieldsDontHaveValuesInConstructorWithBaseClass) { + // See above, but for a class with a base class. + std::string Code = R"( + struct Base { + int BaseVal; + }; + + struct target : public Base { + target() { + 0; + // [[p]] + // Mention the fields so they are modeled. + BaseVal; + Val; + } + + int Val; + }; + )"; + runDataflow( + Code, + [](const llvm::StringMap> &Results, + ASTContext &ASTCtx) { + const Environment &Env = getEnvironmentAtAnnotation(Results, "p"); + // FIXME: The field of the base class should already have been + // initialized with a value by the base constructor. This test documents + // the current buggy behavior. + EXPECT_EQ(getFieldValue(Env.getThisPointeeStorageLocation(), "BaseVal", + ASTCtx, Env), + nullptr); + EXPECT_EQ(getFieldValue(Env.getThisPointeeStorageLocation(), "Val", + ASTCtx, Env), + nullptr); + }); +} + TEST(TransferTest, StructModeledFieldsWithAccessor) { std::string Code = R"( class S {