Skip to content

Revert "Improve stack usage to increase recursive initialization depth" #89006

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.

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 1 commit into from
Apr 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 0 additions & 6 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -203,12 +203,6 @@ Non-comprehensive list of changes in this release
- ``__typeof_unqual__`` is available in all C modes as an extension, which behaves
like ``typeof_unqual`` from C23, similar to ``__typeof__`` and ``typeof``.

- Improved stack usage with C++ initialization code. This allows significantly
more levels of recursive initialization before reaching stack exhaustion
limits. This will positively impact recursive template instantiation code,
but should also reduce memory overhead for initializations in general.
Fixes #GH88330

New Compiler Flags
------------------
- ``-fsanitize=implicit-bitfield-conversion`` checks implicit truncation and
Expand Down
6 changes: 2 additions & 4 deletions clang/include/clang/Sema/Initialization.h
Original file line number Diff line number Diff line change
Expand Up @@ -1134,7 +1134,7 @@ class InitializationSequence {
OverloadingResult FailedOverloadResult;

/// The candidate set created when initialization failed.
std::unique_ptr<OverloadCandidateSet> FailedCandidateSet;
OverloadCandidateSet FailedCandidateSet;

/// The incomplete type that caused a failure.
QualType FailedIncompleteType;
Expand Down Expand Up @@ -1403,9 +1403,7 @@ class InitializationSequence {
/// Retrieve a reference to the candidate set when overload
/// resolution fails.
OverloadCandidateSet &getFailedCandidateSet() {
assert(FailedCandidateSet &&
"this should have been allocated in the constructor!");
return *FailedCandidateSet;
return FailedCandidateSet;
}

/// Get the overloading result, for when the initialization
Expand Down
70 changes: 51 additions & 19 deletions clang/include/clang/Sema/Overload.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@
#include <cassert>
#include <cstddef>
#include <cstdint>
#include <memory>
#include <utility>

namespace clang {
Expand Down Expand Up @@ -875,8 +874,7 @@ class Sema;
ConversionFixItGenerator Fix;

/// Viable - True to indicate that this overload candidate is viable.
LLVM_PREFERRED_TYPE(bool)
unsigned Viable : 1;
bool Viable : 1;

/// Whether this candidate is the best viable function, or tied for being
/// the best viable function.
Expand All @@ -885,14 +883,12 @@ class Sema;
/// was part of the ambiguity kernel: the minimal non-empty set of viable
/// candidates such that all elements of the ambiguity kernel are better
/// than all viable candidates not in the ambiguity kernel.
LLVM_PREFERRED_TYPE(bool)
unsigned Best : 1;
bool Best : 1;

/// IsSurrogate - True to indicate that this candidate is a
/// surrogate for a conversion to a function pointer or reference
/// (C++ [over.call.object]).
LLVM_PREFERRED_TYPE(bool)
unsigned IsSurrogate : 1;
bool IsSurrogate : 1;

/// IgnoreObjectArgument - True to indicate that the first
/// argument's conversion, which for this function represents the
Expand All @@ -901,20 +897,18 @@ class Sema;
/// implicit object argument is just a placeholder) or a
/// non-static member function when the call doesn't have an
/// object argument.
LLVM_PREFERRED_TYPE(bool)
unsigned IgnoreObjectArgument : 1;
bool IgnoreObjectArgument : 1;

/// True if the candidate was found using ADL.
LLVM_PREFERRED_TYPE(CallExpr::ADLCallKind)
unsigned IsADLCandidate : 1;
CallExpr::ADLCallKind IsADLCandidate : 1;

/// Whether this is a rewritten candidate, and if so, of what kind?
LLVM_PREFERRED_TYPE(OverloadCandidateRewriteKind)
unsigned RewriteKind : 2;

/// FailureKind - The reason why this candidate is not viable.
LLVM_PREFERRED_TYPE(OverloadFailureKind)
unsigned FailureKind : 5;
/// Actually an OverloadFailureKind.
unsigned char FailureKind;

/// The number of call arguments that were explicitly provided,
/// to be used while performing partial ordering of function templates.
Expand Down Expand Up @@ -978,9 +972,7 @@ class Sema;
private:
friend class OverloadCandidateSet;
OverloadCandidate()
: IsSurrogate(false),
IsADLCandidate(static_cast<unsigned>(CallExpr::NotADL)),
RewriteKind(CRK_None) {}
: IsSurrogate(false), IsADLCandidate(CallExpr::NotADL), RewriteKind(CRK_None) {}
};

/// OverloadCandidateSet - A set of overload candidates, used in C++
Expand Down Expand Up @@ -1078,16 +1070,51 @@ class Sema;
};

private:
SmallVector<OverloadCandidate, 4> Candidates;
llvm::SmallPtrSet<uintptr_t, 4> Functions;
SmallVector<OverloadCandidate, 16> Candidates;
llvm::SmallPtrSet<uintptr_t, 16> Functions;

// Allocator for ConversionSequenceLists. We store the first few of these
// inline to avoid allocation for small sets.
llvm::BumpPtrAllocator SlabAllocator;

SourceLocation Loc;
CandidateSetKind Kind;
OperatorRewriteInfo RewriteInfo;

constexpr static unsigned NumInlineBytes =
24 * sizeof(ImplicitConversionSequence);
unsigned NumInlineBytesUsed = 0;
alignas(void *) char InlineSpace[NumInlineBytes];

// Address space of the object being constructed.
LangAS DestAS = LangAS::Default;

/// If we have space, allocates from inline storage. Otherwise, allocates
/// from the slab allocator.
/// FIXME: It would probably be nice to have a SmallBumpPtrAllocator
/// instead.
/// FIXME: Now that this only allocates ImplicitConversionSequences, do we
/// want to un-generalize this?
template <typename T>
T *slabAllocate(unsigned N) {
// It's simpler if this doesn't need to consider alignment.
static_assert(alignof(T) == alignof(void *),
"Only works for pointer-aligned types.");
static_assert(std::is_trivial<T>::value ||
std::is_same<ImplicitConversionSequence, T>::value,
"Add destruction logic to OverloadCandidateSet::clear().");

unsigned NBytes = sizeof(T) * N;
if (NBytes > NumInlineBytes - NumInlineBytesUsed)
return SlabAllocator.Allocate<T>(N);
char *FreeSpaceStart = InlineSpace + NumInlineBytesUsed;
assert(uintptr_t(FreeSpaceStart) % alignof(void *) == 0 &&
"Misaligned storage!");

NumInlineBytesUsed += NBytes;
return reinterpret_cast<T *>(FreeSpaceStart);
}

void destroyCandidates();

public:
Expand Down Expand Up @@ -1136,7 +1163,12 @@ class Sema;
ConversionSequenceList
allocateConversionSequences(unsigned NumConversions) {
ImplicitConversionSequence *Conversions =
new ImplicitConversionSequence[NumConversions];
slabAllocate<ImplicitConversionSequence>(NumConversions);

// Construct the new objects.
for (unsigned I = 0; I != NumConversions; ++I)
new (&Conversions[I]) ImplicitConversionSequence();

return ConversionSequenceList(Conversions, NumConversions);
}

Expand Down
26 changes: 12 additions & 14 deletions clang/lib/Sema/SemaInit.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6114,8 +6114,7 @@ InitializationSequence::InitializationSequence(
Sema &S, const InitializedEntity &Entity, const InitializationKind &Kind,
MultiExprArg Args, bool TopLevelOfInitList, bool TreatUnavailableAsInvalid)
: FailedOverloadResult(OR_Success),
FailedCandidateSet(new OverloadCandidateSet(
Kind.getLocation(), OverloadCandidateSet::CSK_Normal)) {
FailedCandidateSet(Kind.getLocation(), OverloadCandidateSet::CSK_Normal) {
InitializeFrom(S, Entity, Kind, Args, TopLevelOfInitList,
TreatUnavailableAsInvalid);
}
Expand Down Expand Up @@ -9736,7 +9735,7 @@ bool InitializationSequence::Diagnose(Sema &S,
switch (FailedOverloadResult) {
case OR_Ambiguous:

FailedCandidateSet->NoteCandidates(
FailedCandidateSet.NoteCandidates(
PartialDiagnosticAt(
Kind.getLocation(),
Failure == FK_UserConversionOverloadFailed
Expand All @@ -9750,8 +9749,7 @@ bool InitializationSequence::Diagnose(Sema &S,
break;

case OR_No_Viable_Function: {
auto Cands =
FailedCandidateSet->CompleteCandidates(S, OCD_AllCandidates, Args);
auto Cands = FailedCandidateSet.CompleteCandidates(S, OCD_AllCandidates, Args);
if (!S.RequireCompleteType(Kind.getLocation(),
DestType.getNonReferenceType(),
diag::err_typecheck_nonviable_condition_incomplete,
Expand All @@ -9761,13 +9759,13 @@ bool InitializationSequence::Diagnose(Sema &S,
<< OnlyArg->getType() << Args[0]->getSourceRange()
<< DestType.getNonReferenceType();

FailedCandidateSet->NoteCandidates(S, Args, Cands);
FailedCandidateSet.NoteCandidates(S, Args, Cands);
break;
}
case OR_Deleted: {
OverloadCandidateSet::iterator Best;
OverloadingResult Ovl =
FailedCandidateSet->BestViableFunction(S, Kind.getLocation(), Best);
OverloadingResult Ovl
= FailedCandidateSet.BestViableFunction(S, Kind.getLocation(), Best);

StringLiteral *Msg = Best->Function->getDeletedMessage();
S.Diag(Kind.getLocation(), diag::err_typecheck_deleted_function)
Expand Down Expand Up @@ -9951,7 +9949,7 @@ bool InitializationSequence::Diagnose(Sema &S,
// bad.
switch (FailedOverloadResult) {
case OR_Ambiguous:
FailedCandidateSet->NoteCandidates(
FailedCandidateSet.NoteCandidates(
PartialDiagnosticAt(Kind.getLocation(),
S.PDiag(diag::err_ovl_ambiguous_init)
<< DestType << ArgsRange),
Expand Down Expand Up @@ -10005,7 +10003,7 @@ bool InitializationSequence::Diagnose(Sema &S,
break;
}

FailedCandidateSet->NoteCandidates(
FailedCandidateSet.NoteCandidates(
PartialDiagnosticAt(
Kind.getLocation(),
S.PDiag(diag::err_ovl_no_viable_function_in_init)
Expand All @@ -10015,8 +10013,8 @@ bool InitializationSequence::Diagnose(Sema &S,

case OR_Deleted: {
OverloadCandidateSet::iterator Best;
OverloadingResult Ovl =
FailedCandidateSet->BestViableFunction(S, Kind.getLocation(), Best);
OverloadingResult Ovl
= FailedCandidateSet.BestViableFunction(S, Kind.getLocation(), Best);
if (Ovl != OR_Deleted) {
S.Diag(Kind.getLocation(), diag::err_ovl_deleted_init)
<< DestType << ArgsRange;
Expand Down Expand Up @@ -10095,8 +10093,8 @@ bool InitializationSequence::Diagnose(Sema &S,
S.Diag(Kind.getLocation(), diag::err_selected_explicit_constructor)
<< Args[0]->getSourceRange();
OverloadCandidateSet::iterator Best;
OverloadingResult Ovl =
FailedCandidateSet->BestViableFunction(S, Kind.getLocation(), Best);
OverloadingResult Ovl
= FailedCandidateSet.BestViableFunction(S, Kind.getLocation(), Best);
(void)Ovl;
assert(Ovl == OR_Success && "Inconsistent overload resolution");
CXXConstructorDecl *CtorDecl = cast<CXXConstructorDecl>(Best->Function);
Expand Down
21 changes: 10 additions & 11 deletions clang/lib/Sema/SemaOverload.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1057,14 +1057,17 @@ bool OverloadCandidateSet::OperatorRewriteInfo::shouldAddReversed(

void OverloadCandidateSet::destroyCandidates() {
for (iterator i = begin(), e = end(); i != e; ++i) {
delete[] i->Conversions.data();
for (auto &C : i->Conversions)
C.~ImplicitConversionSequence();
if (!i->Viable && i->FailureKind == ovl_fail_bad_deduction)
i->DeductionFailure.Destroy();
}
}

void OverloadCandidateSet::clear(CandidateSetKind CSK) {
destroyCandidates();
SlabAllocator.Reset();
NumInlineBytesUsed = 0;
Candidates.clear();
Functions.clear();
Kind = CSK;
Expand Down Expand Up @@ -6980,7 +6983,7 @@ void Sema::AddOverloadCandidate(
Candidate.RewriteKind =
CandidateSet.getRewriteInfo().getRewriteKind(Function, PO);
Candidate.IsSurrogate = false;
Candidate.IsADLCandidate = static_cast<unsigned>(IsADLCandidate);
Candidate.IsADLCandidate = IsADLCandidate;
Candidate.IgnoreObjectArgument = false;
Candidate.ExplicitCallArguments = Args.size();

Expand Down Expand Up @@ -7812,7 +7815,7 @@ void Sema::AddTemplateOverloadCandidate(
Candidate.RewriteKind =
CandidateSet.getRewriteInfo().getRewriteKind(Candidate.Function, PO);
Candidate.IsSurrogate = false;
Candidate.IsADLCandidate = static_cast<unsigned>(IsADLCandidate);
Candidate.IsADLCandidate = IsADLCandidate;
// Ignore the object argument if there is one, since we don't have an object
// type.
Candidate.IgnoreObjectArgument =
Expand Down Expand Up @@ -14122,8 +14125,7 @@ static ExprResult FinishOverloadedCallExpr(Sema &SemaRef, Scope *S, Expr *Fn,
return ExprError();
return SemaRef.BuildResolvedCallExpr(
Res.get(), FDecl, LParenLoc, Args, RParenLoc, ExecConfig,
/*IsExecConfig=*/false,
static_cast<CallExpr::ADLCallKind>((*Best)->IsADLCandidate));
/*IsExecConfig=*/false, (*Best)->IsADLCandidate);
}

case OR_No_Viable_Function: {
Expand Down Expand Up @@ -14182,8 +14184,7 @@ static ExprResult FinishOverloadedCallExpr(Sema &SemaRef, Scope *S, Expr *Fn,
return ExprError();
return SemaRef.BuildResolvedCallExpr(
Res.get(), FDecl, LParenLoc, Args, RParenLoc, ExecConfig,
/*IsExecConfig=*/false,
static_cast<CallExpr::ADLCallKind>((*Best)->IsADLCandidate));
/*IsExecConfig=*/false, (*Best)->IsADLCandidate);
}
}

Expand Down Expand Up @@ -14490,8 +14491,7 @@ Sema::CreateOverloadedUnaryOp(SourceLocation OpLoc, UnaryOperatorKind Opc,
Args[0] = Input;
CallExpr *TheCall = CXXOperatorCallExpr::Create(
Context, Op, FnExpr.get(), ArgsArray, ResultTy, VK, OpLoc,
CurFPFeatureOverrides(),
static_cast<CallExpr::ADLCallKind>(Best->IsADLCandidate));
CurFPFeatureOverrides(), Best->IsADLCandidate);

if (CheckCallReturnType(FnDecl->getReturnType(), OpLoc, TheCall, FnDecl))
return ExprError();
Expand Down Expand Up @@ -14909,8 +14909,7 @@ ExprResult Sema::CreateOverloadedBinOp(SourceLocation OpLoc,
// members; CodeGen should take care not to emit the this pointer.
TheCall = CXXOperatorCallExpr::Create(
Context, ChosenOp, FnExpr.get(), Args, ResultTy, VK, OpLoc,
CurFPFeatureOverrides(),
static_cast<CallExpr::ADLCallKind>(Best->IsADLCandidate));
CurFPFeatureOverrides(), Best->IsADLCandidate);

if (const auto *Method = dyn_cast<CXXMethodDecl>(FnDecl);
Method && Method->isImplicitObjectMemberFunction()) {
Expand Down