[clang] [Serialization] Check for stack exhaustion when reading declarations (PR #79875)
Ilya Biryukov via cfe-commits
cfe-commits at lists.llvm.org
Mon Jun 3 08:07:03 PDT 2024
https://github.com/ilya-biryukov updated https://github.com/llvm/llvm-project/pull/79875
>From db8b091568c924a9550052ea643f718ac11d3e73 Mon Sep 17 00:00:00 2001
From: Ilya Biryukov <ibiryukov at google.com>
Date: Mon, 29 Jan 2024 18:55:53 +0100
Subject: [PATCH 1/4] [Serialization] Check for stack exhaustion when reading
declarations
Particular example that lead to this is a very long chain of
`UsingShadowDecl`s that we hit in our codebase in generated code.
To avoid that, check for stack exhaustion when deserializing the
declaration. At that point, we can point to source location of a
particular declaration that is being deserialized.
---
clang/lib/Serialization/ASTReaderDecl.cpp | 4 +-
.../Modules/lots-of-using-shadow-decls.cpp | 43 +++++++++++++++++++
2 files changed, 46 insertions(+), 1 deletion(-)
create mode 100644 clang/test/Modules/lots-of-using-shadow-decls.cpp
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index 61cc99d4df687..ba363c0d773a1 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -4127,7 +4127,9 @@ Decl *ASTReader::ReadDeclRecord(GlobalDeclID ID) {
// calls to Decl::getASTContext() by Decl's methods will find the
// TranslationUnitDecl without crashing.
D->setDeclContext(Context.getTranslationUnitDecl());
- Reader.Visit(D);
+
+ // Reading some declarations can result in deep recursion.
+ SemaObj->runWithSufficientStackSpace(DeclLoc, [&] { Reader.Visit(D); });
// If this declaration is also a declaration context, get the
// offsets for its tables of lexical and visible declarations.
diff --git a/clang/test/Modules/lots-of-using-shadow-decls.cpp b/clang/test/Modules/lots-of-using-shadow-decls.cpp
new file mode 100644
index 0000000000000..c3048352842e3
--- /dev/null
+++ b/clang/test/Modules/lots-of-using-shadow-decls.cpp
@@ -0,0 +1,43 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/usings.cppm -o %t/usings.pcm
+// RUN: %clang_cc1 -std=c++20 -fmodule-file=usings=%t/usings.pcm %t/use.cpp -verify -fsyntax-only -Wno-stack-exhausted
+
+// expected-no-diagnostics
+
+//--- usings.cppm
+export module usings;
+
+#define TYPES1(NAME) DECLARE(NAME##a) DECLARE(NAME##b) DECLARE(NAME##c) \
+ DECLARE(NAME##d) DECLARE(NAME##e) DECLARE(NAME##f) DECLARE(NAME##g) \
+ DECLARE(NAME##h) DECLARE(NAME##i) DECLARE(NAME##j)
+#define TYPES2(NAME) TYPES1(NAME##a) TYPES1(NAME##b) TYPES1(NAME##c) \
+ TYPES1(NAME##d) TYPES1(NAME##e) TYPES1(NAME##f) TYPES1(NAME##g) \
+ TYPES1(NAME##h) TYPES1(NAME##i) TYPES1(NAME##j)
+#define TYPES3(NAME) TYPES2(NAME##a) TYPES2(NAME##b) TYPES2(NAME##c) \
+ TYPES2(NAME##d) TYPES2(NAME##e) TYPES2(NAME##f) TYPES2(NAME##g) \
+ TYPES2(NAME##h) TYPES2(NAME##i) TYPES2(NAME##j)
+#define TYPES4(NAME) TYPES3(NAME##a) TYPES3(NAME##b) TYPES3(NAME##c) \
+ TYPES3(NAME##d) TYPES3(NAME##e) TYPES3(NAME##f) TYPES3(NAME##g)
+
+#define DECLARE(NAME) struct NAME {};
+TYPES4(Type)
+
+export struct Base {
+#undef DECLARE
+#define DECLARE(NAME) void func(NAME*);
+TYPES4(Type)
+};
+
+export struct Derived : Base {
+ using Base::func;
+};
+
+//--- use.cpp
+import usings;
+void test() {
+ Derived().func(nullptr); // expected-error{{ambiguous}}
+ // expected-note@* + {{candidate function}}
+}
>From fac778f48eca1cf7ec1456465f5c7a7b0691323d Mon Sep 17 00:00:00 2001
From: Ilya Biryukov <ibiryukov at google.com>
Date: Wed, 29 May 2024 17:05:18 +0200
Subject: [PATCH 2/4] fixup! [Serialization] Check for stack exhaustion when
reading declarations
Use clang::runWithSufficientStackSpace, fallback to Sema only when
available.
---
clang/include/clang/Serialization/ASTReader.h | 5 +++++
clang/lib/Serialization/ASTReader.cpp | 15 +++++++++++++++
clang/lib/Serialization/ASTReaderDecl.cpp | 4 +++-
3 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h
index 4ece4593f0738..bea58d60d36c4 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -429,6 +429,9 @@ class ASTReader
FileManager &FileMgr;
const PCHContainerReader &PCHContainerRdr;
DiagnosticsEngine &Diags;
+ // Sema has duplicate logic, but SemaObj can sometimes be null so ASTReader
+ // has its own version.
+ bool WarnedStackExhausted = false;
/// The semantic analysis object that will be processing the
/// AST files and the translation unit that uses it.
@@ -2135,6 +2138,8 @@ class ASTReader
/// Report a diagnostic.
DiagnosticBuilder Diag(SourceLocation Loc, unsigned DiagID) const;
+ void warnStackExhausted(SourceLocation Loc);
+
IdentifierInfo *DecodeIdentifierInfo(serialization::IdentifierID ID);
IdentifierInfo *readIdentifier(ModuleFile &M, const RecordData &Record,
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index ae72fcdd26119..a62164a0bb40d 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -44,6 +44,7 @@
#include "clang/Basic/CommentOptions.h"
#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/DiagnosticError.h"
+#include "clang/Basic/DiagnosticIDs.h"
#include "clang/Basic/DiagnosticOptions.h"
#include "clang/Basic/DiagnosticSema.h"
#include "clang/Basic/ExceptionSpecificationType.h"
@@ -9404,6 +9405,20 @@ DiagnosticBuilder ASTReader::Diag(SourceLocation Loc, unsigned DiagID) const {
return Diags.Report(Loc, DiagID);
}
+void ASTReader::warnStackExhausted(SourceLocation Loc) {
+ // When Sema is available, avoid duplicate errors. This should be rare.
+ if (SemaObj) {
+ SemaObj->warnStackExhausted(Loc);
+ return;
+ }
+
+ if (WarnedStackExhausted)
+ return;
+ WarnedStackExhausted = true;
+
+ Diag(Loc, diag::warn_stack_exhausted);
+}
+
/// Retrieve the identifier table associated with the
/// preprocessor.
IdentifierTable &ASTReader::getIdentifierTable() {
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index ba363c0d773a1..765c083ce8df8 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -49,6 +49,7 @@
#include "clang/Basic/PragmaKinds.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/Specifiers.h"
+#include "clang/Basic/Stack.h"
#include "clang/Sema/IdentifierResolver.h"
#include "clang/Serialization/ASTBitCodes.h"
#include "clang/Serialization/ASTRecordReader.h"
@@ -4129,7 +4130,8 @@ Decl *ASTReader::ReadDeclRecord(GlobalDeclID ID) {
D->setDeclContext(Context.getTranslationUnitDecl());
// Reading some declarations can result in deep recursion.
- SemaObj->runWithSufficientStackSpace(DeclLoc, [&] { Reader.Visit(D); });
+ clang::runWithSufficientStackSpace([&] { warnStackExhausted(DeclLoc); },
+ [&] { Reader.Visit(D); });
// If this declaration is also a declaration context, get the
// offsets for its tables of lexical and visible declarations.
>From 7913bc8dfad40eb2d02432cda80da26efddf4c33 Mon Sep 17 00:00:00 2001
From: Ilya Biryukov <ibiryukov at google.com>
Date: Wed, 29 May 2024 17:11:11 +0200
Subject: [PATCH 3/4] fixup! [Serialization] Check for stack exhaustion when
reading declarations
remove the test, it was there for illustrative purposes
---
.../Modules/lots-of-using-shadow-decls.cpp | 43 -------------------
1 file changed, 43 deletions(-)
delete mode 100644 clang/test/Modules/lots-of-using-shadow-decls.cpp
diff --git a/clang/test/Modules/lots-of-using-shadow-decls.cpp b/clang/test/Modules/lots-of-using-shadow-decls.cpp
deleted file mode 100644
index c3048352842e3..0000000000000
--- a/clang/test/Modules/lots-of-using-shadow-decls.cpp
+++ /dev/null
@@ -1,43 +0,0 @@
-// RUN: rm -rf %t
-// RUN: mkdir -p %t
-// RUN: split-file %s %t
-
-// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/usings.cppm -o %t/usings.pcm
-// RUN: %clang_cc1 -std=c++20 -fmodule-file=usings=%t/usings.pcm %t/use.cpp -verify -fsyntax-only -Wno-stack-exhausted
-
-// expected-no-diagnostics
-
-//--- usings.cppm
-export module usings;
-
-#define TYPES1(NAME) DECLARE(NAME##a) DECLARE(NAME##b) DECLARE(NAME##c) \
- DECLARE(NAME##d) DECLARE(NAME##e) DECLARE(NAME##f) DECLARE(NAME##g) \
- DECLARE(NAME##h) DECLARE(NAME##i) DECLARE(NAME##j)
-#define TYPES2(NAME) TYPES1(NAME##a) TYPES1(NAME##b) TYPES1(NAME##c) \
- TYPES1(NAME##d) TYPES1(NAME##e) TYPES1(NAME##f) TYPES1(NAME##g) \
- TYPES1(NAME##h) TYPES1(NAME##i) TYPES1(NAME##j)
-#define TYPES3(NAME) TYPES2(NAME##a) TYPES2(NAME##b) TYPES2(NAME##c) \
- TYPES2(NAME##d) TYPES2(NAME##e) TYPES2(NAME##f) TYPES2(NAME##g) \
- TYPES2(NAME##h) TYPES2(NAME##i) TYPES2(NAME##j)
-#define TYPES4(NAME) TYPES3(NAME##a) TYPES3(NAME##b) TYPES3(NAME##c) \
- TYPES3(NAME##d) TYPES3(NAME##e) TYPES3(NAME##f) TYPES3(NAME##g)
-
-#define DECLARE(NAME) struct NAME {};
-TYPES4(Type)
-
-export struct Base {
-#undef DECLARE
-#define DECLARE(NAME) void func(NAME*);
-TYPES4(Type)
-};
-
-export struct Derived : Base {
- using Base::func;
-};
-
-//--- use.cpp
-import usings;
-void test() {
- Derived().func(nullptr); // expected-error{{ambiguous}}
- // expected-note@* + {{candidate function}}
-}
>From de978cf091688ee9a85752ddb058379d44570c8a Mon Sep 17 00:00:00 2001
From: Ilya Biryukov <ibiryukov at google.com>
Date: Mon, 3 Jun 2024 16:56:53 +0200
Subject: [PATCH 4/4] fixup! [Serialization] Check for stack exhaustion when
reading declarations
Update the comment according to the review suggestion.
---
clang/lib/Serialization/ASTReader.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index a62164a0bb40d..2bc00e1bb3e38 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -9406,7 +9406,7 @@ DiagnosticBuilder ASTReader::Diag(SourceLocation Loc, unsigned DiagID) const {
}
void ASTReader::warnStackExhausted(SourceLocation Loc) {
- // When Sema is available, avoid duplicate errors. This should be rare.
+ // When Sema is available, avoid duplicate errors.
if (SemaObj) {
SemaObj->warnStackExhausted(Loc);
return;
More information about the cfe-commits
mailing list