https://github.com/zahiraam updated https://github.com/llvm/llvm-project/pull/208256
>From b925b2c3fa0f15b20898bd6456329eddfdb027ea Mon Sep 17 00:00:00 2001 From: Zahira Ammarguellat <[email protected]> Date: Wed, 20 May 2026 05:48:13 -0700 Subject: [PATCH 1/3] [OpenMP] Prevent parser infinite loop on unimplemented clauses --- clang/lib/Basic/OpenMPKinds.cpp | 1 + clang/lib/Parse/ParseOpenMP.cpp | 9 ++ clang/lib/Sema/SemaOpenMP.cpp | 4 + .../OpenMP/unimplemented_clause_messages.cpp | 93 +++++++++++++++++++ 4 files changed, 107 insertions(+) create mode 100644 clang/test/OpenMP/unimplemented_clause_messages.cpp diff --git a/clang/lib/Basic/OpenMPKinds.cpp b/clang/lib/Basic/OpenMPKinds.cpp index 287eb217ba458..675d86349c933 100644 --- a/clang/lib/Basic/OpenMPKinds.cpp +++ b/clang/lib/Basic/OpenMPKinds.cpp @@ -965,6 +965,7 @@ void clang::getOpenMPCaptureRegions( case OMPD_simd: case OMPD_single: case OMPD_target_data: + case OMPD_taskgraph: case OMPD_taskgroup: case OMPD_stripe: // These directives (when standalone) use OMPD_unknown as the region, diff --git a/clang/lib/Parse/ParseOpenMP.cpp b/clang/lib/Parse/ParseOpenMP.cpp index 45a47ec797f01..ba3d3113700ff 100644 --- a/clang/lib/Parse/ParseOpenMP.cpp +++ b/clang/lib/Parse/ParseOpenMP.cpp @@ -2388,6 +2388,8 @@ StmtResult Parser::ParseOpenMPExecutableDirective( ImplicitClauseAllowed = false; Actions.OpenMP().StartOpenMPClause(CKind); HasImplicitClause = false; + SourceLocation ClauseLoc = Tok.getLocation(); + OMPClause *Clause = ParseOpenMPClause(DKind, CKind, !SeenClauses[unsigned(CKind)]); SeenClauses[unsigned(CKind)] = true; @@ -2398,6 +2400,13 @@ StmtResult Parser::ParseOpenMPExecutableDirective( if (Tok.is(tok::comma)) ConsumeToken(); Actions.OpenMP().EndOpenMPClause(); + + // If ParseOpenMPClause returned without consuming any tokens, skip + // to end to avoid an infinite loop. + if (Tok.getLocation() == ClauseLoc) { + skipUntilPragmaOpenMPEnd(DKind); + break; + } } // End location of the directive. EndLoc = Tok.getLocation(); diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp index d6f6bc919a31b..76b40a5039180 100644 --- a/clang/lib/Sema/SemaOpenMP.cpp +++ b/clang/lib/Sema/SemaOpenMP.cpp @@ -6778,6 +6778,10 @@ StmtResult SemaOpenMP::ActOnOpenMPExecutableDirective( case OMPD_begin_declare_variant: case OMPD_end_declare_variant: llvm_unreachable("OpenMP Directive is not allowed"); + case OMPD_taskgraph: + Diag(StartLoc, diag::err_omp_unexpected_directive) + << 1 << getOpenMPDirectiveName(OMPD_taskgraph); + return StmtError(); case OMPD_unknown: default: llvm_unreachable("Unknown OpenMP directive"); diff --git a/clang/test/OpenMP/unimplemented_clause_messages.cpp b/clang/test/OpenMP/unimplemented_clause_messages.cpp new file mode 100644 index 0000000000000..172203ea5d040 --- /dev/null +++ b/clang/test/OpenMP/unimplemented_clause_messages.cpp @@ -0,0 +1,93 @@ +// RUN: %clang_cc1 -verify=expected,omp60 -fopenmp -fopenmp-version=60 %s +// RUN: %clang_cc1 -verify=expected,omp51 -fopenmp -fopenmp-version=51 %s +// RUN: %clang_cc1 -verify=expected,omp60 -fopenmp-simd -fopenmp-version=60 %s +// RUN: %clang_cc1 -verify=expected,omp51 -fopenmp-simd -fopenmp-version=51 %s + + +void test_induction_basic() { + int i; + // omp60-warning@+4{{extra tokens at the end of '#pragma omp parallel for' are ignored}} + // omp60-error@+3{{unexpected OpenMP clause 'induction' in directive '#pragma omp parallel for'}} + // omp51-warning@+2{{extra tokens at the end of '#pragma omp parallel for' are ignored}} + // omp51-error@+1{{unexpected OpenMP clause 'induction' in directive '#pragma omp parallel for'}} +#pragma omp parallel for induction(i) + for (i = 0; i < 10; ++i) + ; +} + +void test_apply() { + // omp60-warning@+4{{extra tokens at the end of '#pragma omp tile' are ignored}} + // omp60-error@+3{{unexpected OpenMP clause 'apply' in directive '#pragma omp tile'}} + // omp51-error@+2{{unexpected OpenMP clause 'apply' in directive '#pragma omp tile'}} + // omp51-warning@+1{{extra tokens at the end of '#pragma omp tile' are ignored}} +#pragma omp tile sizes(10) apply(intratile: unroll) + for (int i = 0; i < 10; ++i) + ; +} + +void test_empty_apply() { + // omp60-warning@+4{{extra tokens at the end of '#pragma omp tile' are ignored}} + // omp60-error@+3{{unexpected OpenMP clause 'apply' in directive '#pragma omp tile'}} + // omp51-error@+2{{unexpected OpenMP clause 'apply' in directive '#pragma omp tile'}} + // omp51-warning@+1{{extra tokens at the end of '#pragma omp tile' are ignored}} +#pragma omp tile sizes(10) apply() + for (int i = 0; i < 10; ++i) + ; +} + +void test_nested_apply() +{ + // omp60-error@+5{{unexpected OpenMP clause 'apply' in directive '#pragma omp tile'}} + // omp60-warning@+4{{extra tokens at the end of '#pragma omp tile' are ignored}} + //omp51-error@+3{{unexpected OpenMP clause 'apply' in directive '#pragma omp tile'}} + // omp51-warning@+2{{extra tokens at the end of '#pragma omp tile' are ignored}} +#pragma omp tile sizes(10) \ + apply(intratile: unroll partial(2) apply(reverse)) + for (int i = 0; i < 100; ++i) + ; +} + +void test_induction_with_following_clause() { + int i; + // omp60-warning@+4{{extra tokens at the end of '#pragma omp parallel for' are ignored}} + // omp60-error@+3{{unexpected OpenMP clause 'induction' in directive '#pragma omp parallel for'}} + // omp51-error@+2{{unexpected OpenMP clause 'induction' in directive '#pragma omp parallel for'}} + // omp51-warning@+1{{extra tokens at the end of '#pragma omp parallel for' are ignored}} +#pragma omp parallel for induction(i) num_threads(4) + for (i = 0; i < 10; ++i) + ; +} + +class Point { + float x, y, m; + char color; + +}; + +void processPointsInLine() { + float separation; + // omp60-error@+4{{unexpected OpenMP clause 'induction' in directive '#pragma omp parallel for'}} + // omp60-warning@+3{{extra tokens at the end of '#pragma omp parallel for' are ignored}} + // omp51-error@+2{{unexpected OpenMP clause 'induction' in directive '#pragma omp parallel for'}} + // omp51-warning@+1{{extra tokens at the end of '#pragma omp parallel for' are ignored}} +#pragma omp parallel for induction(step(Separation)) + for (int i = 0; i < 10; ++i) { + ; + } +} + +// Make sure test doesn't crash. +void test_tasgraph() +{ + // omp60-error@+2{{unexpected OpenMP directive '#pragma omp taskgraph'}} + // omp51-error@+1{{unexpected OpenMP directive '#pragma omp taskgraph'}} +#pragma omp taskgraph + for (int i = 0; i < 10; ++i) + ; +} + +void test_implemented_clause() { +#pragma omp tile sizes(10) + for (int i = 0; i < 10; ++i) + ; +} >From 51b5dd59154f0213e4052d9a8cb7365c7afde0fd Mon Sep 17 00:00:00 2001 From: Ammarguellat <[email protected]> Date: Wed, 8 Jul 2026 09:24:56 -0700 Subject: [PATCH 2/3] [Clang] Fix x86_fp80 misalignment with pragma pack on Windows --- clang/lib/AST/RecordLayoutBuilder.cpp | 27 ++++++++- clang/test/CodeGen/x86_fp80-alignment-win.cpp | 58 +++++++++++++++++++ 2 files changed, 82 insertions(+), 3 deletions(-) create mode 100644 clang/test/CodeGen/x86_fp80-alignment-win.cpp diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp index 854f88f20b2b6..aa9161f5dccea 100644 --- a/clang/lib/AST/RecordLayoutBuilder.cpp +++ b/clang/lib/AST/RecordLayoutBuilder.cpp @@ -2693,6 +2693,25 @@ MicrosoftRecordLayoutBuilder::getAdjustedElementInfo( auto TInfo = Context.getTypeInfoInChars(FD->getType()->getUnqualifiedDesugaredType()); ElementInfo Info{TInfo.Width, TInfo.Align}; + // Types like x86_fp80 (long double with /Qlong-double) require 16-byte + // (or greater) for correctness. + bool ContainsX86FP80 = false; + std::function<bool(QualType)> CheckForX86FP80 = [&](QualType Ty) -> bool { + const Type *BaseType = Ty->getBaseElementTypeUnsafe(); + if (const auto *BT = dyn_cast<BuiltinType>(BaseType)) { + return BT->getKind() == BuiltinType::LongDouble && + &Context.getTargetInfo().getLongDoubleFormat() == + &llvm::APFloat::x87DoubleExtended(); + } + if (const auto *RT = dyn_cast<RecordType>(BaseType)) { + for (const auto *Field : RT->getDecl()->fields()) { + if (CheckForX86FP80(Field->getType())) + return true; + } + } + return false; + }; + ContainsX86FP80 = CheckForX86FP80(FD->getType()); // Respect align attributes on the field. CharUnits FieldRequiredAlignment = Context.toCharUnitsFromBits(FD->getMaxAlignment()); @@ -2717,10 +2736,12 @@ MicrosoftRecordLayoutBuilder::getAdjustedElementInfo( // Capture required alignment as a side-effect. RequiredAlignment = std::max(RequiredAlignment, FieldRequiredAlignment); } - // Respect pragma pack, attribute pack and declspec align - if (!MaxFieldAlignment.isZero()) + // Respect pragma pack, attribute pack and declspec align, but not for types + // that require specific alignment for correctness (e.g., x86_fp80 needs + // 16-byte alignment for movaps instructions). + if (!MaxFieldAlignment.isZero() && !ContainsX86FP80) Info.Alignment = std::min(Info.Alignment, MaxFieldAlignment); - if (FD->hasAttr<PackedAttr>()) + if (FD->hasAttr<PackedAttr>() && !ContainsX86FP80) Info.Alignment = CharUnits::One(); Info.Alignment = std::max(Info.Alignment, FieldRequiredAlignment); return Info; diff --git a/clang/test/CodeGen/x86_fp80-alignment-win.cpp b/clang/test/CodeGen/x86_fp80-alignment-win.cpp new file mode 100644 index 0000000000000..f446474c598be --- /dev/null +++ b/clang/test/CodeGen/x86_fp80-alignment-win.cpp @@ -0,0 +1,58 @@ +// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -mlong-double-80 -emit-llvm -o - %s | FileCheck %s + +// Test that x86_fp80 (long double with /Qlong-double flag) maintains +// 16-byte alignment for correctness (required by movaps instructions), +// even when #pragma pack would normally reduce it. + +struct Klass { + long double a; +}; + +// Simulate std::array without including headers +template<typename T, unsigned N> +struct array { + T _Elems[N]; +}; + +// CHECK-LABEL: define {{.*}} @{{.*}}test_single_klass +// CHECK: %k = alloca %struct.Klass, align 16 +// CHECK-NOT: align 8 +// CHECK: store x86_fp80 {{.*}}, ptr {{.*}}, align 16 +void test_single_klass() { + Klass k; + k.a = 0.0L; +} + +// CHECK-LABEL: define {{.*}} @{{.*}}test_struct_array +// CHECK: %matrix = alloca %struct.array, align 16 +// CHECK-NOT: align 8 +// CHECK: store x86_fp80 {{.*}}, ptr {{.*}}, align 16 +void test_struct_array() { + array<Klass, 16> matrix; + for (int i = 0; i < 16; i++) + matrix._Elems[i].a = 0.0L; +} + +// CHECK-LABEL: define {{.*}} @{{.*}}test_direct_array +// CHECK: %arr = alloca [16 x %struct.Klass], align 16 +void test_direct_array() { + Klass arr[16]; + for (int i = 0; i < 16; i++) + arr[i].a = 0.0L; +} + +// Test with explicit pragma pack(8) - should still maintain 16-byte alignment +#pragma pack(push, 8) +struct PackedKlass { + long double b; +}; + +// CHECK-LABEL: define {{.*}} @{{.*}}test_explicit_pack +// CHECK: %pk = alloca %struct.PackedKlass, align 16 +// CHECK-NOT: align 8 +// CHECK: store x86_fp80 {{.*}}, ptr {{.*}}, align 16 +void test_explicit_pack() { + PackedKlass pk; + pk.b = 0.0L; +} +#pragma pack(pop) >From 6f8a42b0e9bc5aaf0e33b185751d9e9acd9d06c2 Mon Sep 17 00:00:00 2001 From: Ammarguellat <[email protected]> Date: Fri, 10 Jul 2026 07:26:10 -0700 Subject: [PATCH 3/3] Leveraged FieldRequiredAlignment --- clang/lib/AST/RecordLayoutBuilder.cpp | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/clang/lib/AST/RecordLayoutBuilder.cpp b/clang/lib/AST/RecordLayoutBuilder.cpp index aa9161f5dccea..f0d9caf3c2469 100644 --- a/clang/lib/AST/RecordLayoutBuilder.cpp +++ b/clang/lib/AST/RecordLayoutBuilder.cpp @@ -2693,9 +2693,12 @@ MicrosoftRecordLayoutBuilder::getAdjustedElementInfo( auto TInfo = Context.getTypeInfoInChars(FD->getType()->getUnqualifiedDesugaredType()); ElementInfo Info{TInfo.Width, TInfo.Align}; + // Respect align attributes on the field. + CharUnits FieldRequiredAlignment = + Context.toCharUnitsFromBits(FD->getMaxAlignment()); + // Types like x86_fp80 (long double with /Qlong-double) require 16-byte // (or greater) for correctness. - bool ContainsX86FP80 = false; std::function<bool(QualType)> CheckForX86FP80 = [&](QualType Ty) -> bool { const Type *BaseType = Ty->getBaseElementTypeUnsafe(); if (const auto *BT = dyn_cast<BuiltinType>(BaseType)) { @@ -2711,10 +2714,10 @@ MicrosoftRecordLayoutBuilder::getAdjustedElementInfo( } return false; }; - ContainsX86FP80 = CheckForX86FP80(FD->getType()); - // Respect align attributes on the field. - CharUnits FieldRequiredAlignment = - Context.toCharUnitsFromBits(FD->getMaxAlignment()); + if (CheckForX86FP80(FD->getType())) + FieldRequiredAlignment = + std::max(FieldRequiredAlignment, CharUnits::fromQuantity(16)); + // Respect align attributes on the type. if (Context.isAlignmentRequired(FD->getType())) FieldRequiredAlignment = std::max( @@ -2739,9 +2742,9 @@ MicrosoftRecordLayoutBuilder::getAdjustedElementInfo( // Respect pragma pack, attribute pack and declspec align, but not for types // that require specific alignment for correctness (e.g., x86_fp80 needs // 16-byte alignment for movaps instructions). - if (!MaxFieldAlignment.isZero() && !ContainsX86FP80) + if (!MaxFieldAlignment.isZero()) Info.Alignment = std::min(Info.Alignment, MaxFieldAlignment); - if (FD->hasAttr<PackedAttr>() && !ContainsX86FP80) + if (FD->hasAttr<PackedAttr>()) Info.Alignment = CharUnits::One(); Info.Alignment = std::max(Info.Alignment, FieldRequiredAlignment); return Info; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
