[clang] 58f831d - More robust fix for crash on invalid range-based for statement.
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Mon Jun 8 13:11:56 PDT 2020
Author: Richard Smith
Date: 2020-06-08T13:11:23-07:00
New Revision: 58f831d2b3885bbbc2366045e46211c507fa5f8b
URL: https://github.com/llvm/llvm-project/commit/58f831d2b3885bbbc2366045e46211c507fa5f8b
DIFF: https://github.com/llvm/llvm-project/commit/58f831d2b3885bbbc2366045e46211c507fa5f8b.diff
LOG: More robust fix for crash on invalid range-based for statement.
Reliably mark the loop variable declaration in a range for as having an
invalid initializer if anything goes wrong building the initializer. We
previously based this determination on whether an error was emitted,
which is not a reliable signal due to error suppression (during error
recovery etc).
Also, properly mark the variable as having initializer errors rather
than simply marking it invalid. This is necessary to mark any structured
bindings as invalid too.
This generalizes the previous fix in
936ec89e91e2dda8b6110b1fd1f9920509d7a17b.
Added:
Modified:
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaStmt.cpp
clang/lib/Sema/TreeTransform.h
clang/test/SemaCXX/cxx11-crashes.cpp
clang/test/SemaCXX/for-range-crash.cpp
Removed:
################################################################################
diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 025b09de0ad1..64e93963727e 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -12317,7 +12317,7 @@ void Sema::ActOnInitializerError(Decl *D) {
BD->setInvalidDecl();
// Auto types are meaningless if we can't make sense of the initializer.
- if (ParsingInitForAutoVars.count(D)) {
+ if (VD->getType()->isUndeducedType()) {
D->setInvalidDecl();
return;
}
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index 8d32fc094494..9d5672cff00a 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -2127,18 +2127,22 @@ StmtResult Sema::ActOnCXXForRangeStmt(Scope *S, SourceLocation ForLoc,
return StmtError();
}
+ // This function is responsible for attaching an initializer to LoopVar. We
+ // must call ActOnInitializerError if we fail to do so.
Decl *LoopVar = DS->getSingleDecl();
if (LoopVar->isInvalidDecl() || !Range ||
DiagnoseUnexpandedParameterPack(Range, UPPC_Expression)) {
- LoopVar->setInvalidDecl();
+ ActOnInitializerError(LoopVar);
return StmtError();
}
// Build the coroutine state immediately and not later during template
// instantiation
if (!CoawaitLoc.isInvalid()) {
- if (!ActOnCoroutineBodyStart(S, CoawaitLoc, "co_await"))
+ if (!ActOnCoroutineBodyStart(S, CoawaitLoc, "co_await")) {
+ ActOnInitializerError(LoopVar);
return StmtError();
+ }
}
// Build auto && __range = range-init
@@ -2150,7 +2154,7 @@ StmtResult Sema::ActOnCXXForRangeStmt(Scope *S, SourceLocation ForLoc,
std::string("__range") + DepthStr);
if (FinishForRangeVarDecl(*this, RangeVar, Range, RangeLoc,
diag::err_for_range_deduction_failure)) {
- LoopVar->setInvalidDecl();
+ ActOnInitializerError(LoopVar);
return StmtError();
}
@@ -2159,14 +2163,20 @@ StmtResult Sema::ActOnCXXForRangeStmt(Scope *S, SourceLocation ForLoc,
BuildDeclaratorGroup(MutableArrayRef<Decl *>((Decl **)&RangeVar, 1));
StmtResult RangeDecl = ActOnDeclStmt(RangeGroup, RangeLoc, RangeLoc);
if (RangeDecl.isInvalid()) {
- LoopVar->setInvalidDecl();
+ ActOnInitializerError(LoopVar);
return StmtError();
}
- return BuildCXXForRangeStmt(
+ StmtResult R = BuildCXXForRangeStmt(
ForLoc, CoawaitLoc, InitStmt, ColonLoc, RangeDecl.get(),
/*BeginStmt=*/nullptr, /*EndStmt=*/nullptr,
/*Cond=*/nullptr, /*Inc=*/nullptr, DS, RParenLoc, Kind);
+ if (R.isInvalid()) {
+ ActOnInitializerError(LoopVar);
+ return StmtError();
+ }
+
+ return R;
}
/// Create the initialization, compare, and increment steps for
@@ -2349,22 +2359,6 @@ static StmtResult RebuildForRangeWithDereference(Sema &SemaRef, Scope *S,
AdjustedRange.get(), RParenLoc, Sema::BFRK_Rebuild);
}
-namespace {
-/// RAII object to automatically invalidate a declaration if an error occurs.
-struct InvalidateOnErrorScope {
- InvalidateOnErrorScope(Sema &SemaRef, Decl *D, bool Enabled)
- : Trap(SemaRef.Diags), D(D), Enabled(Enabled) {}
- ~InvalidateOnErrorScope() {
- if (Enabled && Trap.hasErrorOccurred())
- D->setInvalidDecl();
- }
-
- DiagnosticErrorTrap Trap;
- Decl *D;
- bool Enabled;
-};
-}
-
/// BuildCXXForRangeStmt - Build or instantiate a C++11 for-range statement.
StmtResult Sema::BuildCXXForRangeStmt(SourceLocation ForLoc,
SourceLocation CoawaitLoc, Stmt *InitStmt,
@@ -2391,11 +2385,6 @@ StmtResult Sema::BuildCXXForRangeStmt(SourceLocation ForLoc,
DeclStmt *LoopVarDS = cast<DeclStmt>(LoopVarDecl);
VarDecl *LoopVar = cast<VarDecl>(LoopVarDS->getSingleDecl());
- // If we hit any errors, mark the loop variable as invalid if its type
- // contains 'auto'.
- InvalidateOnErrorScope Invalidate(*this, LoopVar,
- LoopVar->getType()->isUndeducedType());
-
StmtResult BeginDeclStmt = Begin;
StmtResult EndDeclStmt = End;
ExprResult NotEqExpr = Cond, IncrExpr = Inc;
@@ -2434,11 +2423,8 @@ StmtResult Sema::BuildCXXForRangeStmt(SourceLocation ForLoc,
QualType RangeType = Range->getType();
if (RequireCompleteType(RangeLoc, RangeType,
- diag::err_for_range_incomplete_type)) {
- if (LoopVar->getType()->isUndeducedType())
- LoopVar->setInvalidDecl();
+ diag::err_for_range_incomplete_type))
return StmtError();
- }
// Build auto __begin = begin-expr, __end = end-expr.
// Divide by 2, since the variables are in the inner scope (loop body).
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index bfbdfbf14379..ff9d4d610660 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -8086,8 +8086,12 @@ TreeTransform<Derived>::TransformCXXForRangeStmt(CXXForRangeStmt *S) {
Cond.get(),
Inc.get(), LoopVar.get(),
S->getRParenLoc());
- if (NewStmt.isInvalid())
+ if (NewStmt.isInvalid() && LoopVar.get() != S->getLoopVarStmt()) {
+ // Might not have attached any initializer to the loop variable.
+ getSema().ActOnInitializerError(
+ cast<DeclStmt>(LoopVar.get())->getSingleDecl());
return StmtError();
+ }
}
StmtResult Body = getDerived().TransformStmt(S->getBody());
diff --git a/clang/test/SemaCXX/cxx11-crashes.cpp b/clang/test/SemaCXX/cxx11-crashes.cpp
index 19205db85b6c..d60782df35f4 100644
--- a/clang/test/SemaCXX/cxx11-crashes.cpp
+++ b/clang/test/SemaCXX/cxx11-crashes.cpp
@@ -71,6 +71,7 @@ namespace b6981007 {
// We used to attempt to evaluate the initializer of this variable,
// and crash because it has an undeduced type.
const int &n(x);
+ constexpr int k = sizeof(x);
}
}
}
diff --git a/clang/test/SemaCXX/for-range-crash.cpp b/clang/test/SemaCXX/for-range-crash.cpp
index 21637264a2bf..766f34e50ed4 100644
--- a/clang/test/SemaCXX/for-range-crash.cpp
+++ b/clang/test/SemaCXX/for-range-crash.cpp
@@ -1,5 +1,10 @@
// RUN: %clang_cc1 -fsyntax-only -verify -std=c++17 %s
+// Ensure that we don't crash if errors are suppressed by an error limit.
+// RUN: not %clang_cc1 -fsyntax-only -std=c++17 -ferror-limit=1 %s
+
+error e; // expected-error {{unknown type name}}
+
template <typename>
class Bar {
Bar<int> *variables_to_modify;
@@ -8,3 +13,18 @@ class Bar {
delete c;
}
};
+
+void foo() {
+ int a;
+ struct X; // expected-note {{forward declaration}}
+ for (X x // expected-error {{incomplete type}}
+ : a) { // expected-error {{range expression of type 'int'}}
+ constexpr int n = sizeof(x);
+ }
+
+ struct S { int x, y; };
+ for (S [x, y] // expected-error {{must be 'auto'}}
+ : a) { // expected-error {{range expression}}
+ typename decltype(x)::a b;
+ }
+}
More information about the cfe-commits
mailing list