Skip to content

Commit 844f833

Browse files
authored
Fix crash with modules and constexpr destructor (#69076)
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 #68702
1 parent ce1f946 commit 844f833

File tree

3 files changed

+83
-4
lines changed

3 files changed

+83
-4
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -958,6 +958,8 @@ Miscellaneous Clang Crashes Fixed
958958
- Fixed a crash when a lambda marked as ``static`` referenced a captured
959959
variable in an expression.
960960
`Issue 74608 <https://github.com/llvm/llvm-project/issues/74608>`_
961+
- Fixed a crash with modules and a ``constexpr`` destructor.
962+
`Issue 68702 <https://github.com/llvm/llvm-project/issues/68702>`_
961963

962964

963965
OpenACC Specific Changes

clang/lib/AST/ExprConstant.cpp

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15754,10 +15754,22 @@ bool Expr::EvaluateAsInitializer(APValue &Value, const ASTContext &Ctx,
1575415754
LValue LVal;
1575515755
LVal.set(VD);
1575615756

15757-
if (!EvaluateInPlace(Value, Info, LVal, this,
15758-
/*AllowNonLiteralTypes=*/true) ||
15759-
EStatus.HasSideEffects)
15760-
return false;
15757+
{
15758+
// C++23 [intro.execution]/p5
15759+
// A full-expression is ... an init-declarator ([dcl.decl]) or a
15760+
// mem-initializer.
15761+
// So we need to make sure temporary objects are destroyed after having
15762+
// evaluated the expression (per C++23 [class.temporary]/p4).
15763+
//
15764+
// FIXME: Otherwise this may break test/Modules/pr68702.cpp because the
15765+
// serialization code calls ParmVarDecl::getDefaultArg() which strips the
15766+
// outermost FullExpr, such as ExprWithCleanups.
15767+
FullExpressionRAII Scope(Info);
15768+
if (!EvaluateInPlace(Value, Info, LVal, this,
15769+
/*AllowNonLiteralTypes=*/true) ||
15770+
EStatus.HasSideEffects)
15771+
return false;
15772+
}
1576115773

1576215774
// At this point, any lifetime-extended temporaries are completely
1576315775
// initialized.

clang/test/Modules/pr68702.cpp

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
// RUN: rm -rf %t
2+
// RUN: mkdir %t
3+
// RUN: split-file %s %t
4+
5+
// RUN: %clang_cc1 -std=c++20 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t %t/main.cpp -o %t/main.o
6+
7+
//--- V.h
8+
#ifndef V_H
9+
#define V_H
10+
11+
class A {
12+
public:
13+
constexpr A() { }
14+
constexpr ~A() { }
15+
};
16+
17+
template <typename T>
18+
class V {
19+
public:
20+
V() = default;
21+
22+
constexpr V(int n, const A& a = A()) {}
23+
};
24+
25+
#endif
26+
27+
//--- inst1.h
28+
#include "V.h"
29+
30+
static void inst1() {
31+
V<int> v;
32+
}
33+
34+
//--- inst2.h
35+
#include "V.h"
36+
37+
static void inst2() {
38+
V<int> v(100);
39+
}
40+
41+
//--- module.modulemap
42+
module "M" {
43+
export *
44+
module "V.h" {
45+
export *
46+
header "V.h"
47+
}
48+
module "inst1.h" {
49+
export *
50+
header "inst1.h"
51+
}
52+
}
53+
54+
module "inst2.h" {
55+
export *
56+
header "inst2.h"
57+
}
58+
59+
//--- main.cpp
60+
#include "V.h"
61+
#include "inst2.h"
62+
63+
static void m() {
64+
static V<int> v(100);
65+
}

0 commit comments

Comments
 (0)