https://github.com/hahnjo updated https://github.com/llvm/llvm-project/pull/69076
>From a55ca99a373b17501d56d18af9e8aa2dc2cbcea0 Mon Sep 17 00:00:00 2001 From: Jonas Hahnfeld <jonas.hahnf...@cern.ch> Date: Sat, 14 Oct 2023 20:10:28 +0200 Subject: [PATCH 1/3] Fix crash with modules and constexpr destructor With modules, serialization might omit the outer ExprWithCleanups as it calls ParmVarDecl::getDefaultArg(). Complementary to fixing this in a separate change, make the code more robust by adding a FullExpressionRAII and avoid the llvm_unreachable in the added test clang/test/Modules/pr68702.cpp. Closes https://github.com/llvm/llvm-project/issues/68702 --- clang/lib/AST/ExprConstant.cpp | 16 ++++++--- clang/test/Modules/pr68702.cpp | 65 ++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 4 deletions(-) create mode 100644 clang/test/Modules/pr68702.cpp diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index f6aeee1a4e935d..416d48ae82933f 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -15754,10 +15754,18 @@ bool Expr::EvaluateAsInitializer(APValue &Value, const ASTContext &Ctx, LValue LVal; LVal.set(VD); - if (!EvaluateInPlace(Value, Info, LVal, this, - /*AllowNonLiteralTypes=*/true) || - EStatus.HasSideEffects) - return false; + { + // C++23 [intro.execution]/p5 + // A full-expression is ... an init-declarator ([dcl.decl]) or a + // mem-initializer. + // So we need to make sure temporary objects are destroyed after having + // evaluated the expression (per C++23 [class.temporary]/p4). + FullExpressionRAII Scope(Info); + if (!EvaluateInPlace(Value, Info, LVal, this, + /*AllowNonLiteralTypes=*/true) || + EStatus.HasSideEffects) + return false; + } // At this point, any lifetime-extended temporaries are completely // initialized. diff --git a/clang/test/Modules/pr68702.cpp b/clang/test/Modules/pr68702.cpp new file mode 100644 index 00000000000000..d32f946910f4fb --- /dev/null +++ b/clang/test/Modules/pr68702.cpp @@ -0,0 +1,65 @@ +// RUN: rm -rf %t +// RUN: mkdir %t +// RUN: split-file %s %t + +// RUN: %clang_cc1 -std=c++20 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %t/main.cpp -o %t/main.o + +//--- V.h +#ifndef V_H +#define V_H + +class A { +public: + constexpr A() { } + constexpr ~A() { } +}; + +template <typename T> +class V { +public: + V() = default; + + constexpr V(int n, const A& a = A()) {} +}; + +#endif + +//--- inst1.h +#include "V.h" + +static void inst1() { + V<int> v; +} + +//--- inst2.h +#include "V.h" + +static void inst2() { + V<int> v(100); +} + +//--- module.modulemap +module "M" { + export * + module "V.h" { + export * + header "V.h" + } + module "inst1.h" { + export * + header "inst1.h" + } +} + +module "inst2.h" { + export * + header "inst2.h" +} + +//--- main.cpp +#include "V.h" +#include "inst2.h" + +static void m() { + static V<int> v(100); +} >From 8381d35fc3e1dd57ba0dd2a76aea2931c659e419 Mon Sep 17 00:00:00 2001 From: Jonas Hahnfeld <jonas.hahnf...@cern.ch> Date: Wed, 10 Jan 2024 08:41:59 +0100 Subject: [PATCH 2/3] Expand comment --- clang/lib/AST/ExprConstant.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp index 416d48ae82933f..f20850d14c0c86 100644 --- a/clang/lib/AST/ExprConstant.cpp +++ b/clang/lib/AST/ExprConstant.cpp @@ -15760,6 +15760,10 @@ bool Expr::EvaluateAsInitializer(APValue &Value, const ASTContext &Ctx, // mem-initializer. // So we need to make sure temporary objects are destroyed after having // evaluated the expression (per C++23 [class.temporary]/p4). + // + // FIXME: Otherwise this may break test/Modules/pr68702.cpp because the + // serialization code calls ParmVarDecl::getDefaultArg() which strips the + // outermost FullExpr, such as ExprWithCleanups. FullExpressionRAII Scope(Info); if (!EvaluateInPlace(Value, Info, LVal, this, /*AllowNonLiteralTypes=*/true) || >From 676c7ea4ad35ae9114023573d5698a22adeb1460 Mon Sep 17 00:00:00 2001 From: Jonas Hahnfeld <jonas.hahnf...@cern.ch> Date: Wed, 10 Jan 2024 08:47:50 +0100 Subject: [PATCH 3/3] Add a release note --- clang/docs/ReleaseNotes.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index ee211c16a48ac8..aa3252b1d4f5f4 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -877,6 +877,8 @@ Miscellaneous Clang Crashes Fixed - Fixed a crash when a lambda marked as ``static`` referenced a captured variable in an expression. `Issue 74608 <https://github.com/llvm/llvm-project/issues/74608>`_ +- Fixed a crash with modules and a ``constexpr`` destructor. + `Issue 68702 <https://github.com/llvm/llvm-project/issues/68702>`_ OpenACC Specific Changes _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits