https://github.com/akash-manna-sky updated https://github.com/llvm/llvm-project/pull/235809
>From cf75d3bac32c3eb76c693672f3b699d7fa3424f9 Mon Sep 17 00:00:00 2001 From: Akash Manna <[email protected]> Date: Sun, 25 Oct 2026 08:59:04 +0530 Subject: [PATCH] [clang][CodeGen] Fix crash on break out of a for loop with a C89 init declaration Since #156643 a variable declared in the init-statement of a for loop in C89 mode no longer gets its own LexicalScope, so its cleanup lives on the enclosing scope's cleanup stack. The loop exit was still created before the init-statement, so its recorded cleanup depth did not include that cleanup. A break therefore looked like a jump out through a pending cleanup: EmitBranchThroughCleanup rerouted it into the cleanup block and recorded for.end as a branch-after, for.end ended up with no users and was deleted at the end of the loop, and FinishFunction later built a branch to the freed block when it popped the cleanup. Create the loop exit after the init-statement in C89 mode, so it records the depth that includes the variable's cleanup and leaving the loop branches to for.end directly, as the variable belongs to the enclosing scope. The C99/C++ path is unchanged. Fixes #170243 --- clang/docs/ReleaseNotes.md | 2 + clang/lib/CodeGen/CGStmt.cpp | 19 ++- clang/test/CodeGen/attr-cleanup.c | 142 ++++++++++++++++++ clang/test/CodeGen/c89-for-init-lifetime.c | 163 +++++++++++++++++++++ 4 files changed, 320 insertions(+), 6 deletions(-) create mode 100644 clang/test/CodeGen/c89-for-init-lifetime.c diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index e6b71ba2a0b565..63b51c1d41bb01 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -748,6 +748,8 @@ features cannot lower the translation-unit ABI level; - Fixed a crash when a local register variable bound to a pseudo or status register by an `asm` label, such as `register long double t __asm("17");` (the flags register), was used as an inline asm operand. Such declarations are now rejected, matching GCC. (#GH225033) +- Fixed a crash on `break` out of a `for` loop whose init-statement declares a variable in C89 mode (a Clang + extension), which happened at `-O1` and above or when the variable had a `cleanup` attribute. (#GH170243) - Fixed an ICE hat occurred when using `__imag int/float` as lvalue in assignment. (#GH119498) - Fixed a C23 rejects-valid where `auto T x;` (with `T` a typedef-name) was rejected as a missing initializer. (#GH164930) - Fixed an assertion failure in `-Wsign-compare` when a negated or complemented vector of unsigned integers was compared against a signed constant. (#GH203575) diff --git a/clang/lib/CodeGen/CGStmt.cpp b/clang/lib/CodeGen/CGStmt.cpp index 42bbbf88a3f07b..863f2b73c2e735 100644 --- a/clang/lib/CodeGen/CGStmt.cpp +++ b/clang/lib/CodeGen/CGStmt.cpp @@ -1245,15 +1245,22 @@ void CodeGenFunction::EmitDoStmt(const DoStmt &S, void CodeGenFunction::EmitForStmt(const ForStmt &S, ArrayRef<const Attr *> ForAttrs) { - JumpDest LoopExit = getJumpDestInCurrentScope("for.end"); - std::optional<LexicalScope> ForScope; - if (getLangOpts().C99 || getLangOpts().CPlusPlus) + JumpDest LoopExit; + if (getLangOpts().C99 || getLangOpts().CPlusPlus) { + LoopExit = getJumpDestInCurrentScope("for.end"); ForScope.emplace(*this, S.getSourceRange()); - // Evaluate the first part before the loop. - if (S.getInit()) - EmitStmt(S.getInit()); + // Evaluate the first part before the loop. + if (S.getInit()) + EmitStmt(S.getInit()); + } else { + // Before C99 the init variable belongs to the enclosing scope, so create + // the loop exit after its cleanups: leaving the loop must not run them. + if (S.getInit()) + EmitStmt(S.getInit()); + LoopExit = getJumpDestInCurrentScope("for.end"); + } // Start the loop with a block that tests the condition. // If there's an increment, the continue scope will be overwritten diff --git a/clang/test/CodeGen/attr-cleanup.c b/clang/test/CodeGen/attr-cleanup.c index 05723a8ab60ab4..5abfca9d2b2d07 100644 --- a/clang/test/CodeGen/attr-cleanup.c +++ b/clang/test/CodeGen/attr-cleanup.c @@ -25,3 +25,145 @@ void test_nested_for_loop_cleanup(void) { // C99: for.cond.cleanup: // C99-NEXT: call void @cleaner(ptr noundef %j) // C99-NEXT: br label %for.end + +// GH170243: in C89 a for-init variable belongs to the enclosing scope, so +// leaving the loop must not run its cleanup; in C99 the loop owns it. + +// C89-LABEL: define{{.*}} void @test_for_init_break() +// C99-LABEL: define{{.*}} void @test_for_init_break() +void test_for_init_break(void) { + for (__attribute__((cleanup(cleaner))) int i = 0;;) + break; +} +// C89: for.cond: +// C89-NEXT: br label %for.end +// C89: for.end: +// C89-NEXT: call void @cleaner(ptr noundef %i) +// C89-NEXT: ret void + +// C99: for.cond: +// C99-NEXT: call void @cleaner(ptr noundef %i) +// C99-NEXT: br label %for.end +// C99: for.end: +// C99-NEXT: ret void + +// C89-LABEL: define{{.*}} void @test_for_init_continue( +// C99-LABEL: define{{.*}} void @test_for_init_continue( +void test_for_init_continue(int n) { + for (__attribute__((cleanup(cleaner))) int i = 0; i < n; i++) + continue; +} +// C89: br i1 %cmp, label %for.body, label %for.end +// C89: for.body: +// C89-NEXT: br label %for.inc +// C89: for.end: +// C89-NEXT: call void @cleaner(ptr noundef %i) +// C89-NEXT: ret void + +// C99: br i1 %cmp, label %for.body, label %for.cond.cleanup +// C99: for.cond.cleanup: +// C99-NEXT: call void @cleaner(ptr noundef %i) +// C99-NEXT: br label %for.end +// C99: for.body: +// C99-NEXT: br label %for.inc +// C99: for.end: +// C99-NEXT: ret void + +// C89-LABEL: define{{.*}} void @test_for_init_goto() +// C99-LABEL: define{{.*}} void @test_for_init_goto() +void test_for_init_goto(void) { + for (__attribute__((cleanup(cleaner))) int i = 0;;) + goto out; +out: + return; +} +// C89: for.cond: +// C89-NEXT: br label %out +// C89: out: +// C89-NEXT: call void @cleaner(ptr noundef %i) +// C89-NEXT: ret void + +// C99: for.cond: +// C99-NEXT: store i32 {{[0-9]+}}, ptr %cleanup.dest.slot +// C99-NEXT: call void @cleaner(ptr noundef %i) +// C99-NEXT: %cleanup.dest = load i32, ptr %cleanup.dest.slot +// C99-NEXT: switch i32 %cleanup.dest, label %unreachable [ +// C99-NEXT: i32 {{[0-9]+}}, label %out +// C99-NEXT: ] +// C99: out: +// C99-NEXT: ret void + +// C89-LABEL: define{{.*}} i32 @test_for_init_return() +// C99-LABEL: define{{.*}} i32 @test_for_init_return() +int test_for_init_return(void) { + for (__attribute__((cleanup(cleaner))) int i = 1;;) + return i; +} +// C89: for.cond: +// C89-NEXT: %[[V:.*]] = load i32, ptr %i +// C89-NEXT: call void @cleaner(ptr noundef %i) +// C89-NEXT: ret i32 %[[V]] + +// C99: for.cond: +// C99-NEXT: %[[V:.*]] = load i32, ptr %i +// C99-NEXT: call void @cleaner(ptr noundef %i) +// C99-NEXT: ret i32 %[[V]] + +// C89-LABEL: define{{.*}} void @test_for_init_nested( +// C99-LABEL: define{{.*}} void @test_for_init_nested( +void test_for_init_nested(int n) { + for (__attribute__((cleanup(cleaner))) int a = 1;; a = 0) { + for (__attribute__((cleanup(cleaner))) int s = 0; s < n; ++s) + ; + if (!a) + break; + } +} +// C89: br i1 %cmp, label %for.body, label %[[INNER_END:for.end[0-9]*]] +// C89: [[INNER_END]]: +// C89: br i1 %tobool, label %if.end, label %if.then +// C89: if.then: +// C89-NEXT: store i32 {{[0-9]+}}, ptr %cleanup.dest.slot +// C89-NEXT: br label %cleanup +// C89: cleanup: +// C89-NEXT: call void @cleaner(ptr noundef %s) +// C89-NEXT: %cleanup.dest = load i32, ptr %cleanup.dest.slot +// C89-NEXT: switch i32 %cleanup.dest, label %unreachable [ +// C89-NEXT: i32 0, label %cleanup.cont +// C89-NEXT: i32 {{[0-9]+}}, label %[[OUTER_END:for.end[0-9]*]] +// C89-NEXT: ] +// C89: [[OUTER_END]]: +// C89-NEXT: call void @cleaner(ptr noundef %a) +// C89-NEXT: ret void + +// C99: br i1 %cmp, label %for.body, label %for.cond.cleanup +// C99: for.cond.cleanup: +// C99-NEXT: call void @cleaner(ptr noundef %s) +// C99-NEXT: br label %[[INNER_END:for.end[0-9]*]] +// C99: [[INNER_END]]: +// C99: br i1 %tobool, label %if.end, label %if.then +// C99: if.then: +// C99-NEXT: call void @cleaner(ptr noundef %a) +// C99-NEXT: br label %[[OUTER_END:for.end[0-9]*]] +// C99: [[OUTER_END]]: +// C99-NEXT: ret void + +// C89-LABEL: define{{.*}} void @test_for_init_in_block( +// C99-LABEL: define{{.*}} void @test_for_init_in_block( +void test_for_init_in_block(int n) { + if (n) { + for (__attribute__((cleanup(cleaner))) int i = 0;;) + break; + } +} +// C89: for.cond: +// C89-NEXT: br label %for.end +// C89: for.end: +// C89-NEXT: call void @cleaner(ptr noundef %i) +// C89-NEXT: br label %if.end + +// C99: for.cond: +// C99-NEXT: call void @cleaner(ptr noundef %i) +// C99-NEXT: br label %for.end +// C99: for.end: +// C99-NEXT: br label %if.end diff --git a/clang/test/CodeGen/c89-for-init-lifetime.c b/clang/test/CodeGen/c89-for-init-lifetime.c new file mode 100644 index 00000000000000..3e1367dd0ef26f --- /dev/null +++ b/clang/test/CodeGen/c89-for-init-lifetime.c @@ -0,0 +1,163 @@ +// RUN: %clang_cc1 -triple x86_64-linux-gnu -std=c89 -O1 -disable-llvm-passes -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,C89 +// RUN: %clang_cc1 -triple x86_64-linux-gnu -std=c99 -O1 -disable-llvm-passes -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,C99 +// RUN: %clang_cc1 -triple x86_64-linux-gnu -x c++ -std=c++17 -O1 -disable-llvm-passes -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,C99 + +// GH170243: before C99 a variable declared in a for-init-statement belongs to +// the enclosing scope, so leaving the loop must not end its lifetime. In C99 +// and C++ the loop owns it and its lifetime ends on every exit path. + +#ifdef __cplusplus +extern "C" { +#endif + +#ifndef __cplusplus +void *bar(int); + +// C89-LABEL: define{{.*}} ptr @foo() +void *foo(void) { + char *c = "abc"; + for (int a = 1;; a = 0) { + for (char *s = c; *s; ++s) { + } + if (!a) + break; + } +} +// C89: br i1 %tobool, label %for.body, label %[[INNER_END:for.end[0-9]*]] +// C89: [[INNER_END]]: +// C89: br i1 %tobool{{[0-9]*}}, label %if.end, label %if.then +// C89: if.then: +// C89-NEXT: store i32 {{[0-9]+}}, ptr %cleanup.dest.slot +// C89-NEXT: br label %cleanup +// C89: cleanup: +// C89-NEXT: call void @llvm.lifetime.end.p0(ptr %s) +// C89-NEXT: %cleanup.dest = load i32, ptr %cleanup.dest.slot +// C89-NEXT: switch i32 %cleanup.dest, label %unreachable [ +// C89-NEXT: i32 0, label %cleanup.cont +// C89-NEXT: i32 {{[0-9]+}}, label %[[OUTER_END:for.end[0-9]*]] +// C89-NEXT: ] +// C89: [[OUTER_END]]: +// C89-NEXT: call void @llvm.lifetime.end.p0(ptr %a) +// C89-NEXT: call void @llvm.lifetime.end.p0(ptr %c) +// C89: ret ptr + +// C89-LABEL: define{{.*}} i32 @main() +int main(void) { + for (int i;;) + break; +} +// C89: call void @llvm.lifetime.start.p0(ptr %i) +// C89-NEXT: br label %for.cond +// C89: for.cond: +// C89-NEXT: br label %for.end +// C89: for.end: +// C89-NEXT: call void @llvm.lifetime.end.p0(ptr %i) +// C89: ret i32 +#endif + +// CHECK-LABEL: define{{.*}} void @test_break() +void test_break(void) { + for (int i = 0;;) + break; +} +// CHECK: call void @llvm.lifetime.start.p0(ptr %i) +// CHECK-NEXT: store i32 0, ptr %i +// CHECK-NEXT: br label %for.cond +// CHECK: for.cond: +// C89-NEXT: br label %for.end +// C89: for.end: +// C89-NEXT: call void @llvm.lifetime.end.p0(ptr %i) +// C89-NEXT: ret void +// C99-NEXT: call void @llvm.lifetime.end.p0(ptr %i) +// C99-NEXT: br label %for.end +// C99: for.end: +// C99-NEXT: ret void + +// CHECK-LABEL: define{{.*}} void @test_continue( +void test_continue(int n) { + for (int i = 0; i < n; i++) + continue; +} +// C89: br i1 %cmp, label %for.body, label %for.end +// C99: br i1 %cmp, label %for.body, label %for.cond.cleanup +// C99: for.cond.cleanup: +// C99-NEXT: call void @llvm.lifetime.end.p0(ptr %i) +// C99-NEXT: br label %for.end +// CHECK: for.body: +// CHECK-NEXT: br label %for.inc +// CHECK: for.end: +// C89-NEXT: call void @llvm.lifetime.end.p0(ptr %i) +// CHECK-NEXT: ret void + +// CHECK-LABEL: define{{.*}} void @test_goto() +void test_goto(void) { + for (int i = 0;;) + goto out; +out: + return; +} +// CHECK: for.cond: +// C89-NEXT: br label %out +// C89: out: +// C89-NEXT: call void @llvm.lifetime.end.p0(ptr %i) +// C89-NEXT: ret void +// C99-NEXT: store i32 {{[0-9]+}}, ptr %cleanup.dest.slot +// C99-NEXT: call void @llvm.lifetime.end.p0(ptr %i) +// C99-NEXT: %cleanup.dest = load i32, ptr %cleanup.dest.slot +// C99-NEXT: switch i32 %cleanup.dest, label %unreachable [ +// C99-NEXT: i32 {{[0-9]+}}, label %out +// C99-NEXT: ] +// C99: out: +// C99-NEXT: ret void + +// CHECK-LABEL: define{{.*}} i32 @test_return() +int test_return(void) { + for (int i = 1;;) + return i; +} +// CHECK: for.cond: +// CHECK-NEXT: %[[V:.*]] = load i32, ptr %i +// CHECK-NEXT: call void @llvm.lifetime.end.p0(ptr %i) +// CHECK-NEXT: ret i32 %[[V]] + +// CHECK-LABEL: define{{.*}} void @test_nested( +void test_nested(int n) { + for (int a = 1;; a = 0) { + for (int s = 0; s < n; ++s) + ; + if (!a) + break; + } +} +// C89: br i1 %cmp, label %for.body, label %[[INNER_END:for.end[0-9]*]] +// C89: [[INNER_END]]: +// C89: br i1 %tobool, label %if.end, label %if.then +// C89: if.then: +// C89-NEXT: store i32 {{[0-9]+}}, ptr %cleanup.dest.slot +// C89-NEXT: br label %cleanup +// C89: cleanup: +// C89-NEXT: call void @llvm.lifetime.end.p0(ptr %s) +// C89-NEXT: %cleanup.dest = load i32, ptr %cleanup.dest.slot +// C89-NEXT: switch i32 %cleanup.dest, label %unreachable [ +// C89-NEXT: i32 0, label %cleanup.cont +// C89-NEXT: i32 {{[0-9]+}}, label %[[OUTER_END:for.end[0-9]*]] +// C89-NEXT: ] +// C89: [[OUTER_END]]: +// C89-NEXT: call void @llvm.lifetime.end.p0(ptr %a) +// C89-NEXT: ret void + +// C99: br i1 %cmp, label %for.body, label %for.cond.cleanup +// C99: for.cond.cleanup: +// C99-NEXT: call void @llvm.lifetime.end.p0(ptr %s) +// C99-NEXT: br label %[[INNER_END:for.end[0-9]*]] +// C99: [[INNER_END]]: +// C99: br i1 %tobool, label %if.end, label %if.then +// C99: if.then: +// C99-NEXT: call void @llvm.lifetime.end.p0(ptr %a) +// C99-NEXT: br label %[[OUTER_END:for.end[0-9]*]] +// C99: [[OUTER_END]]: +// C99-NEXT: ret void + +#ifdef __cplusplus +} +#endif _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
