https://github.com/bricknerb updated https://github.com/llvm/llvm-project/pull/124128
>From 06bf61b6e3900edb99bcbe1c48e1581463f1ec11 Mon Sep 17 00:00:00 2001 From: Boaz Brickner <brick...@google.com> Date: Thu, 23 Jan 2025 16:03:22 +0100 Subject: [PATCH 1/2] [clang] Fix segmentation fault caused by VarBypassDetector stack overflow on deeply nested expressions This happens when using -O2. Added a test that reproduces it based on a test that was reverted in #111701: https://github.com/bricknerb/llvm-project/blob/93e4a7386ec897e53d7330c6206d38759a858be2/clang/test/CodeGen/deeply-nested-expressions.cpp However, this test is slow and likely to be hard to maintained as discussed in https://github.com/llvm/llvm-project/pull/111701/files/1a63281b6c240352653fd2e4299755c1f32a76f4#r1795518779 so unless there are objections I plan to remove this test before merging. --- clang/lib/CodeGen/CodeGenFunction.cpp | 2 +- clang/lib/CodeGen/VarBypassDetector.cpp | 23 ++++++++------ clang/lib/CodeGen/VarBypassDetector.h | 9 ++++-- .../CodeGen/deeply-nested-expressions.cpp | 30 +++++++++++++++++++ 4 files changed, 51 insertions(+), 13 deletions(-) create mode 100644 clang/test/CodeGen/deeply-nested-expressions.cpp diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp index 11fdddba1144b..6bdfbbd401303 100644 --- a/clang/lib/CodeGen/CodeGenFunction.cpp +++ b/clang/lib/CodeGen/CodeGenFunction.cpp @@ -1533,7 +1533,7 @@ void CodeGenFunction::GenerateCode(GlobalDecl GD, llvm::Function *Fn, // Initialize helper which will detect jumps which can cause invalid // lifetime markers. if (ShouldEmitLifetimeMarkers) - Bypasses.Init(Body); + Bypasses.Init(CGM, Body); } // Emit the standard function prologue. diff --git a/clang/lib/CodeGen/VarBypassDetector.cpp b/clang/lib/CodeGen/VarBypassDetector.cpp index 6eda83dfdef2f..7b2b3542928ad 100644 --- a/clang/lib/CodeGen/VarBypassDetector.cpp +++ b/clang/lib/CodeGen/VarBypassDetector.cpp @@ -8,6 +8,7 @@ #include "VarBypassDetector.h" +#include "CodeGenModule.h" #include "clang/AST/Decl.h" #include "clang/AST/Expr.h" #include "clang/AST/Stmt.h" @@ -17,13 +18,13 @@ using namespace CodeGen; /// Clear the object and pre-process for the given statement, usually function /// body statement. -void VarBypassDetector::Init(const Stmt *Body) { +void VarBypassDetector::Init(CodeGenModule &CGM, const Stmt *Body) { FromScopes.clear(); ToScopes.clear(); Bypasses.clear(); Scopes = {{~0U, nullptr}}; unsigned ParentScope = 0; - AlwaysBypassed = !BuildScopeInformation(Body, ParentScope); + AlwaysBypassed = !BuildScopeInformation(CGM, Body, ParentScope); if (!AlwaysBypassed) Detect(); } @@ -31,7 +32,7 @@ void VarBypassDetector::Init(const Stmt *Body) { /// Build scope information for a declaration that is part of a DeclStmt. /// Returns false if we failed to build scope information and can't tell for /// which vars are being bypassed. -bool VarBypassDetector::BuildScopeInformation(const Decl *D, +bool VarBypassDetector::BuildScopeInformation(CodeGenModule &CGM, const Decl *D, unsigned &ParentScope) { const VarDecl *VD = dyn_cast<VarDecl>(D); if (VD && VD->hasLocalStorage()) { @@ -41,7 +42,7 @@ bool VarBypassDetector::BuildScopeInformation(const Decl *D, if (const VarDecl *VD = dyn_cast<VarDecl>(D)) if (const Expr *Init = VD->getInit()) - return BuildScopeInformation(Init, ParentScope); + return BuildScopeInformation(CGM, Init, ParentScope); return true; } @@ -50,7 +51,7 @@ bool VarBypassDetector::BuildScopeInformation(const Decl *D, /// LabelAndGotoScopes and recursively walking the AST as needed. /// Returns false if we failed to build scope information and can't tell for /// which vars are being bypassed. -bool VarBypassDetector::BuildScopeInformation(const Stmt *S, +bool VarBypassDetector::BuildScopeInformation(CodeGenModule &CGM, const Stmt *S, unsigned &origParentScope) { // If this is a statement, rather than an expression, scopes within it don't // propagate out into the enclosing scope. Otherwise we have to worry about @@ -68,12 +69,12 @@ bool VarBypassDetector::BuildScopeInformation(const Stmt *S, case Stmt::SwitchStmtClass: if (const Stmt *Init = cast<SwitchStmt>(S)->getInit()) { - if (!BuildScopeInformation(Init, ParentScope)) + if (!BuildScopeInformation(CGM, Init, ParentScope)) return false; ++StmtsToSkip; } if (const VarDecl *Var = cast<SwitchStmt>(S)->getConditionVariable()) { - if (!BuildScopeInformation(Var, ParentScope)) + if (!BuildScopeInformation(CGM, Var, ParentScope)) return false; ++StmtsToSkip; } @@ -86,7 +87,7 @@ bool VarBypassDetector::BuildScopeInformation(const Stmt *S, case Stmt::DeclStmtClass: { const DeclStmt *DS = cast<DeclStmt>(S); for (auto *I : DS->decls()) - if (!BuildScopeInformation(I, origParentScope)) + if (!BuildScopeInformation(CGM, I, origParentScope)) return false; return true; } @@ -126,7 +127,11 @@ bool VarBypassDetector::BuildScopeInformation(const Stmt *S, } // Recursively walk the AST. - if (!BuildScopeInformation(SubStmt, ParentScope)) + bool Result; + CGM.runWithSufficientStackSpace(S->getEndLoc(), [&] { + Result = BuildScopeInformation(CGM, SubStmt, ParentScope); + }); + if (!Result) return false; } return true; diff --git a/clang/lib/CodeGen/VarBypassDetector.h b/clang/lib/CodeGen/VarBypassDetector.h index 164e88c0b2f1b..cc4d387aeaa5b 100644 --- a/clang/lib/CodeGen/VarBypassDetector.h +++ b/clang/lib/CodeGen/VarBypassDetector.h @@ -14,6 +14,7 @@ #ifndef LLVM_CLANG_LIB_CODEGEN_VARBYPASSDETECTOR_H #define LLVM_CLANG_LIB_CODEGEN_VARBYPASSDETECTOR_H +#include "CodeGenModule.h" #include "clang/AST/Decl.h" #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/DenseSet.h" @@ -50,7 +51,7 @@ class VarBypassDetector { bool AlwaysBypassed = false; public: - void Init(const Stmt *Body); + void Init(CodeGenModule &CGM, const Stmt *Body); /// Returns true if the variable declaration was by bypassed by any goto or /// switch statement. @@ -59,8 +60,10 @@ class VarBypassDetector { } private: - bool BuildScopeInformation(const Decl *D, unsigned &ParentScope); - bool BuildScopeInformation(const Stmt *S, unsigned &origParentScope); + bool BuildScopeInformation(CodeGenModule &CGM, const Decl *D, + unsigned &ParentScope); + bool BuildScopeInformation(CodeGenModule &CGM, const Stmt *S, + unsigned &origParentScope); void Detect(); void Detect(unsigned From, unsigned To); }; diff --git a/clang/test/CodeGen/deeply-nested-expressions.cpp b/clang/test/CodeGen/deeply-nested-expressions.cpp new file mode 100644 index 0000000000000..98980be750648 --- /dev/null +++ b/clang/test/CodeGen/deeply-nested-expressions.cpp @@ -0,0 +1,30 @@ +// RUN: %clang_cc1 %s -emit-llvm -o - -Wstack-exhausted +// RUN: %clang_cc1 %s -emit-llvm -o - -Wstack-exhausted -O2 + +class AClass { +public: + AClass() {} + AClass &f() { return *this; } +}; + +#define CALLS1 f +#define CALLS2 CALLS1().CALLS1 +#define CALLS4 CALLS2().CALLS2 +#define CALLS8 CALLS4().CALLS4 +#define CALLS16 CALLS8().CALLS8 +#define CALLS32 CALLS16().CALLS16 +#define CALLS64 CALLS32().CALLS32 +#define CALLS128 CALLS64().CALLS64 +#define CALLS256 CALLS128().CALLS128 +#define CALLS512 CALLS256().CALLS256 +#define CALLS1024 CALLS512().CALLS512 +#define CALLS2048 CALLS1024().CALLS1024 +#define CALLS4096 CALLS2048().CALLS2048 +#define CALLS8192 CALLS4096().CALLS4096 +#define CALLS16384 CALLS8192().CALLS8192 +#define CALLS32768 CALLS16384().CALLS16384 + +void test_bar() { + AClass a; + a.CALLS32768(); +} >From d6b5576940d38aadb3293acbff680d1f5d22486c Mon Sep 17 00:00:00 2001 From: Boaz Brickner <brick...@google.com> Date: Mon, 10 Mar 2025 09:28:55 +0100 Subject: [PATCH 2/2] Remove test since it is slow and likely to be hard to maintain --- .../CodeGen/deeply-nested-expressions.cpp | 30 ------------------- 1 file changed, 30 deletions(-) delete mode 100644 clang/test/CodeGen/deeply-nested-expressions.cpp diff --git a/clang/test/CodeGen/deeply-nested-expressions.cpp b/clang/test/CodeGen/deeply-nested-expressions.cpp deleted file mode 100644 index 98980be750648..0000000000000 --- a/clang/test/CodeGen/deeply-nested-expressions.cpp +++ /dev/null @@ -1,30 +0,0 @@ -// RUN: %clang_cc1 %s -emit-llvm -o - -Wstack-exhausted -// RUN: %clang_cc1 %s -emit-llvm -o - -Wstack-exhausted -O2 - -class AClass { -public: - AClass() {} - AClass &f() { return *this; } -}; - -#define CALLS1 f -#define CALLS2 CALLS1().CALLS1 -#define CALLS4 CALLS2().CALLS2 -#define CALLS8 CALLS4().CALLS4 -#define CALLS16 CALLS8().CALLS8 -#define CALLS32 CALLS16().CALLS16 -#define CALLS64 CALLS32().CALLS32 -#define CALLS128 CALLS64().CALLS64 -#define CALLS256 CALLS128().CALLS128 -#define CALLS512 CALLS256().CALLS256 -#define CALLS1024 CALLS512().CALLS512 -#define CALLS2048 CALLS1024().CALLS1024 -#define CALLS4096 CALLS2048().CALLS2048 -#define CALLS8192 CALLS4096().CALLS4096 -#define CALLS16384 CALLS8192().CALLS8192 -#define CALLS32768 CALLS16384().CALLS16384 - -void test_bar() { - AClass a; - a.CALLS32768(); -} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
