Mordante created this revision.
Mordante added reviewers: rnk, rsmith, aaron.ballman.
Mordante added a project: clang.
When a long set of expressions is chained it may overflow the stack. This warns
about the issue.
Note I'm not sure whether `AnalyzeImplicitConversions` is the best place to add
this test, but it was the best I could find. Also not sure what the naming
convention for these helpers is. Another option is to put the original code in
a lambda in `AnalyzeImplicitConversions` and thus remove the extra function.
Fixes https://bugs.llvm.org/show_bug.cgi?id=14030
Depends on D69478 <https://reviews.llvm.org/D69478> (if this patch is unwanted
it is possible to remove the dependency.)
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D69479
Files:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaChecking.cpp
clang/test/Sema/stack-overflow-expr-bool.cpp
clang/test/Sema/stack-overflow-expr-double.c
clang/test/Sema/stack-overflow-expr-int.c
Index: clang/test/Sema/stack-overflow-expr-int.c
===================================================================
--- /dev/null
+++ clang/test/Sema/stack-overflow-expr-int.c
@@ -0,0 +1,21 @@
+// The segmentation fault gives exit code 139
+// RUN: not not %clang_cc1 %s -fsyntax-only 2>&1 | FileCheck %s
+// REQUIRES: thread_support
+
+#define A10 ARG ARG ARG ARG ARG ARG ARG ARG ARG ARG
+#define A100 A10 A10 A10 A10 A10 A10 A10 A10 A10 A10
+#define A1000 A10 A10 A10 A10 A10 A10 A10 A10 A10 A10
+#define A10000 A100 A100 A100 A100 A100 A100 A100 A100 A100 A100
+#define A100000 A1000 A1000 A1000 A1000 A1000 A1000 A1000 A1000 A1000 A1000
+#define A1000000 A10000 A10000 A10000 A10000 A10000 A10000 A10000 A10000 A10000 A10000
+#define A10000000 A100000 A100000 A100000 A100000 A100000 A100000 A100000 A100000 A100000 A100000
+#define A100000000 A1000000 A1000000 A1000000 A1000000 A1000000 A1000000 A1000000 A1000000 A1000000 A1000000
+
+#define ARG 1 +
+void foo() {
+ int i = A100000000 1;
+}
+
+// CHECK: warning: stack nearly exhausted; compilation time may suffer, and crashes due to stack overflow are likely
+// CHECK: note: Reduce the number of chained expressions, using helper variables or parens
+// CHECK: Stack dump:
Index: clang/test/Sema/stack-overflow-expr-double.c
===================================================================
--- /dev/null
+++ clang/test/Sema/stack-overflow-expr-double.c
@@ -0,0 +1,21 @@
+// The segmentation fault gives exit code 139
+// RUN: not not %clang_cc1 %s -fsyntax-only 2>&1 | FileCheck %s
+// REQUIRES: thread_support
+
+#define A10 ARG ARG ARG ARG ARG ARG ARG ARG ARG ARG
+#define A100 A10 A10 A10 A10 A10 A10 A10 A10 A10 A10
+#define A1000 A10 A10 A10 A10 A10 A10 A10 A10 A10 A10
+#define A10000 A100 A100 A100 A100 A100 A100 A100 A100 A100 A100
+#define A100000 A1000 A1000 A1000 A1000 A1000 A1000 A1000 A1000 A1000 A1000
+#define A1000000 A10000 A10000 A10000 A10000 A10000 A10000 A10000 A10000 A10000 A10000
+#define A10000000 A100000 A100000 A100000 A100000 A100000 A100000 A100000 A100000 A100000 A100000
+#define A100000000 A1000000 A1000000 A1000000 A1000000 A1000000 A1000000 A1000000 A1000000 A1000000 A1000000
+
+#define ARG 1. +
+void foo() {
+ double f = A100000000 1.;
+}
+
+// CHECK: warning: stack nearly exhausted; compilation time may suffer, and crashes due to stack overflow are likely
+// CHECK: note: Reduce the number of chained expressions, using helper variables or parens
+// CHECK: Stack dump:
Index: clang/test/Sema/stack-overflow-expr-bool.cpp
===================================================================
--- /dev/null
+++ clang/test/Sema/stack-overflow-expr-bool.cpp
@@ -0,0 +1,21 @@
+// The segmentation fault gives exit code 139
+// RUN: not not %clang_cc1 %s -fsyntax-only 2>&1 | FileCheck %s
+// REQUIRES: thread_support
+
+#define A10 ARG ARG ARG ARG ARG ARG ARG ARG ARG ARG
+#define A100 A10 A10 A10 A10 A10 A10 A10 A10 A10 A10
+#define A1000 A10 A10 A10 A10 A10 A10 A10 A10 A10 A10
+#define A10000 A100 A100 A100 A100 A100 A100 A100 A100 A100 A100
+#define A100000 A1000 A1000 A1000 A1000 A1000 A1000 A1000 A1000 A1000 A1000
+#define A1000000 A10000 A10000 A10000 A10000 A10000 A10000 A10000 A10000 A10000 A10000
+#define A10000000 A100000 A100000 A100000 A100000 A100000 A100000 A100000 A100000 A100000 A100000
+#define A100000000 A1000000 A1000000 A1000000 A1000000 A1000000 A1000000 A1000000 A1000000 A1000000 A1000000
+
+#define ARG true &&
+void foo() {
+ bool b = A100000000 true;
+}
+
+// CHECK: warning: stack nearly exhausted; compilation time may suffer, and crashes due to stack overflow are likely
+// CHECK: note: Reduce the number of chained expressions, using helper variables or parens
+// CHECK: Stack dump:
Index: clang/lib/Sema/SemaChecking.cpp
===================================================================
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -10368,8 +10368,8 @@
IsSameFloatAfterCast(value.getComplexFloatImag(), Src, Tgt));
}
-static void AnalyzeImplicitConversions(Sema &S, Expr *E, SourceLocation CC,
- bool IsListInit = false);
+static void analyzeImplicitConversionsWithSufficientStackSpace(
+ Sema &S, Expr *E, SourceLocation CC, bool IsListInit = false);
static bool IsEnumConstOrFromMacro(Sema &S, Expr *E) {
// Suppress cases where we are comparing against an enum constant.
@@ -10666,8 +10666,10 @@
/// Analyze the operands of the given comparison. Implements the
/// fallback case from AnalyzeComparison.
static void AnalyzeImpConvsInComparison(Sema &S, BinaryOperator *E) {
- AnalyzeImplicitConversions(S, E->getLHS(), E->getOperatorLoc());
- AnalyzeImplicitConversions(S, E->getRHS(), E->getOperatorLoc());
+ analyzeImplicitConversionsWithSufficientStackSpace(S, E->getLHS(),
+ E->getOperatorLoc());
+ analyzeImplicitConversionsWithSufficientStackSpace(S, E->getRHS(),
+ E->getOperatorLoc());
}
/// Implements -Wsign-compare.
@@ -10756,8 +10758,10 @@
// Go ahead and analyze implicit conversions in the operands. Note
// that we skip the implicit conversions on both sides.
- AnalyzeImplicitConversions(S, LHS, E->getOperatorLoc());
- AnalyzeImplicitConversions(S, RHS, E->getOperatorLoc());
+ analyzeImplicitConversionsWithSufficientStackSpace(S, LHS,
+ E->getOperatorLoc());
+ analyzeImplicitConversionsWithSufficientStackSpace(S, RHS,
+ E->getOperatorLoc());
// If the signed range is non-negative, -Wsign-compare won't fire.
if (signedRange.NonNegative)
@@ -10924,7 +10928,8 @@
/// operations.
static void AnalyzeAssignment(Sema &S, BinaryOperator *E) {
// Just recurse on the LHS.
- AnalyzeImplicitConversions(S, E->getLHS(), E->getOperatorLoc());
+ analyzeImplicitConversionsWithSufficientStackSpace(S, E->getLHS(),
+ E->getOperatorLoc());
// We want to recurse on the RHS as normal unless we're assigning to
// a bitfield.
@@ -10932,12 +10937,13 @@
if (AnalyzeBitFieldAssignment(S, Bitfield, E->getRHS(),
E->getOperatorLoc())) {
// Recurse, ignoring any implicit conversions on the RHS.
- return AnalyzeImplicitConversions(S, E->getRHS()->IgnoreParenImpCasts(),
- E->getOperatorLoc());
+ return analyzeImplicitConversionsWithSufficientStackSpace(
+ S, E->getRHS()->IgnoreParenImpCasts(), E->getOperatorLoc());
}
}
- AnalyzeImplicitConversions(S, E->getRHS(), E->getOperatorLoc());
+ analyzeImplicitConversionsWithSufficientStackSpace(S, E->getRHS(),
+ E->getOperatorLoc());
// Diagnose implicitly sequentially-consistent atomic assignment.
if (E->getLHS()->getType()->isAtomicType())
@@ -11108,8 +11114,10 @@
assert(isa<CompoundAssignOperator>(E) &&
"Must be compound assignment operation");
// Recurse on the LHS and RHS in here
- AnalyzeImplicitConversions(S, E->getLHS(), E->getOperatorLoc());
- AnalyzeImplicitConversions(S, E->getRHS(), E->getOperatorLoc());
+ analyzeImplicitConversionsWithSufficientStackSpace(S, E->getLHS(),
+ E->getOperatorLoc());
+ analyzeImplicitConversionsWithSufficientStackSpace(S, E->getRHS(),
+ E->getOperatorLoc());
if (E->getLHS()->getType()->isAtomicType())
S.Diag(E->getOperatorLoc(), diag::warn_atomic_implicit_seq_cst);
@@ -11876,14 +11884,15 @@
if (isa<ConditionalOperator>(E))
return CheckConditionalOperator(S, cast<ConditionalOperator>(E), CC, T);
- AnalyzeImplicitConversions(S, E, CC);
+ analyzeImplicitConversionsWithSufficientStackSpace(S, E, CC);
if (E->getType() != T)
return CheckImplicitConversion(S, E, T, CC, &ICContext);
}
static void CheckConditionalOperator(Sema &S, ConditionalOperator *E,
SourceLocation CC, QualType T) {
- AnalyzeImplicitConversions(S, E->getCond(), E->getQuestionLoc());
+ analyzeImplicitConversionsWithSufficientStackSpace(S, E->getCond(),
+ E->getQuestionLoc());
bool Suspicious = false;
CheckConditionalOperand(S, E->getTrueExpr(), T, CC, Suspicious);
@@ -11927,8 +11936,11 @@
/// AnalyzeImplicitConversions - Find and report any interesting
/// implicit conversions in the given expression. There are a couple
/// of competing diagnostics here, -Wconversion and -Wsign-compare.
+///
+/// \note Do not call this function direct but use
+/// \ref analyzeImplicitConversionsWithSufficientStackSpace
static void AnalyzeImplicitConversions(Sema &S, Expr *OrigE, SourceLocation CC,
- bool IsListInit/*= false*/) {
+ bool IsListInit) {
QualType T = OrigE->getType();
Expr *E = OrigE->IgnoreParenImpCasts();
@@ -11974,7 +11986,8 @@
// FIXME: Use a more uniform representation for this.
for (auto *SE : POE->semantics())
if (auto *OVE = dyn_cast<OpaqueValueExpr>(SE))
- AnalyzeImplicitConversions(S, OVE->getSourceExpr(), CC, IsListInit);
+ analyzeImplicitConversionsWithSufficientStackSpace(
+ S, OVE->getSourceExpr(), CC, IsListInit);
}
// Skip past explicit casts.
@@ -11982,7 +11995,8 @@
E = CE->getSubExpr()->IgnoreParenImpCasts();
if (!CE->getType()->isVoidType() && E->getType()->isAtomicType())
S.Diag(E->getBeginLoc(), diag::warn_atomic_implicit_seq_cst);
- return AnalyzeImplicitConversions(S, E, CC, IsListInit);
+ return analyzeImplicitConversionsWithSufficientStackSpace(S, E, CC,
+ IsListInit);
}
if (BinaryOperator *BO = dyn_cast<BinaryOperator>(E)) {
@@ -12021,7 +12035,8 @@
// Ignore checking string literals that are in logical and operators.
// This is a common pattern for asserts.
continue;
- AnalyzeImplicitConversions(S, ChildExpr, CC, IsListInit);
+ analyzeImplicitConversionsWithSufficientStackSpace(S, ChildExpr, CC,
+ IsListInit);
}
if (BO && BO->isLogicalOp()) {
@@ -12045,6 +12060,16 @@
}
}
+/// analyzeImplicitConversionsWithSufficientStackSpace - Find and report any
+/// interesting implicit conversions in the given expression. There are a
+/// couple of competing diagnostics here, -Wconversion and -Wsign-compare.
+static void analyzeImplicitConversionsWithSufficientStackSpace(
+ Sema &S, Expr *OrigE, SourceLocation CC, bool IsListInit /*= false*/) {
+ S.runWithSufficientStackSpace(
+ CC, [&] { AnalyzeImplicitConversions(S, OrigE, CC, IsListInit); },
+ diag::note_reduce_expression_chaining);
+}
+
/// Diagnose integer type and any valid implicit conversion to it.
static bool checkOpenCLEnqueueIntType(Sema &S, Expr *E, const QualType &IntT) {
// Taking into account implicit conversions,
@@ -12321,7 +12346,7 @@
CheckArrayAccess(E);
// This is not the right CC for (e.g.) a variable initialization.
- AnalyzeImplicitConversions(*this, E, CC);
+ analyzeImplicitConversionsWithSufficientStackSpace(*this, E, CC);
}
/// CheckBoolLikeConversion - Check conversion of given expression to boolean.
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3477,6 +3477,9 @@
"cast to %1 from smaller integer type %0">,
InGroup<IntToVoidPointerCast>;
+def note_reduce_expression_chaining : Note<
+ "Reduce the number of chained expressions, using helper variables or parens">;
+
def warn_attribute_ignored_for_field_of_type : Warning<
"%0 attribute ignored for field of type %1">,
InGroup<IgnoredAttributes>;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
