mboehme updated this revision to Diff 531734.
mboehme added a comment.
Avoid including llvm/Testing/Support/Error.h in
clang/unittests/Analysis/FlowSensitive/TestingSupport.h.
This can lead to version conflicts in projects that want to include
TestingSupport.h but use a different version of gtest than the one vendored with
LLVM.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D153006/new/
https://reviews.llvm.org/D153006
Files:
clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
clang/include/clang/Analysis/FlowSensitive/RecordOps.h
clang/include/clang/Analysis/FlowSensitive/StorageLocation.h
clang/include/clang/Analysis/FlowSensitive/Value.h
clang/lib/Analysis/FlowSensitive/CMakeLists.txt
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
clang/lib/Analysis/FlowSensitive/RecordOps.cpp
clang/lib/Analysis/FlowSensitive/Transfer.cpp
clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
clang/unittests/Analysis/FlowSensitive/TestingSupport.h
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
@@ -12,7 +12,7 @@
#include "clang/ASTMatchers/ASTMatchers.h"
#include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
-#include "clang/Analysis/FlowSensitive/NoopAnalysis.h"
+#include "clang/Analysis/FlowSensitive/RecordOps.h"
#include "clang/Analysis/FlowSensitive/StorageLocation.h"
#include "clang/Analysis/FlowSensitive/Value.h"
#include "clang/Basic/LangStandard.h"
@@ -38,59 +38,22 @@
using ::testing::NotNull;
using ::testing::UnorderedElementsAre;
-using BuiltinOptions = DataflowAnalysisContext::Options;
-
-template <typename Matcher>
-llvm::Error
-runDataflowReturnError(llvm::StringRef Code, Matcher Match,
- DataflowAnalysisOptions Options,
- LangStandard::Kind Std = LangStandard::lang_cxx17,
- llvm::StringRef TargetFun = "target") {
- using ast_matchers::hasName;
- llvm::SmallVector<std::string, 3> ASTBuildArgs = {
- // -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",
- "-std=" +
- std::string(LangStandard::getLangStandardForKind(Std).getName())};
- AnalysisInputs<NoopAnalysis> AI(
- Code, hasName(TargetFun),
- [UseBuiltinModel = Options.BuiltinOpts.has_value()](ASTContext &C,
- Environment &Env) {
- return NoopAnalysis(
- C,
- DataflowAnalysisOptions{
- UseBuiltinModel ? Env.getDataflowAnalysisContext().getOptions()
- : std::optional<BuiltinOptions>()});
- });
- AI.ASTBuildArgs = ASTBuildArgs;
- if (Options.BuiltinOpts)
- AI.BuiltinOptions = *Options.BuiltinOpts;
- return checkDataflow<NoopAnalysis>(
- std::move(AI),
- /*VerifyResults=*/
- [&Match](
- const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
- const AnalysisOutputs &AO) { Match(Results, AO.ASTCtx); });
-}
-
-template <typename Matcher>
-void runDataflow(llvm::StringRef Code, Matcher Match,
+template <typename VerifyResultsT>
+void runDataflow(llvm::StringRef Code, VerifyResultsT VerifyResults,
DataflowAnalysisOptions Options,
LangStandard::Kind Std = LangStandard::lang_cxx17,
llvm::StringRef TargetFun = "target") {
ASSERT_THAT_ERROR(
- runDataflowReturnError(Code, Match, Options, Std, TargetFun),
+ runDataflowReturnError(Code, VerifyResults, Options, Std, TargetFun),
llvm::Succeeded());
}
-template <typename Matcher>
-void runDataflow(llvm::StringRef Code, Matcher Match,
+template <typename VerifyResultsT>
+void runDataflow(llvm::StringRef Code, VerifyResultsT VerifyResults,
LangStandard::Kind Std = LangStandard::lang_cxx17,
bool ApplyBuiltinTransfer = true,
llvm::StringRef TargetFun = "target") {
- runDataflow(Code, std::move(Match),
+ runDataflow(Code, std::move(VerifyResults),
{ApplyBuiltinTransfer ? BuiltinOptions{}
: std::optional<BuiltinOptions>()},
Std, TargetFun);
@@ -1987,22 +1950,20 @@
};
void target() {
- A Foo;
- A Bar;
- (void)Foo.Baz;
+ A Foo = { 1 };
+ A Bar = { 2 };
// [[p1]]
Foo = Bar;
// [[p2]]
+ int val = 3;
+ Foo.Baz = val;
+ // [[p3]]
}
)";
runDataflow(
Code,
[](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
ASTContext &ASTCtx) {
- ASSERT_THAT(Results.keys(), UnorderedElementsAre("p1", "p2"));
- const Environment &Env1 = getEnvironmentAtAnnotation(Results, "p1");
- const Environment &Env2 = getEnvironmentAtAnnotation(Results, "p2");
-
const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
ASSERT_THAT(FooDecl, NotNull());
@@ -2012,35 +1973,61 @@
const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
ASSERT_THAT(BazDecl, NotNull());
- const auto *FooLoc1 =
- cast<AggregateStorageLocation>(Env1.getStorageLocation(*FooDecl));
- const auto *BarLoc1 =
- cast<AggregateStorageLocation>(Env1.getStorageLocation(*BarDecl));
+ // Before copy assignment.
+ {
+ const Environment &Env1 = getEnvironmentAtAnnotation(Results, "p1");
+
+ const auto *FooLoc1 =
+ cast<AggregateStorageLocation>(Env1.getStorageLocation(*FooDecl));
+ const auto *BarLoc1 =
+ cast<AggregateStorageLocation>(Env1.getStorageLocation(*BarDecl));
+ EXPECT_FALSE(recordsEqual(*FooLoc1, *BarLoc1, Env1));
+
+ const auto *FooBazVal1 =
+ cast<IntegerValue>(Env1.getValue(FooLoc1->getChild(*BazDecl)));
+ const auto *BarBazVal1 =
+ cast<IntegerValue>(Env1.getValue(BarLoc1->getChild(*BazDecl)));
+ EXPECT_NE(FooBazVal1, BarBazVal1);
+ }
- const auto *FooVal1 = cast<StructValue>(Env1.getValue(*FooLoc1));
- const auto *BarVal1 = cast<StructValue>(Env1.getValue(*BarLoc1));
- EXPECT_NE(FooVal1, BarVal1);
+ // After copy assignment.
+ {
+ const Environment &Env2 = getEnvironmentAtAnnotation(Results, "p2");
- const auto *FooBazVal1 =
- cast<IntegerValue>(Env1.getValue(FooLoc1->getChild(*BazDecl)));
- const auto *BarBazVal1 =
- cast<IntegerValue>(Env1.getValue(BarLoc1->getChild(*BazDecl)));
- EXPECT_NE(FooBazVal1, BarBazVal1);
+ const auto *FooLoc2 =
+ cast<AggregateStorageLocation>(Env2.getStorageLocation(*FooDecl));
+ const auto *BarLoc2 =
+ cast<AggregateStorageLocation>(Env2.getStorageLocation(*BarDecl));
- const auto *FooLoc2 =
- cast<AggregateStorageLocation>(Env2.getStorageLocation(*FooDecl));
- const auto *BarLoc2 =
- cast<AggregateStorageLocation>(Env2.getStorageLocation(*BarDecl));
+ const auto *FooVal2 = cast<StructValue>(Env2.getValue(*FooLoc2));
+ const auto *BarVal2 = cast<StructValue>(Env2.getValue(*BarLoc2));
+ EXPECT_NE(FooVal2, BarVal2);
- const auto *FooVal2 = cast<StructValue>(Env2.getValue(*FooLoc2));
- const auto *BarVal2 = cast<StructValue>(Env2.getValue(*BarLoc2));
- EXPECT_EQ(FooVal2, BarVal2);
+ EXPECT_TRUE(recordsEqual(*FooLoc2, *BarLoc2, Env2));
- const auto *FooBazVal2 =
- cast<IntegerValue>(Env2.getValue(FooLoc1->getChild(*BazDecl)));
- const auto *BarBazVal2 =
- cast<IntegerValue>(Env2.getValue(BarLoc1->getChild(*BazDecl)));
- EXPECT_EQ(FooBazVal2, BarBazVal2);
+ const auto *FooBazVal2 =
+ cast<IntegerValue>(Env2.getValue(FooLoc2->getChild(*BazDecl)));
+ const auto *BarBazVal2 =
+ cast<IntegerValue>(Env2.getValue(BarLoc2->getChild(*BazDecl)));
+ EXPECT_EQ(FooBazVal2, BarBazVal2);
+ }
+
+ // After value update.
+ {
+ const Environment &Env3 = getEnvironmentAtAnnotation(Results, "p3");
+
+ const auto *FooLoc3 =
+ cast<AggregateStorageLocation>(Env3.getStorageLocation(*FooDecl));
+ const auto *BarLoc3 =
+ cast<AggregateStorageLocation>(Env3.getStorageLocation(*BarDecl));
+ EXPECT_FALSE(recordsEqual(*FooLoc3, *BarLoc3, Env3));
+
+ const auto *FooBazVal3 =
+ cast<IntegerValue>(Env3.getValue(FooLoc3->getChild(*BazDecl)));
+ const auto *BarBazVal3 =
+ cast<IntegerValue>(Env3.getValue(BarLoc3->getChild(*BazDecl)));
+ EXPECT_NE(FooBazVal3, BarBazVal3);
+ }
});
}
@@ -2051,19 +2038,20 @@
};
void target() {
- A Foo;
- (void)Foo.Baz;
+ A Foo = { 1 };
A Bar = Foo;
- // [[p]]
+ // [[after_copy]]
+ // FIXME: Integer literals don't yet produce `Value`s, so we need to use
+ // a variable to assign from here.
+ int val = 2;
+ Foo.Baz = val;
+ // [[after_update]]
}
)";
runDataflow(
Code,
[](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
ASTContext &ASTCtx) {
- ASSERT_THAT(Results.keys(), UnorderedElementsAre("p"));
- const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
-
const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
ASSERT_THAT(FooDecl, NotNull());
@@ -2073,20 +2061,50 @@
const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
ASSERT_THAT(BazDecl, NotNull());
- const auto *FooLoc =
- cast<AggregateStorageLocation>(Env.getStorageLocation(*FooDecl));
- const auto *BarLoc =
- cast<AggregateStorageLocation>(Env.getStorageLocation(*BarDecl));
+ // after_copy
+ {
+ const Environment &Env =
+ getEnvironmentAtAnnotation(Results, "after_copy");
+
+ const auto *FooLoc =
+ cast<AggregateStorageLocation>(Env.getStorageLocation(*FooDecl));
+ const auto *BarLoc =
+ cast<AggregateStorageLocation>(Env.getStorageLocation(*BarDecl));
+
+ // `Foo` and `Bar` have different `StructValue`s associated with them.
+ const auto *FooVal = cast<StructValue>(Env.getValue(*FooLoc));
+ const auto *BarVal = cast<StructValue>(Env.getValue(*BarLoc));
+ EXPECT_NE(FooVal, BarVal);
+
+ // But the records compare equal.
+ EXPECT_TRUE(recordsEqual(*FooLoc, *BarLoc, Env));
+
+ // In particular, the value of `Baz` in both records is the same.
+ const auto *FooBazVal =
+ cast<IntegerValue>(Env.getValue(FooLoc->getChild(*BazDecl)));
+ const auto *BarBazVal =
+ cast<IntegerValue>(Env.getValue(BarLoc->getChild(*BazDecl)));
+ EXPECT_EQ(FooBazVal, BarBazVal);
+ }
- const auto *FooVal = cast<StructValue>(Env.getValue(*FooLoc));
- const auto *BarVal = cast<StructValue>(Env.getValue(*BarLoc));
- EXPECT_EQ(FooVal, BarVal);
+ // after_update
+ {
+ const Environment &Env =
+ getEnvironmentAtAnnotation(Results, "after_update");
- const auto *FooBazVal =
- cast<IntegerValue>(Env.getValue(FooLoc->getChild(*BazDecl)));
- const auto *BarBazVal =
- cast<IntegerValue>(Env.getValue(BarLoc->getChild(*BazDecl)));
- EXPECT_EQ(FooBazVal, BarBazVal);
+ const auto *FooLoc =
+ cast<AggregateStorageLocation>(Env.getStorageLocation(*FooDecl));
+ const auto *BarLoc =
+ cast<AggregateStorageLocation>(Env.getStorageLocation(*BarDecl));
+
+ EXPECT_FALSE(recordsEqual(*FooLoc, *BarLoc, Env));
+
+ const auto *FooBazVal =
+ cast<IntegerValue>(Env.getValue(FooLoc->getChild(*BazDecl)));
+ const auto *BarBazVal =
+ cast<IntegerValue>(Env.getValue(BarLoc->getChild(*BazDecl)));
+ EXPECT_NE(FooBazVal, BarBazVal);
+ }
});
}
@@ -2125,10 +2143,7 @@
cast<AggregateStorageLocation>(Env.getStorageLocation(*FooDecl));
const auto *BarLoc =
cast<AggregateStorageLocation>(Env.getStorageLocation(*BarDecl));
-
- const auto *FooVal = cast<StructValue>(Env.getValue(*FooLoc));
- const auto *BarVal = cast<StructValue>(Env.getValue(*BarLoc));
- EXPECT_EQ(FooVal, BarVal);
+ EXPECT_TRUE(recordsEqual(*FooLoc, *BarLoc, Env));
const auto *FooBazVal =
cast<IntegerValue>(Env.getValue(FooLoc->getChild(*BazDecl)));
@@ -2171,10 +2186,7 @@
cast<AggregateStorageLocation>(Env.getStorageLocation(*FooDecl));
const auto *BarLoc =
cast<AggregateStorageLocation>(Env.getStorageLocation(*BarDecl));
-
- const auto *FooVal = cast<StructValue>(Env.getValue(*FooLoc));
- const auto *BarVal = cast<StructValue>(Env.getValue(*BarLoc));
- EXPECT_EQ(FooVal, BarVal);
+ EXPECT_TRUE(recordsEqual(*FooLoc, *BarLoc, Env));
const auto *FooBazVal =
cast<IntegerValue>(Env.getValue(FooLoc->getChild(*BazDecl)));
@@ -2239,6 +2251,8 @@
const auto *BarVal1 = cast<StructValue>(Env1.getValue(*BarLoc1));
EXPECT_NE(FooVal1, BarVal1);
+ EXPECT_FALSE(recordsEqual(*FooLoc1, *BarLoc1, Env1));
+
const auto *FooBazVal1 =
cast<IntegerValue>(Env1.getValue(FooLoc1->getChild(*BazDecl)));
const auto *BarBazVal1 =
@@ -2248,7 +2262,8 @@
const auto *FooLoc2 =
cast<AggregateStorageLocation>(Env2.getStorageLocation(*FooDecl));
const auto *FooVal2 = cast<StructValue>(Env2.getValue(*FooLoc2));
- EXPECT_EQ(FooVal2, BarVal1);
+ EXPECT_NE(FooVal2, BarVal1);
+ EXPECT_TRUE(recordsEqual(*FooLoc2, Env2, *BarLoc1, Env1));
const auto *FooBazVal2 =
cast<IntegerValue>(Env2.getValue(FooLoc1->getChild(*BazDecl)));
Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===================================================================
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -33,6 +33,7 @@
#include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
#include "clang/Analysis/FlowSensitive/MatchSwitch.h"
+#include "clang/Analysis/FlowSensitive/NoopAnalysis.h"
#include "clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h"
#include "clang/Basic/LLVM.h"
#include "clang/Serialization/PCHContainerOperations.h"
@@ -381,6 +382,45 @@
});
}
+using BuiltinOptions = DataflowAnalysisContext::Options;
+
+/// Runs dataflow on `Code` with a `NoopAnalysis` and calls `VerifyResults` to
+/// verify the results.
+template <typename VerifyResultsT>
+llvm::Error
+runDataflowReturnError(llvm::StringRef Code, VerifyResultsT VerifyResults,
+ DataflowAnalysisOptions Options,
+ LangStandard::Kind Std = LangStandard::lang_cxx17,
+ llvm::StringRef TargetFun = "target") {
+ using ast_matchers::hasName;
+ llvm::SmallVector<std::string, 3> ASTBuildArgs = {
+ // -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",
+ "-std=" +
+ std::string(LangStandard::getLangStandardForKind(Std).getName())};
+ AnalysisInputs<NoopAnalysis> AI(
+ Code, hasName(TargetFun),
+ [UseBuiltinModel = Options.BuiltinOpts.has_value()](ASTContext &C,
+ Environment &Env) {
+ return NoopAnalysis(
+ C,
+ DataflowAnalysisOptions{
+ UseBuiltinModel ? Env.getDataflowAnalysisContext().getOptions()
+ : std::optional<BuiltinOptions>()});
+ });
+ AI.ASTBuildArgs = ASTBuildArgs;
+ if (Options.BuiltinOpts)
+ AI.BuiltinOptions = *Options.BuiltinOpts;
+ return checkDataflow<NoopAnalysis>(
+ std::move(AI),
+ /*VerifyResults=*/
+ [&VerifyResults](
+ const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+ const AnalysisOutputs &AO) { VerifyResults(Results, AO.ASTCtx); });
+}
+
/// Returns the `ValueDecl` for the given identifier.
///
/// Requirements:
@@ -388,6 +428,21 @@
/// `Name` must be unique in `ASTCtx`.
const ValueDecl *findValueDecl(ASTContext &ASTCtx, llvm::StringRef Name);
+/// Returns the storage location (of type `LocT`) for the given identifier.
+/// `LocT` must be a subclass of `StorageLocation` and must be of the
+/// appropriate type.
+///
+/// Requirements:
+///
+/// `Name` must be unique in `ASTCtx`.
+template <class LocT>
+LocT &getLocForDecl(ASTContext &ASTCtx, const Environment &Env,
+ llvm::StringRef Name) {
+ const ValueDecl *VD = findValueDecl(ASTCtx, Name);
+ assert(VD != nullptr);
+ return *cast<LocT>(Env.getStorageLocation(*VD));
+}
+
/// Returns the value (of type `ValueT`) for the given identifier.
/// `ValueT` must be a subclass of `Value` and must be of the appropriate type.
///
Index: clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
===================================================================
--- /dev/null
+++ clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
@@ -0,0 +1,205 @@
+//===- unittests/Analysis/FlowSensitive/RecordOpsTest.cpp -----------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/Analysis/FlowSensitive/RecordOps.h"
+#include "TestingSupport.h"
+#include "llvm/Testing/Support/Error.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace dataflow {
+namespace test {
+namespace {
+
+template <typename VerifyResultsT>
+void runDataflow(llvm::StringRef Code, VerifyResultsT VerifyResults,
+ LangStandard::Kind Std = LangStandard::lang_cxx17,
+ llvm::StringRef TargetFun = "target") {
+ ASSERT_THAT_ERROR(
+ runDataflowReturnError(Code, VerifyResults,
+ DataflowAnalysisOptions{BuiltinOptions{}}, Std,
+ TargetFun),
+ llvm::Succeeded());
+}
+
+TEST(RecordOpsTest, CopyRecord) {
+ std::string Code = R"(
+ struct S {
+ int outer_int;
+ int &ref;
+ struct {
+ int inner_int;
+ } inner;
+ };
+ void target(S s1, S s2) {
+ (void)s1.outer_int;
+ (void)s1.ref;
+ (void)s1.inner.inner_int;
+ // [[p]]
+ }
+ )";
+ runDataflow(
+ Code,
+ [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+ ASTContext &ASTCtx) {
+ Environment Env = getEnvironmentAtAnnotation(Results, "p");
+
+ const ValueDecl *OuterIntDecl = findValueDecl(ASTCtx, "outer_int");
+ const ValueDecl *RefDecl = findValueDecl(ASTCtx, "ref");
+ const ValueDecl *InnerDecl = findValueDecl(ASTCtx, "inner");
+ const ValueDecl *InnerIntDecl = findValueDecl(ASTCtx, "inner_int");
+
+ auto &S1 = getLocForDecl<AggregateStorageLocation>(ASTCtx, Env, "s1");
+ auto &S2 = getLocForDecl<AggregateStorageLocation>(ASTCtx, Env, "s2");
+ auto &Inner1 = cast<AggregateStorageLocation>(S1.getChild(*InnerDecl));
+ auto &Inner2 = cast<AggregateStorageLocation>(S2.getChild(*InnerDecl));
+
+ EXPECT_NE(Env.getValue(S1.getChild(*OuterIntDecl)),
+ Env.getValue(S2.getChild(*OuterIntDecl)));
+ EXPECT_NE(Env.getValue(S1.getChild(*RefDecl)),
+ Env.getValue(S2.getChild(*RefDecl)));
+ EXPECT_NE(Env.getValue(Inner1.getChild(*InnerIntDecl)),
+ Env.getValue(Inner2.getChild(*InnerIntDecl)));
+
+ auto *S1Val = cast<StructValue>(Env.getValue(S1));
+ auto *S2Val = cast<StructValue>(Env.getValue(S2));
+ EXPECT_NE(S1Val, S2Val);
+
+ S1Val->setProperty("prop", Env.getBoolLiteralValue(true));
+
+ copyRecord(S1, S2, Env);
+
+ EXPECT_EQ(Env.getValue(S1.getChild(*OuterIntDecl)),
+ Env.getValue(S2.getChild(*OuterIntDecl)));
+ EXPECT_EQ(Env.getValue(S1.getChild(*RefDecl)),
+ Env.getValue(S2.getChild(*RefDecl)));
+ EXPECT_EQ(Env.getValue(Inner1.getChild(*InnerIntDecl)),
+ Env.getValue(Inner2.getChild(*InnerIntDecl)));
+
+ S1Val = cast<StructValue>(Env.getValue(S1));
+ S2Val = cast<StructValue>(Env.getValue(S2));
+ EXPECT_NE(S1Val, S2Val);
+
+ EXPECT_EQ(S2Val->getProperty("prop"), &Env.getBoolLiteralValue(true));
+ });
+}
+
+TEST(RecordOpsTest, RecordsEqual) {
+ std::string Code = R"(
+ struct S {
+ int outer_int;
+ int &ref;
+ struct {
+ int inner_int;
+ } inner;
+ };
+ void target(S s1, S s2) {
+ (void)s1.outer_int;
+ (void)s1.ref;
+ (void)s1.inner.inner_int;
+ // [[p]]
+ }
+ )";
+ runDataflow(
+ Code,
+ [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+ ASTContext &ASTCtx) {
+ Environment Env = getEnvironmentAtAnnotation(Results, "p");
+
+ const ValueDecl *OuterIntDecl = findValueDecl(ASTCtx, "outer_int");
+ const ValueDecl *RefDecl = findValueDecl(ASTCtx, "ref");
+ const ValueDecl *InnerDecl = findValueDecl(ASTCtx, "inner");
+ const ValueDecl *InnerIntDecl = findValueDecl(ASTCtx, "inner_int");
+
+ auto &S1 = getLocForDecl<AggregateStorageLocation>(ASTCtx, Env, "s1");
+ auto &S2 = getLocForDecl<AggregateStorageLocation>(ASTCtx, Env, "s2");
+ auto &Inner2 = cast<AggregateStorageLocation>(S2.getChild(*InnerDecl));
+
+ cast<StructValue>(Env.getValue(S1))
+ ->setProperty("prop", Env.getBoolLiteralValue(true));
+
+ // Strategy: Create two equal records, then verify each of the various
+ // ways in which records can differ causes recordsEqual to return false.
+ // changes we can make to the record.
+
+ // This test reuses the same objects for multiple checks, which isn't
+ // great, but seems better than duplicating the setup code for every
+ // check.
+
+ copyRecord(S1, S2, Env);
+ EXPECT_TRUE(recordsEqual(S1, S2, Env));
+
+ // S2 has a different outer_int.
+ Env.setValue(S2.getChild(*OuterIntDecl), Env.create<IntegerValue>());
+ EXPECT_FALSE(recordsEqual(S1, S2, Env));
+ copyRecord(S1, S2, Env);
+ EXPECT_TRUE(recordsEqual(S1, S2, Env));
+
+ // S2 doesn't have outer_int at all.
+ Env.clearValue(S2.getChild(*OuterIntDecl));
+ EXPECT_FALSE(recordsEqual(S1, S2, Env));
+ copyRecord(S1, S2, Env);
+ EXPECT_TRUE(recordsEqual(S1, S2, Env));
+
+ // S2 has a different ref.
+ Env.setValue(S2.getChild(*RefDecl),
+ Env.create<ReferenceValue>(Env.createStorageLocation(
+ RefDecl->getType().getNonReferenceType())));
+ EXPECT_FALSE(recordsEqual(S1, S2, Env));
+ copyRecord(S1, S2, Env);
+ EXPECT_TRUE(recordsEqual(S1, S2, Env));
+
+ // S2 doesn't have ref at all.
+ Env.clearValue(S2.getChild(*RefDecl));
+ EXPECT_FALSE(recordsEqual(S1, S2, Env));
+ copyRecord(S1, S2, Env);
+ EXPECT_TRUE(recordsEqual(S1, S2, Env));
+
+ // S2 as a different inner_int.
+ Env.setValue(Inner2.getChild(*InnerIntDecl),
+ Env.create<IntegerValue>());
+ EXPECT_FALSE(recordsEqual(S1, S2, Env));
+ copyRecord(S1, S2, Env);
+ EXPECT_TRUE(recordsEqual(S1, S2, Env));
+
+ // S1 and S2 have the same property with different values.
+ cast<StructValue>(Env.getValue(S2))
+ ->setProperty("prop", Env.getBoolLiteralValue(false));
+ EXPECT_FALSE(recordsEqual(S1, S2, Env));
+ copyRecord(S1, S2, Env);
+ EXPECT_TRUE(recordsEqual(S1, S2, Env));
+
+ // S1 has a property that S2 doesn't have.
+ cast<StructValue>(Env.getValue(S1))
+ ->setProperty("other_prop", Env.getBoolLiteralValue(false));
+ EXPECT_FALSE(recordsEqual(S1, S2, Env));
+ // We modified S1 this time, so need to copy back the other way.
+ copyRecord(S2, S1, Env);
+ EXPECT_TRUE(recordsEqual(S1, S2, Env));
+
+ // S2 has a property that S1 doesn't have.
+ cast<StructValue>(Env.getValue(S2))
+ ->setProperty("other_prop", Env.getBoolLiteralValue(false));
+ EXPECT_FALSE(recordsEqual(S1, S2, Env));
+ copyRecord(S1, S2, Env);
+ EXPECT_TRUE(recordsEqual(S1, S2, Env));
+
+ // S1 and S2 have the same number of properties, but with different
+ // names.
+ cast<StructValue>(Env.getValue(S1))
+ ->setProperty("prop1", Env.getBoolLiteralValue(false));
+ cast<StructValue>(Env.getValue(S2))
+ ->setProperty("prop2", Env.getBoolLiteralValue(false));
+ EXPECT_FALSE(recordsEqual(S1, S2, Env));
+ });
+}
+
+} // namespace
+} // namespace test
+} // namespace dataflow
+} // namespace clang
Index: clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
===================================================================
--- clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
+++ clang/unittests/Analysis/FlowSensitive/CMakeLists.txt
@@ -14,6 +14,7 @@
MapLatticeTest.cpp
MatchSwitchTest.cpp
MultiVarConstantPropagationTest.cpp
+ RecordOpsTest.cpp
SignAnalysisTest.cpp
SingleVarConstantPropagationTest.cpp
SolverTest.cpp
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -23,6 +23,7 @@
#include "clang/Analysis/FlowSensitive/ControlFlowContext.h"
#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
#include "clang/Analysis/FlowSensitive/NoopAnalysis.h"
+#include "clang/Analysis/FlowSensitive/RecordOps.h"
#include "clang/Analysis/FlowSensitive/Value.h"
#include "clang/Basic/Builtins.h"
#include "clang/Basic/OperatorKinds.h"
@@ -590,16 +591,21 @@
const Expr *Arg = S->getArg(0);
assert(Arg != nullptr);
- if (S->isElidable()) {
- auto *ArgLoc = Env.getStorageLocation(*Arg, SkipPast::Reference);
- if (ArgLoc == nullptr)
- return;
+ auto *ArgLoc = cast<AggregateStorageLocation>(
+ Env.getStorageLocation(*Arg, SkipPast::Reference));
+ if (ArgLoc == nullptr)
+ return;
+ if (S->isElidable()) {
Env.setStorageLocation(*S, *ArgLoc);
- } else if (auto *ArgVal = Env.getValue(*Arg, SkipPast::Reference)) {
- auto &Loc = Env.createStorageLocation(*S);
+ } else {
+ auto &Loc =
+ cast<AggregateStorageLocation>(Env.createStorageLocation(*S));
Env.setStorageLocation(*S, Loc);
- Env.setValue(Loc, *ArgVal);
+ if (Value *Val = Env.createValue(S->getType())) {
+ Env.setValue(Loc, *Val);
+ copyRecord(*ArgLoc, Loc, Env);
+ }
}
return;
}
@@ -631,16 +637,17 @@
!Method->isMoveAssignmentOperator())
return;
- auto *ObjectLoc = Env.getStorageLocation(*Arg0, SkipPast::Reference);
+ auto *ObjectLoc = cast<AggregateStorageLocation>(
+ Env.getStorageLocation(*Arg0, SkipPast::Reference));
if (ObjectLoc == nullptr)
return;
- auto *Val = Env.getValue(*Arg1, SkipPast::Reference);
- if (Val == nullptr)
+ auto *ArgLoc = cast<AggregateStorageLocation>(
+ Env.getStorageLocation(*Arg1, SkipPast::Reference));
+ if (ArgLoc == nullptr)
return;
- // Assign a value to the storage location of the object.
- Env.setValue(*ObjectLoc, *Val);
+ copyRecord(*ArgLoc, *ObjectLoc, Env);
// FIXME: Add a test for the value of the whole expression.
// Assign a storage location for the whole expression.
Index: clang/lib/Analysis/FlowSensitive/RecordOps.cpp
===================================================================
--- /dev/null
+++ clang/lib/Analysis/FlowSensitive/RecordOps.cpp
@@ -0,0 +1,135 @@
+//===-- RecordOps.cpp -------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// Operations on records (structs, classes, and unions).
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/Analysis/FlowSensitive/RecordOps.h"
+
+#define DEBUG_TYPE "dataflow"
+
+namespace clang {
+namespace dataflow {
+
+void copyRecord(AggregateStorageLocation &Src, AggregateStorageLocation &Dst,
+ Environment &Env) {
+ QualType Type = Src.getType();
+
+ LLVM_DEBUG({
+ if (Dst.getType().getCanonicalType().getUnqualifiedType() !=
+ Type.getCanonicalType().getUnqualifiedType()) {
+ llvm::dbgs() << "Source type " << Type << "\n";
+ llvm::dbgs() << "Destination type " << Dst.getType() << "\n";
+ }
+ });
+ assert(Dst.getType().getCanonicalType().getUnqualifiedType() ==
+ Type.getCanonicalType().getUnqualifiedType());
+
+ for (auto [Field, SrcFieldLoc] : Src.children()) {
+ assert(SrcFieldLoc != nullptr);
+
+ StorageLocation &DstFieldLoc = Dst.getChild(*Field);
+
+ if (Field->getType()->isRecordType()) {
+ copyRecord(cast<AggregateStorageLocation>(*SrcFieldLoc),
+ cast<AggregateStorageLocation>(DstFieldLoc), Env);
+ } else {
+ if (Value *Val = Env.getValue(*SrcFieldLoc))
+ Env.setValue(DstFieldLoc, *Val);
+ else
+ Env.clearValue(DstFieldLoc);
+ }
+ }
+
+ StructValue *SrcVal = cast_or_null<StructValue>(Env.getValue(Src));
+ StructValue *DstVal = cast_or_null<StructValue>(Env.getValue(Dst));
+
+ if (SrcVal == nullptr || DstVal == nullptr)
+ return;
+
+ llvm::iterator_range<
+ llvm::DenseMap<const ValueDecl *, Value *>::const_iterator>
+ DstChildren = DstVal->children();
+ DstVal = &Env.create<StructValue>(llvm::DenseMap<const ValueDecl *, Value *>(
+ DstChildren.begin(), DstChildren.end()));
+ Env.setValue(Dst, *DstVal);
+
+ for (const auto &[Name, Value] : SrcVal->properties()) {
+ if (Value != nullptr)
+ DstVal->setProperty(Name, *Value);
+ }
+}
+
+bool recordsEqual(const AggregateStorageLocation &Loc1, const Environment &Env1,
+ const AggregateStorageLocation &Loc2,
+ const Environment &Env2) {
+ QualType Type = Loc1.getType();
+
+ LLVM_DEBUG({
+ if (Loc2.getType().getCanonicalType().getUnqualifiedType() !=
+ Type.getCanonicalType().getUnqualifiedType()) {
+ llvm::dbgs() << "Loc1 type " << Type << "\n";
+ llvm::dbgs() << "Loc2 type " << Loc2.getType() << "\n";
+ }
+ });
+ assert(Loc2.getType().getCanonicalType().getUnqualifiedType() ==
+ Type.getCanonicalType().getUnqualifiedType());
+
+ for (auto [Field, FieldLoc1] : Loc1.children()) {
+ assert(FieldLoc1 != nullptr);
+
+ StorageLocation &FieldLoc2 = Loc2.getChild(*Field);
+
+ if (Field->getType()->isRecordType()) {
+ if (!recordsEqual(cast<AggregateStorageLocation>(*FieldLoc1), Env1,
+ cast<AggregateStorageLocation>(FieldLoc2), Env2))
+ return false;
+ } else if (Field->getType()->isReferenceType()) {
+ auto *RefVal1 = cast_or_null<ReferenceValue>(Env1.getValue(*FieldLoc1));
+ auto *RefVal2 = cast_or_null<ReferenceValue>(Env1.getValue(FieldLoc2));
+ if (RefVal1 && RefVal2) {
+ if (&RefVal1->getReferentLoc() != &RefVal2->getReferentLoc())
+ return false;
+ } else {
+ // If either of `RefVal1` and `RefVal2` is null, we only consider them
+ // equal if they're both null.
+ if (RefVal1 || RefVal2)
+ return false;
+ }
+ } else {
+ if (Env1.getValue(*FieldLoc1) != Env2.getValue(FieldLoc2))
+ return false;
+ }
+ }
+
+ llvm::StringMap<Value *> Props1, Props2;
+
+ if (StructValue *Val1 = cast_or_null<StructValue>(Env1.getValue(Loc1)))
+ for (const auto &[Name, Value] : Val1->properties())
+ Props1[Name] = Value;
+ if (StructValue *Val2 = cast_or_null<StructValue>(Env2.getValue(Loc2)))
+ for (const auto &[Name, Value] : Val2->properties())
+ Props2[Name] = Value;
+
+ if (Props1.size() != Props2.size())
+ return false;
+
+ for (const auto &[Name, Value] : Props1) {
+ auto It = Props2.find(Name);
+ if (It == Props2.end())
+ return false;
+ if (Value != It->second)
+ return false;
+ }
+
+ return true;
+}
+
+} // namespace dataflow
+} // namespace clang
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -748,6 +748,24 @@
}
}
+void Environment::clearValue(const StorageLocation &Loc) {
+ LocToVal.erase(&Loc);
+
+ auto It = MemberLocToStruct.find(&Loc);
+ if (It != MemberLocToStruct.end()) {
+ // `Loc` is the location of a struct member so we need to also clear the
+ // member in the corresponding `StructValue`.
+
+ assert(It->second.first != nullptr);
+ StructValue &StructVal = *It->second.first;
+
+ assert(It->second.second != nullptr);
+ const ValueDecl &Member = *It->second.second;
+
+ StructVal.clearChild(Member);
+ }
+}
+
void Environment::setValueStrict(const Expr &E, Value &Val) {
assert(E.isPRValue());
assert(!isa<ReferenceValue>(Val));
Index: clang/lib/Analysis/FlowSensitive/CMakeLists.txt
===================================================================
--- clang/lib/Analysis/FlowSensitive/CMakeLists.txt
+++ clang/lib/Analysis/FlowSensitive/CMakeLists.txt
@@ -5,6 +5,7 @@
DataflowEnvironment.cpp
HTMLLogger.cpp
Logger.cpp
+ RecordOps.cpp
Transfer.cpp
TypeErasedDataflowAnalysis.cpp
Value.cpp
Index: clang/include/clang/Analysis/FlowSensitive/Value.h
===================================================================
--- clang/include/clang/Analysis/FlowSensitive/Value.h
+++ clang/include/clang/Analysis/FlowSensitive/Value.h
@@ -312,6 +312,9 @@
/// Assigns `Val` as the child value for `D`.
void setChild(const ValueDecl &D, Value &Val) { Children[&D] = &Val; }
+ /// Clears any value assigned as the child value for `D`.
+ void clearChild(const ValueDecl &D) { Children.erase(&D); }
+
llvm::iterator_range<
llvm::DenseMap<const ValueDecl *, Value *>::const_iterator>
children() const {
Index: clang/include/clang/Analysis/FlowSensitive/StorageLocation.h
===================================================================
--- clang/include/clang/Analysis/FlowSensitive/StorageLocation.h
+++ clang/include/clang/Analysis/FlowSensitive/StorageLocation.h
@@ -70,13 +70,12 @@
/// that all of the members of a given union have the same storage location.
class AggregateStorageLocation final : public StorageLocation {
public:
+ using FieldToLoc = llvm::DenseMap<const ValueDecl *, StorageLocation *>;
+
explicit AggregateStorageLocation(QualType Type)
- : AggregateStorageLocation(
- Type, llvm::DenseMap<const ValueDecl *, StorageLocation *>()) {}
+ : AggregateStorageLocation(Type, FieldToLoc()) {}
- AggregateStorageLocation(
- QualType Type,
- llvm::DenseMap<const ValueDecl *, StorageLocation *> Children)
+ AggregateStorageLocation(QualType Type, FieldToLoc Children)
: StorageLocation(Kind::Aggregate, Type), Children(std::move(Children)) {}
static bool classof(const StorageLocation *Loc) {
@@ -90,8 +89,12 @@
return *It->second;
}
+ llvm::iterator_range<FieldToLoc::const_iterator> children() const {
+ return {Children.begin(), Children.end()};
+ }
+
private:
- llvm::DenseMap<const ValueDecl *, StorageLocation *> Children;
+ FieldToLoc Children;
};
} // namespace dataflow
Index: clang/include/clang/Analysis/FlowSensitive/RecordOps.h
===================================================================
--- /dev/null
+++ clang/include/clang/Analysis/FlowSensitive/RecordOps.h
@@ -0,0 +1,71 @@
+//===-- RecordOps.h ---------------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// Operations on records (structs, classes, and unions).
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_ANALYSIS_FLOWSENSITIVE_RECORDOPS_H
+#define LLVM_CLANG_ANALYSIS_FLOWSENSITIVE_RECORDOPS_H
+
+#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
+#include "clang/Analysis/FlowSensitive/StorageLocation.h"
+
+namespace clang {
+namespace dataflow {
+
+/// Copies a record (struct, class, or union) from `Src` to `Dst`.
+///
+/// This performs a deep copy, i.e. it copies every field and recurses on
+/// fields of record type. It also copies properties from the `StructValue`
+/// associated with `Dst` to the `StructValue` associated with `Src` (if these
+/// `StructValue`s exist).
+///
+/// If there is a `StructValue` associated with `Dst` in the environment, this
+/// function creates a new `StructValue` and associates it with `Dst`; clients
+/// need to be aware of this and must not assume that the `StructValue`
+/// associated with `Dst` remains the same after the call.
+///
+/// We create a new `StructValue` rather than modifying properties on the old
+/// `StructValue` because the old `StructValue` may be shared with other
+/// `Environment`s, and we don't want changes to properties to be visible there.
+///
+/// Requirements:
+///
+/// `Src` and `Dst` must have the same canonical unqualified type.
+void copyRecord(AggregateStorageLocation &Src, AggregateStorageLocation &Dst,
+ Environment &Env);
+
+/// Returns whether the records `Loc1` and `Loc2` are equal.
+///
+/// Values for `Loc1` are retrieved from `Env1`, and values for `Loc2` are
+/// retrieved from `Env2`. A convenience overload retrieves values for `Loc1`
+/// and `Loc2` from the same environment.
+///
+/// This performs a deep comparison, i.e. it compares every field and recurses
+/// on fields of record type. Fields of reference type compare equal if they
+/// refer to the same storage location. If `StructValue`s are associated with
+/// `Loc1` and `Loc2`, it also compares the properties on those `StructValue`s.
+///
+/// Requirements:
+///
+/// `Src` and `Dst` must have the same canonical unqualified type.
+bool recordsEqual(const AggregateStorageLocation &Loc1, const Environment &Env1,
+ const AggregateStorageLocation &Loc2,
+ const Environment &Env2);
+
+inline bool recordsEqual(const AggregateStorageLocation &Loc1,
+ const AggregateStorageLocation &Loc2,
+ const Environment &Env) {
+ return recordsEqual(Loc1, Env, Loc2, Env);
+}
+
+} // namespace dataflow
+} // namespace clang
+
+#endif // LLVM_CLANG_ANALYSIS_FLOWSENSITIVE_RECORDOPS_H
Index: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
===================================================================
--- clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -387,6 +387,9 @@
/// Assigns `Val` as the value of `Loc` in the environment.
void setValue(const StorageLocation &Loc, Value &Val);
+ /// Clears any association between `Loc` and a value in the environment.
+ void clearValue(const StorageLocation &Loc);
+
/// Assigns `Val` as the value of the prvalue `E` in the environment.
///
/// If `E` is not yet associated with a storage location, associates it with
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
