https://github.com/alejandro-alvarez-sonarsource updated
https://github.com/llvm/llvm-project/pull/121550
From fe4adf2f95aeb72436eead3567767c8c539811c3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
<alejandro.alva...@sonarsource.com>
Date: Fri, 3 Jan 2025 08:58:23 +0100
Subject: [PATCH 1/6] [clang] Do not serialize function definitions without a
body
An instantiated templated function definition may not have a body due
to parsing errors inside the templated function. When serializing, an
assert is tripped inside `ASTRecordWriter::AddFunctionDefinition` when
building with assertions enabled.
The instantiation may happen on an intermediate module.
The test case was reduced from `mp-units`.
---
clang/lib/Serialization/ASTWriter.cpp | 6 ++-
clang/test/Modules/missing-body-in-import.cpp | 42 +++++++++++++++++++
2 files changed, 46 insertions(+), 2 deletions(-)
create mode 100644 clang/test/Modules/missing-body-in-import.cpp
diff --git a/clang/lib/Serialization/ASTWriter.cpp
b/clang/lib/Serialization/ASTWriter.cpp
index 4a6027943072c0..36b9e3e2ba1720 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -6227,8 +6227,10 @@ void ASTWriter::WriteDeclUpdatesBlocks(ASTContext
&Context,
// Add a trailing update record, if any. These must go last because we
// lazily load their attached statement.
if (!GeneratingReducedBMI || !CanElideDeclDef(D)) {
- if (HasUpdatedBody) {
- const auto *Def = cast<FunctionDecl>(D);
+ assert(!(HasUpdatedBody && HasAddedVarDefinition) &&
+ "Declaration can not be both a FunctionDecl and a VarDecl");
+ if (const auto *Def = dyn_cast<FunctionDecl>(D);
+ HasUpdatedBody && Def->doesThisDeclarationHaveABody()) {
Record.push_back(UPD_CXX_ADDED_FUNCTION_DEFINITION);
Record.push_back(Def->isInlined());
Record.AddSourceLocation(Def->getInnerLocStart());
diff --git a/clang/test/Modules/missing-body-in-import.cpp
b/clang/test/Modules/missing-body-in-import.cpp
new file mode 100644
index 00000000000000..b52ebba15087a3
--- /dev/null
+++ b/clang/test/Modules/missing-body-in-import.cpp
@@ -0,0 +1,42 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: cd %t
+
+// RUN: %clang_cc1 -std=c++23 mod1.cppm -emit-module-interface -o mod1.pcm
-fallow-pcm-with-compiler-errors -verify
+// RUN: %clang_cc1 -std=c++23 mod2.cppm -emit-module-interface -o mod2.pcm
-fmodule-file=mod1=mod1.pcm -verify -fallow-pcm-with-compiler-errors
+// RUN: %clang_cc1 -std=c++23 mod3.cppm -emit-module-interface -o mod3.pcm
-fmodule-file=mod1=mod1.pcm -fmodule-file=mod2=mod2.pcm -verify
-fallow-pcm-with-compiler-errors
+// RUN: %clang_cc1 -std=c++23 main.cpp -fmodule-file=mod1=mod1.pcm
-fmodule-file=mod2=mod2.pcm -fmodule-file=mod3=mod3.pcm -verify
-fallow-pcm-with-compiler-errors -ast-dump-all
+
+//--- mod1.cppm
+export module mod1;
+
+export template <unsigned N, unsigned M>
+class A {
+public:
+ constexpr A(const char[], const char[]) {
+ auto x = BrokenExpr; // expected-error {{use of undeclared identifier
'BrokenExpr'}}
+ }
+};
+
+export template<A<1,1> NTTP>
+struct B {};
+
+template < unsigned N, unsigned M >
+A(const char (&)[N], const char (&)[M]) -> A< 1, 1 >;
+
+//--- mod2.cppm
+export module mod2;
+import mod1;
+
+struct C: B <A{"a", "b"}> { // expected-error {{non-type template argument is
not a constant expression}}
+ constexpr C(int a) { }
+};
+
+//--- mod3.cppm
+// expected-no-diagnostics
+export module mod3;
+export import mod2;
+
+//--- main.cpp
+// expected-no-diagnostics
+import mod3; // no crash
From 300707b5c11f576a35ba98404cad019af511a216 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
<alejandro.alva...@sonarsource.com>
Date: Mon, 6 Jan 2025 09:18:20 +0100
Subject: [PATCH 2/6] Do not set `HasUpdatedBody`
---
clang/lib/Serialization/ASTWriter.cpp | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/clang/lib/Serialization/ASTWriter.cpp
b/clang/lib/Serialization/ASTWriter.cpp
index 36b9e3e2ba1720..1ef794100e49e6 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -6092,9 +6092,10 @@ void ASTWriter::WriteDeclUpdatesBlocks(ASTContext
&Context,
// An updated body is emitted last, so that the reader doesn't need
// to skip over the lazy body to reach statements for other records.
- if (Kind == UPD_CXX_ADDED_FUNCTION_DEFINITION)
- HasUpdatedBody = true;
- else if (Kind == UPD_CXX_ADDED_VAR_DEFINITION)
+ if (Kind == UPD_CXX_ADDED_FUNCTION_DEFINITION) {
+ assert(isa<FunctionDecl>(D) && "expected FunctionDecl");
+ HasUpdatedBody = dyn_cast<FunctionDecl>(D)->hasBody();
+ } else if (Kind == UPD_CXX_ADDED_VAR_DEFINITION)
HasAddedVarDefinition = true;
else
Record.push_back(Kind);
@@ -6227,10 +6228,8 @@ void ASTWriter::WriteDeclUpdatesBlocks(ASTContext
&Context,
// Add a trailing update record, if any. These must go last because we
// lazily load their attached statement.
if (!GeneratingReducedBMI || !CanElideDeclDef(D)) {
- assert(!(HasUpdatedBody && HasAddedVarDefinition) &&
- "Declaration can not be both a FunctionDecl and a VarDecl");
- if (const auto *Def = dyn_cast<FunctionDecl>(D);
- HasUpdatedBody && Def->doesThisDeclarationHaveABody()) {
+ if (HasUpdatedBody) {
+ const auto *Def = cast<FunctionDecl>(D);
Record.push_back(UPD_CXX_ADDED_FUNCTION_DEFINITION);
Record.push_back(Def->isInlined());
Record.AddSourceLocation(Def->getInnerLocStart());
From 6e0dbf52044cfa97157ace00237ef979b9eab19f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
<alejandro.alva...@sonarsource.com>
Date: Mon, 6 Jan 2025 09:24:58 +0100
Subject: [PATCH 3/6] Update clang/lib/Serialization/ASTWriter.cpp
Co-authored-by: Chuanqi Xu <yedeng...@linux.alibaba.com>
---
clang/lib/Serialization/ASTWriter.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/lib/Serialization/ASTWriter.cpp
b/clang/lib/Serialization/ASTWriter.cpp
index 1ef794100e49e6..3bbb278acfa6c5 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -6094,7 +6094,7 @@ void ASTWriter::WriteDeclUpdatesBlocks(ASTContext
&Context,
// to skip over the lazy body to reach statements for other records.
if (Kind == UPD_CXX_ADDED_FUNCTION_DEFINITION) {
assert(isa<FunctionDecl>(D) && "expected FunctionDecl");
- HasUpdatedBody = dyn_cast<FunctionDecl>(D)->hasBody();
+ HasUpdatedBody = cast<FunctionDecl>(D)->hasBody();
} else if (Kind == UPD_CXX_ADDED_VAR_DEFINITION)
HasAddedVarDefinition = true;
else
From f0a8e435ef7a19591fc15c0974014cfddc0604f4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
<alejandro.alva...@sonarsource.com>
Date: Mon, 6 Jan 2025 09:27:37 +0100
Subject: [PATCH 4/6] Add missing braces
---
clang/lib/Serialization/ASTWriter.cpp | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Serialization/ASTWriter.cpp
b/clang/lib/Serialization/ASTWriter.cpp
index 3bbb278acfa6c5..099f050c438265 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -6095,10 +6095,11 @@ void ASTWriter::WriteDeclUpdatesBlocks(ASTContext
&Context,
if (Kind == UPD_CXX_ADDED_FUNCTION_DEFINITION) {
assert(isa<FunctionDecl>(D) && "expected FunctionDecl");
HasUpdatedBody = cast<FunctionDecl>(D)->hasBody();
- } else if (Kind == UPD_CXX_ADDED_VAR_DEFINITION)
+ } else if (Kind == UPD_CXX_ADDED_VAR_DEFINITION) {
HasAddedVarDefinition = true;
- else
+ } else {
Record.push_back(Kind);
+ }
switch (Kind) {
case UPD_CXX_ADDED_IMPLICIT_MEMBER:
From 3e2c300ba13bd299b3f0bce52f62729519174504 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
<alejandro.alva...@sonarsource.com>
Date: Mon, 6 Jan 2025 09:52:18 +0100
Subject: [PATCH 5/6] Move the check to FunctionDefinitionInstantiated and
CompletedImplicitDefinition
---
clang/lib/Serialization/ASTWriter.cpp | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/clang/lib/Serialization/ASTWriter.cpp
b/clang/lib/Serialization/ASTWriter.cpp
index 099f050c438265..fc66ac01d97f6c 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -6092,14 +6092,12 @@ void ASTWriter::WriteDeclUpdatesBlocks(ASTContext
&Context,
// An updated body is emitted last, so that the reader doesn't need
// to skip over the lazy body to reach statements for other records.
- if (Kind == UPD_CXX_ADDED_FUNCTION_DEFINITION) {
- assert(isa<FunctionDecl>(D) && "expected FunctionDecl");
- HasUpdatedBody = cast<FunctionDecl>(D)->hasBody();
- } else if (Kind == UPD_CXX_ADDED_VAR_DEFINITION) {
+ if (Kind == UPD_CXX_ADDED_FUNCTION_DEFINITION)
+ HasUpdatedBody = true;
+ else if (Kind == UPD_CXX_ADDED_VAR_DEFINITION)
HasAddedVarDefinition = true;
- } else {
+ else
Record.push_back(Kind);
- }
switch (Kind) {
case UPD_CXX_ADDED_IMPLICIT_MEMBER:
@@ -7232,6 +7230,9 @@ void ASTWriter::CompletedImplicitDefinition(const
FunctionDecl *D) {
if (!D->isFromASTFile())
return; // Declaration not imported from PCH.
+ if (!D->doesThisDeclarationHaveABody())
+ return; // The function definition may not have a body due to parsing
errors.
+
// Implicit function decl from a PCH was defined.
DeclUpdates[D].push_back(DeclUpdate(UPD_CXX_ADDED_FUNCTION_DEFINITION));
}
@@ -7250,6 +7251,8 @@ void ASTWriter::FunctionDefinitionInstantiated(const
FunctionDecl *D) {
assert(!WritingAST && "Already writing the AST!");
if (!D->isFromASTFile())
return;
+ if (!D->doesThisDeclarationHaveABody())
+ return; // The function definition may not have a body due to parsing
errors.
DeclUpdates[D].push_back(DeclUpdate(UPD_CXX_ADDED_FUNCTION_DEFINITION));
}
From cefbcf08a6b3ab7b4ccebd0efff23fad06e938a7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20=C3=81lvarez=20Ayll=C3=B3n?=
<alejandro.alva...@sonarsource.com>
Date: Mon, 6 Jan 2025 09:59:13 +0100
Subject: [PATCH 6/6] Move comment above
---
clang/lib/Serialization/ASTWriter.cpp | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/clang/lib/Serialization/ASTWriter.cpp
b/clang/lib/Serialization/ASTWriter.cpp
index fc66ac01d97f6c..7fa9322e7551f7 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -7230,8 +7230,9 @@ void ASTWriter::CompletedImplicitDefinition(const
FunctionDecl *D) {
if (!D->isFromASTFile())
return; // Declaration not imported from PCH.
+ // The function definition may not have a body due to parsing errors.
if (!D->doesThisDeclarationHaveABody())
- return; // The function definition may not have a body due to parsing
errors.
+ return;
// Implicit function decl from a PCH was defined.
DeclUpdates[D].push_back(DeclUpdate(UPD_CXX_ADDED_FUNCTION_DEFINITION));
@@ -7251,8 +7252,10 @@ void ASTWriter::FunctionDefinitionInstantiated(const
FunctionDecl *D) {
assert(!WritingAST && "Already writing the AST!");
if (!D->isFromASTFile())
return;
+
+ // The function definition may not have a body due to parsing errors.
if (!D->doesThisDeclarationHaveABody())
- return; // The function definition may not have a body due to parsing
errors.
+ return;
DeclUpdates[D].push_back(DeclUpdate(UPD_CXX_ADDED_FUNCTION_DEFINITION));
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
