Skip to content

Commit a48dc2e

Browse files
committed
[clang][dataflow] Correctly treat empty initializer lists for unions.
This fixes a crash introduced by #82348 but also adds additional handling to make sure that we treat empty initializer lists for both unions and structs/classes correctly (see tests added in this patch).
1 parent e630a45 commit a48dc2e

File tree

3 files changed

+86
-4
lines changed

3 files changed

+86
-4
lines changed

clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -983,7 +983,7 @@ StorageLocation &Environment::createObjectInternal(const ValueDecl *D,
983983
}
984984

985985
Value *Val = nullptr;
986-
if (InitExpr)
986+
if (InitExpr) {
987987
// In the (few) cases where an expression is intentionally
988988
// "uninterpreted", `InitExpr` is not associated with a value. There are
989989
// two ways to handle this situation: propagate the status, so that
@@ -998,6 +998,11 @@ StorageLocation &Environment::createObjectInternal(const ValueDecl *D,
998998
// default value (assuming we don't update the environment API to return
999999
// references).
10001000
Val = getValue(*InitExpr);
1001+
1002+
if (!Val && isa<ImplicitValueInitExpr>(InitExpr) &&
1003+
InitExpr->getType()->isPointerType())
1004+
Val = &getOrCreateNullPointerValue(InitExpr->getType()->getPointeeType());
1005+
}
10011006
if (!Val)
10021007
Val = createValue(Ty);
10031008

clang/lib/Analysis/FlowSensitive/Transfer.cpp

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -685,9 +685,22 @@ class TransferVisitor : public ConstStmtVisitor<TransferVisitor> {
685685

686686
// `S->inits()` contains all the initializer expressions, including the
687687
// ones for direct base classes.
688-
auto Inits = S->inits();
688+
ArrayRef<Expr *> Inits = S->inits();
689689
size_t InitIdx = 0;
690690

691+
// Unions initialized with an empty initializer list need special treatment.
692+
// For structs/classes initialized with an empty initializer list, Clang
693+
// puts `ImplicitValueInitExpr`s in `InitListExpr::inits()`, but for unions,
694+
// it doesn't do this -- so we create an `ImplicitValueInitExpr` ourselves.
695+
std::optional<ImplicitValueInitExpr> ImplicitValueInitForUnion;
696+
SmallVector<Expr *> InitsForUnion;
697+
if (S->getType()->isUnionType() && Inits.empty()) {
698+
assert(FieldsForInit.size() == 1);
699+
ImplicitValueInitForUnion.emplace(FieldsForInit.front()->getType());
700+
InitsForUnion.push_back(&*ImplicitValueInitForUnion);
701+
Inits = InitsForUnion;
702+
}
703+
691704
// Initialize base classes.
692705
if (auto* R = S->getType()->getAsCXXRecordDecl()) {
693706
assert(FieldsForInit.size() + R->getNumBases() == Inits.size());

clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Lines changed: 66 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2393,8 +2393,72 @@ TEST(TransferTest, InitListExprAsUnion) {
23932393
auto &FLoc = getFieldLoc<RecordStorageLocation>(
23942394
*Env.getThisPointeeStorageLocation(), "F", ASTCtx);
23952395
auto *AVal = cast<PointerValue>(getFieldValue(&FLoc, "a", ASTCtx, Env));
2396-
ASSERT_EQ(AVal, &getValueForDecl<PointerValue>(ASTCtx, Env, "null"));
2397-
ASSERT_EQ(getFieldValue(&FLoc, "b", ASTCtx, Env), nullptr);
2396+
EXPECT_EQ(AVal, &getValueForDecl<PointerValue>(ASTCtx, Env, "null"));
2397+
EXPECT_EQ(getFieldValue(&FLoc, "b", ASTCtx, Env), nullptr);
2398+
});
2399+
}
2400+
2401+
TEST(TransferTest, EmptyInitListExprForUnion) {
2402+
// This is a crash repro.
2403+
std::string Code = R"cc(
2404+
class target {
2405+
union {
2406+
int *a;
2407+
bool *b;
2408+
} F;
2409+
2410+
public:
2411+
constexpr target() : F{} {
2412+
int *null = nullptr;
2413+
F.b; // Make sure we reference 'b' so it is modeled.
2414+
// [[p]]
2415+
}
2416+
};
2417+
)cc";
2418+
runDataflow(
2419+
Code,
2420+
[](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
2421+
ASTContext &ASTCtx) {
2422+
const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
2423+
2424+
auto &FLoc = getFieldLoc<RecordStorageLocation>(
2425+
*Env.getThisPointeeStorageLocation(), "F", ASTCtx);
2426+
auto *AVal = cast<PointerValue>(getFieldValue(&FLoc, "a", ASTCtx, Env));
2427+
EXPECT_EQ(AVal, &getValueForDecl<PointerValue>(ASTCtx, Env, "null"));
2428+
EXPECT_EQ(getFieldValue(&FLoc, "b", ASTCtx, Env), nullptr);
2429+
});
2430+
}
2431+
2432+
TEST(TransferTest, EmptyInitListExprForStruct) {
2433+
std::string Code = R"cc(
2434+
class target {
2435+
struct {
2436+
int *a;
2437+
bool *b;
2438+
} F;
2439+
2440+
public:
2441+
constexpr target() : F{} {
2442+
int *NullIntPtr = nullptr;
2443+
bool *NullBoolPtr = nullptr;
2444+
// [[p]]
2445+
}
2446+
};
2447+
)cc";
2448+
runDataflow(
2449+
Code,
2450+
[](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
2451+
ASTContext &ASTCtx) {
2452+
const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
2453+
2454+
auto &FLoc = getFieldLoc<RecordStorageLocation>(
2455+
*Env.getThisPointeeStorageLocation(), "F", ASTCtx);
2456+
auto *AVal = cast<PointerValue>(getFieldValue(&FLoc, "a", ASTCtx, Env));
2457+
ASSERT_EQ(AVal,
2458+
&getValueForDecl<PointerValue>(ASTCtx, Env, "NullIntPtr"));
2459+
auto *BVal = cast<PointerValue>(getFieldValue(&FLoc, "b", ASTCtx, Env));
2460+
ASSERT_EQ(BVal,
2461+
&getValueForDecl<PointerValue>(ASTCtx, Env, "NullBoolPtr"));
23982462
});
23992463
}
24002464

0 commit comments

Comments
 (0)