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