Skip to content

Commit 78b7f6a

Browse files
[Sema] Avoid deep recursion in AnalyzeImplicitConversions (llvm#145734)
The function already exposes a work list to avoid deep recursion, this commit starts utilizing it in a helper that could also lead to a deep recursion. We have observed this crash on `clang/test/C/C99/n590.c` with our internal builds that enable aggressive optimizations and hit the limit earlier than default release builds of Clang. See the added test for an example with a deeper recursion that used to crash in upstream Clang before this change with the following stack trace: ``` #0 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) /usr/local/google/home/ibiryukov/code/llvm-project/llvm/lib/Support/Unix/Signals.inc:804:13 #1 llvm::sys::RunSignalHandlers() /usr/local/google/home/ibiryukov/code/llvm-project/llvm/lib/Support/Signals.cpp:106:18 #2 SignalHandler(int, siginfo_t*, void*) /usr/local/google/home/ibiryukov/code/llvm-project/llvm/lib/Support/Unix/Signals.inc:0:3 #3 (/lib/x86_64-linux-gnu/libc.so.6+0x3fdf0) #4 AnalyzeImplicitConversions(clang::Sema&, clang::Expr*, clang::SourceLocation, bool) /usr/local/google/home/ibiryukov/code/llvm-project/clang/lib/Sema/SemaChecking.cpp:12772:0 #5 CheckCommaOperand /usr/local/google/home/ibiryukov/code/llvm-project/clang/lib/Sema/SemaChecking.cpp:0:3 #6 AnalyzeImplicitConversions /usr/local/google/home/ibiryukov/code/llvm-project/clang/lib/Sema/SemaChecking.cpp:12644:7 #7 AnalyzeImplicitConversions(clang::Sema&, clang::Expr*, clang::SourceLocation, bool) /usr/local/google/home/ibiryukov/code/llvm-project/clang/lib/Sema/SemaChecking.cpp:12776:5 #8 CheckCommaOperand /usr/local/google/home/ibiryukov/code/llvm-project/clang/lib/Sema/SemaChecking.cpp:0:3 #9 AnalyzeImplicitConversions /usr/local/google/home/ibiryukov/code/llvm-project/clang/lib/Sema/SemaChecking.cpp:12644:7 #10 AnalyzeImplicitConversions(clang::Sema&, clang::Expr*, clang::SourceLocation, bool) /usr/local/google/home/ibiryukov/code/llvm-project/clang/lib/Sema/SemaChecking.cpp:12776:5 #11 CheckCommaOperand /usr/local/google/home/ibiryukov/code/llvm-project/clang/lib/Sema/SemaChecking.cpp:0:3 #12 AnalyzeImplicitConversions /usr/local/google/home/ibiryukov/code/llvm-project/clang/lib/Sema/SemaChecking.cpp:12644:7 #13 AnalyzeImplicitConversions(clang::Sema&, clang::Expr*, clang::SourceLocation, bool) /usr/local/google/home/ibiryukov/code/llvm-project/clang/lib/Sema/SemaChecking.cpp:12776:5 #14 CheckCommaOperand /usr/local/google/home/ibiryukov/code/llvm-project/clang/lib/Sema/SemaChecking.cpp:0:3 #15 AnalyzeImplicitConversions /usr/local/google/home/ibiryukov/code/llvm-project/clang/lib/Sema/SemaChecking.cpp:12644:7 #16 AnalyzeImplicitConversions(clang::Sema&, clang::Expr*, clang::SourceLocation, bool) /usr/local/google/home/ibiryukov/code/llvm-project/clang/lib/Sema/SemaChecking.cpp:12776:5 #17 CheckCommaOperand /usr/local/google/home/ibiryukov/code/llvm-project/clang/lib/Sema/SemaChecking.cpp:0:3 #18 AnalyzeImplicitConversions /usr/local/google/home/ibiryukov/code/llvm-project/clang/lib/Sema/SemaChecking.cpp:12644:7 #19 AnalyzeImplicitConversions(clang::Sema&, clang::Expr*, clang::SourceLocation, bool) /usr/local/google/home/ibiryukov/code/llvm-project/clang/lib/Sema/SemaChecking.cpp:12776:5 ... 700+ more stack frames. ```
1 parent c225d6d commit 78b7f6a

File tree

2 files changed

+424
-11
lines changed

2 files changed

+424
-11
lines changed

clang/lib/Sema/SemaChecking.cpp

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11684,15 +11684,6 @@ static void DiagnoseFloatingImpCast(Sema &S, const Expr *E, QualType T,
1168411684
}
1168511685
}
1168611686

11687-
static void CheckCommaOperand(Sema &S, Expr *E, QualType T, SourceLocation CC,
11688-
bool ExtraCheckForImplicitConversion) {
11689-
E = E->IgnoreParenImpCasts();
11690-
AnalyzeImplicitConversions(S, E, CC);
11691-
11692-
if (ExtraCheckForImplicitConversion && E->getType() != T)
11693-
S.CheckImplicitConversion(E, T, CC);
11694-
}
11695-
1169611687
/// Analyze the given compound assignment for the possible losing of
1169711688
/// floating-point precision.
1169811689
static void AnalyzeCompoundAssignment(Sema &S, BinaryOperator *E) {
@@ -12567,6 +12558,17 @@ struct AnalyzeImplicitConversionsWorkItem {
1256712558
};
1256812559
}
1256912560

12561+
static void CheckCommaOperand(
12562+
Sema &S, Expr *E, QualType T, SourceLocation CC,
12563+
bool ExtraCheckForImplicitConversion,
12564+
llvm::SmallVectorImpl<AnalyzeImplicitConversionsWorkItem> &WorkList) {
12565+
E = E->IgnoreParenImpCasts();
12566+
WorkList.push_back({E, CC, false});
12567+
12568+
if (ExtraCheckForImplicitConversion && E->getType() != T)
12569+
S.CheckImplicitConversion(E, T, CC);
12570+
}
12571+
1257012572
/// Data recursive variant of AnalyzeImplicitConversions. Subexpressions
1257112573
/// that should be visited are added to WorkList.
1257212574
static void AnalyzeImplicitConversions(
@@ -12642,9 +12644,9 @@ static void AnalyzeImplicitConversions(
1264212644
/// how CheckConditionalOperand behaves; it's as-if the correct operand
1264312645
/// were directly used for the implicit conversion check.
1264412646
CheckCommaOperand(S, BO->getLHS(), T, BO->getOperatorLoc(),
12645-
/*ExtraCheckForImplicitConversion=*/false);
12647+
/*ExtraCheckForImplicitConversion=*/false, WorkList);
1264612648
CheckCommaOperand(S, BO->getRHS(), T, BO->getOperatorLoc(),
12647-
/*ExtraCheckForImplicitConversion=*/true);
12649+
/*ExtraCheckForImplicitConversion=*/true, WorkList);
1264812650
return;
1264912651
}
1265012652
}

0 commit comments

Comments
 (0)