[clang] e7a811b - PR45133: Don't crash if the active member of a union changes while it's
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Tue Mar 17 20:37:22 PDT 2020
Author: Richard Smith
Date: 2020-03-17T20:37:14-07:00
New Revision: e7a811b3193ace664e02e07924305e0b9e3c3fd7
URL: https://github.com/llvm/llvm-project/commit/e7a811b3193ace664e02e07924305e0b9e3c3fd7
DIFF: https://github.com/llvm/llvm-project/commit/e7a811b3193ace664e02e07924305e0b9e3c3fd7.diff
LOG: PR45133: Don't crash if the active member of a union changes while it's
in the process of being initialized.
Added:
Modified:
clang/include/clang/Basic/DiagnosticASTKinds.td
clang/lib/AST/ExprConstant.cpp
clang/test/SemaCXX/constant-expression-cxx2a.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Basic/DiagnosticASTKinds.td b/clang/include/clang/Basic/DiagnosticASTKinds.td
index 48bafeeff208..544573edffdf 100644
--- a/clang/include/clang/Basic/DiagnosticASTKinds.td
+++ b/clang/include/clang/Basic/DiagnosticASTKinds.td
@@ -186,6 +186,9 @@ def note_constexpr_access_inactive_union_member : Note<
"construction of subobject of|destruction of}0 "
"member %1 of union with %select{active member %3|no active member}2 "
"is not allowed in a constant expression">;
+def note_constexpr_union_member_change_during_init : Note<
+ "assignment would change active union member during the initialization of "
+ "a
diff erent member of the same union">;
def note_constexpr_access_static_temporary : Note<
"%select{read of|read of|assignment to|increment of|decrement of|"
"member call on|dynamic_cast of|typeid applied to|reconstruction of|"
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 5bb9fc319d48..f5c37ad44ebd 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -674,6 +674,7 @@ namespace {
None,
Bases,
AfterBases,
+ AfterFields,
Destroying,
DestroyingBases
};
@@ -821,6 +822,9 @@ namespace {
void finishedConstructingBases() {
EI.ObjectsUnderConstruction[Object] = ConstructionPhase::AfterBases;
}
+ void finishedConstructingFields() {
+ EI.ObjectsUnderConstruction[Object] = ConstructionPhase::AfterFields;
+ }
~EvaluatingConstructorRAII() {
if (DidInsert) EI.ObjectsUnderConstruction.erase(Object);
}
@@ -5121,6 +5125,7 @@ static Optional<DynamicType> ComputeDynamicType(EvalInfo &Info, const Expr *E,
case ConstructionPhase::None:
case ConstructionPhase::AfterBases:
+ case ConstructionPhase::AfterFields:
case ConstructionPhase::Destroying:
// We've finished constructing the base classes and not yet started
// destroying them again, so this is the dynamic type.
@@ -5339,7 +5344,10 @@ static bool HandleDynamicCast(EvalInfo &Info, const ExplicitCastExpr *E,
namespace {
struct StartLifetimeOfUnionMemberHandler {
+ EvalInfo &Info;
+ const Expr *LHSExpr;
const FieldDecl *Field;
+ bool DuringInit;
static const AccessKinds AccessKind = AK_Assign;
@@ -5355,9 +5363,21 @@ struct StartLifetimeOfUnionMemberHandler {
// * No variant members' lifetimes begin
// * All scalar subobjects whose lifetimes begin have indeterminate values
assert(SubobjType->isUnionType());
- if (!declaresSameEntity(Subobj.getUnionField(), Field) ||
- !Subobj.getUnionValue().hasValue())
- Subobj.setUnion(Field, getDefaultInitValue(Field->getType()));
+ if (declaresSameEntity(Subobj.getUnionField(), Field)) {
+ // This union member is already active. If it's also in-lifetime, there's
+ // nothing to do.
+ if (Subobj.getUnionValue().hasValue())
+ return true;
+ } else if (DuringInit) {
+ // We're currently in the process of initializing a
diff erent union
+ // member. If we carried on, that initialization would attempt to
+ // store to an inactive union member, resulting in undefined behavior.
+ Info.FFDiag(LHSExpr,
+ diag::note_constexpr_union_member_change_during_init);
+ return false;
+ }
+
+ Subobj.setUnion(Field, getDefaultInitValue(Field->getType()));
return true;
}
bool found(APSInt &Value, QualType SubobjType) {
@@ -5460,7 +5480,10 @@ static bool HandleUnionActiveMemberChange(EvalInfo &Info, const Expr *LHSExpr,
SubobjectDesignator D = LHS.Designator;
D.truncate(Info.Ctx, LHS.Base, LengthAndField.first);
- StartLifetimeOfUnionMemberHandler StartLifetime{LengthAndField.second};
+ bool DuringInit = Info.isEvaluatingCtorDtor(LHS.Base, D.Entries) ==
+ ConstructionPhase::AfterBases;
+ StartLifetimeOfUnionMemberHandler StartLifetime{
+ Info, LHSExpr, LengthAndField.second, DuringInit};
if (!findSubobject(Info, LHSExpr, Obj, D, StartLifetime))
return false;
}
@@ -5778,6 +5801,8 @@ static bool HandleConstructorCall(const Expr *E, const LValue &This,
}
}
+ EvalObj.finishedConstructingFields();
+
return Success &&
EvaluateStmt(Ret, Info, Definition->getBody()) != ESR_Failed &&
LifetimeExtendedScope.destroy();
@@ -9180,6 +9205,8 @@ bool RecordExprEvaluator::VisitInitListExpr(const InitListExpr *E) {
}
}
+ EvalObj.finishedConstructingFields();
+
return Success;
}
diff --git a/clang/test/SemaCXX/constant-expression-cxx2a.cpp b/clang/test/SemaCXX/constant-expression-cxx2a.cpp
index 01d2a7f58d48..42d7300c5fde 100644
--- a/clang/test/SemaCXX/constant-expression-cxx2a.cpp
+++ b/clang/test/SemaCXX/constant-expression-cxx2a.cpp
@@ -1346,3 +1346,38 @@ namespace mutable_subobjects {
auto &zti = typeid(z.y);
static_assert(&zti == &typeid(Y));
}
+
+namespace PR45133 {
+ struct A { long x; };
+
+ union U;
+ constexpr A foo(U *up);
+
+ union U {
+ A a = foo(this); // expected-note {{in call to 'foo(&u)'}}
+ int y;
+ };
+
+ constexpr A foo(U *up) {
+ up->y = 11; // expected-note {{assignment would change active union member during the initialization of a
diff erent member}}
+ return {42};
+ }
+
+ constinit U u = {}; // expected-error {{constant init}} expected-note {{constinit}}
+
+ template<int> struct X {};
+
+ union V {
+ int a, b;
+ constexpr V(X<0>) : a(a = 1) {} // ok
+ constexpr V(X<1>) : a(b = 1) {} // expected-note {{assignment would change active union member during the initialization of a
diff erent member}}
+ constexpr V(X<2>) : a() { b = 1; } // ok
+ // This case (changing the active member then changing it back) is debatable,
+ // but it seems appropriate to reject.
+ constexpr V(X<3>) : a((b = 1, a = 1)) {} // expected-note {{assignment would change active union member during the initialization of a
diff erent member}}
+ };
+ constinit V v0 = X<0>();
+ constinit V v1 = X<1>(); // expected-error {{constant init}} expected-note {{constinit}} expected-note {{in call}}
+ constinit V v2 = X<2>();
+ constinit V v3 = X<3>(); // expected-error {{constant init}} expected-note {{constinit}} expected-note {{in call}}
+}
More information about the cfe-commits
mailing list