[clang] Diagnose use of VLAs in a coroutine (PR #70341)
https://github.com/AaronBallman created https://github.com/llvm/llvm-project/pull/70341 Fixes https://github.com/llvm/llvm-project/issues/65858 >From f068b471efa81d60d1d6e2c98da56be58958a015 Mon Sep 17 00:00:00 2001 From: Aaron Ballman Date: Thu, 26 Oct 2023 10:53:06 -0400 Subject: [PATCH] Diagnose use of VLAs in a coroutine Fixes https://github.com/llvm/llvm-project/issues/65858 --- .../clang/Basic/DiagnosticSemaKinds.td| 2 ++ clang/include/clang/Sema/ScopeInfo.h | 8 + clang/lib/Sema/SemaCoroutine.cpp | 5 clang/lib/Sema/SemaType.cpp | 18 clang/test/SemaCXX/coroutine-vla.cpp | 29 +++ 5 files changed, 56 insertions(+), 6 deletions(-) create mode 100644 clang/test/SemaCXX/coroutine-vla.cpp diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index a673ce726d6c220..453bd8a9a340425 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -166,6 +166,8 @@ def ext_vla_folded_to_constant : ExtWarn< InGroup; def err_vla_unsupported : Error< "variable length arrays are not supported for %select{the current target|'%1'}0">; +def err_vla_in_coroutine_unsupported : Error< + "variable length arrays in a coroutine are not supported">; def note_vla_unsupported : Note< "variable length arrays are not supported for the current target">; diff --git a/clang/include/clang/Sema/ScopeInfo.h b/clang/include/clang/Sema/ScopeInfo.h index 02b22af89ff035d..b2f6e3289f41fce 100644 --- a/clang/include/clang/Sema/ScopeInfo.h +++ b/clang/include/clang/Sema/ScopeInfo.h @@ -189,6 +189,9 @@ class FunctionScopeInfo { /// First SEH '__try' statement in the current function. SourceLocation FirstSEHTryLoc; + /// First use of a VLA within the current function. + SourceLocation FirstVLALoc; + private: /// Used to determine if errors occurred in this function or block. DiagnosticErrorTrap ErrorTrap; @@ -473,6 +476,11 @@ class FunctionScopeInfo { FirstSEHTryLoc = TryLoc; } + void setHasVLA(SourceLocation VLALoc) { +if (FirstVLALoc.isInvalid()) + FirstVLALoc = VLALoc; + } + bool NeedsScopeChecking() const { return !HasDroppedStmt && (HasIndirectGoto || HasMustTail || (HasBranchProtectedScope && HasBranchIntoScope)); diff --git a/clang/lib/Sema/SemaCoroutine.cpp b/clang/lib/Sema/SemaCoroutine.cpp index d2b0922a4bb9c4c..e1f3b5acb3cadec 100644 --- a/clang/lib/Sema/SemaCoroutine.cpp +++ b/clang/lib/Sema/SemaCoroutine.cpp @@ -1198,6 +1198,11 @@ void Sema::CheckCompletedCoroutineBody(FunctionDecl *FD, Stmt *&Body) { if (FD->hasAttr()) Diag(FD->getLocation(), diag::warn_always_inline_coroutine); + // We don't allow use of VLAs within a coroutine, so diagnose if we've seen + // a VLA in the body of this function. + if (Fn->FirstVLALoc.isValid()) +Diag(Fn->FirstVLALoc, diag::err_vla_in_coroutine_unsupported); + // [stmt.return.coroutine]p1: // A coroutine shall not enclose a return statement ([stmt.return]). if (Fn->FirstReturnLoc.isValid()) { diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp index 28b81c1768a3004..dea77fae4cadb59 100644 --- a/clang/lib/Sema/SemaType.cpp +++ b/clang/lib/Sema/SemaType.cpp @@ -2706,12 +2706,18 @@ QualType Sema::BuildArrayType(QualType T, ArrayType::ArraySizeModifier ASM, } } - if (T->isVariableArrayType() && !Context.getTargetInfo().isVLASupported()) { -// CUDA device code and some other targets don't support VLAs. -bool IsCUDADevice = (getLangOpts().CUDA && getLangOpts().CUDAIsDevice); -targetDiag(Loc, - IsCUDADevice ? diag::err_cuda_vla : diag::err_vla_unsupported) -<< (IsCUDADevice ? CurrentCUDATarget() : 0); + if (T->isVariableArrayType()) { +if (!Context.getTargetInfo().isVLASupported()) { + // CUDA device code and some other targets don't support VLAs. + bool IsCUDADevice = (getLangOpts().CUDA && getLangOpts().CUDAIsDevice); + targetDiag(Loc, + IsCUDADevice ? diag::err_cuda_vla : diag::err_vla_unsupported) + << (IsCUDADevice ? CurrentCUDATarget() : 0); +} else if (sema::FunctionScopeInfo *FSI = getCurFunction()) { + // VLAs are supported on this target, but we may need to do delayed + // checking that the VLA is not being used within a coroutine. + FSI->setHasVLA(Loc); +} } // If this is not C99, diagnose array size modifiers on non-VLAs. diff --git a/clang/test/SemaCXX/coroutine-vla.cpp b/clang/test/SemaCXX/coroutine-vla.cpp new file mode 100644 index 000..176e35f346e2b45 --- /dev/null +++ b/clang/test/SemaCXX/coroutine-vla.cpp @@ -0,0 +1,29 @@ +// RUN: %clang_cc1 %s -std=c++20 -fsyntax-only -Wno-vla-cxx-extension -verify +#include "Inputs/std-coroutine.h" + +struct promise; + +struct coroutine : std::coroutine_ha
[clang] Diagnose use of VLAs in a coroutine (PR #70341)
llvmbot wrote: @llvm/pr-subscribers-coroutines Author: Aaron Ballman (AaronBallman) Changes Fixes https://github.com/llvm/llvm-project/issues/65858 --- Full diff: https://github.com/llvm/llvm-project/pull/70341.diff 5 Files Affected: - (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+2) - (modified) clang/include/clang/Sema/ScopeInfo.h (+8) - (modified) clang/lib/Sema/SemaCoroutine.cpp (+5) - (modified) clang/lib/Sema/SemaType.cpp (+12-6) - (added) clang/test/SemaCXX/coroutine-vla.cpp (+29) ``diff diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index a673ce726d6c220..453bd8a9a340425 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -166,6 +166,8 @@ def ext_vla_folded_to_constant : ExtWarn< InGroup; def err_vla_unsupported : Error< "variable length arrays are not supported for %select{the current target|'%1'}0">; +def err_vla_in_coroutine_unsupported : Error< + "variable length arrays in a coroutine are not supported">; def note_vla_unsupported : Note< "variable length arrays are not supported for the current target">; diff --git a/clang/include/clang/Sema/ScopeInfo.h b/clang/include/clang/Sema/ScopeInfo.h index 02b22af89ff035d..b2f6e3289f41fce 100644 --- a/clang/include/clang/Sema/ScopeInfo.h +++ b/clang/include/clang/Sema/ScopeInfo.h @@ -189,6 +189,9 @@ class FunctionScopeInfo { /// First SEH '__try' statement in the current function. SourceLocation FirstSEHTryLoc; + /// First use of a VLA within the current function. + SourceLocation FirstVLALoc; + private: /// Used to determine if errors occurred in this function or block. DiagnosticErrorTrap ErrorTrap; @@ -473,6 +476,11 @@ class FunctionScopeInfo { FirstSEHTryLoc = TryLoc; } + void setHasVLA(SourceLocation VLALoc) { +if (FirstVLALoc.isInvalid()) + FirstVLALoc = VLALoc; + } + bool NeedsScopeChecking() const { return !HasDroppedStmt && (HasIndirectGoto || HasMustTail || (HasBranchProtectedScope && HasBranchIntoScope)); diff --git a/clang/lib/Sema/SemaCoroutine.cpp b/clang/lib/Sema/SemaCoroutine.cpp index d2b0922a4bb9c4c..e1f3b5acb3cadec 100644 --- a/clang/lib/Sema/SemaCoroutine.cpp +++ b/clang/lib/Sema/SemaCoroutine.cpp @@ -1198,6 +1198,11 @@ void Sema::CheckCompletedCoroutineBody(FunctionDecl *FD, Stmt *&Body) { if (FD->hasAttr()) Diag(FD->getLocation(), diag::warn_always_inline_coroutine); + // We don't allow use of VLAs within a coroutine, so diagnose if we've seen + // a VLA in the body of this function. + if (Fn->FirstVLALoc.isValid()) +Diag(Fn->FirstVLALoc, diag::err_vla_in_coroutine_unsupported); + // [stmt.return.coroutine]p1: // A coroutine shall not enclose a return statement ([stmt.return]). if (Fn->FirstReturnLoc.isValid()) { diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp index 28b81c1768a3004..dea77fae4cadb59 100644 --- a/clang/lib/Sema/SemaType.cpp +++ b/clang/lib/Sema/SemaType.cpp @@ -2706,12 +2706,18 @@ QualType Sema::BuildArrayType(QualType T, ArrayType::ArraySizeModifier ASM, } } - if (T->isVariableArrayType() && !Context.getTargetInfo().isVLASupported()) { -// CUDA device code and some other targets don't support VLAs. -bool IsCUDADevice = (getLangOpts().CUDA && getLangOpts().CUDAIsDevice); -targetDiag(Loc, - IsCUDADevice ? diag::err_cuda_vla : diag::err_vla_unsupported) -<< (IsCUDADevice ? CurrentCUDATarget() : 0); + if (T->isVariableArrayType()) { +if (!Context.getTargetInfo().isVLASupported()) { + // CUDA device code and some other targets don't support VLAs. + bool IsCUDADevice = (getLangOpts().CUDA && getLangOpts().CUDAIsDevice); + targetDiag(Loc, + IsCUDADevice ? diag::err_cuda_vla : diag::err_vla_unsupported) + << (IsCUDADevice ? CurrentCUDATarget() : 0); +} else if (sema::FunctionScopeInfo *FSI = getCurFunction()) { + // VLAs are supported on this target, but we may need to do delayed + // checking that the VLA is not being used within a coroutine. + FSI->setHasVLA(Loc); +} } // If this is not C99, diagnose array size modifiers on non-VLAs. diff --git a/clang/test/SemaCXX/coroutine-vla.cpp b/clang/test/SemaCXX/coroutine-vla.cpp new file mode 100644 index 000..176e35f346e2b45 --- /dev/null +++ b/clang/test/SemaCXX/coroutine-vla.cpp @@ -0,0 +1,29 @@ +// RUN: %clang_cc1 %s -std=c++20 -fsyntax-only -Wno-vla-cxx-extension -verify +#include "Inputs/std-coroutine.h" + +struct promise; + +struct coroutine : std::coroutine_handle { + using promise_type = ::promise; +}; + +struct promise +{ +coroutine get_return_object(); +std::suspend_always initial_suspend() noexcept; +std::suspend_always final_suspend() noexcept; +void return_void(); +void unhandled_exception();
[clang] Diagnose use of VLAs in a coroutine (PR #70341)
cor3ntin wrote: LGTM but it is missing a release note :) https://github.com/llvm/llvm-project/pull/70341 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Diagnose use of VLAs in a coroutine (PR #70341)
https://github.com/cor3ntin approved this pull request. https://github.com/llvm/llvm-project/pull/70341 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Diagnose use of VLAs in a coroutine (PR #70341)
https://github.com/ChuanqiXu9 approved this pull request. Thanks! LGTM with a nit. https://github.com/llvm/llvm-project/pull/70341 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Diagnose use of VLAs in a coroutine (PR #70341)
@@ -1198,6 +1198,11 @@ void Sema::CheckCompletedCoroutineBody(FunctionDecl *FD, Stmt *&Body) { if (FD->hasAttr()) Diag(FD->getLocation(), diag::warn_always_inline_coroutine); + // We don't allow use of VLAs within a coroutine, so diagnose if we've seen ChuanqiXu9 wrote: ```suggestion // VLAs are not allowed within a coroutine, so diagnose if we've seen ``` nit: it is not us (clang/LLVM) who decide not to support VLA in coroutines. But C++20 coroutines itself doesn't support VLA by its design naturally. https://github.com/llvm/llvm-project/pull/70341 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Diagnose use of VLAs in a coroutine (PR #70341)
https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/70341 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Diagnose use of VLAs in a coroutine (PR #70341)
ChuanqiXu9 wrote: > LGTM but it is missing a release note :) it may be fine to not mention it clearly since VLAs in coroutines never got compiled.. user might meet backend crashes.. https://github.com/llvm/llvm-project/pull/70341 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Diagnose use of VLAs in a coroutine (PR #70341)
https://github.com/ChuanqiXu9 edited https://github.com/llvm/llvm-project/pull/70341 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Diagnose use of VLAs in a coroutine (PR #70341)
https://github.com/erichkeane approved this pull request. The comment suggestion makes sense to me, else just needs a release note. https://github.com/llvm/llvm-project/pull/70341 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] Diagnose use of VLAs in a coroutine (PR #70341)
https://github.com/AaronBallman updated https://github.com/llvm/llvm-project/pull/70341 >From f068b471efa81d60d1d6e2c98da56be58958a015 Mon Sep 17 00:00:00 2001 From: Aaron Ballman Date: Thu, 26 Oct 2023 10:53:06 -0400 Subject: [PATCH 1/2] Diagnose use of VLAs in a coroutine Fixes https://github.com/llvm/llvm-project/issues/65858 --- .../clang/Basic/DiagnosticSemaKinds.td| 2 ++ clang/include/clang/Sema/ScopeInfo.h | 8 + clang/lib/Sema/SemaCoroutine.cpp | 5 clang/lib/Sema/SemaType.cpp | 18 clang/test/SemaCXX/coroutine-vla.cpp | 29 +++ 5 files changed, 56 insertions(+), 6 deletions(-) create mode 100644 clang/test/SemaCXX/coroutine-vla.cpp diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index a673ce726d6c220..453bd8a9a340425 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -166,6 +166,8 @@ def ext_vla_folded_to_constant : ExtWarn< InGroup; def err_vla_unsupported : Error< "variable length arrays are not supported for %select{the current target|'%1'}0">; +def err_vla_in_coroutine_unsupported : Error< + "variable length arrays in a coroutine are not supported">; def note_vla_unsupported : Note< "variable length arrays are not supported for the current target">; diff --git a/clang/include/clang/Sema/ScopeInfo.h b/clang/include/clang/Sema/ScopeInfo.h index 02b22af89ff035d..b2f6e3289f41fce 100644 --- a/clang/include/clang/Sema/ScopeInfo.h +++ b/clang/include/clang/Sema/ScopeInfo.h @@ -189,6 +189,9 @@ class FunctionScopeInfo { /// First SEH '__try' statement in the current function. SourceLocation FirstSEHTryLoc; + /// First use of a VLA within the current function. + SourceLocation FirstVLALoc; + private: /// Used to determine if errors occurred in this function or block. DiagnosticErrorTrap ErrorTrap; @@ -473,6 +476,11 @@ class FunctionScopeInfo { FirstSEHTryLoc = TryLoc; } + void setHasVLA(SourceLocation VLALoc) { +if (FirstVLALoc.isInvalid()) + FirstVLALoc = VLALoc; + } + bool NeedsScopeChecking() const { return !HasDroppedStmt && (HasIndirectGoto || HasMustTail || (HasBranchProtectedScope && HasBranchIntoScope)); diff --git a/clang/lib/Sema/SemaCoroutine.cpp b/clang/lib/Sema/SemaCoroutine.cpp index d2b0922a4bb9c4c..e1f3b5acb3cadec 100644 --- a/clang/lib/Sema/SemaCoroutine.cpp +++ b/clang/lib/Sema/SemaCoroutine.cpp @@ -1198,6 +1198,11 @@ void Sema::CheckCompletedCoroutineBody(FunctionDecl *FD, Stmt *&Body) { if (FD->hasAttr()) Diag(FD->getLocation(), diag::warn_always_inline_coroutine); + // We don't allow use of VLAs within a coroutine, so diagnose if we've seen + // a VLA in the body of this function. + if (Fn->FirstVLALoc.isValid()) +Diag(Fn->FirstVLALoc, diag::err_vla_in_coroutine_unsupported); + // [stmt.return.coroutine]p1: // A coroutine shall not enclose a return statement ([stmt.return]). if (Fn->FirstReturnLoc.isValid()) { diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp index 28b81c1768a3004..dea77fae4cadb59 100644 --- a/clang/lib/Sema/SemaType.cpp +++ b/clang/lib/Sema/SemaType.cpp @@ -2706,12 +2706,18 @@ QualType Sema::BuildArrayType(QualType T, ArrayType::ArraySizeModifier ASM, } } - if (T->isVariableArrayType() && !Context.getTargetInfo().isVLASupported()) { -// CUDA device code and some other targets don't support VLAs. -bool IsCUDADevice = (getLangOpts().CUDA && getLangOpts().CUDAIsDevice); -targetDiag(Loc, - IsCUDADevice ? diag::err_cuda_vla : diag::err_vla_unsupported) -<< (IsCUDADevice ? CurrentCUDATarget() : 0); + if (T->isVariableArrayType()) { +if (!Context.getTargetInfo().isVLASupported()) { + // CUDA device code and some other targets don't support VLAs. + bool IsCUDADevice = (getLangOpts().CUDA && getLangOpts().CUDAIsDevice); + targetDiag(Loc, + IsCUDADevice ? diag::err_cuda_vla : diag::err_vla_unsupported) + << (IsCUDADevice ? CurrentCUDATarget() : 0); +} else if (sema::FunctionScopeInfo *FSI = getCurFunction()) { + // VLAs are supported on this target, but we may need to do delayed + // checking that the VLA is not being used within a coroutine. + FSI->setHasVLA(Loc); +} } // If this is not C99, diagnose array size modifiers on non-VLAs. diff --git a/clang/test/SemaCXX/coroutine-vla.cpp b/clang/test/SemaCXX/coroutine-vla.cpp new file mode 100644 index 000..176e35f346e2b45 --- /dev/null +++ b/clang/test/SemaCXX/coroutine-vla.cpp @@ -0,0 +1,29 @@ +// RUN: %clang_cc1 %s -std=c++20 -fsyntax-only -Wno-vla-cxx-extension -verify +#include "Inputs/std-coroutine.h" + +struct promise; + +struct coroutine : std::coroutine_handle { + using promise_type = ::promise; +}; + +stru
[clang] Diagnose use of VLAs in a coroutine (PR #70341)
https://github.com/erichkeane approved this pull request. https://github.com/llvm/llvm-project/pull/70341 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits