Author: Akash Manna Date: 2026-11-02T04:51:07Z New Revision: 6048b5d1724e868332074c69118a63f5215613fb
URL: https://github.com/llvm/llvm-project/commit/6048b5d1724e868332074c69118a63f5215613fb DIFF: https://github.com/llvm/llvm-project/commit/6048b5d1724e868332074c69118a63f5215613fb.diff LOG: [Clang][CodeGen] Fix crash on break out of a for loop with a `C89` `init` declaration (#235809) Fixes #170243 Since #156643 a variable declared in the init-statement of a C89 `for` loop has no `LexicalScope` of its own, so its cleanup sits on the enclosing scope's cleanup stack. `for.end` was still created before the init-statement though, so its recorded cleanup depth didn't include that cleanup. A `break` then had to be threaded through it, which left `for.end` with no users, so it got deleted at the end of the loop while the cleanup still pointed at it, and `FinishFunction` later emitted a branch to the freed block. In C89 mode the loop exit is now created after the init-statement, so it records the right depth and leaving the loop branches to `for.end` directly. That's the intended behaviour, the variable belongs to the enclosing scope and its cleanup runs when that scope ends. The C99/C++ path is untouched. AI Assisted. Added: clang/test/CodeGen/c89-for-init-lifetime.c Modified: clang/docs/ReleaseNotes.md clang/lib/CodeGen/CGStmt.cpp clang/test/CodeGen/attr-cleanup.c Removed: ################################################################################ diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index a367196a601bc..e769ad6a24e6b 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -811,6 +811,8 @@ documentation](CIR/index.md) for more information regarding ClangIR's status and - 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 347bf6b23cbc8..b50ac00bac4eb 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 05723a8ab60ab..5abfca9d2b2d0 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 0000000000000..3e1367dd0ef26 --- /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
