https://github.com/avunit1 updated https://github.com/llvm/llvm-project/pull/208569
>From b28d5668d2dc9b8c4d4986e70e8a4d1eb032e725 Mon Sep 17 00:00:00 2001 From: avunit1 <[email protected]> Date: Thu, 9 Jul 2026 21:23:26 +0000 Subject: [PATCH] Model coroutine promise construction in the CFG The Static Analyzer reported core.UndefinedBinaryOperatorResult false positives whenever a coroutine's promise_type::await_transform() (or any other promise member function reachable from the coroutine body) read a data member that is guaranteed to be initialized, e.g. via an in-class default member initializer: struct promise_type { unsigned storage_ = 0; Ready await_transform(Ready a) noexcept { (void)(storage_ & ~3u); // false positive: garbage value return a; } }; Root cause: AnalysisDeclContext::getBody() unwraps a CoroutineBodyStmt down to just CoroutineBodyStmt::getBody() -- the as-written body -- before the CFG is built from it (CFG::buildCFG). This is intentional for CFG-based Sema warnings such as -Wunreachable-code, which don't want to reason about compiler-synthesized coroutine machinery. But it means CoroutineBodyStmt::getPromiseDeclStmt() -- the DeclStmt whose initializer constructs the promise object, including evaluation of any default member initializers -- never becomes part of the CFG that the Static Analyzer walks. Since the promise variable's DeclStmt is never visited, its constructor call is never evaluated, so RegionStore never binds a value to e.g. 'storage_', and the field is treated as uninitialized garbage the first time it's read from await_transform(). Fix: teach CFGBuilder::buildCFG() to optionally prepend the promise object's DeclStmt to the CFG for a coroutine, behind a new opt-in CFG::BuildOptions flag (AddImplicitCoroutinePromiseConstruction, off by default). The Static Analyzer's AnalysisDeclContextManager (constructed in AnalysisManager::AnalysisManager) turns the flag on; every other CFG client (Sema's flow-sensitive warnings, libclang, etc.) is unaffected, since AnalysisDeclContext::getBody() itself is untouched and the flag defaults to off. This reuses the existing DeclStmt/CXXConstructExpr visitation machinery that already correctly models default member initializers, constructor bodies, brace-init, etc. for any ordinary local variable -- no special casing for the reproducer, and no attempt to model coroutine suspension itself (initial_suspend/final_suspend remain unmodeled, as before). Adds clang/test/Analysis/coroutine-promise-init.cpp, covering: the original reproducer, multiple default member initializers, a user-provided promise constructor, brace-init, and a negative control where a genuinely-uninitialized promise member must still be flagged. Mock <coroutine> declarations live in clang/test/Analysis/Inputs/system-header-simulator-cxx-coroutines.h, following the existing system-header-simulator-cxx-*.h convention. The reproducer's std::uintptr_t was replaced with a plain unsigned int, since the exact integer type is irrelevant to the bug and __UINTPTR_TYPE__'s portability across all targets isn't documented. Fixes https://github.com/llvm/llvm-project/issues/208531 --- .../clang/Analysis/AnalysisDeclContext.h | 1 + clang/include/clang/Analysis/CFG.h | 17 ++ clang/lib/Analysis/AnalysisDeclContext.cpp | 5 +- clang/lib/Analysis/CFG.cpp | 21 +++ .../StaticAnalyzer/Core/AnalysisManager.cpp | 4 +- .../system-header-simulator-cxx-coroutines.h | 31 ++++ .../test/Analysis/coroutine-promise-init.cpp | 154 ++++++++++++++++++ 7 files changed, 231 insertions(+), 2 deletions(-) create mode 100644 clang/test/Analysis/Inputs/system-header-simulator-cxx-coroutines.h create mode 100644 clang/test/Analysis/coroutine-promise-init.cpp diff --git a/clang/include/clang/Analysis/AnalysisDeclContext.h b/clang/include/clang/Analysis/AnalysisDeclContext.h index 84877136f842b..a6fdd40e6c7c3 100644 --- a/clang/include/clang/Analysis/AnalysisDeclContext.h +++ b/clang/include/clang/Analysis/AnalysisDeclContext.h @@ -375,6 +375,7 @@ class AnalysisDeclContextManager { bool synthesizeBodies = false, bool addStaticInitBranches = false, bool addCXXNewAllocator = true, bool addRichCXXConstructors = true, bool markElidedCXXConstructors = true, bool addVirtualBaseBranches = true, + bool addImplicitCoroutinePromiseConstruction = false, std::unique_ptr<CodeInjector> injector = nullptr); AnalysisDeclContext *getContext(const Decl *D); diff --git a/clang/include/clang/Analysis/CFG.h b/clang/include/clang/Analysis/CFG.h index 749ed3fc76452..91bef47b024ea 100644 --- a/clang/include/clang/Analysis/CFG.h +++ b/clang/include/clang/Analysis/CFG.h @@ -1306,6 +1306,23 @@ class CFG { bool AddVirtualBaseBranches = false; bool OmitImplicitValueInitializers = false; bool AssumeReachableDefaultInSwitchStatements = false; + // Add the construction (including evaluation of any default member + // initializers) of a coroutine's promise object to the CFG being built + // for the coroutine. `AnalysisDeclContext::getBody()` returns only the + // as-written body of a coroutine (i.e. `CoroutineBodyStmt::getBody()`) + // so that CFG-based Sema warnings (e.g. -Wunreachable-code) don't have + // to reason about compiler-synthesized coroutine machinery. Because of + // that, by default, nothing in the CFG models the initialization of the + // promise object (`CoroutineBodyStmt::getPromiseDeclStmt()`), even + // though the promise is guaranteed to be constructed, with all of its + // default member initializers applied, before any of the coroutine's + // body -- including the first `co_await` -- executes. Clients that + // symbolically execute the CFG (namely the Static Analyzer) need this + // modeled or they will treat the promise's data members as + // uninitialized. This option is off by default to preserve the CFG + // shape relied upon by the existing Sema-level, non-path-sensitive + // warnings. + bool AddImplicitCoroutinePromiseConstruction = false; BuildOptions() = default; diff --git a/clang/lib/Analysis/AnalysisDeclContext.cpp b/clang/lib/Analysis/AnalysisDeclContext.cpp index 437360a775275..6db7d6d6c0a8f 100644 --- a/clang/lib/Analysis/AnalysisDeclContext.cpp +++ b/clang/lib/Analysis/AnalysisDeclContext.cpp @@ -69,7 +69,8 @@ AnalysisDeclContextManager::AnalysisDeclContextManager( bool addLoopExit, bool addScopes, bool synthesizeBodies, bool addStaticInitBranch, bool addCXXNewAllocator, bool addRichCXXConstructors, bool markElidedCXXConstructors, - bool addVirtualBaseBranches, std::unique_ptr<CodeInjector> injector) + bool addVirtualBaseBranches, bool addImplicitCoroutinePromiseConstruction, + std::unique_ptr<CodeInjector> injector) : Injector(std::move(injector)), FunctionBodyFarm(ASTCtx, Injector.get()), SynthesizeBodies(synthesizeBodies) { cfgBuildOptions.PruneTriviallyFalseEdges = !useUnoptimizedCFG; @@ -84,6 +85,8 @@ AnalysisDeclContextManager::AnalysisDeclContextManager( cfgBuildOptions.AddRichCXXConstructors = addRichCXXConstructors; cfgBuildOptions.MarkElidedCXXConstructors = markElidedCXXConstructors; cfgBuildOptions.AddVirtualBaseBranches = addVirtualBaseBranches; + cfgBuildOptions.AddImplicitCoroutinePromiseConstruction = + addImplicitCoroutinePromiseConstruction; } void AnalysisDeclContextManager::clear() { Contexts.clear(); } diff --git a/clang/lib/Analysis/CFG.cpp b/clang/lib/Analysis/CFG.cpp index bb8e98a894dab..0bfb7a85121eb 100644 --- a/clang/lib/Analysis/CFG.cpp +++ b/clang/lib/Analysis/CFG.cpp @@ -1740,6 +1740,27 @@ std::unique_ptr<CFG> CFGBuilder::buildCFG(const Decl *D, Stmt *Statement) { } } + // For a coroutine, model the construction of the promise object -- + // including the evaluation of any of its default member initializers -- + // as part of this CFG, if requested. `Statement` here is only the + // coroutine's as-written body (see `AnalysisDeclContext::getBody()`), so + // without this the promise object's fields would incorrectly appear + // never-initialized to any client that symbolically evaluates the CFG, + // even though the promise is always fully constructed by the time the + // coroutine body -- and thus the first `co_await` -- runs. + if (BuildOpts.AddImplicitCoroutinePromiseConstruction) { + if (const auto *FD = dyn_cast_or_null<FunctionDecl>(D)) { + if (const auto *CoroBody = + dyn_cast_or_null<CoroutineBodyStmt>(FD->getBody())) { + if (B) + Succ = B; + B = addStmt(CoroBody->getPromiseDeclStmt()); + if (badCFG) + return nullptr; + } + } + } + if (B) Succ = B; diff --git a/clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp b/clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp index 8a16716f951b8..704d73f822dc4 100644 --- a/clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp +++ b/clang/lib/StaticAnalyzer/Core/AnalysisManager.cpp @@ -33,7 +33,9 @@ AnalysisManager::AnalysisManager(ASTContext &ASTCtx, Preprocessor &PP, /*addCXXNewAllocator=*/true, Options.ShouldIncludeRichConstructorsInCFG, Options.ShouldElideConstructors, - /*addVirtualBaseBranches=*/true, std::move(injector)), + /*addVirtualBaseBranches=*/true, + /*addImplicitCoroutinePromiseConstruction=*/true, + std::move(injector)), Ctx(ASTCtx), PP(PP), LangOpts(ASTCtx.getLangOpts()), PathConsumers(std::move(PDC)), CreateStoreMgr(storemgr), CreateConstraintMgr(constraintmgr), CheckerMgr(checkerMgr), diff --git a/clang/test/Analysis/Inputs/system-header-simulator-cxx-coroutines.h b/clang/test/Analysis/Inputs/system-header-simulator-cxx-coroutines.h new file mode 100644 index 0000000000000..79c024656d239 --- /dev/null +++ b/clang/test/Analysis/Inputs/system-header-simulator-cxx-coroutines.h @@ -0,0 +1,31 @@ +#pragma clang system_header + +// Like the compiler-provided <coroutine>, but cut down to the bare minimum +// needed to write coroutines in tests. + +namespace std { + +template <typename R, typename...> struct coroutine_traits { + using promise_type = typename R::promise_type; +}; + +template <typename Promise = void> struct coroutine_handle { + static coroutine_handle from_address(void *addr) noexcept { return {}; } + static coroutine_handle from_promise(Promise &promise) { return {}; } + constexpr coroutine_handle() noexcept = default; +}; + +template <> struct coroutine_handle<void> { + template <typename Promise> + coroutine_handle(coroutine_handle<Promise>) noexcept {} + static coroutine_handle from_address(void *addr) noexcept { return {}; } + constexpr coroutine_handle() noexcept = default; +}; + +struct suspend_never { + bool await_ready() noexcept { return true; } + void await_suspend(coroutine_handle<>) noexcept {} + void await_resume() noexcept {} +}; + +} // namespace std diff --git a/clang/test/Analysis/coroutine-promise-init.cpp b/clang/test/Analysis/coroutine-promise-init.cpp new file mode 100644 index 0000000000000..d8e1067bd0042 --- /dev/null +++ b/clang/test/Analysis/coroutine-promise-init.cpp @@ -0,0 +1,154 @@ +// RUN: %clang_analyze_cc1 -std=c++20 -analyzer-checker=core -verify %s + +// Regression test for the Clang Static Analyzer treating a coroutine +// promise object's data members as uninitialized garbage even when they +// are guaranteed to be initialized (via in-class default member +// initializers, a user-provided constructor, brace-init, etc.), because +// the promise object's construction was never represented in the CFG that +// the analyzer walks. See the CFG.cpp change to +// CFG::BuildOptions::AddImplicitCoroutinePromiseConstruction. + +#include "Inputs/system-header-simulator-cxx-coroutines.h" + +struct Ready { + bool await_ready() noexcept { return true; } + void await_suspend(std::coroutine_handle<>) noexcept {} + void await_resume() noexcept {} +}; + +//===----------------------------------------------------------------------===// +// 1. Exact reproducer from the bug report: in-class default member +// initializer on a scalar member. The original report used +// std::uintptr_t (as in real pointer-tagging code), but the exact +// integer type is irrelevant to the bug, so a plain, unambiguously +// portable type is used here instead. +//===----------------------------------------------------------------------===// + +struct PointerTask { + struct promise_type { + unsigned storage_ = 0; + + PointerTask get_return_object() noexcept { return {}; } + std::suspend_never initial_suspend() noexcept { return {}; } + std::suspend_never final_suspend() noexcept { return {}; } + void return_void() noexcept {} + void unhandled_exception() noexcept {} + + Ready await_transform(Ready a) noexcept { + (void)(storage_ & ~3u); // no-warning + return a; + } + }; +}; + +PointerTask pointer_repro() { + co_await Ready{}; +} + +//===----------------------------------------------------------------------===// +// 2. Multiple members, each with its own default member initializer. +//===----------------------------------------------------------------------===// + +struct MultiMemberTask { + struct promise_type { + int a_ = 1; + int b_ = 2; + bool flag_ = false; + + MultiMemberTask get_return_object() noexcept { return {}; } + std::suspend_never initial_suspend() noexcept { return {}; } + std::suspend_never final_suspend() noexcept { return {}; } + void return_void() noexcept {} + void unhandled_exception() noexcept {} + + Ready await_transform(Ready a) noexcept { + if (flag_) {} // no-warning + (void)(a_ + b_); // no-warning + return a; + } + }; +}; + +MultiMemberTask multi_member_repro() { + co_await Ready{}; +} + +//===----------------------------------------------------------------------===// +// 3. Member initialized by the promise type's user-provided (non-default) +// constructor body, rather than an in-class initializer. +//===----------------------------------------------------------------------===// + +struct CtorInitTask { + struct promise_type { + int value_; + + promise_type() : value_(0) {} + + CtorInitTask get_return_object() noexcept { return {}; } + std::suspend_never initial_suspend() noexcept { return {}; } + std::suspend_never final_suspend() noexcept { return {}; } + void return_void() noexcept {} + void unhandled_exception() noexcept {} + + Ready await_transform(Ready a) noexcept { + (void)(value_ & 1); // no-warning + return a; + } + }; +}; + +CtorInitTask ctor_init_repro() { + co_await Ready{}; +} + +//===----------------------------------------------------------------------===// +// 4. Brace / zero initialization of a scalar member. +//===----------------------------------------------------------------------===// + +struct BraceInitTask { + struct promise_type { + int value_{}; + + BraceInitTask get_return_object() noexcept { return {}; } + std::suspend_never initial_suspend() noexcept { return {}; } + std::suspend_never final_suspend() noexcept { return {}; } + void return_void() noexcept {} + void unhandled_exception() noexcept {} + + Ready await_transform(Ready a) noexcept { + (void)(value_ | 1); // no-warning + return a; + } + }; +}; + +BraceInitTask brace_init_repro() { + co_await Ready{}; +} + +//===----------------------------------------------------------------------===// +// 5. Negative control: a promise member that is genuinely never +// initialized must still be flagged. This guards against the fix +// degenerating into a blanket suppression for promise types. +//===----------------------------------------------------------------------===// + +struct UninitTask { + struct promise_type { + int uninitialized_; + + UninitTask get_return_object() noexcept { return {}; } + std::suspend_never initial_suspend() noexcept { return {}; } + std::suspend_never final_suspend() noexcept { return {}; } + void return_void() noexcept {} + void unhandled_exception() noexcept {} + + Ready await_transform(Ready a) noexcept { + (void)(uninitialized_ & 1); // expected-warning{{The left operand of '&' is a garbage value}} + return a; + } + }; +}; + +UninitTask uninit_repro() { + co_await Ready{}; +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
