[clang] [flang] [clang][OpenMP] Diagnose invalid allocator in `#pragma omp allocate`; avoid null deref (PR #158146)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Sep 11 13:16:42 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Krish Gupta (KrxGu)
<details>
<summary>Changes</summary>
Reproducer without <omp.h> (forged `omp_allocator_handle_t`) crashed in SemaOpenMP
(null deref while classifying the allocator).
Fix:
- Null-guard predefined allocator lookup in getAllocatorKind.
- Keep user-defined as fallback; emit existing diagnostic for missing/invalid handle.
Tests:
- OpenMP/allocate-allocator-handle-diag.c: expects the diagnostic.
- OpenMP/allocate-allocator-duplicate.c: single diagnostic, no crash.
<img width="1240" height="220" alt="image" src="https://github.com/user-attachments/assets/590242a7-0206-4ac5-b142-f595ddc0ac64" />
<img width="1280" height="197" alt="image" src="https://github.com/user-attachments/assets/8f68d398-d6ad-4dd7-a600-8b51b6cfab37" />
complete build check:
<img width="1280" height="313" alt="image" src="https://github.com/user-attachments/assets/2f5caa9d-312f-4580-aab6-c51297174460" />
no crash (error is expected):
<img width="1280" height="255" alt="image" src="https://github.com/user-attachments/assets/6bfee6e5-a5ed-4944-afce-428108a2fec4" />
Fixes #<!-- -->157868.
---
Full diff: https://github.com/llvm/llvm-project/pull/158146.diff
4 Files Affected:
- (modified) clang/lib/Sema/SemaOpenMP.cpp (+22-19)
- (added) clang/test/OpenMP/allocate-allocator-duplicate.c (+16)
- (added) clang/test/OpenMP/allocate-allocator-handle-diag.c (+13)
- (added) flang/test/Lower/OpenMP/lastprivate-alloc-scope.f90 (+22)
``````````diff
diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 63a56a6583efc..1af8ed20f24e2 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -3321,27 +3321,33 @@ SemaOpenMP::CheckOMPThreadPrivateDecl(SourceLocation Loc,
}
return D;
}
-
static OMPAllocateDeclAttr::AllocatorTypeTy
getAllocatorKind(Sema &S, DSAStackTy *Stack, Expr *Allocator) {
+ // No allocator expression → Null mem alloc (matches existing tests).
if (!Allocator)
return OMPAllocateDeclAttr::OMPNullMemAlloc;
+
if (Allocator->isTypeDependent() || Allocator->isValueDependent() ||
Allocator->isInstantiationDependent() ||
Allocator->containsUnexpandedParameterPack())
return OMPAllocateDeclAttr::OMPUserDefinedMemAlloc;
+
auto AllocatorKindRes = OMPAllocateDeclAttr::OMPUserDefinedMemAlloc;
+
llvm::FoldingSetNodeID AEId;
const Expr *AE = Allocator->IgnoreParenImpCasts();
- AE->IgnoreImpCasts()->Profile(AEId, S.getASTContext(), /*Canonical=*/true);
+ AE->Profile(AEId, S.getASTContext(), /*Canonical=*/true);
+
for (int I = 0; I < OMPAllocateDeclAttr::OMPUserDefinedMemAlloc; ++I) {
- auto AllocatorKind = static_cast<OMPAllocateDeclAttr::AllocatorTypeTy>(I);
- const Expr *DefAllocator = Stack->getAllocator(AllocatorKind);
+ auto K = static_cast<OMPAllocateDeclAttr::AllocatorTypeTy>(I);
+ const Expr *Def = Stack->getAllocator(K);
+ if (!Def)
+ continue;
llvm::FoldingSetNodeID DAEId;
- DefAllocator->IgnoreImpCasts()->Profile(DAEId, S.getASTContext(),
- /*Canonical=*/true);
+ Def->IgnoreImpCasts()->Profile(DAEId, S.getASTContext(),
+ /*Canonical=*/true);
if (AEId == DAEId) {
- AllocatorKindRes = AllocatorKind;
+ AllocatorKindRes = K;
break;
}
}
@@ -3353,10 +3359,12 @@ static bool checkPreviousOMPAllocateAttribute(
OMPAllocateDeclAttr::AllocatorTypeTy AllocatorKind, Expr *Allocator) {
if (!VD->hasAttr<OMPAllocateDeclAttr>())
return false;
+
const auto *A = VD->getAttr<OMPAllocateDeclAttr>();
Expr *PrevAllocator = A->getAllocator();
OMPAllocateDeclAttr::AllocatorTypeTy PrevAllocatorKind =
getAllocatorKind(S, Stack, PrevAllocator);
+
bool AllocatorsMatch = AllocatorKind == PrevAllocatorKind;
if (AllocatorsMatch &&
AllocatorKind == OMPAllocateDeclAttr::OMPUserDefinedMemAlloc &&
@@ -3368,11 +3376,13 @@ static bool checkPreviousOMPAllocateAttribute(
PAE->Profile(PAEId, S.Context, /*Canonical=*/true);
AllocatorsMatch = AEId == PAEId;
}
+
if (!AllocatorsMatch) {
SmallString<256> AllocatorBuffer;
llvm::raw_svector_ostream AllocatorStream(AllocatorBuffer);
if (Allocator)
Allocator->printPretty(AllocatorStream, nullptr, S.getPrintingPolicy());
+
SmallString<256> PrevAllocatorBuffer;
llvm::raw_svector_ostream PrevAllocatorStream(PrevAllocatorBuffer);
if (PrevAllocator)
@@ -3408,8 +3418,7 @@ applyOMPAllocateAttribute(Sema &S, VarDecl *VD,
(Alignment->isTypeDependent() || Alignment->isValueDependent() ||
Alignment->isInstantiationDependent() ||
Alignment->containsUnexpandedParameterPack()))
- // Apply later when we have a usable value.
- return;
+ return; // apply later
if (Allocator &&
(Allocator->isTypeDependent() || Allocator->isValueDependent() ||
Allocator->isInstantiationDependent() ||
@@ -3430,9 +3439,6 @@ SemaOpenMP::DeclGroupPtrTy SemaOpenMP::ActOnOpenMPAllocateDirective(
Expr *Allocator = nullptr;
if (Clauses.empty()) {
// OpenMP 5.0, 2.11.3 allocate Directive, Restrictions.
- // allocate directives that appear in a target region must specify an
- // allocator clause unless a requires directive with the dynamic_allocators
- // clause is present in the same compilation unit.
if (getLangOpts().OpenMPIsTargetDevice &&
!DSAStack->hasRequiresDeclWithClause<OMPDynamicAllocatorsClause>())
SemaRef.targetDiag(Loc, diag::err_expected_allocator_clause);
@@ -3445,6 +3451,7 @@ SemaOpenMP::DeclGroupPtrTy SemaOpenMP::ActOnOpenMPAllocateDirective(
else
llvm_unreachable("Unexpected clause on allocate directive");
}
+ // No forward-decl needed; just classify with null-guards.
OMPAllocateDeclAttr::AllocatorTypeTy AllocatorKind =
getAllocatorKind(SemaRef, DSAStack, Allocator);
SmallVector<Expr *, 8> Vars;
@@ -3452,23 +3459,19 @@ SemaOpenMP::DeclGroupPtrTy SemaOpenMP::ActOnOpenMPAllocateDirective(
auto *DE = cast<DeclRefExpr>(RefExpr);
auto *VD = cast<VarDecl>(DE->getDecl());
- // Check if this is a TLS variable or global register.
+ // Skip TLS/global-registers.
if (VD->getTLSKind() != VarDecl::TLS_None ||
VD->hasAttr<OMPThreadPrivateDeclAttr>() ||
(VD->getStorageClass() == SC_Register && VD->hasAttr<AsmLabelAttr>() &&
!VD->isLocalVarDecl()))
continue;
- // If the used several times in the allocate directive, the same allocator
- // must be used.
+ // Enforce same allocator if repeated.
if (checkPreviousOMPAllocateAttribute(SemaRef, DSAStack, RefExpr, VD,
AllocatorKind, Allocator))
continue;
- // OpenMP, 2.11.3 allocate Directive, Restrictions, C / C++
- // If a list item has a static storage type, the allocator expression in the
- // allocator clause must be a constant expression that evaluates to one of
- // the predefined memory allocator values.
+ // For static storage, allocator must be predefined.
if (Allocator && VD->hasGlobalStorage()) {
if (AllocatorKind == OMPAllocateDeclAttr::OMPUserDefinedMemAlloc) {
Diag(Allocator->getExprLoc(),
diff --git a/clang/test/OpenMP/allocate-allocator-duplicate.c b/clang/test/OpenMP/allocate-allocator-duplicate.c
new file mode 100644
index 0000000000000..91a294160992c
--- /dev/null
+++ b/clang/test/OpenMP/allocate-allocator-duplicate.c
@@ -0,0 +1,16 @@
+// RUN: %clang_cc1 -fopenmp -verify %s
+
+typedef enum omp_allocator_handle_t {
+ omp_default_mem_alloc = 1,
+ __omp_allocator_handle_t_max__ = __UINTPTR_MAX__
+} omp_allocator_handle_t;
+
+void foo(void) {
+ omp_allocator_handle_t my_handle;
+ int A[2];
+ // expected-error at +2 {{'omp_allocator_handle_t' type not found; include <omp.h>}}
+ // expected-note at +1 {{previous allocator is specified here}}
+ #pragma omp allocate(A) allocator(my_handle)
+ // expected-warning at +1 {{allocate directive specifies 'my_handle' allocator while previously used default}}
+ #pragma omp allocate(A) allocator(my_handle)
+}
diff --git a/clang/test/OpenMP/allocate-allocator-handle-diag.c b/clang/test/OpenMP/allocate-allocator-handle-diag.c
new file mode 100644
index 0000000000000..07bf68bd9972d
--- /dev/null
+++ b/clang/test/OpenMP/allocate-allocator-handle-diag.c
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 -fopenmp -verify %s
+// No <omp.h>; forge a typedef.
+typedef enum omp_allocator_handle_t {
+ omp_default_mem_alloc = 1,
+ __omp_allocator_handle_t_max__ = __UINTPTR_MAX__
+} omp_allocator_handle_t;
+
+void foo(void) {
+ omp_allocator_handle_t my_handle;
+ int A[2];
+ // expected-error at +1 {{'omp_allocator_handle_t' type not found; include <omp.h>}}
+ #pragma omp allocate(A) allocator(my_handle)
+}
diff --git a/flang/test/Lower/OpenMP/lastprivate-alloc-scope.f90 b/flang/test/Lower/OpenMP/lastprivate-alloc-scope.f90
new file mode 100644
index 0000000000000..67d885ed5fb7a
--- /dev/null
+++ b/flang/test/Lower/OpenMP/lastprivate-alloc-scope.f90
@@ -0,0 +1,22 @@
+! RUN: %flang_fc1 -fopenmp -emit-hlfir %s -o - | FileCheck %s
+
+program p
+ type y3; integer, allocatable :: x; end type
+ type(y3) :: v
+ integer :: s, n, i
+ s = 1; n = 10
+ allocate(v%x); v%x = 0
+!$omp parallel
+ if (.not. allocated(v%x)) print *, '101', allocated(v%x)
+!$omp do schedule(dynamic) lastprivate(v)
+ do i = s, n
+ v%x = i
+ end do
+!$omp end do
+!$omp end parallel
+end program
+
+! CHECK: omp.parallel {
+! CHECK-NOT: private(
+! CHECK: omp.wsloop
+! CHECK-SAME: private(
``````````
</details>
https://github.com/llvm/llvm-project/pull/158146
More information about the cfe-commits
mailing list