llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: FlyMan2004 <details> <summary>Changes</summary> Fixes llvm/llvm-project#<!-- -->230932 The optimized switch-case shortcut in `EmitCaseStmt` used the innermost `BreakContinueStack` entry for a case containing only a break. For a named `break` targeting an enclosing loop, this selected the `switch` exit and continued the loop instead of terminating it. Resolve the destination through `GetDestForLoopControlStmt` before checking cleanups. This also redirects fallthrough to the named target and preserves required cleanup paths. Cover `for`, `while`, and do-while in C and C++, plus C++ range-`for`, with ordinary break controls and lifetime checks. The optimized checks use `-O1 -disable-llvm-passes` to isolate frontend code generation. LLM tools were used for this contribution. I've reviewed, built, and tested the change myself before pushing to GitHub. --- Full diff: https://github.com/llvm/llvm-project/pull/230936.diff 4 Files Affected: - (modified) clang/docs/ReleaseNotes.md (+5) - (modified) clang/lib/CodeGen/CGStmt.cpp (+3-2) - (modified) clang/test/CodeGen/labeled-break-continue.c (+110) - (modified) clang/test/CodeGenCXX/labeled-break-continue.cpp (+149) ``````````diff diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index 77a1e73bc8af6..60b08e064b7cc 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -945,6 +945,11 @@ features cannot lower the translation-unit ABI level; #### Miscellaneous Bug Fixes +- Fixed a miscompilation of C2y named `break` statements targeting enclosing loops + from `switch` cases with optimizations enabled. These breaks now exit the named + loop and run required cleanups instead of leaving only the `switch`. + (#GH230932) + #### Miscellaneous Clang Crashes Fixed - Fixed a crash when Microsoft extensions were enabled and an unterminated diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp index 05b3693138b0a..38988a0295b13 100644 --- a/clang/lib/CodeGen/CGStmt.cpp +++ b/clang/lib/CodeGen/CGStmt.cpp @@ -1870,7 +1870,8 @@ void CodeGenFunction::EmitCaseStmt(const CaseStmt &S, if (!CGM.getCodeGenOpts().hasProfileClangInstr() && CGM.getCodeGenOpts().OptimizationLevel > 0 && isa<BreakStmt>(S.getSubStmt())) { - JumpDest Block = BreakContinueStack.back().BreakBlock; + const auto &Break = cast<BreakStmt>(*S.getSubStmt()); + JumpDest Block = GetDestForLoopControlStmt(Break)->BreakBlock; // Only do this optimization if there are no cleanups that need emitting. if (isObviouslyBranchWithoutCleanups(Block)) { @@ -1879,7 +1880,7 @@ void CodeGenFunction::EmitCaseStmt(const CaseStmt &S, SwitchInsn->addCase(CaseVal, Block.getBlock()); // If there was a fallthrough into this case, make sure to redirect it to - // the end of the switch as well. + // the break destination as well. if (Builder.GetInsertBlock()) { Builder.CreateBr(Block.getBlock()); Builder.ClearInsertionPoint(); diff --git a/clang/test/CodeGen/labeled-break-continue.c b/clang/test/CodeGen/labeled-break-continue.c index f307a1bd79ab8..93a9763efcf89 100644 --- a/clang/test/CodeGen/labeled-break-continue.c +++ b/clang/test/CodeGen/labeled-break-continue.c @@ -1,4 +1,5 @@ // RUN: %clang_cc1 -std=c2y -triple x86_64-unknown-linux -emit-llvm -o - %s | FileCheck %s +// RUN: %clang_cc1 -std=c2y -triple x86_64-unknown-linux -O1 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefix=OPT bool g1(); bool g2(); @@ -144,6 +145,12 @@ void f4() { // CHECK: br label %while.cond // CHECK: while.end: // CHECK: ret void +// OPT-LABEL: define {{.*}} void @f5() +// OPT: switch i32 {{.*}}, label %sw.epilog [ +// OPT-NEXT: i32 1, label %while.end +// OPT-NEXT: i32 2, label %sw.epilog +// OPT-NEXT: i32 3, label %sw.bb +// OPT-NEXT: ] void f5() { l1: while (g1()) { l2: switch (g2()) { @@ -272,6 +279,10 @@ void f6() { // CHECK: br label %while.cond // CHECK: while.end: // CHECK: ret void +// OPT-LABEL: define {{.*}} void @f7() +// OPT: switch i32 {{.*}}, label %sw.epilog [ +// OPT-NEXT: i32 1, label %while.end +// OPT-NEXT: ] void f7() { loop: while (g1()) { switch (g2()) { @@ -279,3 +290,102 @@ void f7() { } } } + +// CHECK-LABEL: define {{.*}} i32 @f8() +// CHECK: switch i32 {{.*}}, label %sw.epilog [ +// CHECK-NEXT: i32 0, label %sw.bb +// CHECK-NEXT: ] +// CHECK: sw.bb: +// CHECK-NEXT: br label %for.end +// OPT-LABEL: define {{.*}} i32 @f8() +// OPT: switch i32 {{.*}}, label %sw.epilog [ +// OPT-NEXT: i32 0, label %sw.bb +// OPT-NEXT: ] +// OPT: sw.bb: +// OPT-NEXT: store i32 {{[0-9]+}}, ptr %cleanup.dest.slot +// OPT-NEXT: br label %cleanup +// OPT: cleanup: +// OPT-NEXT: call void @llvm.lifetime.end.p0(ptr %i) +// OPT-NEXT: br label %for.end +int f8() { + int count = 0; + outer: for (int i = 0; i < 2; ++i) { + ++count; + switch (i) { + case 0: break outer; + } + } + return count; +} + +// Check both the switch edge and fallthrough into a named break. +// OPT-LABEL: define {{.*}} void @f9( +// OPT: switch i32 {{.*}}, label %sw.epilog [ +// OPT-NEXT: i32 1, label %sw.bb +// OPT-NEXT: i32 0, label %while.end +// OPT-NEXT: i32 2, label %sw.epilog +// OPT-NEXT: ] +// OPT: sw.bb: +// OPT: call {{.*}} i1 @g2() +// OPT-NEXT: br label %while.end +void f9(int x) { + loop: while (g1()) { + switch (x) { + case 1: g2(); + case 0: break loop; + case 2: break; + } + } +} + +// Without loop-scope cleanups, the named break can go directly to the loop +// exit. An ordinary break must still go to the switch exit. +// CHECK-LABEL: define {{.*}} void @f10( +// CHECK: switch i32 {{.*}}, label %sw.epilog [ +// CHECK-NEXT: i32 0, label %sw.bb +// CHECK-NEXT: i32 1, label %sw.bb1 +// CHECK-NEXT: ] +// CHECK: sw.bb: +// CHECK-NEXT: br label %for.end +// CHECK: sw.bb1: +// CHECK-NEXT: br label %sw.epilog +// OPT-LABEL: define {{.*}} void @f10( +// OPT: switch i32 {{.*}}, label %sw.epilog [ +// OPT-NEXT: i32 0, label %for.end +// OPT-NEXT: i32 1, label %sw.epilog +// OPT-NEXT: ] +// OPT: sw.epilog: +// OPT-NEXT: br label %for.cond +void f10(int x) { + loop: for (; g1();) { + switch (x) { + case 0: break loop; + case 1: break; + } + } +} + +// CHECK-LABEL: define {{.*}} void @f11( +// CHECK: switch i32 {{.*}}, label %sw.epilog [ +// CHECK-NEXT: i32 0, label %sw.bb +// CHECK-NEXT: i32 1, label %sw.bb1 +// CHECK-NEXT: ] +// CHECK: sw.bb: +// CHECK-NEXT: br label %do.end +// CHECK: sw.bb1: +// CHECK-NEXT: br label %sw.epilog +// OPT-LABEL: define {{.*}} void @f11( +// OPT: switch i32 {{.*}}, label %sw.epilog [ +// OPT-NEXT: i32 0, label %do.end +// OPT-NEXT: i32 1, label %sw.epilog +// OPT-NEXT: ] +// OPT: sw.epilog: +// OPT-NEXT: br label %do.cond +void f11(int x) { + loop: do { + switch (x) { + case 0: break loop; + case 1: break; + } + } while (g1()); +} diff --git a/clang/test/CodeGenCXX/labeled-break-continue.cpp b/clang/test/CodeGenCXX/labeled-break-continue.cpp index 50c222a99c25c..7442ccf7e4c02 100644 --- a/clang/test/CodeGenCXX/labeled-break-continue.cpp +++ b/clang/test/CodeGenCXX/labeled-break-continue.cpp @@ -1,4 +1,5 @@ // RUN: %clang_cc1 -fnamed-loops -triple x86_64-unknown-linux -std=c++20 -emit-llvm -o - %s | FileCheck %s +// RUN: %clang_cc1 -fnamed-loops -triple x86_64-unknown-linux -std=c++20 -O1 -disable-llvm-passes -emit-llvm -o - %s | FileCheck %s --check-prefix=OPT static int a[10]{}; struct NonTrivialDestructor { @@ -219,3 +220,151 @@ template void f3<true>(); // CHECK: while.end: // CHECK: ret void template void f3<false>(); + +// A named break out of the loop must run its cleanup, even though breaking out +// of the inner switch would not require one. +// OPT-LABEL: define {{.*}} void @_Z2f4i( +// OPT: switch i32 {{.*}}, label %sw.epilog [ +// OPT-NEXT: i32 0, label %sw.bb +// OPT-NEXT: ] +// OPT: sw.bb: +// OPT-NEXT: store i32 [[EXIT:[0-9]+]], ptr %cleanup.dest.slot +// OPT-NEXT: br label %cleanup +// OPT: cleanup: +// OPT-NEXT: call void @_ZN20NonTrivialDestructorD1Ev( +// OPT: switch i32 %cleanup.dest, label %unreachable [ +// OPT: i32 [[EXIT]], label %while.end +void f4(int x) { + loop: while (g(0)) { + NonTrivialDestructor n; + switch (x) { + case 0: break loop; + } + } +} + +// Without loop-scope cleanups, the named break can go directly to the loop +// exit. An ordinary break must still go to the switch exit. +// CHECK-LABEL: define {{.*}} void @_Z2f5i( +// CHECK: switch i32 {{.*}}, label %sw.epilog [ +// CHECK-NEXT: i32 0, label %sw.bb +// CHECK-NEXT: i32 1, label %sw.bb1 +// CHECK-NEXT: ] +// CHECK: sw.bb: +// CHECK-NEXT: br label %for.end +// CHECK: sw.bb1: +// CHECK-NEXT: br label %sw.epilog +// OPT-LABEL: define {{.*}} void @_Z2f5i( +// OPT: switch i32 {{.*}}, label %sw.epilog [ +// OPT-NEXT: i32 0, label %for.end +// OPT-NEXT: i32 1, label %sw.epilog +// OPT-NEXT: ] +// OPT: sw.epilog: +// OPT-NEXT: br label %for.cond +void f5(int x) { + loop: for (; h();) { + switch (x) { + case 0: break loop; + case 1: break; + } + } +} + +// CHECK-LABEL: define {{.*}} void @_Z2f6i( +// CHECK: switch i32 {{.*}}, label %sw.epilog [ +// CHECK-NEXT: i32 0, label %sw.bb +// CHECK-NEXT: i32 1, label %sw.bb1 +// CHECK-NEXT: ] +// CHECK: sw.bb: +// CHECK-NEXT: br label %while.end +// CHECK: sw.bb1: +// CHECK-NEXT: br label %sw.epilog +// OPT-LABEL: define {{.*}} void @_Z2f6i( +// OPT: switch i32 {{.*}}, label %sw.epilog [ +// OPT-NEXT: i32 0, label %while.end +// OPT-NEXT: i32 1, label %sw.epilog +// OPT-NEXT: ] +// OPT: sw.epilog: +// OPT-NEXT: br label %while.cond +void f6(int x) { + loop: while (h()) { + switch (x) { + case 0: break loop; + case 1: break; + } + } +} + +// CHECK-LABEL: define {{.*}} void @_Z2f7i( +// CHECK: switch i32 {{.*}}, label %sw.epilog [ +// CHECK-NEXT: i32 0, label %sw.bb +// CHECK-NEXT: i32 1, label %sw.bb1 +// CHECK-NEXT: ] +// CHECK: sw.bb: +// CHECK-NEXT: br label %do.end +// CHECK: sw.bb1: +// CHECK-NEXT: br label %sw.epilog +// OPT-LABEL: define {{.*}} void @_Z2f7i( +// OPT: switch i32 {{.*}}, label %sw.epilog [ +// OPT-NEXT: i32 0, label %do.end +// OPT-NEXT: i32 1, label %sw.epilog +// OPT-NEXT: ] +// OPT: sw.epilog: +// OPT-NEXT: br label %do.cond +void f7(int x) { + loop: do { + switch (x) { + case 0: break loop; + case 1: break; + } + } while (h()); +} + +// A named break must end the loop variable and iterator lifetimes before +// leaving the range-for. An ordinary break ends only the loop variable lifetime. +// CHECK-LABEL: define {{.*}} void @_Z2f8i( +// CHECK: switch i32 {{.*}}, label %sw.epilog [ +// CHECK-NEXT: i32 0, label %sw.bb +// CHECK-NEXT: i32 1, label %sw.bb1 +// CHECK-NEXT: ] +// CHECK: sw.bb: +// CHECK-NEXT: br label %for.end +// CHECK: sw.bb1: +// CHECK-NEXT: br label %sw.epilog +// OPT-LABEL: define {{.*}} void @_Z2f8i( +// OPT: for.cond.cleanup: +// OPT-NEXT: store i32 [[RANGE_EXIT:[1-9][0-9]*]], ptr %cleanup.dest.slot +// OPT-NEXT: br label %cleanup1 +// OPT: switch i32 {{.*}}, label %sw.epilog [ +// OPT-NEXT: i32 0, label %sw.bb +// OPT-NEXT: i32 1, label %sw.epilog +// OPT-NEXT: ] +// OPT: sw.bb: +// OPT-NEXT: store i32 [[RANGE_EXIT]], ptr %cleanup.dest.slot +// OPT-NEXT: br label %cleanup +// OPT: sw.epilog: +// OPT-NEXT: store i32 0, ptr %cleanup.dest.slot +// OPT-NEXT: br label %cleanup +// OPT: cleanup: +// OPT-NEXT: call void @llvm.lifetime.end.p0(ptr %i) +// OPT-NEXT: %cleanup.dest = load i32, ptr %cleanup.dest.slot +// OPT-NEXT: switch i32 %cleanup.dest, label %cleanup1 [ +// OPT-NEXT: i32 0, label %cleanup.cont +// OPT-NEXT: ] +// OPT: cleanup.cont: +// OPT-NEXT: br label %for.inc +// OPT: cleanup1: +// OPT-NEXT: call void @llvm.lifetime.end.p0(ptr %__end1) +// OPT-NEXT: call void @llvm.lifetime.end.p0(ptr %__begin1) +// OPT-NEXT: call void @llvm.lifetime.end.p0(ptr %__range1) +// OPT-NEXT: br label %for.end +// OPT: for.end: +// OPT-NEXT: ret void +void f8(int x) { + loop: for (int i : a) { + switch (x) { + case 0: break loop; + case 1: break; + } + } +} `````````` </details> https://github.com/llvm/llvm-project/pull/230936 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
