caio/clang-p2996
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

PR49585: Emit the jump destination for a for loop 'continue' from within the scope of the condition variable.

Browse Source 
The condition variable is in scope in the loop increment, so we need to
emit the jump destination from wthin the scope of the condition
variable.

For GCC compatibility (and compatibility with real-world 'FOR_EACH'
macros), 'continue' is permitted in a statement expression within the
condition of a for loop, though, so there are two cases here:

* If the for loop has no condition variable, we can emit the jump
  destination before emitting the condition.

* If the for loop has a condition variable, we must defer emitting the
  jump destination until after emitting the variable. We diagnose a
  'continue' appearing in the initializer of the condition variable,
  because it would jump past the initializer into the scope of that
  variable.

Reviewed By: rjmccall

Differential Revision: https://reviews.llvm.org/D98816
This commit is contained in:
Richard Smith
2021-03-17 14:00:03 -07:00
parent c615927c8e
commit a875721d8a
10 changed files with 238 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
clang/include/clang/Basic/DiagnosticSemaKinds.td
View File

@@ -5776,6 +5776,9 @@ def note_goto_ms_asm_label : Note<
def warn_unused_label : Warning<"unused label %0">,
InGroup<UnusedLabel>, DefaultIgnore;
def err_continue_from_cond_var_init : Error<
"cannot jump from this continue statement to the loop increment; "
"jump bypasses initialization of loop condition variable">;
def err_goto_into_protected_scope : Error<
"cannot jump from this goto statement to its label">;
def ext_goto_into_protected_scope : ExtWarn<

3
clang/include/clang/Parse/Parser.h
View File

@@ -1991,7 +1991,8 @@ private:
Sema::ConditionResult ParseCXXCondition(StmtResult *InitStmt,
SourceLocation Loc,
Sema::ConditionKind CK,
ForRangeInfo *FRI = nullptr);
ForRangeInfo *FRI = nullptr,
bool EnterForConditionScope = false);
//===--------------------------------------------------------------------===//
// C++ Coroutines

19
clang/include/clang/Sema/Scope.h
View File

@@ -129,11 +129,17 @@ public:
/// This is a compound statement scope.
CompoundStmtScope = 0x400000,
/// We are between inheritance colon and the real class/struct definition scope.
/// We are between inheritance colon and the real class/struct definition
/// scope.
ClassInheritanceScope = 0x800000,
/// This is the scope of a C++ catch statement.
CatchScope = 0x1000000,
/// This is a scope in which a condition variable is currently being
/// parsed. If such a scope is a ContinueScope, it's invalid to jump to the
/// continue block from here.
ConditionVarScope = 0x2000000,
};
private:
@@ -247,6 +253,17 @@ public:
return const_cast<Scope*>(this)->getContinueParent();
}
// Set whether we're in the scope of a condition variable, where 'continue'
// is disallowed despite being a continue scope.
void setIsConditionVarScope(bool InConditionVarScope) {
Flags = (Flags & ~ConditionVarScope) |
(InConditionVarScope ? ConditionVarScope : 0);
}
bool isConditionVarScope() const {
return Flags & ConditionVarScope;
}
/// getBreakParent - Return the closest scope that a break statement
/// would be affected by.
Scope *getBreakParent() {

33
clang/lib/CodeGen/CGStmt.cpp
View File

@@ -948,8 +948,8 @@ void CodeGenFunction::EmitForStmt(const ForStmt &S,
// Start the loop with a block that tests the condition.
// If there's an increment, the continue scope will be overwritten
// later.
JumpDest Continue = getJumpDestInCurrentScope("for.cond");
llvm::BasicBlock *CondBlock = Continue.getBlock();
JumpDest CondDest = getJumpDestInCurrentScope("for.cond");
llvm::BasicBlock *CondBlock = CondDest.getBlock();
EmitBlock(CondBlock);
bool LoopMustProgress = false;
@@ -967,24 +967,33 @@ void CodeGenFunction::EmitForStmt(const ForStmt &S,
SourceLocToDebugLoc(R.getBegin()),
SourceLocToDebugLoc(R.getEnd()), LoopMustProgress);
// If the for loop doesn't have an increment we can just use the
// condition as the continue block. Otherwise we'll need to create
// a block for it (in the current scope, i.e. in the scope of the
// condition), and that we will become our continue block.
if (S.getInc())
Continue = getJumpDestInCurrentScope("for.inc");
// Store the blocks to use for break and continue.
BreakContinueStack.push_back(BreakContinue(LoopExit, Continue));
// Create a cleanup scope for the condition variable cleanups.
LexicalScope ConditionScope(*this, S.getSourceRange());
// If the for loop doesn't have an increment we can just use the condition as
// the continue block. Otherwise, if there is no condition variable, we can
// form the continue block now. If there is a condition variable, we can't
// form the continue block until after we've emitted the condition, because
// the condition is in scope in the increment, but Sema's jump diagnostics
// ensure that there are no continues from the condition variable that jump
// to the loop increment.
JumpDest Continue;
if (!S.getInc())
Continue = CondDest;
else if (!S.getConditionVariable())
Continue = getJumpDestInCurrentScope("for.inc");
BreakContinueStack.push_back(BreakContinue(LoopExit, Continue));
if (S.getCond()) {
// If the for statement has a condition scope, emit the local variable
// declaration.
if (S.getConditionVariable()) {
EmitDecl(*S.getConditionVariable());
// We have entered the condition variable's scope, so we're now able to
// jump to the continue block.
Continue = getJumpDestInCurrentScope("for.inc");
BreakContinueStack.back().ContinueBlock = Continue;
}
llvm::BasicBlock *ExitBlock = LoopExit.getBlock();

30
clang/lib/Parse/ParseExprCXX.cpp
View File

@@ -1987,11 +1987,30 @@ Parser::ParseCXXTypeConstructExpression(const DeclSpec &DS) {
/// \param FRI If non-null, a for range declaration is permitted, and if
/// present will be parsed and stored here, and a null result will be returned.
///
/// \param EnterForConditionScope If true, enter a continue/break scope at the
/// appropriate moment for a 'for' loop.
///
/// \returns The parsed condition.
Sema::ConditionResult Parser::ParseCXXCondition(StmtResult *InitStmt,
SourceLocation Loc,
Sema::ConditionKind CK,
ForRangeInfo *FRI) {
ForRangeInfo *FRI,
bool EnterForConditionScope) {
// Helper to ensure we always enter a continue/break scope if requested.
struct ForConditionScopeRAII {
Scope *S;
void enter(bool IsConditionVariable) {
if (S) {
S->AddFlags(Scope::BreakScope | Scope::ContinueScope);
S->setIsConditionVarScope(IsConditionVariable);
}
}
~ForConditionScopeRAII() {
if (S)
S->setIsConditionVarScope(false);
}
} ForConditionScope{EnterForConditionScope ? getCurScope() : nullptr};
ParenBraceBracketBalancer BalancerRAIIObj(*this);
PreferredType.enterCondition(Actions, Tok.getLocation());
@@ -2014,6 +2033,9 @@ Sema::ConditionResult Parser::ParseCXXCondition(StmtResult *InitStmt,
// Determine what kind of thing we have.
switch (isCXXConditionDeclarationOrInitStatement(InitStmt, FRI)) {
case ConditionOrInitStatement::Expression: {
// If this is a for loop, we're entering its condition.
ForConditionScope.enter(/*IsConditionVariable=*/false);
ProhibitAttributes(attrs);
// We can have an empty expression here.
@@ -2056,6 +2078,9 @@ Sema::ConditionResult Parser::ParseCXXCondition(StmtResult *InitStmt,
}
case ConditionOrInitStatement::ForRangeDecl: {
// This is 'for (init-stmt; for-range-decl : range-expr)'.
// We're not actually in a for loop yet, so 'break' and 'continue' aren't
// permitted here.
assert(FRI && "should not parse a for range declaration here");
SourceLocation DeclStart = Tok.getLocation(), DeclEnd;
DeclGroupPtrTy DG = ParseSimpleDeclaration(DeclaratorContext::ForInit,
@@ -2069,6 +2094,9 @@ Sema::ConditionResult Parser::ParseCXXCondition(StmtResult *InitStmt,
break;
}
// If this is a for loop, we're entering its condition.
ForConditionScope.enter(/*IsConditionVariable=*/true);
// type-specifier-seq
DeclSpec DS(AttrFactory);
DS.takeAttributesFrom(attrs);

12
clang/lib/Parse/ParseStmt.cpp
View File

@@ -1959,7 +1959,6 @@ StmtResult Parser::ParseForStatement(SourceLocation *TrailingElseLoc) {
}
// Parse the second part of the for specifier.
getCurScope()->AddFlags(Scope::BreakScope | Scope::ContinueScope);
if (!ForEach && !ForRangeInfo.ParsedForRangeDecl() &&
!SecondPart.isInvalid()) {
// Parse the second part of the for specifier.
@@ -1975,7 +1974,8 @@ StmtResult Parser::ParseForStatement(SourceLocation *TrailingElseLoc) {
ColonProtectionRAIIObject ColonProtection(*this, MightBeForRangeStmt);
SecondPart =
ParseCXXCondition(nullptr, ForLoc, Sema::ConditionKind::Boolean,
MightBeForRangeStmt ? &ForRangeInfo : nullptr);
MightBeForRangeStmt ? &ForRangeInfo : nullptr,
/*EnterForConditionScope*/ true);
if (ForRangeInfo.ParsedForRangeDecl()) {
Diag(FirstPart.get() ? FirstPart.get()->getBeginLoc()
@@ -1992,6 +1992,9 @@ StmtResult Parser::ParseForStatement(SourceLocation *TrailingElseLoc) {
}
}
} else {
// We permit 'continue' and 'break' in the condition of a for loop.
getCurScope()->AddFlags(Scope::BreakScope | Scope::ContinueScope);
ExprResult SecondExpr = ParseExpression();
if (SecondExpr.isInvalid())
SecondPart = Sema::ConditionError();
@@ -2003,6 +2006,11 @@ StmtResult Parser::ParseForStatement(SourceLocation *TrailingElseLoc) {
}
}
// Enter a break / continue scope, if we didn't already enter one while
// parsing the second part.
if (!(getCurScope()->getFlags() & Scope::ContinueScope))
getCurScope()->AddFlags(Scope::BreakScope | Scope::ContinueScope);
// Parse the third part of the for statement.
if (!ForEach && !ForRangeInfo.ParsedForRangeDecl()) {
if (Tok.isNot(tok::semi)) {

6
clang/lib/Sema/SemaStmt.cpp
View File

@@ -3000,6 +3000,12 @@ Sema::ActOnContinueStmt(SourceLocation ContinueLoc, Scope *CurScope) {
// C99 6.8.6.2p1: A break shall appear only in or as a loop body.
return StmtError(Diag(ContinueLoc, diag::err_continue_not_in_loop));
}
if (S->getFlags() & Scope::ConditionVarScope) {
// We cannot 'continue;' from within a statement expression in the
// initializer of a condition variable because we would jump past the
// initialization of that variable.
return StmtError(Diag(ContinueLoc, diag::err_continue_from_cond_var_init));
}
CheckJumpOutOfSEHFinally(*this, ContinueLoc, *S);
return new (Context) ContinueStmt(ContinueLoc);

125
clang/test/CodeGenCXX/for-cond-var.cpp Normal file
View File

@@ -0,0 +1,125 @@
// RUN: %clang_cc1 -emit-llvm -o - %s -triple x86_64-linux | FileCheck %s
struct A {
A();
A(const A &);
~A();
operator bool();
void *data;
};
A make();
bool cond();
void f(int);
// PR49585: Ensure that 'continue' performs the proper cleanups in the presence
// of a for loop condition variable.
//
// CHECK: define {{.*}} void @_Z7PR49585v(
void PR49585() {
for (
// CHECK: call void @_Z1fi(i32 1)
// CHECK: br label %[[for_cond:.*]]
f(1);
// CHECK: [[for_cond]]:
// CHECK: call {{.*}} @_Z4makev(
// CHECK: call {{.*}} @_ZN1AcvbEv(
// CHECK: br i1 {{.*}}, label %[[for_body:.*]], label %[[for_cond_cleanup:.*]]
A a = make();
// CHECK: [[for_cond_cleanup]]:
// CHECK: store
// CHECK: br label %[[cleanup:.*]]
f(2)) {
// CHECK: [[for_body]]:
// CHECK: call {{.*}} @_Z4condv(
// CHECK: br i1 {{.*}}, label %[[if_then:.*]], label %[[if_end:.*]]
if (cond()) {
// CHECK: [[if_then]]:
// CHECK: call {{.*}} @_Z1fi(i32 3)
// CHECK: br label %[[for_inc:.*]]
f(3);
continue;
}
// CHECK: [[if_end]]:
// CHECK: call {{.*}} @_Z1fi(i32 4)
// CHECK: br label %[[for_inc]]
f(4);
}
// CHECK: [[for_inc]]:
// CHECK: call void @_Z1fi(i32 2)
// CHECK: store
// CHECK: br label %[[cleanup]]
// CHECK: [[cleanup]]:
// CHECK: call void @_ZN1AD1Ev(
// CHECK: load
// CHECK: switch {{.*}} label
// CHECK-NEXT: label %[[cleanup_cont:.*]]
// CHECK-NEXT: label %[[for_end:.*]]
// CHECK: [[cleanup_cont]]:
// CHECK: br label %[[for_cond]]
// CHECK [[for_end]]:
// CHECK: ret void
}
// CHECK: define {{.*}} void @_Z13PR49585_breakv(
void PR49585_break() {
for (
// CHECK: call void @_Z1fi(i32 1)
// CHECK: br label %[[for_cond:.*]]
f(1);
// CHECK: [[for_cond]]:
// CHECK: call {{.*}} @_Z4makev(
// CHECK: call {{.*}} @_ZN1AcvbEv(
// CHECK: br i1 {{.*}}, label %[[for_body:.*]], label %[[for_cond_cleanup:.*]]
A a = make();
// CHECK: [[for_cond_cleanup]]:
// CHECK: store
// CHECK: br label %[[cleanup:.*]]
f(2)) {
// CHECK: [[for_body]]:
// CHECK: call {{.*}} @_Z4condv(
// CHECK: br i1 {{.*}}, label %[[if_then:.*]], label %[[if_end:.*]]
if (cond()) {
// CHECK: [[if_then]]:
// CHECK: call {{.*}} @_Z1fi(i32 3)
// CHECK: store
// CHECK: br label %[[cleanup:.*]]
f(3);
break;
}
// CHECK: [[if_end]]:
// CHECK: call {{.*}} @_Z1fi(i32 4)
// CHECK: br label %[[for_inc]]
f(4);
}
// CHECK: [[for_inc]]:
// CHECK: call void @_Z1fi(i32 2)
// CHECK: store
// CHECK: br label %[[cleanup]]
// CHECK: [[cleanup]]:
// CHECK: call void @_ZN1AD1Ev(
// CHECK: load
// CHECK: switch {{.*}} label
// CHECK-NEXT: label %[[cleanup_cont:.*]]
// CHECK-NEXT: label %[[for_end:.*]]
// CHECK: [[cleanup_cont]]:
// CHECK: br label %[[for_cond]]
// CHECK [[for_end]]:
// CHECK: ret void
}

8
clang/test/Parser/cxx2a-init-statement.cpp
View File

@@ -31,4 +31,12 @@ void f() {
for (int n = 0; static int m = 0; ++n) {} // expected-error {{type name does not allow storage class}}
for (int n = 0; static int m : arr1) {} // expected-error {{loop variable 'm' may not be declared 'static'}}
// The init-statement and range are not break / continue scopes. (But the body is.)
for (int n = ({ break; 0; }); int m : arr1) {} // expected-error {{not in loop}}
for (int n = ({ continue; 0; }); int m : arr1) {} // expected-error {{not in loop}}
for (int arr[3]; int n : *({ break; &arr; })) {} // expected-error {{not in loop}}
for (int arr[3]; int n : *({ continue; &arr; })) {} // expected-error {{not in loop}}
for (int n = 0; int m : arr1) { break; }
for (int n = 0; int m : arr1) { continue; }
}

16
clang/test/SemaCXX/scope-check.cpp
View File

@@ -629,3 +629,19 @@ l: // expected-note 4 {{possible target of indirect goto statement}}
}
} // namespace seh
void continue_scope_check() {
// These are OK.
for (; ({break; true;});) {}
for (; ({continue; true;});) {}
for (; int n = ({break; 0;});) {}
for (; int n = 0; ({break;})) {}
for (; int n = 0; ({continue;})) {}
// This would jump past the initialization of 'n' to the increment (where 'n'
// is in scope).
for (; int n = ({continue; 0;});) {} // expected-error {{cannot jump from this continue statement to the loop increment; jump bypasses initialization of loop condition variable}}
// An intervening loop makes it OK again.
for (; int n = ({while (true) continue; 0;});) {}
}