From e0442bdfa5a5cd02431955cb18b67194d2b82faf Mon Sep 17 00:00:00 2001 From: Boaz Brickner Date: Mon, 10 Mar 2025 09:33:00 +0100 Subject: [PATCH] [Clang] Fix segmentation fault caused by `VarBypassDetector` stack overflow on deeply nested expressions (#124128) This happens when using `-O2`. Similarly to #111701 ([test](https://github.com/bricknerb/llvm-project/blob/93e4a7386ec897e53d7330c6206d38759a858be2/clang/test/CodeGen/deeply-nested-expressions.cpp)), not adding a test that reproduces since this test is slow and likely to be hard to maintained as discussed here and in [previous discussion](https://github.com/llvm/llvm-project/pull/111701/files/1a63281b6c240352653fd2e4299755c1f32a76f4#r1795518779). Test that was reverted here: https://github.com/llvm/llvm-project/pull/124128/commits/d6b5576940d38aadb3293acbff680d1f5d22486c --- clang/lib/CodeGen/CodeGenFunction.cpp | 2 +- clang/lib/CodeGen/VarBypassDetector.cpp | 23 ++++++++++++++--------- clang/lib/CodeGen/VarBypassDetector.h | 9 ++++++--- 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp index 08165e0b28406..63f0bf533fd45 100644 --- a/clang/lib/CodeGen/CodeGenFunction.cpp +++ b/clang/lib/CodeGen/CodeGenFunction.cpp @@ -1534,7 +1534,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(D); if (VD && VD->hasLocalStorage()) { @@ -41,7 +42,7 @@ bool VarBypassDetector::BuildScopeInformation(const Decl *D, if (const VarDecl *VD = dyn_cast(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(S)->getInit()) { - if (!BuildScopeInformation(Init, ParentScope)) + if (!BuildScopeInformation(CGM, Init, ParentScope)) return false; ++StmtsToSkip; } if (const VarDecl *Var = cast(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(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); };