https://github.com/steakhal created
https://github.com/llvm/llvm-project/pull/212138
A GCC statement-expression containing control flow finishes the current CFG
block and nulls out the builder's `Block`. Several places assumed `Block`
survives `addStmt()`, so clang crashed on:
```c++
struct S { explicit operator bool(); };
void foo(int n) {
while (S s = ({ while (S t{}) {} S{}; }))
--n;
}
```
`VisitWhileStmt` and `VisitForStmt` asserted `Block == EntryConditionBlock`
after adding the condition variable's initializer. Both already stored the
result of `addStmt(Init)` in `EntryConditionBlock` and used it correctly
afterwards, so only the assert was wrong; thread the returned block instead, as
`VisitSwitchStmt` already does.
`VisitDeclSubExpr` tracked `LastBlock` precisely because `Block` can be nulled
out here, but then passed the stale `Block` to
`maybeAddScopeBeginForVarDecl()`, which dereferences it. With `cfg-scopes=true`
that is a null dereference rather than a failed assert, so it crashed release
builds too, affecting `if` and `switch` condition variables and C++17
init-statements.
Document the invariant on `addStmt()` and point the callers at it.
Also tighten the adjacent `if (Block && badCFG)` guards to `if (badCFG)`, so a
`badCFG` set while traversing the initializer is not missed once `Block` is
null. This is hardening, not part of the crash fix.
Not analyzer-only: Sema's `AnalysisBasedWarnings` builds a CFG, so this also
reproduces on a plain compile with `-fsyntax-only -Wunreachable-code`.
Fixes #211976
Assisted-by: claude
From 7542cabf5175985e39e86ad1d9836417ad043c48 Mon Sep 17 00:00:00 2001
From: Balazs Benics <[email protected]>
Date: Sun, 26 Jul 2026 15:42:44 +0100
Subject: [PATCH] [clang][CFG] Fix crash on statement-expression in condition
variable
A GCC statement-expression containing control flow finishes the current CFG
block and nulls out the builder's `Block`. Several places assumed `Block`
survives `addStmt()`, so clang crashed on:
struct S { explicit operator bool(); };
void foo(int n) {
while (S s = ({ while (S t{}) {} S{}; }))
--n;
}
`VisitWhileStmt` and `VisitForStmt` asserted `Block == EntryConditionBlock`
after adding the condition variable's initializer. Both already stored the
result of `addStmt(Init)` in `EntryConditionBlock` and used it correctly
afterwards, so only the assert was wrong; thread the returned block instead,
as `VisitSwitchStmt` already does.
`VisitDeclSubExpr` tracked `LastBlock` precisely because `Block` can be nulled
out here, but then passed the stale `Block` to `maybeAddScopeBeginForVarDecl()`,
which dereferences it. With `cfg-scopes=true` that is a null dereference rather
than a failed assert, so it crashed release builds too, affecting `if` and
`switch` condition variables and C++17 init-statements.
Document the invariant on `addStmt()` and point the callers at it.
Also tighten the adjacent `if (Block && badCFG)` guards to `if (badCFG)`, so a
`badCFG` set while traversing the initializer is not missed once `Block` is
null. This is hardening, not part of the crash fix.
Not analyzer-only: Sema's `AnalysisBasedWarnings` builds a CFG, so this also
reproduces on a plain compile with `-fsyntax-only -Wunreachable-code`.
Fixes #211976
Assisted-by: claude
---
clang/docs/ReleaseNotes.md | 5 +
clang/lib/Analysis/CFG.cpp | 34 +++--
clang/test/Analysis/cfg.cpp | 124 +++++++++++++++++-
clang/test/Analysis/scopes-cfg-output.cpp | 69 ++++++++++
clang/test/SemaCXX/stmt-expr-in-loop-cond.cpp | 73 +++++++++++
5 files changed, 291 insertions(+), 14 deletions(-)
create mode 100644 clang/test/SemaCXX/stmt-expr-in-loop-cond.cpp
diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md
index 61e8378b1704a..ae6295115de26 100644
--- a/clang/docs/ReleaseNotes.md
+++ b/clang/docs/ReleaseNotes.md
@@ -459,6 +459,11 @@ features cannot lower the translation-unit ABI level;
#### Crash and bug fixes
+- Fixed a crash during CFG construction when a condition variable or an
+ init-statement of a `while`, `for`, `if` or `switch` was initialized by a
+ statement-expression containing control flow, e.g.
+ `while (S s = ({ while (...) {} S{}; }))`. (#GH211976)
+
% comment:
% This is for the Static Analyzer.
% Use `####` headings for subsections:
diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp
index efd1da9c9a4db..df44821ab3c99 100644
--- a/clang/lib/Analysis/CFG.cpp
+++ b/clang/lib/Analysis/CFG.cpp
@@ -797,6 +797,21 @@ class CFGBuilder {
CFGBlock *createBlock(bool add_successor = true);
CFGBlock *createNoReturnBlock();
+ /// Add a statement to the CFG, returning the block it was added to.
+ ///
+ /// Beware that this does not necessarily leave \c Block current, because the
+ /// statement may itself contain control flow. In particular, a GCC
+ /// statement-expression holding a loop (e.g. \c ({ while (...) {} v; }))
ends
+ /// the block it started in, so afterwards \c Block may be null and the
+ /// returned block may differ from the one on entry. Callers that need the
+ /// block a statement ended up in must therefore use the returned value
rather
+ /// than \c Block, and keep the last non-null result when adding several
+ /// statements in a row. The same caveat applies to \c Visit(), which this
+ /// wraps.
+ ///
+ /// May also return null when no block was created, e.g. for a NullStmt when
+ /// \c Block is not current; callers therefore have to keep the previous
+ /// non-null block rather than overwrite it unconditionally.
CFGBlock *addStmt(Stmt *S) {
return Visit(S, AddStmtChoice::AlwaysAdd);
}
@@ -3209,9 +3224,8 @@ CFGBlock *CFGBuilder::VisitDeclSubExpr(DeclStmt *DS) {
ConstructionContextLayer::create(cfg->getBumpVectorContext(), DS),
AILE ? AILE->getSubExpr() : Init);
- // Keep track of the last non-null block, as 'Block' can be nulled out
- // if the initializer expression is something like a 'while' in a
- // statement-expression.
+ // Keep track of the last non-null block, as 'Block' can be nulled out by
+ // the initializer. See the comment on addStmt().
CFGBlock *LastBlock = Block;
if (Init) {
@@ -3237,7 +3251,7 @@ CFGBlock *CFGBuilder::VisitDeclSubExpr(DeclStmt *DS) {
LastBlock = newBlock;
}
- maybeAddScopeBeginForVarDecl(Block, VD, DS);
+ maybeAddScopeBeginForVarDecl(LastBlock, VD, DS);
// Remove variable from local scope.
if (ScopePos && VD == *ScopePos)
@@ -3823,13 +3837,13 @@ CFGBlock *CFGBuilder::VisitForStmt(ForStmt *F) {
ConstructionContextLayer::create(cfg->getBumpVectorContext(),
DS),
Init);
appendStmt(Block, DS);
- EntryConditionBlock = addStmt(Init);
- assert(Block == EntryConditionBlock);
+ if (CFGBlock *NewBlock = addStmt(Init))
+ EntryConditionBlock = NewBlock;
maybeAddScopeBeginForVarDecl(EntryConditionBlock, VD, C);
}
}
- if (Block && badCFG)
+ if (badCFG)
return nullptr;
KnownVal = tryEvaluateBool(C);
@@ -4149,13 +4163,13 @@ CFGBlock *CFGBuilder::VisitWhileStmt(WhileStmt *W) {
const_cast<DeclStmt *>(DS)),
Init);
appendStmt(Block, DS);
- EntryConditionBlock = addStmt(Init);
- assert(Block == EntryConditionBlock);
+ if (CFGBlock *NewBlock = addStmt(Init))
+ EntryConditionBlock = NewBlock;
maybeAddScopeBeginForVarDecl(EntryConditionBlock, VD, C);
}
}
- if (Block && badCFG)
+ if (badCFG)
return nullptr;
// See if this is a known constant.
diff --git a/clang/test/Analysis/cfg.cpp b/clang/test/Analysis/cfg.cpp
index 2a88b73d27756..f6330168495df 100644
--- a/clang/test/Analysis/cfg.cpp
+++ b/clang/test/Analysis/cfg.cpp
@@ -1,7 +1,9 @@
-// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple
x86_64-apple-darwin12 -Wno-error=invalid-gnu-asm-cast -std=c++11
-analyzer-config cfg-rich-constructors=false %s > %t 2>&1
-// RUN: FileCheck --input-file=%t -check-prefixes=CHECK,WARNINGS %s
-// RUN: %clang_analyze_cc1 -analyzer-checker=debug.DumpCFG -triple
x86_64-apple-darwin12 -Wno-error=invalid-gnu-asm-cast -std=c++11
-analyzer-config cfg-rich-constructors=true %s > %t 2>&1
-// RUN: FileCheck --input-file=%t -check-prefixes=CHECK,ANALYZER %s
+
+// DEFINE: %{clang-dump-cfg} = %clang_analyze_cc1
-analyzer-checker=debug.DumpCFG -triple x86_64-apple-darwin12
-Wno-error=invalid-gnu-asm-cast -std=c++11
+// RUN: %{clang-dump-cfg} -analyzer-config cfg-rich-constructors=false %s 2>&1
| \
+// RUN: FileCheck -check-prefixes=CHECK,WARNINGS %s
+// RUN: %{clang-dump-cfg} -analyzer-config cfg-rich-constructors=true %s 2>&1
| \
+// RUN: FileCheck -check-prefixes=CHECK,ANALYZER %s
// This file tests how we construct two different flavors of the Clang CFG -
// the CFG used by the Sema analysis-based warnings and the CFG used by the
@@ -491,6 +493,120 @@ int foo() {
}
} // namespace statement_expression_in_return
+// A statement-expression initializing a loop condition variable finishes the
+// current block, so the entry to the condition is the block the nested loop
+// starts in. The loop-back edge has to point there, not at the block holding
+// the condition variable.
+namespace statement_expression_in_loop_condition_variable {
+struct S { explicit operator bool(); };
+
+// CHECK-LABEL: void while_loop(int n)
+// CHECK: [B6 (ENTRY)]
+// CHECK-NEXT: Succs (1): B5
+// CHECK: [B1]
+// CHECK-NEXT: Preds (1): B2
+// CHECK-NEXT: Succs (1): B5
+// CHECK: [B2]
+// CHECK-NEXT: 1: n
+// CHECK-NEXT: 2: --[B2.1]
+// CHECK-NEXT: Preds (1): B3
+// CHECK-NEXT: Succs (1): B1
+// CHECK: [B3]
+// CHECK-NEXT: 1: {}
+// CHECK-NEXT: 2: S[B3.1] (CXXFunctionalCastExpr, NoOp, S)
+// CHECK-NEXT: 3: [B3.2]
+// CHECK-NEXT: 4: [B3.3] (CXXConstructExpr, S)
+// CHECK-NEXT: 5: ({ ... ; })
+// CHECK-NEXT: 6: [B3.5]
+// WARNINGS-NEXT: 7: [B3.6] (CXXConstructExpr, S)
+// ANALYZER-NEXT: 7: [B3.6] (CXXConstructExpr, [B3.8], S)
+// CHECK-NEXT: 8: S s = ({
+// CHECK-NEXT: while (S t{})
+// CHECK-NEXT: {
+// CHECK-NEXT: }
+// CHECK-NEXT: S{};
+// CHECK-NEXT: });
+// CHECK-NEXT: 9: s
+// CHECK-NEXT: 10: [B3.9].operator bool
+// CHECK-NEXT: 11: [B3.9]
+// CHECK-NEXT: 12: [B3.11] (ImplicitCastExpr, UserDefinedConversion, _Bool)
+// CHECK-NEXT: T: while [B3.12]
+// CHECK-NEXT: Preds (1): B5
+// CHECK-NEXT: Succs (2): B2 B0
+// CHECK: [B4]
+// CHECK-NEXT: Preds (1): B5
+// CHECK-NEXT: Succs (1): B5
+// CHECK: [B5]
+// CHECK-NEXT: 1: {}
+// CHECK-NEXT: 2: S t{};
+// CHECK-NEXT: 3: t
+// CHECK-NEXT: 4: [B5.3].operator bool
+// CHECK-NEXT: 5: [B5.3]
+// CHECK-NEXT: 6: [B5.5] (ImplicitCastExpr, UserDefinedConversion, _Bool)
+// CHECK-NEXT: T: while [B5.6]
+// CHECK-NEXT: Preds (3): B4 B1 B6
+// CHECK-NEXT: Succs (2): B4 B3
+// CHECK: [B0 (EXIT)]
+// CHECK-NEXT: Preds (1): B3
+void while_loop(int n) {
+ while (S s = ({ while (S t{}) {} S{}; }))
+ --n;
+}
+
+// CHECK-LABEL: void for_loop(int n)
+// CHECK: [B6 (ENTRY)]
+// CHECK-NEXT: Succs (1): B5
+// CHECK: [B1]
+// CHECK-NEXT: Preds (1): B2
+// CHECK-NEXT: Succs (1): B5
+// CHECK: [B2]
+// CHECK-NEXT: 1: n
+// CHECK-NEXT: 2: --[B2.1]
+// CHECK-NEXT: Preds (1): B3
+// CHECK-NEXT: Succs (1): B1
+// CHECK: [B3]
+// CHECK-NEXT: 1: {}
+// CHECK-NEXT: 2: S[B3.1] (CXXFunctionalCastExpr, NoOp, S)
+// CHECK-NEXT: 3: [B3.2]
+// CHECK-NEXT: 4: [B3.3] (CXXConstructExpr, S)
+// CHECK-NEXT: 5: ({ ... ; })
+// CHECK-NEXT: 6: [B3.5]
+// WARNINGS-NEXT: 7: [B3.6] (CXXConstructExpr, S)
+// ANALYZER-NEXT: 7: [B3.6] (CXXConstructExpr, [B3.8], S)
+// CHECK-NEXT: 8: S s = ({
+// CHECK-NEXT: while (S t{})
+// CHECK-NEXT: {
+// CHECK-NEXT: }
+// CHECK-NEXT: S{};
+// CHECK-NEXT: });
+// CHECK-NEXT: 9: s
+// CHECK-NEXT: 10: [B3.9].operator bool
+// CHECK-NEXT: 11: [B3.9]
+// CHECK-NEXT: 12: [B3.11] (ImplicitCastExpr, UserDefinedConversion, _Bool)
+// CHECK-NEXT: T: for (; [B3.12]; )
+// CHECK-NEXT: Preds (1): B5
+// CHECK-NEXT: Succs (2): B2 B0
+// CHECK: [B4]
+// CHECK-NEXT: Preds (1): B5
+// CHECK-NEXT: Succs (1): B5
+// CHECK: [B5]
+// CHECK-NEXT: 1: {}
+// CHECK-NEXT: 2: S t{};
+// CHECK-NEXT: 3: t
+// CHECK-NEXT: 4: [B5.3].operator bool
+// CHECK-NEXT: 5: [B5.3]
+// CHECK-NEXT: 6: [B5.5] (ImplicitCastExpr, UserDefinedConversion, _Bool)
+// CHECK-NEXT: T: while [B5.6]
+// CHECK-NEXT: Preds (3): B4 B1 B6
+// CHECK-NEXT: Succs (2): B4 B3
+// CHECK: [B0 (EXIT)]
+// CHECK-NEXT: Preds (1): B3
+void for_loop(int n) {
+ for (; S s = ({ while (S t{}) {} S{}; });)
+ --n;
+}
+} // namespace statement_expression_in_loop_condition_variable
+
// CHECK-LABEL: void vla_simple(int x)
// CHECK: [B1]
// CHECK-NEXT: 1: x
diff --git a/clang/test/Analysis/scopes-cfg-output.cpp
b/clang/test/Analysis/scopes-cfg-output.cpp
index ac8ff1179a994..2297d33443faa 100644
--- a/clang/test/Analysis/scopes-cfg-output.cpp
+++ b/clang/test/Analysis/scopes-cfg-output.cpp
@@ -1484,3 +1484,72 @@ void cleanup_F(F *f);
void test() {
F f __attribute((cleanup(cleanup_F)));
}
+
+// An initializer that is a statement-expression containing control flow
+// finishes the current block, so the scope of the condition variable has to
+// begin in the block the initializer actually starts in.
+// CHECK: [B7 (ENTRY)]
+// CHECK-NEXT: Succs (1): B6
+// CHECK: [B1]
+// CHECK-NEXT: 1: [B3.7].~D() (Implicit destructor)
+// CHECK-NEXT: 2: CFGScopeEnd(d)
+// CHECK-NEXT: Preds (2): B2 B3
+// CHECK-NEXT: Succs (1): B0
+// CHECK: [B2]
+// CHECK-NEXT: 1: n
+// CHECK-NEXT: 2: --[B2.1]
+// CHECK-NEXT: Preds (1): B3
+// CHECK-NEXT: Succs (1): B1
+// CHECK: [B3]
+// CHECK-NEXT: 1: [B6.4].~D() (Implicit destructor)
+// CHECK-NEXT: 2: CFGScopeEnd(t)
+// CHECK-NEXT: 3: D{} (CXXConstructExpr, [B3.4], D)
+// CHECK-NEXT: 4: [B3.3] (BindTemporary)
+// CHECK-NEXT: 5: ({ ... ; })
+// CHECK-NEXT: 6: [B3.5] (BindTemporary)
+// CHECK-NEXT: 7: D d = ({
+// CHECK-NEXT: while (D t{})
+// CHECK-NEXT: {
+// CHECK-NEXT: }
+// CHECK-NEXT: D{};
+// CHECK-NEXT: });
+// CHECK-NEXT: 8: d
+// CHECK-NEXT: 9: [B3.8].operator bool
+// CHECK-NEXT: 10: [B3.8]
+// CHECK-NEXT: 11: [B3.10] (ImplicitCastExpr, UserDefinedConversion, _Bool)
+// CHECK-NEXT: T: if [B3.11]
+// CHECK-NEXT: Preds (1): B6
+// CHECK-NEXT: Succs (2): B2 B1
+// CHECK: [B4]
+// CHECK-NEXT: Preds (1): B5
+// CHECK-NEXT: Succs (1): B6
+// CHECK: [B5]
+// CHECK-NEXT: 1: [B6.4].~D() (Implicit destructor)
+// CHECK-NEXT: 2: CFGScopeEnd(t)
+// CHECK-NEXT: Preds (1): B6
+// CHECK-NEXT: Succs (1): B4
+// CHECK: [B6]
+// CHECK-NEXT: 1: CFGScopeBegin(d)
+// CHECK-NEXT: 2: CFGScopeBegin(t)
+// CHECK-NEXT: 3: {} (CXXConstructExpr, [B6.4], D)
+// CHECK-NEXT: 4: D t{};
+// CHECK-NEXT: 5: t
+// CHECK-NEXT: 6: [B6.5].operator bool
+// CHECK-NEXT: 7: [B6.5]
+// CHECK-NEXT: 8: [B6.7] (ImplicitCastExpr, UserDefinedConversion, _Bool)
+// CHECK-NEXT: T: while [B6.8]
+// CHECK-NEXT: Preds (2): B4 B7
+// CHECK-NEXT: Succs (2): B5 B3
+// CHECK: [B0 (EXIT)]
+// CHECK-NEXT: Preds (1): B1
+class D {
+public:
+ D();
+ ~D();
+ explicit operator bool();
+};
+
+void if_cond_var_stmt_expr(int n) {
+ if (D d = ({ while (D t{}) {} D{}; }))
+ --n;
+}
diff --git a/clang/test/SemaCXX/stmt-expr-in-loop-cond.cpp
b/clang/test/SemaCXX/stmt-expr-in-loop-cond.cpp
new file mode 100644
index 0000000000000..fb1def062b214
--- /dev/null
+++ b/clang/test/SemaCXX/stmt-expr-in-loop-cond.cpp
@@ -0,0 +1,73 @@
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -Wunreachable-code -verify %s
+
+// Do not crash when a condition variable or an init-statement is initialized
+// by a statement-expression containing control flow. Such an initializer
+// finishes the current CFG block, so the builder must not assume the block it
+// started with is still current.
+// https://github.com/llvm/llvm-project/issues/211976
+
+// expected-no-diagnostics
+
+struct S { explicit operator bool(); };
+
+// A non-trivial destructor exercises the scope/destructor bookkeeping.
+struct D { D(); ~D(); explicit operator bool(); };
+struct I { I(); ~I(); operator int(); };
+
+void while_loop(int n) {
+ while (S s = ({ while (S t{}) {} S{}; }))
+ --n;
+}
+
+void for_loop(int n) {
+ for (; S s = ({ while (S t{}) {} S{}; });)
+ --n;
+}
+
+void for_loop_with_init_and_inc(int n) {
+ for (int i = 0; S s = ({ while (S t{}) {} S{}; }); ++i) {
+ if (i > n)
+ break;
+ continue;
+ }
+}
+
+void do_loop(int n) {
+ do {
+ --n;
+ } while (({ while (S t{}) {} n; }));
+}
+
+void if_condition_variable(int n) {
+ if (D d = ({ while (D t{}) {} D{}; }))
+ --n;
+}
+
+void if_init_statement(int n) {
+ if (D d = ({ while (D t{}) {} D{}; }); n)
+ --n;
+}
+
+void switch_condition_variable(int n) {
+ switch (I i = ({ while (D t{}) {} I{}; })) {
+ default:
+ break;
+ }
+}
+
+void switch_init_statement(int n) {
+ switch (D d = ({ while (D t{}) {} D{}; }); n) {
+ default:
+ break;
+ }
+}
+
+// The condition variable is usable in the body, and the nested loop must be
+// re-evaluated on every iteration.
+void condition_variable_used_in_body(int n) {
+ while (D d = ({ while (D t{}) {} D{}; })) {
+ if (d)
+ continue;
+ break;
+ }
+}
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
