ymandel updated this revision to Diff 484830.
ymandel added a comment.
Herald added a subscriber: martong.
Herald added a reviewer: NoQ.
Reviving this patch, addressing some of the earlier concerns. However, I may
have a better solution which makes this patch irrelevant. So, still WIP.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D123032/new/
https://reviews.llvm.org/D123032
Files:
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
Index: clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
===================================================================
--- clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
@@ -23,6 +23,7 @@
using namespace clang;
using namespace dataflow;
using ::testing::ElementsAre;
+using ::testing::IsNull;
using ::testing::NotNull;
using ::testing::Pair;
@@ -102,7 +103,6 @@
auto &Context = Unit->getASTContext();
ASSERT_EQ(Context.getDiagnostics().getClient()->getNumErrors(), 0U);
-
auto Results =
match(decl(anyOf(varDecl(hasName("Global")).bind("global"),
functionDecl(hasName("Target")).bind("target"))),
@@ -133,7 +133,6 @@
auto &Context = Unit->getASTContext();
ASSERT_EQ(Context.getDiagnostics().getClient()->getNumErrors(), 0U);
-
auto Results =
match(decl(anyOf(
varDecl(hasName("Global")).bind("global"),
@@ -149,4 +148,117 @@
EXPECT_THAT(Env.getValue(*Var, SkipPast::None), NotNull());
}
+TEST(ExcludeProtobufTypesTest, ExcludeEnabled) {
+ using namespace ast_matchers;
+
+ DataflowAnalysisContext DAContext(std::make_unique<WatchedLiteralsSolver>(),
+ {/*ExcludeGoogleProtobufs=*/true});
+ Environment Env(DAContext);
+
+ std::string Code = R"cc(
+ namespace google {
+ namespace protobuf {
+ struct Message {};
+ }
+ }
+
+ namespace other {
+ namespace google {
+ namespace protobuf {
+ struct Message {};
+ }
+ }
+ }
+
+ struct Bar : public google::protobuf::Message {
+ bool Field;
+ };
+
+ // Not a protobuf, but looks like it. Verify that it is *not* excluded.
+ struct Zab : public other::google::protobuf::Message {
+ bool Field;
+ };
+
+ void target() {
+ Bar B;
+ Zab Z;
+ (void)0;
+ /*[[check]]*/
+ }
+ )cc";
+
+ auto Unit =
+ tooling::buildASTFromCodeWithArgs(Code, {"-fsyntax-only", "-std=c++11"});
+ auto &Context = Unit->getASTContext();
+
+ ASSERT_EQ(Context.getDiagnostics().getClient()->getNumErrors(), 0U);
+ auto Results = match(varDecl(eachOf(varDecl(hasName("B")).bind("varB"),
+ varDecl(hasName("Z")).bind("varZ"))),
+ Context);
+ const auto *B = selectFirst<VarDecl>("varB", Results);
+ ASSERT_TRUE(B != nullptr);
+ const auto *Z = selectFirst<VarDecl>("varZ", Results);
+ ASSERT_TRUE(Z != nullptr);
+
+ EXPECT_THAT(Env.createValue(B->getType()), IsNull());
+ EXPECT_THAT(Env.createValue(Z->getType()), NotNull());
+}
+
+TEST(ExcludeProtobufTypesTest, ExcludeDisabled) {
+ using namespace ast_matchers;
+
+ DataflowAnalysisContext DAContext(std::make_unique<WatchedLiteralsSolver>(),
+ {/*ExcludeGoogleProtobufs=*/false});
+ Environment Env(DAContext);
+
+ std::string Code = R"cc(
+ namespace google {
+ namespace protobuf {
+ struct Message {};
+ }
+ }
+
+ namespace other {
+ namespace google {
+ namespace protobuf {
+ struct Message {};
+ }
+ }
+ }
+
+ struct Bar : public google::protobuf::Message {
+ bool Field;
+ };
+
+ // Not a protobuf, but looks like it. Verify that it is *not* excluded.
+ struct Zab : public other::google::protobuf::Message {
+ bool Field;
+ };
+
+ void target() {
+ Bar B;
+ Zab Z;
+ (void)0;
+ /*[[check]]*/
+ }
+ )cc";
+
+ auto Unit =
+ tooling::buildASTFromCodeWithArgs(Code, {"-fsyntax-only", "-std=c++11"});
+ auto &Context = Unit->getASTContext();
+
+ ASSERT_EQ(Context.getDiagnostics().getClient()->getNumErrors(), 0U);
+
+ auto Results = match(varDecl(eachOf(varDecl(hasName("B")).bind("varB"),
+ varDecl(hasName("Z")).bind("varZ"))),
+ Context);
+ const auto *B = selectFirst<VarDecl>("varB", Results);
+ ASSERT_TRUE(B != nullptr);
+ const auto *Z = selectFirst<VarDecl>("varZ", Results);
+ ASSERT_TRUE(Z != nullptr);
+
+ EXPECT_THAT(Env.createValue(B->getType()), NotNull());
+ EXPECT_THAT(Env.createValue(Z->getType()), NotNull());
+}
+
} // namespace
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -672,7 +672,8 @@
return &takeOwnership(std::make_unique<PointerValue>(PointeeLoc));
}
- if (Type->isStructureOrClassType()) {
+ if (Type->isStructureOrClassType() &&
+ !DACtx->isExcludedRecordType(*Type->getAs<RecordType>())) {
CreatedValuesCount++;
// FIXME: Initialize only fields that are accessed in the context that is
// being analyzed.
Index: clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
@@ -355,6 +355,66 @@
return nullptr;
}
+// Hard code certain types to exclude from modeling. Currently, we limit to
+// Google protobufs, since they can be very large, and have no value in
+// modeling.
+//
+// FIXME: Remove this specialized exclusion once a more general mechanism is
+// implemented for only modeling accessed fields. Otherwise, consider memoizing
+// this function or caching some of the information on which it relies (like the
+// protobuf `Message` base type) at the TU level.
+bool DataflowAnalysisContext::isExcludedRecordType(const RecordType &Ty) {
+ if (!Options.ExcludeGoogleProtobufs)
+ return false;
+
+ const auto *CD = dyn_cast<CXXRecordDecl>(Ty.getDecl());
+ if (CD == nullptr || !CD->hasDefinition() || CD->getNumBases() != 1)
+ return false;
+
+ QualType BQ = CD->bases_begin()->getType();
+ if (BQ.isNull())
+ return false;
+
+ const RecordType *BaseTy = BQ->getAs<RecordType>();
+ if (BaseTy == nullptr)
+ return false;
+
+ if (ProtobufBaseTy != nullptr)
+ return BaseTy == ProtobufBaseTy;
+
+ const RecordDecl *RD = BaseTy->getDecl();
+ assert(RD != nullptr);
+ IdentifierInfo *II = RD->getIdentifier();
+ if (II == nullptr || !II->isStr("Message"))
+ return false;
+
+ const auto *ND = dyn_cast<NamespaceDecl>(RD->getDeclContext());
+ if (ND == nullptr)
+ return false;
+ IdentifierInfo *NamespaceII = ND->getIdentifier();
+ if (NamespaceII == nullptr)
+ return false;
+ if (NamespaceII->isStr("proto2")) {
+ ProtobufBaseTy = BaseTy;
+ return true;
+ }
+
+ // Check for `::google::protobuf`:
+ if (!NamespaceII->isStr("protobuf"))
+ return false;
+
+ ND = dyn_cast<NamespaceDecl>(ND->getDeclContext());
+ if (ND == nullptr || !ND->getParent()->isTranslationUnit())
+ return false;
+ NamespaceII = ND->getIdentifier();
+ if (NamespaceII != nullptr && NamespaceII->isStr("google")) {
+ ProtobufBaseTy = BaseTy;
+ return true;
+ }
+
+ return false;
+}
+
} // namespace dataflow
} // namespace clang
Index: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
===================================================================
--- clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
+++ clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
@@ -54,14 +54,20 @@
/// is used during dataflow analysis.
class DataflowAnalysisContext {
public:
+ struct Options {
+ // FIXME: add comments and change to std::vector<string>
+ bool ExcludeGoogleProtobufs;
+ };
+
/// Constructs a dataflow analysis context.
///
/// Requirements:
///
/// `S` must not be null.
- DataflowAnalysisContext(std::unique_ptr<Solver> S)
+ DataflowAnalysisContext(std::unique_ptr<Solver> S,
+ Options Opts = {/*ExcludeGoogleProtobufs=*/false})
: S(std::move(S)), TrueVal(createAtomicBoolValue()),
- FalseVal(createAtomicBoolValue()) {
+ FalseVal(createAtomicBoolValue()), Options(Opts) {
assert(this->S != nullptr);
}
@@ -253,6 +259,8 @@
/// returns null.
const ControlFlowContext *getControlFlowContext(const FunctionDecl *F);
+ bool isExcludedRecordType(const RecordType &Ty);
+
private:
struct NullableQualTypeDenseMapInfo : private llvm::DenseMapInfo<QualType> {
static QualType getEmptyKey() {
@@ -330,6 +338,8 @@
AtomicBoolValue &TrueVal;
AtomicBoolValue &FalseVal;
+ Options Options;
+
// Indices that are used to avoid recreating the same composite boolean
// values.
llvm::DenseMap<std::pair<BoolValue *, BoolValue *>, ConjunctionValue *>
@@ -359,6 +369,8 @@
llvm::DenseMap<AtomicBoolValue *, BoolValue *> FlowConditionConstraints;
llvm::DenseMap<const FunctionDecl *, ControlFlowContext> FunctionContexts;
+
+ const RecordType *ProtobufBaseTy = nullptr;
};
} // namespace dataflow
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
