Skip to content

Commit eb2131b

Browse files
committed
[clang][dataflow] Do not crash on missing Value for struct-typed variable init.
Remove constraint that an initializing expression of struct type must have an associated `Value`. This invariant is not and will not be guaranteed by the framework, because of potentially uninitialized fields. Differential Revision: https://reviews.llvm.org/D123961
1 parent 489894f commit eb2131b

File tree

2 files changed

+63
-17
lines changed

2 files changed

+63
-17
lines changed

clang/lib/Analysis/FlowSensitive/Transfer.cpp

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -168,27 +168,25 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
168168
auto &Val =
169169
Env.takeOwnership(std::make_unique<ReferenceValue>(*InitExprLoc));
170170
Env.setValue(Loc, Val);
171-
} else {
172-
// FIXME: The initializer expression must always be assigned a value.
173-
// Replace this with an assert when we have sufficient coverage of
174-
// language features.
175-
if (Value *Val = Env.createValue(D.getType()))
176-
Env.setValue(Loc, *Val);
171+
return;
177172
}
173+
} else if (auto *InitExprVal = Env.getValue(*InitExpr, SkipPast::None)) {
174+
Env.setValue(Loc, *InitExprVal);
178175
return;
179176
}
180177

181-
if (auto *InitExprVal = Env.getValue(*InitExpr, SkipPast::None)) {
182-
Env.setValue(Loc, *InitExprVal);
183-
} else if (!D.getType()->isStructureOrClassType()) {
184-
// FIXME: The initializer expression must always be assigned a value.
185-
// Replace this with an assert when we have sufficient coverage of
186-
// language features.
187-
if (Value *Val = Env.createValue(D.getType()))
188-
Env.setValue(Loc, *Val);
189-
} else {
190-
llvm_unreachable("structs and classes must always be assigned values");
191-
}
178+
// We arrive here in (the few) cases where an expression is intentionally
179+
// "uninterpreted". There are two ways to handle this situation: propagate
180+
// the status, so that uninterpreted initializers result in uninterpreted
181+
// variables, or provide a default value. We choose the latter so that later
182+
// refinements of the variable can be used for reasoning about the
183+
// surrounding code.
184+
//
185+
// FIXME. If and when we interpret all language cases, change this to assert
186+
// that `InitExpr` is interpreted, rather than supplying a default value
187+
// (assuming we don't update the environment API to return references).
188+
if (Value *Val = Env.createValue(D.getType()))
189+
Env.setValue(Loc, *Val);
192190
}
193191

194192
void VisitImplicitCastExpr(const ImplicitCastExpr *S) {

clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,54 @@ TEST_F(TransferTest, StructVarDecl) {
187187
});
188188
}
189189

190+
TEST_F(TransferTest, StructVarDeclWithInit) {
191+
std::string Code = R"(
192+
struct A {
193+
int Bar;
194+
};
195+
196+
A Gen();
197+
198+
void target() {
199+
A Foo = Gen();
200+
// [[p]]
201+
}
202+
)";
203+
runDataflow(
204+
Code, [](llvm::ArrayRef<
205+
std::pair<std::string, DataflowAnalysisState<NoopLattice>>>
206+
Results,
207+
ASTContext &ASTCtx) {
208+
ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
209+
const Environment &Env = Results[0].second.Env;
210+
211+
const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
212+
ASSERT_THAT(FooDecl, NotNull());
213+
214+
ASSERT_TRUE(FooDecl->getType()->isStructureType());
215+
auto FooFields = FooDecl->getType()->getAsRecordDecl()->fields();
216+
217+
FieldDecl *BarDecl = nullptr;
218+
for (FieldDecl *Field : FooFields) {
219+
if (Field->getNameAsString() == "Bar") {
220+
BarDecl = Field;
221+
} else {
222+
FAIL() << "Unexpected field: " << Field->getNameAsString();
223+
}
224+
}
225+
ASSERT_THAT(BarDecl, NotNull());
226+
227+
const auto *FooLoc = cast<AggregateStorageLocation>(
228+
Env.getStorageLocation(*FooDecl, SkipPast::None));
229+
const auto *BarLoc =
230+
cast<ScalarStorageLocation>(&FooLoc->getChild(*BarDecl));
231+
232+
const auto *FooVal = cast<StructValue>(Env.getValue(*FooLoc));
233+
const auto *BarVal = cast<IntegerValue>(FooVal->getChild(*BarDecl));
234+
EXPECT_EQ(Env.getValue(*BarLoc), BarVal);
235+
});
236+
}
237+
190238
TEST_F(TransferTest, ClassVarDecl) {
191239
std::string Code = R"(
192240
class A {

0 commit comments

Comments
 (0)