[clang] Fix crash with modules and constexpr destructor (PR #69076)

[Jonas Hahnfeld via cfe-commits]

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.hahnfeld at 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.hahnfeld at 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.hahnfeld at 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