Skip to content

Commit 607c26e

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 cd64b0e commit 607c26e

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
@@ -2408,8 +2408,72 @@ TEST(TransferTest, InitListExprAsUnion) {
24082408
auto &FLoc = getFieldLoc<RecordStorageLocation>(
24092409
*Env.getThisPointeeStorageLocation(), "F", ASTCtx);
24102410
auto *AVal = cast<PointerValue>(getFieldValue(&FLoc, "a", ASTCtx, Env));
2411-
ASSERT_EQ(AVal, &getValueForDecl<PointerValue>(ASTCtx, Env, "null"));
2412-
ASSERT_EQ(getFieldValue(&FLoc, "b", ASTCtx, Env), nullptr);
2411+
EXPECT_EQ(AVal, &getValueForDecl<PointerValue>(ASTCtx, Env, "null"));
2412+
EXPECT_EQ(getFieldValue(&FLoc, "b", ASTCtx, Env), nullptr);
2413+
});
2414+
}
2415+
2416+
TEST(TransferTest, EmptyInitListExprForUnion) {
2417+
// This is a crash repro.
2418+
std::string Code = R"cc(
2419+
class target {
2420+
union {
2421+
int *a;
2422+
bool *b;
2423+
} F;
2424+
2425+
public:
2426+
constexpr target() : F{} {
2427+
int *null = nullptr;
2428+
F.b; // Make sure we reference 'b' so it is modeled.
2429+
// [[p]]
2430+
}
2431+
};
2432+
)cc";
2433+
runDataflow(
2434+
Code,
2435+
[](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
2436+
ASTContext &ASTCtx) {
2437+
const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
2438+
2439+
auto &FLoc = getFieldLoc<RecordStorageLocation>(
2440+
*Env.getThisPointeeStorageLocation(), "F", ASTCtx);
2441+
auto *AVal = cast<PointerValue>(getFieldValue(&FLoc, "a", ASTCtx, Env));
2442+
EXPECT_EQ(AVal, &getValueForDecl<PointerValue>(ASTCtx, Env, "null"));
2443+
EXPECT_EQ(getFieldValue(&FLoc, "b", ASTCtx, Env), nullptr);
2444+
});
2445+
}
2446+
2447+
TEST(TransferTest, EmptyInitListExprForStruct) {
2448+
std::string Code = R"cc(
2449+
class target {
2450+
struct {
2451+
int *a;
2452+
bool *b;
2453+
} F;
2454+
2455+
public:
2456+
constexpr target() : F{} {
2457+
int *NullIntPtr = nullptr;
2458+
bool *NullBoolPtr = nullptr;
2459+
// [[p]]
2460+
}
2461+
};
2462+
)cc";
2463+
runDataflow(
2464+
Code,
2465+
[](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
2466+
ASTContext &ASTCtx) {
2467+
const Environment &Env = getEnvironmentAtAnnotation(Results, "p");
2468+
2469+
auto &FLoc = getFieldLoc<RecordStorageLocation>(
2470+
*Env.getThisPointeeStorageLocation(), "F", ASTCtx);
2471+
auto *AVal = cast<PointerValue>(getFieldValue(&FLoc, "a", ASTCtx, Env));
2472+
ASSERT_EQ(AVal,
2473+
&getValueForDecl<PointerValue>(ASTCtx, Env, "NullIntPtr"));
2474+
auto *BVal = cast<PointerValue>(getFieldValue(&FLoc, "b", ASTCtx, Env));
2475+
ASSERT_EQ(BVal,
2476+
&getValueForDecl<PointerValue>(ASTCtx, Env, "NullBoolPtr"));
24132477
});
24142478
}
24152479

0 commit comments

Comments
 (0)