Skip to content

[clang] Fix segmentation fault caused by stack overflow on deeply nested expressions #111701

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Oct 14, 2024
Merged

[clang] Fix segmentation fault caused by stack overflow on deeply nested expressions #111701

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
1a63281
[clang] Fix segmentation fault caused by stack overflow on deeply nes…
bricknerb Oct 9, 2024
0100615
[clang] Move the call to clang::runWithSufficientStackSpace() from Co…
bricknerb Oct 11, 2024
d43be53
[clang] Double the depth in the deeply nested expressions test to cat…
bricknerb Oct 11, 2024
93e4a73
[clang] Use macros to shorten the deeply-nested-expressions test.
bricknerb Oct 14, 2024
84996d6
[clang] Remove out of stack warning check since it depends on the pla…
bricknerb Oct 14, 2024
da8b92c
Merge branch 'llvm:main' into stack3
bricknerb Oct 14, 2024
50bec16
[clang] Add missing trailing newline.
bricknerb Oct 14, 2024
856e26d
[clang] Deleting the test for deeply nested expressions for CodeGen t…
bricknerb Oct 14, 2024
6f5ef5f
[clang] Fix formatting when calling CGM.runWithSufficientStackSpace().
bricknerb Oct 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion clang/lib/CodeGen/CGExpr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5817,7 +5817,10 @@ LValue CodeGenFunction::EmitHLSLArrayAssignLValue(const BinaryOperator *E) {

LValue CodeGenFunction::EmitCallExprLValue(const CallExpr *E,
llvm::CallBase **CallOrInvoke) {
RValue RV = EmitCallExpr(E, ReturnValueSlot(), CallOrInvoke);
RValue RV;
CGM.runWithSufficientStackSpace(E->getExprLoc(), [&] {
RV = EmitCallExpr(E, ReturnValueSlot(), CallOrInvoke);
});

if (!RV.isScalar())
return MakeAddrLValue(RV.getAggregateAddress(), E->getType(),
Expand Down
14 changes: 14 additions & 0 deletions clang/lib/CodeGen/CodeGenModule.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
#include "clang/Basic/FileManager.h"
#include "clang/Basic/Module.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Basic/Stack.h"
#include "clang/Basic/TargetInfo.h"
#include "clang/Basic/Version.h"
#include "clang/CodeGen/BackendUtil.h"
Expand Down Expand Up @@ -1596,6 +1597,19 @@ void CodeGenModule::ErrorUnsupported(const Decl *D, const char *Type) {
getDiags().Report(Context.getFullLoc(D->getLocation()), DiagID) << Msg;
}

void CodeGenModule::warnStackExhausted(SourceLocation Loc) {
// Only warn about this once.
if (!WarnedStackExhausted) {
getDiags().Report(Loc, diag::warn_stack_exhausted);
WarnedStackExhausted = true;
}
}
Copy link
Contributor

@cor3ntin cor3ntin Oct 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to avoid the duplication with Sema here?

Copy link
Contributor Author

@bricknerb bricknerb Oct 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking that as a follow up I will extract this small logic into a class which both Sema and CodeGen would own.
I also have #111799 to make them more similar.

Copy link
Contributor

@ilya-biryukov ilya-biryukov Oct 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM and also feels like something that we could do in a follow-up, even if the code here is duplicated for some short period of time.

I suspect there's some wiring up to figure out for sharing this in the same class, so it'd be good to land this while that's still happening.


void CodeGenModule::runWithSufficientStackSpace(SourceLocation Loc,
llvm::function_ref<void()> Fn) {
clang::runWithSufficientStackSpace([&] { warnStackExhausted(Loc); }, Fn);
}

llvm::ConstantInt *CodeGenModule::getSize(CharUnits size) {
return llvm::ConstantInt::get(SizeTy, size.getQuantity());
}
Expand Down
11 changes: 11 additions & 0 deletions clang/lib/CodeGen/CodeGenModule.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,7 @@ class CodeGenModule : public CodeGenTypeCache {
std::unique_ptr<llvm::IndexedInstrProfReader> PGOReader;
InstrProfStats PGOStats;
std::unique_ptr<llvm::SanitizerStatReport> SanStats;
bool WarnedStackExhausted = false;

// A set of references that have only been seen via a weakref so far. This is
// used to remove the weak of the reference if we ever see a direct reference
Expand Down Expand Up @@ -1297,6 +1298,16 @@ class CodeGenModule : public CodeGenTypeCache {
/// Print out an error that codegen doesn't support the specified decl yet.
void ErrorUnsupported(const Decl *D, const char *Type);

/// Warn that the stack is nearly exhausted.
void warnStackExhausted(SourceLocation Loc);

/// Run some code with "sufficient" stack space. (Currently, at least 256K is
/// guaranteed). Produces a warning if we're low on stack space and allocates
/// more in that case. Use this in code that may recurse deeply to avoid stack
/// overflow.
void runWithSufficientStackSpace(SourceLocation Loc,
llvm::function_ref<void()> Fn);

/// Set the attributes on the LLVM function for the given decl and function
/// info. This applies attributes necessary for handling the ABI as well as
/// user specified attributes like section.
Expand Down
Loading