kinu updated this revision to Diff 555842.
kinu marked 5 inline comments as done.
kinu added a comment.
Updated again... very easy to overlook unaddressed comments!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D159284/new/
https://reviews.llvm.org/D159284
Files:
clang/lib/Analysis/FlowSensitive/Transfer.cpp
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
@@ -1436,6 +1436,99 @@
llvm::Succeeded());
}
+TEST(TransferTest, StructModeledFieldsWithComplicatedInheritance) {
+ std::string Code = R"(
+ struct Base1 {
+ int base1_1;
+ int base1_2;
+ };
+ struct Intermediate : Base1 {
+ int intermediate_1;
+ int intermediate_2;
+ };
+ struct Base2 {
+ int base2_1;
+ int base2_2;
+ };
+ struct MostDerived : public Intermediate, Base2 {
+ int most_derived_1;
+ int most_derived_2;
+ };
+
+ void target() {
+ MostDerived MD;
+ MD.base1_2 = 1;
+ MD.intermediate_2 = 1;
+ MD.base2_2 = 1;
+ MD.most_derived_2 = 1;
+ // [[p]]
+ }
+ )";
+ runDataflow(
+ Code,
+ [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+ ASTContext &ASTCtx) {
+ const Environment &Env =
+ getEnvironmentAtAnnotation(Results, "p");
+
+ // Only the accessed fields should exist in the model.
+ auto &MDLoc = getLocForDecl<RecordStorageLocation>(ASTCtx, Env, "MD");
+ std::vector<const ValueDecl*> Fields;
+ for (auto [Field, _] : MDLoc.children())
+ Fields.push_back(Field);
+ ASSERT_THAT(Fields, UnorderedElementsAre(
+ findValueDecl(ASTCtx, "base1_2"),
+ findValueDecl(ASTCtx, "intermediate_2"),
+ findValueDecl(ASTCtx, "base2_2"),
+ findValueDecl(ASTCtx, "most_derived_2")));
+ });
+}
+
+TEST(TransferTest, StructInitializerListWithComplicatedInheritance) {
+ std::string Code = R"(
+ struct Base1 {
+ int base1;
+ };
+ struct Intermediate : Base1 {
+ int intermediate;
+ };
+ struct Base2 {
+ int base2;
+ };
+ struct MostDerived : public Intermediate, Base2 {
+ int most_derived;
+ };
+
+ void target() {
+ MostDerived MD = {};
+ // [[p]]
+ }
+ )";
+ runDataflow(
+ Code,
+ [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+ ASTContext &ASTCtx) {
+ const Environment &Env =
+ getEnvironmentAtAnnotation(Results, "p");
+
+ // When a struct is initialized with a initializer list, all the
+ // fields are considered "accessed", and therefore do exist.
+ auto &MD = getLocForDecl<RecordStorageLocation>(ASTCtx, Env, "MD");
+ ASSERT_THAT(cast<IntegerValue>(
+ getFieldValue(&MD, *findValueDecl(ASTCtx, "base1"), Env)),
+ NotNull());
+ ASSERT_THAT(cast<IntegerValue>(
+ getFieldValue(&MD, *findValueDecl(ASTCtx, "intermediate"), Env)),
+ NotNull());
+ ASSERT_THAT(cast<IntegerValue>(
+ getFieldValue(&MD, *findValueDecl(ASTCtx, "base2"), Env)),
+ NotNull());
+ ASSERT_THAT(cast<IntegerValue>(
+ getFieldValue(&MD, *findValueDecl(ASTCtx, "most_derived"), Env)),
+ NotNull());
+ });
+}
+
TEST(TransferTest, ReferenceMember) {
std::string Code = R"(
struct A {
@@ -2041,6 +2134,26 @@
});
}
+TEST(TransferTest, AssignmentOperatorWithInitAndInheritance) {
+ // This is a crash repro.
+ std::string Code = R"(
+ struct B { int Foo; };
+ struct S : public B {};
+ void target() {
+ S S1 = { 1 };
+ S S2;
+ S S3;
+ S1 = S2; // Only Dst has InitListExpr.
+ S3 = S1; // Only Src has InitListExpr.
+ // [[p]]
+ }
+ )";
+ runDataflow(
+ Code,
+ [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+ ASTContext &ASTCtx) {});
+}
+
TEST(TransferTest, CopyConstructor) {
std::string Code = R"(
struct A {
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===================================================================
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -27,12 +27,12 @@
#include "clang/Analysis/FlowSensitive/Value.h"
#include "clang/Basic/Builtins.h"
#include "clang/Basic/OperatorKinds.h"
-#include "llvm/ADT/STLExtras.h"
#include "llvm/Support/Casting.h"
-#include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/Debug.h"
+#include <assert.h>
#include <cassert>
-#include <memory>
-#include <tuple>
+
+#define DEBUG_TYPE "dataflow"
namespace clang {
namespace dataflow {
@@ -629,17 +629,66 @@
return;
}
- std::vector<FieldDecl *> Fields =
- getFieldsForInitListExpr(Type->getAsRecordDecl());
llvm::DenseMap<const ValueDecl *, StorageLocation *> FieldLocs;
- for (auto [Field, Init] : llvm::zip(Fields, S->inits())) {
- assert(Field != nullptr);
- assert(Init != nullptr);
+ // This only contains the direct fields for the given type.
+ std::vector<FieldDecl *> FieldsForInit =
+ getFieldsForInitListExpr(Type->getAsRecordDecl());
- FieldLocs.insert({Field, &Env.createObject(Field->getType(), Init)});
+ // `S->inits()` contains all the initializer epressions, including the
+ // ones for direct base classes.
+ auto Inits = S->inits();
+ size_t InitIdx = 0;
+
+ // Initialize base classes.
+ if (auto* R = S->getType()->getAsCXXRecordDecl()) {
+ assert(FieldsForInit.size() + R->getNumBases() == Inits.size());
+ for ([[maybe_unused]] const CXXBaseSpecifier &Base : R->bases()) {
+ assert(InitIdx < Inits.size());
+ auto Init = Inits[InitIdx++];
+ assert(Base.getType().getCanonicalType() ==
+ Init->getType().getCanonicalType());
+ auto* BaseVal = cast_or_null<RecordValue>(Env.getValue(*Init));
+ if (!BaseVal)
+ BaseVal = cast<RecordValue>(Env.createValue(Init->getType()));
+ // Take ownership of the fields of the `RecordValue` for the base class
+ // and incorporate them into the "flattened" set of fields for the
+ // derived class.
+ auto Children = BaseVal->getLoc().children();
+ FieldLocs.insert(Children.begin(), Children.end());
+ }
}
+ assert(FieldsForInit.size() == Inits.size() - InitIdx);
+ for (auto Field : FieldsForInit) {
+ assert(InitIdx < Inits.size());
+ auto Init = Inits[InitIdx++];
+ assert(
+ // The types are same, or
+ Field->getType().getCanonicalType().getUnqualifiedType() ==
+ Init->getType().getCanonicalType() ||
+ // The field's type is T&, and initializer is T
+ (Field->getType()->isReferenceType() &&
+ Field->getType().getCanonicalType()->getPointeeType() ==
+ Init->getType().getCanonicalType()));
+ auto& Loc = Env.createObject(Field->getType(), Init);
+ FieldLocs.insert({Field, &Loc});
+ }
+
+ LLVM_DEBUG({
+ // Check that we satisfy the invariant that a `RecordStorageLoation`
+ // contains exactly the set of modeled fields for that type.
+ // `ModeledFields` includes fields from all the bases, but only the
+ // modeled ones. However, if a class type is initialized with an
+ // `InitListExpr`, all fields in the class, including those from base
+ // classes, are included in the set of modeled fields. The code above
+ // should therefore populate exactly the modeled fields.
+ auto ModeledFields = Env.getDataflowAnalysisContext().getModeledFields(Type);
+ assert(ModeledFields.size() == FieldLocs.size());
+ for ([[maybe_unused]] auto [Field, Loc] : FieldLocs)
+ assert(ModeledFields.contains(cast_or_null<FieldDecl>(Field)));
+ });
+
auto &Loc =
Env.getDataflowAnalysisContext().arena().create<RecordStorageLocation>(
Type, std::move(FieldLocs));
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
