https://github.com/zahiraam updated https://github.com/llvm/llvm-project/pull/219265
>From f8eb385de3dca8934ea8ea4c1a28fd1b067facd7 Mon Sep 17 00:00:00 2001 From: Zahira Ammarguellat <[email protected]> Date: Thu, 27 Aug 2026 10:19:04 -0700 Subject: [PATCH 1/2] [OpenMP] Fix OpenMP reduction segfault with non-copyable types and user initializers Applied reviewer's suggestion and fixed format Addressed review comments Applied suggestions Used standard functions Removed codegen changes --- clang/lib/Sema/SemaOpenMP.cpp | 13 ++++ .../test/OpenMP/for_reduction_codegen_UDR.cpp | 22 ++++--- ...eduction_non_copyable_with_initializer.cpp | 65 +++++++++++++++++++ 3 files changed, 90 insertions(+), 10 deletions(-) create mode 100644 clang/test/OpenMP/reduction_non_copyable_with_initializer.cpp diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp index c8d8b1d46d90cd..5b647c3e25cbe1 100644 --- a/clang/lib/Sema/SemaOpenMP.cpp +++ b/clang/lib/Sema/SemaOpenMP.cpp @@ -25,6 +25,7 @@ #include "clang/AST/DynamicRecursiveASTVisitor.h" #include "clang/AST/OpenMPClause.h" #include "clang/AST/RecursiveASTVisitor.h" +#include "clang/AST/Stmt.h" #include "clang/AST/StmtCXX.h" #include "clang/AST/StmtOpenMP.h" #include "clang/AST/StmtVisitor.h" @@ -25618,6 +25619,18 @@ VarDecl *SemaOpenMP::ActOnOpenMPDeclareReductionInitializerStart(Scope *S, void SemaOpenMP::ActOnOpenMPDeclareReductionInitializerEnd( Decl *D, Expr *Initializer, VarDecl *OmpPrivParm) { auto *DRD = cast<OMPDeclareReductionDecl>(D); + + // Ensure OmpPrivParm is default-constructed before the user initializer runs + // (required for class types with non-trivial default constructors). + if (Initializer && !DRD->getDeclContext()->isDependentContext()) { + QualType ReductionType = DRD->getType(); + if (CXXRecordDecl *RD = ReductionType->getAsCXXRecordDecl()) { + CXXConstructorDecl *DefaultCtor = SemaRef.LookupDefaultConstructor(RD); + if (DefaultCtor && !DefaultCtor->isDeleted() && !DefaultCtor->isTrivial()) + SemaRef.ActOnUninitializedDecl(OmpPrivParm); + } + } + SemaRef.DiscardCleanupsInEvaluationContext(); SemaRef.PopExpressionEvaluationContext(); diff --git a/clang/test/OpenMP/for_reduction_codegen_UDR.cpp b/clang/test/OpenMP/for_reduction_codegen_UDR.cpp index 08c90f0094f508..a94a8bb0396403 100644 --- a/clang/test/OpenMP/for_reduction_codegen_UDR.cpp +++ b/clang/test/OpenMP/for_reduction_codegen_UDR.cpp @@ -970,6 +970,7 @@ int main() { // CHECK1-NEXT: store ptr [[TMP1]], ptr [[DOTADDR1]], align 8 // CHECK1-NEXT: [[TMP2:%.*]] = load ptr, ptr [[DOTADDR1]], align 8 // CHECK1-NEXT: [[TMP3:%.*]] = load ptr, ptr [[DOTADDR]], align 8 +// CHECK1-NEXT: call void @_ZN1SIfEC1Ev(ptr noundef nonnull align 4 dereferenceable(12) [[TMP3]]) // CHECK1-NEXT: [[ADD_PTR:%.*]] = getelementptr inbounds i8, ptr [[TMP3]], i64 4 // CHECK1-NEXT: [[ADD_PTR2:%.*]] = getelementptr inbounds i8, ptr [[TMP2]], i64 4 // CHECK1-NEXT: call void @_Z5init1R6BaseS1RKS_(ptr noundef nonnull align 4 dereferenceable(4) [[ADD_PTR]], ptr noundef nonnull align 4 dereferenceable(4) [[ADD_PTR2]]) @@ -2724,6 +2725,7 @@ int main() { // CHECK1-NEXT: store ptr [[TMP1]], ptr [[DOTADDR1]], align 8 // CHECK1-NEXT: [[TMP2:%.*]] = load ptr, ptr [[DOTADDR1]], align 8 // CHECK1-NEXT: [[TMP3:%.*]] = load ptr, ptr [[DOTADDR]], align 8 +// CHECK1-NEXT: call void @_ZN1SIiEC1Ev(ptr noundef nonnull align 4 dereferenceable(12) [[TMP3]]) // CHECK1-NEXT: [[ADD_PTR:%.*]] = getelementptr inbounds i8, ptr [[TMP3]], i64 4 // CHECK1-NEXT: [[ADD_PTR2:%.*]] = getelementptr inbounds i8, ptr [[TMP2]], i64 4 // CHECK1-NEXT: call void @_Z5init2R6BaseS1RKS_(ptr noundef nonnull align 4 dereferenceable(4) [[ADD_PTR]], ptr noundef nonnull align 4 dereferenceable(4) [[ADD_PTR2]]) @@ -3714,21 +3716,21 @@ int main() { // CHECK3-NEXT: ret void // // -// CHECK3-LABEL: define {{[^@]+}}@_ZN1SIiEC2Ev +// CHECK3-LABEL: define {{[^@]+}}@_ZN1SIfEC2Ev // CHECK3-SAME: (ptr noundef nonnull align 4 dereferenceable(12) [[THIS:%.*]]) unnamed_addr #[[ATTR0]] align 2 { // CHECK3-NEXT: entry: // CHECK3-NEXT: [[THIS_ADDR:%.*]] = alloca ptr, align 8 // CHECK3-NEXT: store ptr [[THIS]], ptr [[THIS_ADDR]], align 8 // CHECK3-NEXT: [[THIS1:%.*]] = load ptr, ptr [[THIS_ADDR]], align 8 // CHECK3-NEXT: [[TMP0:%.*]] = getelementptr inbounds i8, ptr [[THIS1]], i64 4 -// CHECK3-NEXT: [[F:%.*]] = getelementptr inbounds nuw [[STRUCT_S:%.*]], ptr [[THIS1]], i32 0, i32 2 +// CHECK3-NEXT: [[F:%.*]] = getelementptr inbounds nuw [[STRUCT_S_0:%.*]], ptr [[THIS1]], i32 0, i32 2 // CHECK3-NEXT: [[TMP1:%.*]] = load volatile double, ptr @g, align 8 -// CHECK3-NEXT: [[CONV:%.*]] = fptosi double [[TMP1]] to i32 -// CHECK3-NEXT: store i32 [[CONV]], ptr [[F]], align 4 +// CHECK3-NEXT: [[CONV:%.*]] = fptrunc double [[TMP1]] to float +// CHECK3-NEXT: store float [[CONV]], ptr [[F]], align 4 // CHECK3-NEXT: ret void // // -// CHECK3-LABEL: define {{[^@]+}}@_ZN1SIiED2Ev +// CHECK3-LABEL: define {{[^@]+}}@_ZN1SIfED2Ev // CHECK3-SAME: (ptr noundef nonnull align 4 dead_on_return(12) dereferenceable(12) [[THIS:%.*]]) unnamed_addr #[[ATTR0]] align 2 { // CHECK3-NEXT: entry: // CHECK3-NEXT: [[THIS_ADDR:%.*]] = alloca ptr, align 8 @@ -3737,21 +3739,21 @@ int main() { // CHECK3-NEXT: ret void // // -// CHECK3-LABEL: define {{[^@]+}}@_ZN1SIfEC2Ev +// CHECK3-LABEL: define {{[^@]+}}@_ZN1SIiEC2Ev // CHECK3-SAME: (ptr noundef nonnull align 4 dereferenceable(12) [[THIS:%.*]]) unnamed_addr #[[ATTR0]] align 2 { // CHECK3-NEXT: entry: // CHECK3-NEXT: [[THIS_ADDR:%.*]] = alloca ptr, align 8 // CHECK3-NEXT: store ptr [[THIS]], ptr [[THIS_ADDR]], align 8 // CHECK3-NEXT: [[THIS1:%.*]] = load ptr, ptr [[THIS_ADDR]], align 8 // CHECK3-NEXT: [[TMP0:%.*]] = getelementptr inbounds i8, ptr [[THIS1]], i64 4 -// CHECK3-NEXT: [[F:%.*]] = getelementptr inbounds nuw [[STRUCT_S_0:%.*]], ptr [[THIS1]], i32 0, i32 2 +// CHECK3-NEXT: [[F:%.*]] = getelementptr inbounds nuw [[STRUCT_S:%.*]], ptr [[THIS1]], i32 0, i32 2 // CHECK3-NEXT: [[TMP1:%.*]] = load volatile double, ptr @g, align 8 -// CHECK3-NEXT: [[CONV:%.*]] = fptrunc double [[TMP1]] to float -// CHECK3-NEXT: store float [[CONV]], ptr [[F]], align 4 +// CHECK3-NEXT: [[CONV:%.*]] = fptosi double [[TMP1]] to i32 +// CHECK3-NEXT: store i32 [[CONV]], ptr [[F]], align 4 // CHECK3-NEXT: ret void // // -// CHECK3-LABEL: define {{[^@]+}}@_ZN1SIfED2Ev +// CHECK3-LABEL: define {{[^@]+}}@_ZN1SIiED2Ev // CHECK3-SAME: (ptr noundef nonnull align 4 dead_on_return(12) dereferenceable(12) [[THIS:%.*]]) unnamed_addr #[[ATTR0]] align 2 { // CHECK3-NEXT: entry: // CHECK3-NEXT: [[THIS_ADDR:%.*]] = alloca ptr, align 8 diff --git a/clang/test/OpenMP/reduction_non_copyable_with_initializer.cpp b/clang/test/OpenMP/reduction_non_copyable_with_initializer.cpp new file mode 100644 index 00000000000000..c5926648291abc --- /dev/null +++ b/clang/test/OpenMP/reduction_non_copyable_with_initializer.cpp @@ -0,0 +1,65 @@ +// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=60 -x c++ -triple x86_64-unknown-linux-gnu -emit-llvm %s -o - | FileCheck %s +// expected-no-diagnostics + +// Simple wrapper class that requires construction before assignment +struct Wrapper { + int* ptr; + Wrapper() : ptr(new int(0)) {} + ~Wrapper() { delete ptr; } + + // Non-copyable + Wrapper(const Wrapper&) = delete; + Wrapper& operator=(const Wrapper&) = delete; + + void assign(int val) { *ptr = val; } + int get() const { return *ptr; } +}; + +struct my_struct { + int a; + int b; + Wrapper w; // Non-trivial member that needs construction + + my_struct() : a(1), b(1), w() {} + + // Non-copyable: deleted copy constructor and assignment + my_struct(const my_struct&) = delete; + my_struct& operator=(const my_struct&) = delete; +}; + +void my_init_default(my_struct& t) { + t.a = 0; + t.b = 0; + t.w.assign(0); // This requires w to be constructed first +} + +void my_add(my_struct& lhs, const my_struct& rhs) { + lhs.a += rhs.a; + lhs.b += rhs.b; +} + +static my_struct x; +my_struct y; + +// Custom reduction with user-defined initializer. +// The initializer uses operations that require the object to be properly +// constructed first (especially the non-trivial Wrapper member). +// Without emitting the default constructor first, this would crash. +#pragma omp declare reduction(my_reduction_add : my_struct : my_add(omp_out, omp_in)) \ + initializer(my_init_default(omp_priv)) + +void foo() { + #pragma omp parallel reduction(my_reduction_add:y) + my_add(y, x); +} + +// Verify that the .omp_initializer function emits the default constructor +// BEFORE calling the user-defined initializer function. This is critical for +// types with non-trivial members (like Wrapper with its pointer management). +// Without the constructor call first, the user initializer would operate on +// uninitialized memory, causing crashes. + +// CHECK-LABEL: define internal void @.omp_initializer. +// CHECK: call {{.*}} @_ZN9my_structC1Ev(ptr {{[^)]+}}) +// CHECK-NEXT: call void @_Z15my_init_defaultR9my_struct(ptr {{[^)]+}}) +// CHECK-NEXT: ret void >From d18f7a40803f0e6760b0ba5e41f0197b593a000d Mon Sep 17 00:00:00 2001 From: Zahira Ammarguellat <[email protected]> Date: Wed, 23 Sep 2026 13:54:59 -0700 Subject: [PATCH 2/2] Applied suggestion --- clang/lib/Sema/SemaOpenMP.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp index 5b647c3e25cbe1..0cde256817d5f8 100644 --- a/clang/lib/Sema/SemaOpenMP.cpp +++ b/clang/lib/Sema/SemaOpenMP.cpp @@ -25625,8 +25625,7 @@ void SemaOpenMP::ActOnOpenMPDeclareReductionInitializerEnd( if (Initializer && !DRD->getDeclContext()->isDependentContext()) { QualType ReductionType = DRD->getType(); if (CXXRecordDecl *RD = ReductionType->getAsCXXRecordDecl()) { - CXXConstructorDecl *DefaultCtor = SemaRef.LookupDefaultConstructor(RD); - if (DefaultCtor && !DefaultCtor->isDeleted() && !DefaultCtor->isTrivial()) + if (!RD->hasTrivialDefaultConstructor()) SemaRef.ActOnUninitializedDecl(OmpPrivParm); } } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
