https://github.com/ilya-biryukov updated 
https://github.com/llvm/llvm-project/pull/79875

>From 6e474e393e63fd1cb5f3b0bea3c971b96591c57f Mon Sep 17 00:00:00 2001
From: Ilya Biryukov <ibiryu...@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 ede787adb74ded8a5145facd9518fe3d1824604a Mon Sep 17 00:00:00 2001
From: Ilya Biryukov <ibiryu...@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 4a6e1d23161be..5491128789c75 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -43,6 +43,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"
@@ -9403,6 +9404,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 9bad8377233b3f32af51d9930813c0706dd0715c Mon Sep 17 00:00:00 2001
From: Ilya Biryukov <ibiryu...@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 549f165bed9dc7af7a9b8ed0e03a0e6bbde19dfb Mon Sep 17 00:00:00 2001
From: Ilya Biryukov <ibiryu...@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 5491128789c75..0dbbd2733f953 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -9405,7 +9405,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;

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to