Skip to content

Commit 6421171

Browse files
cor3ntincor3ntin
authored
[Clang] Implement P2809: Trivial infinite loops are not Undefined Behavior (#90066)
https://wg21.link/P2809R3 This is applied as a DR to C++11 (C++98 did not guarantee forward progress and is left untouched) As an extension (and to preserve existing behavior in C), we consider all controlling expression that can be constant folded in the front end, not just standard constant expressions.
1 parent 5850f6b commit 6421171

File tree

8 files changed

+181
-83
lines changed

8 files changed

+181
-83
lines changed

‎clang/docs/ReleaseNotes.rst

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,9 @@ C++2c Feature Support
190190

191191
- Implemented `P2748R5 Disallow Binding a Returned Glvalue to a Temporary <https://wg21.link/P2748R5>`_.
192192

193+
- Implemented `P2809R3: Trivial infinite loops are not Undefined Behavior <https://wg21.link/P2809R3>`_.
194+
195+
193196
Resolutions to C++ Defect Reports
194197
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
195198
- Substitute template parameter pack, when it is not explicitly specified
@@ -334,6 +337,10 @@ Modified Compiler Flags
334337
- Carved out ``-Wformat`` warning about scoped enums into a subwarning and
335338
make it controlled by ``-Wformat-pedantic``. Fixes #GH88595.
336339

340+
- Trivial infinite loops (i.e loops with a constant controlling expresion
341+
evaluating to ``true`` and an empty body such as ``while(1);``)
342+
are considered infinite, even when the ``-ffinite-loop`` flag is set.
343+
337344
Removed Compiler Flags
338345
-------------------------
339346

‎clang/include/clang/Driver/Options.td

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3971,7 +3971,7 @@ def funroll_loops : Flag<["-"], "funroll-loops">, Group<f_Group>,
39713971
def fno_unroll_loops : Flag<["-"], "fno-unroll-loops">, Group<f_Group>,
39723972
HelpText<"Turn off loop unroller">, Visibility<[ClangOption, CC1Option]>;
39733973
def ffinite_loops: Flag<["-"], "ffinite-loops">, Group<f_Group>,
3974-
HelpText<"Assume all loops are finite.">, Visibility<[ClangOption, CC1Option]>;
3974+
HelpText<"Assume all non-trivial loops are finite.">, Visibility<[ClangOption, CC1Option]>;
39753975
def fno_finite_loops: Flag<["-"], "fno-finite-loops">, Group<f_Group>,
39763976
HelpText<"Do not assume that any loop is finite.">,
39773977
Visibility<[ClangOption, CC1Option]>;

‎clang/lib/CodeGen/CGStmt.cpp

Lines changed: 67 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -908,6 +908,69 @@ void CodeGenFunction::EmitIfStmt(const IfStmt &S) {
908908
incrementProfileCounter(&S);
909909
}
910910

911+
bool CodeGenFunction::checkIfLoopMustProgress(const Expr *ControllingExpression,
912+
bool HasEmptyBody) {
913+
if (CGM.getCodeGenOpts().getFiniteLoops() ==
914+
CodeGenOptions::FiniteLoopsKind::Never)
915+
return false;
916+
917+
// Now apply rules for plain C (see 6.8.5.6 in C11).
918+
// Loops with constant conditions do not have to make progress in any C
919+
// version.
920+
// As an extension, we consisider loops whose constant expression
921+
// can be constant-folded.
922+
Expr::EvalResult Result;
923+
bool CondIsConstInt =
924+
!ControllingExpression ||
925+
(ControllingExpression->EvaluateAsInt(Result, getContext()) &&
926+
Result.Val.isInt());
927+
928+
bool CondIsTrue = CondIsConstInt && (!ControllingExpression ||
929+
Result.Val.getInt().getBoolValue());
930+
931+
// Loops with non-constant conditions must make progress in C11 and later.
932+
if (getLangOpts().C11 && !CondIsConstInt)
933+
return true;
934+
935+
// [C++26][intro.progress] (DR)
936+
// The implementation may assume that any thread will eventually do one of the
937+
// following:
938+
// [...]
939+
// - continue execution of a trivial infinite loop ([stmt.iter.general]).
940+
if (CGM.getCodeGenOpts().getFiniteLoops() ==
941+
CodeGenOptions::FiniteLoopsKind::Always ||
942+
getLangOpts().CPlusPlus11) {
943+
if (HasEmptyBody && CondIsTrue) {
944+
CurFn->removeFnAttr(llvm::Attribute::MustProgress);
945+
return false;
946+
}
947+
return true;
948+
}
949+
return false;
950+
}
951+
952+
// [C++26][stmt.iter.general] (DR)
953+
// A trivially empty iteration statement is an iteration statement matching one
954+
// of the following forms:
955+
// - while ( expression ) ;
956+
// - while ( expression ) { }
957+
// - do ; while ( expression ) ;
958+
// - do { } while ( expression ) ;
959+
// - for ( init-statement expression(opt); ) ;
960+
// - for ( init-statement expression(opt); ) { }
961+
template <typename LoopStmt> static bool hasEmptyLoopBody(const LoopStmt &S) {
962+
if constexpr (std::is_same_v<LoopStmt, ForStmt>) {
963+
if (S.getInc())
964+
return false;
965+
}
966+
const Stmt *Body = S.getBody();
967+
if (!Body || isa<NullStmt>(Body))
968+
return true;
969+
if (const CompoundStmt *Compound = dyn_cast<CompoundStmt>(Body))
970+
return Compound->body_empty();
971+
return false;
972+
}
973+
911974
void CodeGenFunction::EmitWhileStmt(const WhileStmt &S,
912975
ArrayRef<const Attr *> WhileAttrs) {
913976
// Emit the header for the loop, which will also become
@@ -942,13 +1005,12 @@ void CodeGenFunction::EmitWhileStmt(const WhileStmt &S,
9421005
// while(1) is common, avoid extra exit blocks. Be sure
9431006
// to correctly handle break/continue though.
9441007
llvm::ConstantInt *C = dyn_cast<llvm::ConstantInt>(BoolCondVal);
945-
bool CondIsConstInt = C != nullptr;
946-
bool EmitBoolCondBranch = !CondIsConstInt || !C->isOne();
1008+
bool EmitBoolCondBranch = !C || !C->isOne();
9471009
const SourceRange &R = S.getSourceRange();
9481010
LoopStack.push(LoopHeader.getBlock(), CGM.getContext(), CGM.getCodeGenOpts(),
9491011
WhileAttrs, SourceLocToDebugLoc(R.getBegin()),
9501012
SourceLocToDebugLoc(R.getEnd()),
951-
checkIfLoopMustProgress(CondIsConstInt));
1013+
checkIfLoopMustProgress(S.getCond(), hasEmptyLoopBody(S)));
9521014

9531015
// When single byte coverage mode is enabled, add a counter to loop condition.
9541016
if (llvm::EnableSingleByteCoverage)
@@ -1059,14 +1121,13 @@ void CodeGenFunction::EmitDoStmt(const DoStmt &S,
10591121
// "do {} while (0)" is common in macros, avoid extra blocks. Be sure
10601122
// to correctly handle break/continue though.
10611123
llvm::ConstantInt *C = dyn_cast<llvm::ConstantInt>(BoolCondVal);
1062-
bool CondIsConstInt = C;
10631124
bool EmitBoolCondBranch = !C || !C->isZero();
10641125

10651126
const SourceRange &R = S.getSourceRange();
10661127
LoopStack.push(LoopBody, CGM.getContext(), CGM.getCodeGenOpts(), DoAttrs,
10671128
SourceLocToDebugLoc(R.getBegin()),
10681129
SourceLocToDebugLoc(R.getEnd()),
1069-
checkIfLoopMustProgress(CondIsConstInt));
1130+
checkIfLoopMustProgress(S.getCond(), hasEmptyLoopBody(S)));
10701131

10711132
// As long as the condition is true, iterate the loop.
10721133
if (EmitBoolCondBranch) {
@@ -1109,15 +1170,11 @@ void CodeGenFunction::EmitForStmt(const ForStmt &S,
11091170
llvm::BasicBlock *CondBlock = CondDest.getBlock();
11101171
EmitBlock(CondBlock);
11111172

1112-
Expr::EvalResult Result;
1113-
bool CondIsConstInt =
1114-
!S.getCond() || S.getCond()->EvaluateAsInt(Result, getContext());
1115-
11161173
const SourceRange &R = S.getSourceRange();
11171174
LoopStack.push(CondBlock, CGM.getContext(), CGM.getCodeGenOpts(), ForAttrs,
11181175
SourceLocToDebugLoc(R.getBegin()),
11191176
SourceLocToDebugLoc(R.getEnd()),
1120-
checkIfLoopMustProgress(CondIsConstInt));
1177+
checkIfLoopMustProgress(S.getCond(), hasEmptyLoopBody(S)));
11211178

11221179
// Create a cleanup scope for the condition variable cleanups.
11231180
LexicalScope ConditionScope(*this, S.getSourceRange());

‎clang/lib/CodeGen/CodeGenFunction.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1471,6 +1471,8 @@ void CodeGenFunction::GenerateCode(GlobalDecl GD, llvm::Function *Fn,
14711471

14721472
// Ensure that the function adheres to the forward progress guarantee, which
14731473
// is required by certain optimizations.
1474+
// In C++11 and up, the attribute will be removed if the body contains a
1475+
// trivial empty loop.
14741476
if (checkIfFunctionMustProgress())
14751477
CurFn->addFnAttr(llvm::Attribute::MustProgress);
14761478

‎clang/lib/CodeGen/CodeGenFunction.h

Lines changed: 1 addition & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -636,28 +636,7 @@ class CodeGenFunction : public CodeGenTypeCache {
636636
/// Returns true if a loop must make progress, which means the mustprogress
637637
/// attribute can be added. \p HasConstantCond indicates whether the branch
638638
/// condition is a known constant.
639-
bool checkIfLoopMustProgress(bool HasConstantCond) {
640-
if (CGM.getCodeGenOpts().getFiniteLoops() ==
641-
CodeGenOptions::FiniteLoopsKind::Always)
642-
return true;
643-
if (CGM.getCodeGenOpts().getFiniteLoops() ==
644-
CodeGenOptions::FiniteLoopsKind::Never)
645-
return false;
646-
647-
// If the containing function must make progress, loops also must make
648-
// progress (as in C++11 and later).
649-
if (checkIfFunctionMustProgress())
650-
return true;
651-
652-
// Now apply rules for plain C (see 6.8.5.6 in C11).
653-
// Loops with constant conditions do not have to make progress in any C
654-
// version.
655-
if (HasConstantCond)
656-
return false;
657-
658-
// Loops with non-constant conditions must make progress in C11 and later.
659-
return getLangOpts().C11;
660-
}
639+
bool checkIfLoopMustProgress(const Expr *, bool HasEmptyBody);
661640

662641
const CodeGen::CGBlockInfo *BlockInfo = nullptr;
663642
llvm::Value *BlockPointer = nullptr;

‎clang/test/CodeGen/attr-mustprogress.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ int b = 0;
3030
// CHECK: for.cond:
3131
// C99-NOT: br {{.*}}!llvm.loop
3232
// C11-NOT: br {{.*}}!llvm.loop
33-
// FINITE-NEXT: br {{.*}}!llvm.loop
33+
// FINITE-NOR: br {{.*}}!llvm.loop
3434
//
3535
void f0(void) {
3636
for (; ;) ;
@@ -45,7 +45,7 @@ void f0(void) {
4545
// CHECK: for.body:
4646
// C99-NOT: br {{.*}}, !llvm.loop
4747
// C11-NOT: br {{.*}}, !llvm.loop
48-
// FINITE-NEXT: br {{.*}}, !llvm.loop
48+
// FINITE-NOT: br {{.*}}, !llvm.loop
4949
// CHECK: for.end:
5050
// CHECK-NEXT: ret void
5151
//
@@ -84,7 +84,7 @@ void f2(void) {
8484
// CHECK: for.body:
8585
// C99-NOT: br {{.*}}, !llvm.loop
8686
// C11-NOT: br {{.*}}, !llvm.loop
87-
// FINITE-NEXT: br {{.*}}, !llvm.loop
87+
// FINITE-NOT: br {{.*}}, !llvm.loop
8888
// CHECK: for.end:
8989
// CHECK-NEXT: br label %for.cond1
9090
// CHECK: for.cond1:
@@ -113,7 +113,7 @@ void F(void) {
113113
// CHECK: while.body:
114114
// C99-NOT: br {{.*}}, !llvm.loop
115115
// C11-NOT: br {{.*}}, !llvm.loop
116-
// FINITE-NEXT: br {{.*}}, !llvm.loop
116+
// FINITE-NOT: br {{.*}}, !llvm.loop
117117
//
118118
void w1(void) {
119119
while (1) {
@@ -159,7 +159,7 @@ void w2(void) {
159159
// CHECK: while.body2:
160160
// C99-NOT: br {{.*}} !llvm.loop
161161
// C11-NOT: br {{.*}} !llvm.loop
162-
// FINITE-NEXT: br {{.*}} !llvm.loop
162+
// FINITE-NOT: br {{.*}} !llvm.loop
163163
//
164164
void W(void) {
165165
while (a == b) {
@@ -177,7 +177,7 @@ void W(void) {
177177
// CHECK: do.cond:
178178
// C99-NOT: br {{.*}}, !llvm.loop
179179
// C11-NOT: br {{.*}}, !llvm.loop
180-
// FINITE-NEXT: br {{.*}}, !llvm.loop
180+
// FINITE-NOT: br {{.*}}, !llvm.loop
181181
// CHECK: do.end:
182182
// CHECK-NEXT: ret void
183183
//

0 commit comments

Comments
 (0)