[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
