[clang] [Clang] Ensure zero-init is not overridden when initializing a base class in a constant expression context (PR #70150)
Shafik Yaghmour via cfe-commits
cfe-commits at lists.llvm.org
Tue Oct 24 21:30:56 PDT 2023
https://github.com/shafik updated https://github.com/llvm/llvm-project/pull/70150
>From 1e7ec004102349a67519966bb7111838668ab2b4 Mon Sep 17 00:00:00 2001
From: Shafik Yaghmour <shafik.yaghmour at intel.com>
Date: Tue, 24 Oct 2023 16:21:30 -0700
Subject: [PATCH] [Clang] Ensure zero-init is not overridden when intializing a
base class in a constant expression context
During constant evaluation when value-initializing a class if the base class
was default-initialized it would undue the previously zero-initialized class
members. This fixes the way we handle default initialzation to avoid
initializing over an already initialized member to an indeterminate value.
Fixes: https://github.com/llvm/llvm-project/issues/69890
---
clang/docs/ReleaseNotes.rst | 4 +++
clang/lib/AST/ExprConstant.cpp | 33 +++++++++++--------
clang/test/AST/Interp/records.cpp | 7 ++--
.../dcl.init/dcl.init.general/p16-cxx20.cpp | 18 ++++++++++
4 files changed, 44 insertions(+), 18 deletions(-)
create mode 100644 clang/test/CXX/dcl.decl/dcl.init/dcl.init.general/p16-cxx20.cpp
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index a063733e96cb74c..ae2f082ad323a78 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -622,6 +622,10 @@ Bug Fixes to C++ Support
(`#46200 <https://github.com/llvm/llvm-project/issues/46200>`_)
(`#57812 <https://github.com/llvm/llvm-project/issues/57812>`_)
+- Fix bug where we were overriding zero-initialization of class members when
+ default initializing a base class in a constant expression context. Fixes:
+ (`#69890 <https://github.com/llvm/llvm-project/issues/69890>`_)
+
Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
- Fixed an import failure of recursive friend class template.
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index 6b47b8a1256477d..eea0827d6f7a8a1 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -4872,8 +4872,13 @@ static bool HandleBaseToDerivedCast(EvalInfo &Info, const CastExpr *E,
/// Get the value to use for a default-initialized object of type T.
/// Return false if it encounters something invalid.
-static bool getDefaultInitValue(QualType T, APValue &Result) {
+static bool handleDefaultInitValue(QualType T, APValue &Result) {
bool Success = true;
+
+ // If there is already a value present don't overwrite it.
+ if (!Result.isAbsent())
+ return true;
+
if (auto *RD = T->getAsCXXRecordDecl()) {
if (RD->isInvalidDecl()) {
Result = APValue();
@@ -4890,13 +4895,14 @@ static bool getDefaultInitValue(QualType T, APValue &Result) {
for (CXXRecordDecl::base_class_const_iterator I = RD->bases_begin(),
End = RD->bases_end();
I != End; ++I, ++Index)
- Success &= getDefaultInitValue(I->getType(), Result.getStructBase(Index));
+ Success &=
+ handleDefaultInitValue(I->getType(), Result.getStructBase(Index));
for (const auto *I : RD->fields()) {
if (I->isUnnamedBitfield())
continue;
- Success &= getDefaultInitValue(I->getType(),
- Result.getStructField(I->getFieldIndex()));
+ Success &= handleDefaultInitValue(
+ I->getType(), Result.getStructField(I->getFieldIndex()));
}
return Success;
}
@@ -4906,7 +4912,7 @@ static bool getDefaultInitValue(QualType T, APValue &Result) {
Result = APValue(APValue::UninitArray(), 0, AT->getSize().getZExtValue());
if (Result.hasArrayFiller())
Success &=
- getDefaultInitValue(AT->getElementType(), Result.getArrayFiller());
+ handleDefaultInitValue(AT->getElementType(), Result.getArrayFiller());
return Success;
}
@@ -4947,7 +4953,7 @@ static bool EvaluateVarDecl(EvalInfo &Info, const VarDecl *VD) {
if (!InitE) {
if (VD->getType()->isDependentType())
return Info.noteSideEffect();
- return getDefaultInitValue(VD->getType(), Val);
+ return handleDefaultInitValue(VD->getType(), Val);
}
if (InitE->isValueDependent())
return false;
@@ -6049,7 +6055,7 @@ struct StartLifetimeOfUnionMemberHandler {
return false;
}
APValue Result;
- Failed = !getDefaultInitValue(Field->getType(), Result);
+ Failed = !handleDefaultInitValue(Field->getType(), Result);
Subobj.setUnion(Field, Result);
return true;
}
@@ -6401,7 +6407,7 @@ static bool HandleConstructorCall(const Expr *E, const LValue &This,
for (; !declaresSameEntity(*FieldIt, FD); ++FieldIt) {
assert(FieldIt != RD->field_end() && "missing field?");
if (!FieldIt->isUnnamedBitfield())
- Success &= getDefaultInitValue(
+ Success &= handleDefaultInitValue(
FieldIt->getType(),
Result.getStructField(FieldIt->getFieldIndex()));
}
@@ -6458,7 +6464,8 @@ static bool HandleConstructorCall(const Expr *E, const LValue &This,
// FIXME: This immediately starts the lifetime of all members of
// an anonymous struct. It would be preferable to strictly start
// member lifetime in initialization order.
- Success &= getDefaultInitValue(Info.Ctx.getRecordType(CD), *Value);
+ Success &=
+ handleDefaultInitValue(Info.Ctx.getRecordType(CD), *Value);
}
// Store Subobject as its parent before updating it for the last element
// in the chain.
@@ -6510,7 +6517,7 @@ static bool HandleConstructorCall(const Expr *E, const LValue &This,
if (!RD->isUnion()) {
for (; FieldIt != RD->field_end(); ++FieldIt) {
if (!FieldIt->isUnnamedBitfield())
- Success &= getDefaultInitValue(
+ Success &= handleDefaultInitValue(
FieldIt->getType(),
Result.getStructField(FieldIt->getFieldIndex()));
}
@@ -9823,7 +9830,7 @@ bool PointerExprEvaluator::VisitCXXNewExpr(const CXXNewExpr *E) {
} else if (Init) {
if (!EvaluateInPlace(*Val, Info, Result, Init))
return false;
- } else if (!getDefaultInitValue(AllocType, *Val)) {
+ } else if (!handleDefaultInitValue(AllocType, *Val)) {
return false;
}
@@ -10233,7 +10240,7 @@ bool RecordExprEvaluator::VisitCXXConstructExpr(const CXXConstructExpr *E,
if (ZeroInit)
return ZeroInitialization(E, T);
- return getDefaultInitValue(T, Result);
+ return handleDefaultInitValue(T, Result);
}
const FunctionDecl *Definition = nullptr;
@@ -15655,7 +15662,7 @@ bool VarDecl::evaluateDestruction(
APValue DestroyedValue;
if (getEvaluatedValue() && !getEvaluatedValue()->isAbsent())
DestroyedValue = *getEvaluatedValue();
- else if (!getDefaultInitValue(getType(), DestroyedValue))
+ else if (!handleDefaultInitValue(getType(), DestroyedValue))
return false;
if (!EvaluateDestruction(getASTContext(), this, std::move(DestroyedValue),
diff --git a/clang/test/AST/Interp/records.cpp b/clang/test/AST/Interp/records.cpp
index a2e878f6132d0ac..e899e37915f0398 100644
--- a/clang/test/AST/Interp/records.cpp
+++ b/clang/test/AST/Interp/records.cpp
@@ -1042,13 +1042,10 @@ namespace TemporaryObjectExpr {
F f{12};
};
constexpr int foo(S x) {
- return x.a; // expected-note {{read of uninitialized object}} \
- // ref-note {{read of uninitialized object}}
+ return x.a; // expected-note {{read of uninitialized object}}
}
static_assert(foo(S()) == 0, ""); // expected-error {{not an integral constant expression}} \
- // expected-note {{in call to}} \
- // ref-error {{not an integral constant expression}} \
- // ref-note {{in call to}}
+ // expected-note {{in call to}}
};
#endif
}
diff --git a/clang/test/CXX/dcl.decl/dcl.init/dcl.init.general/p16-cxx20.cpp b/clang/test/CXX/dcl.decl/dcl.init/dcl.init.general/p16-cxx20.cpp
new file mode 100644
index 000000000000000..1ad205d769c3895
--- /dev/null
+++ b/clang/test/CXX/dcl.decl/dcl.init/dcl.init.general/p16-cxx20.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 %s
+
+// If the initializer is (), the object is value-initialized.
+
+// expected-no-diagnostics
+namespace GH69890 {
+struct A {
+ constexpr A() {}
+ int x;
+};
+
+struct B : A {
+ int y;
+};
+
+static_assert(B().x == 0);
+static_assert(B().y == 0);
+}
More information about the cfe-commits
mailing list