[clang][dataflow] Fix nullptr dereferencing error.

When pre-initializing fields in the environment, the code assumed that all
fields of a struct would be initialized. However, given limits on value
construction, that assumption is incorrect. This patch changes the code to drop
that assumption and thereby avoid dereferencing a nullptr.

Differential Revision: https://reviews.llvm.org/D121158
This commit is contained in:
Yitzhak Mandelbaum 2022-03-07 21:16:28 +00:00
parent e55b9b0d0a
commit 18c84e2d32
4 changed files with 81 additions and 34 deletions

View File

@ -201,11 +201,13 @@ public:
return Val->getKind() == Kind::Struct;
}
/// Returns the child value that is assigned for `D`.
Value &getChild(const ValueDecl &D) const {
/// Returns the child value that is assigned for `D` or null if the child is
/// not initialized.
Value *getChild(const ValueDecl &D) const {
auto It = Children.find(&D);
assert(It != Children.end());
return *It->second;
if (It == Children.end())
return nullptr;
return It->second;
}
/// Assigns `Val` as the child value for `D`.

View File

@ -366,7 +366,8 @@ void Environment::setValue(const StorageLocation &Loc, Value &Val) {
assert(Field != nullptr);
StorageLocation &FieldLoc = AggregateLoc.getChild(*Field);
MemberLocToStruct[&FieldLoc] = std::make_pair(StructVal, Field);
setValue(FieldLoc, StructVal->getChild(*Field));
if (auto *FieldVal = StructVal->getChild(*Field))
setValue(FieldLoc, *FieldVal);
}
}
@ -485,9 +486,9 @@ Value *Environment::createValueUnlessSelfReferential(
continue;
Visited.insert(FieldType.getCanonicalType());
FieldValues.insert(
{Field, createValueUnlessSelfReferential(
FieldType, Visited, Depth + 1, CreatedValuesCount)});
if (auto *FieldValue = createValueUnlessSelfReferential(
FieldType, Visited, Depth + 1, CreatedValuesCount))
FieldValues.insert({Field, FieldValue});
Visited.erase(FieldType.getCanonicalType());
}

View File

@ -7,7 +7,10 @@
//===----------------------------------------------------------------------===//
#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
#include "NoopAnalysis.h"
#include "TestingSupport.h"
#include "clang/Analysis/FlowSensitive/DataflowAnalysisContext.h"
#include "clang/Analysis/FlowSensitive/Value.h"
#include "clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
@ -17,6 +20,8 @@ namespace {
using namespace clang;
using namespace dataflow;
using ::testing::ElementsAre;
using ::testing::Pair;
class EnvironmentTest : public ::testing::Test {
DataflowAnalysisContext Context;
@ -54,4 +59,43 @@ TEST_F(EnvironmentTest, FlowCondition) {
EXPECT_FALSE(Env.flowConditionImplies(NotX));
}
TEST_F(EnvironmentTest, CreateValueRecursiveType) {
using namespace ast_matchers;
std::string Code = R"cc(
struct Recursive {
bool X;
Recursive *R;
};
)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(qualType(hasDeclaration(recordDecl(
hasName("Recursive"),
has(fieldDecl(hasName("R")).bind("field-r")))))
.bind("target"),
Context);
const QualType *Ty = selectFirst<QualType>("target", Results);
const FieldDecl *R = selectFirst<FieldDecl>("field-r", Results);
ASSERT_TRUE(Ty != nullptr && !Ty->isNull());
ASSERT_TRUE(R != nullptr);
// Verify that the struct and the field (`R`) with first appearance of the
// type is created successfully.
Value *Val = Env.createValue(*Ty);
ASSERT_NE(Val, nullptr);
StructValue *SVal = clang::dyn_cast<StructValue>(Val);
ASSERT_NE(SVal, nullptr);
Val = SVal->getChild(*R);
ASSERT_NE(Val, nullptr);
PointerValue *PV = clang::dyn_cast<PointerValue>(Val);
EXPECT_NE(PV, nullptr);
}
} // namespace

View File

@ -181,7 +181,7 @@ TEST_F(TransferTest, StructVarDecl) {
cast<ScalarStorageLocation>(&FooLoc->getChild(*BarDecl));
const auto *FooVal = cast<StructValue>(Env.getValue(*FooLoc));
const auto *BarVal = cast<IntegerValue>(&FooVal->getChild(*BarDecl));
const auto *BarVal = cast<IntegerValue>(FooVal->getChild(*BarDecl));
EXPECT_EQ(Env.getValue(*BarLoc), BarVal);
});
}
@ -227,7 +227,7 @@ TEST_F(TransferTest, ClassVarDecl) {
cast<ScalarStorageLocation>(&FooLoc->getChild(*BarDecl));
const auto *FooVal = cast<StructValue>(Env.getValue(*FooLoc));
const auto *BarVal = cast<IntegerValue>(&FooVal->getChild(*BarDecl));
const auto *BarVal = cast<IntegerValue>(FooVal->getChild(*BarDecl));
EXPECT_EQ(Env.getValue(*BarLoc), BarVal);
});
}
@ -351,27 +351,27 @@ TEST_F(TransferTest, SelfReferentialReferenceVarDecl) {
cast<StructValue>(Env.getValue(FooVal->getPointeeLoc()));
const auto *BarVal =
cast<ReferenceValue>(&FooPointeeVal->getChild(*BarDecl));
cast<ReferenceValue>(FooPointeeVal->getChild(*BarDecl));
const auto *BarPointeeVal =
cast<StructValue>(Env.getValue(BarVal->getPointeeLoc()));
const auto *FooRefVal =
cast<ReferenceValue>(&BarPointeeVal->getChild(*FooRefDecl));
cast<ReferenceValue>(BarPointeeVal->getChild(*FooRefDecl));
const StorageLocation &FooRefPointeeLoc = FooRefVal->getPointeeLoc();
EXPECT_THAT(Env.getValue(FooRefPointeeLoc), IsNull());
const auto *FooPtrVal =
cast<PointerValue>(&BarPointeeVal->getChild(*FooPtrDecl));
cast<PointerValue>(BarPointeeVal->getChild(*FooPtrDecl));
const StorageLocation &FooPtrPointeeLoc = FooPtrVal->getPointeeLoc();
EXPECT_THAT(Env.getValue(FooPtrPointeeLoc), IsNull());
const auto *BazRefVal =
cast<ReferenceValue>(&BarPointeeVal->getChild(*BazRefDecl));
cast<ReferenceValue>(BarPointeeVal->getChild(*BazRefDecl));
const StorageLocation &BazRefPointeeLoc = BazRefVal->getPointeeLoc();
EXPECT_THAT(Env.getValue(BazRefPointeeLoc), NotNull());
const auto *BazPtrVal =
cast<PointerValue>(&BarPointeeVal->getChild(*BazPtrDecl));
cast<PointerValue>(BarPointeeVal->getChild(*BazPtrDecl));
const StorageLocation &BazPtrPointeeLoc = BazPtrVal->getPointeeLoc();
EXPECT_THAT(Env.getValue(BazPtrPointeeLoc), NotNull());
});
@ -508,27 +508,27 @@ TEST_F(TransferTest, SelfReferentialPointerVarDecl) {
cast<StructValue>(Env.getValue(FooVal->getPointeeLoc()));
const auto *BarVal =
cast<PointerValue>(&FooPointeeVal->getChild(*BarDecl));
cast<PointerValue>(FooPointeeVal->getChild(*BarDecl));
const auto *BarPointeeVal =
cast<StructValue>(Env.getValue(BarVal->getPointeeLoc()));
const auto *FooRefVal =
cast<ReferenceValue>(&BarPointeeVal->getChild(*FooRefDecl));
cast<ReferenceValue>(BarPointeeVal->getChild(*FooRefDecl));
const StorageLocation &FooRefPointeeLoc = FooRefVal->getPointeeLoc();
EXPECT_THAT(Env.getValue(FooRefPointeeLoc), IsNull());
const auto *FooPtrVal =
cast<PointerValue>(&BarPointeeVal->getChild(*FooPtrDecl));
cast<PointerValue>(BarPointeeVal->getChild(*FooPtrDecl));
const StorageLocation &FooPtrPointeeLoc = FooPtrVal->getPointeeLoc();
EXPECT_THAT(Env.getValue(FooPtrPointeeLoc), IsNull());
const auto *BazRefVal =
cast<ReferenceValue>(&BarPointeeVal->getChild(*BazRefDecl));
cast<ReferenceValue>(BarPointeeVal->getChild(*BazRefDecl));
const StorageLocation &BazRefPointeeLoc = BazRefVal->getPointeeLoc();
EXPECT_THAT(Env.getValue(BazRefPointeeLoc), NotNull());
const auto *BazPtrVal =
cast<PointerValue>(&BarPointeeVal->getChild(*BazPtrDecl));
cast<PointerValue>(BarPointeeVal->getChild(*BazPtrDecl));
const StorageLocation &BazPtrPointeeLoc = BazPtrVal->getPointeeLoc();
EXPECT_THAT(Env.getValue(BazPtrPointeeLoc), NotNull());
});
@ -888,7 +888,7 @@ TEST_F(TransferTest, StructParamDecl) {
cast<ScalarStorageLocation>(&FooLoc->getChild(*BarDecl));
const auto *FooVal = cast<StructValue>(Env.getValue(*FooLoc));
const auto *BarVal = cast<IntegerValue>(&FooVal->getChild(*BarDecl));
const auto *BarVal = cast<IntegerValue>(FooVal->getChild(*BarDecl));
EXPECT_EQ(Env.getValue(*BarLoc), BarVal);
});
}
@ -1000,7 +1000,7 @@ TEST_F(TransferTest, StructMember) {
const auto *FooLoc = cast<AggregateStorageLocation>(
Env.getStorageLocation(*FooDecl, SkipPast::None));
const auto *FooVal = cast<StructValue>(Env.getValue(*FooLoc));
const auto *BarVal = cast<IntegerValue>(&FooVal->getChild(*BarDecl));
const auto *BarVal = cast<IntegerValue>(FooVal->getChild(*BarDecl));
const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
ASSERT_THAT(BazDecl, NotNull());
@ -1048,7 +1048,7 @@ TEST_F(TransferTest, ClassMember) {
const auto *FooLoc = cast<AggregateStorageLocation>(
Env.getStorageLocation(*FooDecl, SkipPast::None));
const auto *FooVal = cast<StructValue>(Env.getValue(*FooLoc));
const auto *BarVal = cast<IntegerValue>(&FooVal->getChild(*BarDecl));
const auto *BarVal = cast<IntegerValue>(FooVal->getChild(*BarDecl));
const ValueDecl *BazDecl = findValueDecl(ASTCtx, "Baz");
ASSERT_THAT(BazDecl, NotNull());
@ -1095,7 +1095,7 @@ TEST_F(TransferTest, ReferenceMember) {
const auto *FooLoc = cast<AggregateStorageLocation>(
Env.getStorageLocation(*FooDecl, SkipPast::None));
const auto *FooVal = cast<StructValue>(Env.getValue(*FooLoc));
const auto *BarVal = cast<ReferenceValue>(&FooVal->getChild(*BarDecl));
const auto *BarVal = cast<ReferenceValue>(FooVal->getChild(*BarDecl));
const auto *BarPointeeVal =
cast<IntegerValue>(Env.getValue(BarVal->getPointeeLoc()));
@ -1173,7 +1173,7 @@ TEST_F(TransferTest, StructThisMember) {
const auto *BazLoc =
cast<ScalarStorageLocation>(&QuxLoc->getChild(*BazDecl));
const auto *BazVal = cast<IntegerValue>(&QuxVal->getChild(*BazDecl));
const auto *BazVal = cast<IntegerValue>(QuxVal->getChild(*BazDecl));
EXPECT_EQ(Env.getValue(*BazLoc), BazVal);
const ValueDecl *QuuxDecl = findValueDecl(ASTCtx, "Quux");
@ -1249,7 +1249,7 @@ TEST_F(TransferTest, ClassThisMember) {
const auto *BazLoc =
cast<ScalarStorageLocation>(&QuxLoc->getChild(*BazDecl));
const auto *BazVal = cast<IntegerValue>(&QuxVal->getChild(*BazDecl));
const auto *BazVal = cast<IntegerValue>(QuxVal->getChild(*BazDecl));
EXPECT_EQ(Env.getValue(*BazLoc), BazVal);
const ValueDecl *QuuxDecl = findValueDecl(ASTCtx, "Quux");
@ -1399,7 +1399,7 @@ TEST_F(TransferTest, TemporaryObject) {
cast<ScalarStorageLocation>(&FooLoc->getChild(*BarDecl));
const auto *FooVal = cast<StructValue>(Env.getValue(*FooLoc));
const auto *BarVal = cast<IntegerValue>(&FooVal->getChild(*BarDecl));
const auto *BarVal = cast<IntegerValue>(FooVal->getChild(*BarDecl));
EXPECT_EQ(Env.getValue(*BarLoc), BarVal);
});
}
@ -1438,7 +1438,7 @@ TEST_F(TransferTest, ElidableConstructor) {
cast<ScalarStorageLocation>(&FooLoc->getChild(*BarDecl));
const auto *FooVal = cast<StructValue>(Env.getValue(*FooLoc));
const auto *BarVal = cast<IntegerValue>(&FooVal->getChild(*BarDecl));
const auto *BarVal = cast<IntegerValue>(FooVal->getChild(*BarDecl));
EXPECT_EQ(Env.getValue(*BarLoc), BarVal);
},
LangStandard::lang_cxx14);
@ -1706,7 +1706,7 @@ TEST_F(TransferTest, BindTemporary) {
*cast<StructValue>(Env.getValue(*FooDecl, SkipPast::None));
const auto *BarVal =
cast<IntegerValue>(Env.getValue(*BarDecl, SkipPast::None));
EXPECT_EQ(BarVal, &FooVal.getChild(*BazDecl));
EXPECT_EQ(BarVal, FooVal.getChild(*BazDecl));
});
}
@ -1982,12 +1982,12 @@ TEST_F(TransferTest, AggregateInitialization) {
cast<StructValue>(Env.getValue(*QuuxDecl, SkipPast::None));
ASSERT_THAT(QuuxVal, NotNull());
const auto *BazVal = cast<StructValue>(&QuuxVal->getChild(*BazDecl));
const auto *BazVal = cast<StructValue>(QuuxVal->getChild(*BazDecl));
ASSERT_THAT(BazVal, NotNull());
EXPECT_EQ(&QuuxVal->getChild(*BarDecl), BarArgVal);
EXPECT_EQ(&BazVal->getChild(*FooDecl), FooArgVal);
EXPECT_EQ(&QuuxVal->getChild(*QuxDecl), QuxArgVal);
EXPECT_EQ(QuuxVal->getChild(*BarDecl), BarArgVal);
EXPECT_EQ(BazVal->getChild(*FooDecl), FooArgVal);
EXPECT_EQ(QuuxVal->getChild(*QuxDecl), QuxArgVal);
});
}
}
@ -2383,7 +2383,7 @@ TEST_F(TransferTest, AssignMemberBeforeCopy) {
const auto *A2Val =
cast<StructValue>(Env.getValue(*A2Decl, SkipPast::None));
EXPECT_EQ(&A2Val->getChild(*FooDecl), BarVal);
EXPECT_EQ(A2Val->getChild(*FooDecl), BarVal);
});
}