From 4e0cf4e00d4cfd837e9dfd9e6aed88aca1de295a Mon Sep 17 00:00:00 2001 From: Oleksandr Tarasiuk Date: Sun, 6 Jul 2025 00:35:48 +0300 Subject: [PATCH 1/5] [Clang] fix crash in codegen caused by deferred asm diagnostics under -fopenmp --- clang/docs/ReleaseNotes.rst | 2 ++ clang/lib/Sema/SemaStmtAsm.cpp | 34 ++++++++++++++-------------------- clang/test/OpenMP/openmp_asm.c | 28 ++++++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 20 deletions(-) create mode 100644 clang/test/OpenMP/openmp_asm.c diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 9a94c4bcd9980..dcf2ffe43edfd 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -750,6 +750,8 @@ Bug Fixes in This Version - Fixed an infinite recursion when checking constexpr destructors. (#GH141789) - Fixed a crash when a malformed using declaration appears in a ``constexpr`` function. (#GH144264) - Fixed a bug when use unicode character name in macro concatenation. (#GH145240) +- Fixed a crash caused by deferred diagnostics under ``-fopenmp``, + which resulted in passing invalid asm statements to codegen. (#GH140375) Bug Fixes to Compiler Builtins ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/lib/Sema/SemaStmtAsm.cpp b/clang/lib/Sema/SemaStmtAsm.cpp index 4507a21a4c111..b949178f6a938 100644 --- a/clang/lib/Sema/SemaStmtAsm.cpp +++ b/clang/lib/Sema/SemaStmtAsm.cpp @@ -309,10 +309,9 @@ StmtResult Sema::ActOnGCCAsmStmt(SourceLocation AsmLoc, bool IsSimple, TargetInfo::ConstraintInfo Info(ConstraintStr, OutputName); if (!Context.getTargetInfo().validateOutputConstraint(Info) && !(LangOpts.HIPStdPar && LangOpts.CUDAIsDevice)) { - targetDiag(Constraint->getBeginLoc(), - diag::err_asm_invalid_output_constraint) - << Info.getConstraintStr(); - return CreateGCCAsmStmt(); + return StmtError(targetDiag(Constraint->getBeginLoc(), + diag::err_asm_invalid_output_constraint) + << Info.getConstraintStr()); } ExprResult ER = CheckPlaceholderExpr(Exprs[i]); @@ -378,9 +377,9 @@ StmtResult Sema::ActOnGCCAsmStmt(SourceLocation AsmLoc, bool IsSimple, FeatureMap, GCCAsmStmt::ExtractStringFromGCCAsmStmtComponent(Constraint), Size)) { - targetDiag(OutputExpr->getBeginLoc(), diag::err_asm_invalid_output_size) - << Info.getConstraintStr(); - return CreateGCCAsmStmt(); + return StmtError(targetDiag(OutputExpr->getBeginLoc(), + diag::err_asm_invalid_output_size) + << Info.getConstraintStr()); } } @@ -399,10 +398,9 @@ StmtResult Sema::ActOnGCCAsmStmt(SourceLocation AsmLoc, bool IsSimple, TargetInfo::ConstraintInfo Info(ConstraintStr, InputName); if (!Context.getTargetInfo().validateInputConstraint(OutputConstraintInfos, Info)) { - targetDiag(Constraint->getBeginLoc(), - diag::err_asm_invalid_input_constraint) - << Info.getConstraintStr(); - return CreateGCCAsmStmt(); + return StmtError(targetDiag(Constraint->getBeginLoc(), + diag::err_asm_invalid_input_constraint) + << Info.getConstraintStr()); } ExprResult ER = CheckPlaceholderExpr(Exprs[i]); @@ -504,13 +502,9 @@ StmtResult Sema::ActOnGCCAsmStmt(SourceLocation AsmLoc, bool IsSimple, GCCAsmStmt::ExtractStringFromGCCAsmStmtComponent(ClobberExpr); if (!Context.getTargetInfo().isValidClobber(Clobber)) { - targetDiag(ClobberExpr->getBeginLoc(), - diag::err_asm_unknown_register_name) - << Clobber; - return new (Context) GCCAsmStmt( - Context, AsmLoc, IsSimple, IsVolatile, NumOutputs, NumInputs, Names, - constraints.data(), Exprs.data(), asmString, NumClobbers, - clobbers.data(), NumLabels, RParenLoc); + return StmtError(targetDiag(ClobberExpr->getBeginLoc(), + diag::err_asm_unknown_register_name) + << Clobber); } if (Clobber == "unwind") { @@ -520,8 +514,8 @@ StmtResult Sema::ActOnGCCAsmStmt(SourceLocation AsmLoc, bool IsSimple, // Using unwind clobber and asm-goto together is not supported right now. if (UnwindClobberLoc && NumLabels > 0) { - targetDiag(*UnwindClobberLoc, diag::err_asm_unwind_and_goto); - return CreateGCCAsmStmt(); + return StmtError( + targetDiag(*UnwindClobberLoc, diag::err_asm_unwind_and_goto)); } GCCAsmStmt *NS = CreateGCCAsmStmt(); diff --git a/clang/test/OpenMP/openmp_asm.c b/clang/test/OpenMP/openmp_asm.c new file mode 100644 index 0000000000000..f2705d1a8803f --- /dev/null +++ b/clang/test/OpenMP/openmp_asm.c @@ -0,0 +1,28 @@ +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fopenmp -verify=fopenmp -emit-llvm -o - %s +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -verify -emit-llvm -o - %s + +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -x c++ -fopenmp -verify=fopenmp -emit-llvm -o - %s +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -x c++ -verify -emit-llvm -o - %s + +// fopenmp-no-diagnostics + +void t1(int *a, int *b) { + asm volatile("" : "+&r"(a) : ""(b)); // expected-error {{invalid input constraint '' in asm}} +} + +void t2() { + asm ("nop" : : : "foo"); // expected-error {{unknown register name 'foo' in asm}} +} + +void t3() { + asm goto ("" ::: "unwind" : label); // expected-error {{unwind clobber cannot be used with asm goto}} +label: + ; +} + +typedef int vec256 __attribute__((ext_vector_type(8))); +vec256 t4() { + vec256 out; + asm("something %0" : "=y"(out)); // expected-error {{invalid output size for constraint '=y'}} + return out; +} From 540d1793d4b604a7c7790642040ff33ec2ad3e04 Mon Sep 17 00:00:00 2001 From: Oleksandr Tarasiuk Date: Fri, 11 Jul 2025 15:06:17 +0300 Subject: [PATCH 2/5] skip the emitting of host functions with deferred diagnostics --- clang/include/clang/Sema/Sema.h | 4 +-- clang/include/clang/Sema/SemaBase.h | 1 + clang/lib/CodeGen/ModuleBuilder.cpp | 6 ++++- clang/lib/Sema/Sema.cpp | 42 ++++++++++++++++++----------- clang/lib/Sema/SemaStmtAsm.cpp | 30 ++++++++++++--------- 5 files changed, 53 insertions(+), 30 deletions(-) diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index b281b1cfef96a..496fb54ce0759 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -1110,10 +1110,10 @@ class Sema final : public SemaBase { } SemaDiagnosticBuilder targetDiag(SourceLocation Loc, unsigned DiagID, - const FunctionDecl *FD = nullptr); + FunctionDecl *FD = nullptr); SemaDiagnosticBuilder targetDiag(SourceLocation Loc, const PartialDiagnostic &PD, - const FunctionDecl *FD = nullptr) { + FunctionDecl *FD = nullptr) { return targetDiag(Loc, PD.getDiagID(), FD) << PD; } diff --git a/clang/include/clang/Sema/SemaBase.h b/clang/include/clang/Sema/SemaBase.h index 550f530af72f5..c24db4d012413 100644 --- a/clang/include/clang/Sema/SemaBase.h +++ b/clang/include/clang/Sema/SemaBase.h @@ -138,6 +138,7 @@ class SemaBase { ~SemaDiagnosticBuilder(); bool isImmediate() const { return ImmediateDiag.has_value(); } + bool isDeferred() const { return PartialDiagId.has_value(); } /// Convertible to bool: True if we immediately emitted an error, false if /// we didn't emit an error or we created a deferred error. diff --git a/clang/lib/CodeGen/ModuleBuilder.cpp b/clang/lib/CodeGen/ModuleBuilder.cpp index 8c1fee8c974f1..787ab30e0c92c 100644 --- a/clang/lib/CodeGen/ModuleBuilder.cpp +++ b/clang/lib/CodeGen/ModuleBuilder.cpp @@ -186,8 +186,12 @@ namespace { HandlingTopLevelDeclRAII HandlingDecl(*this); // Make sure to emit all elements of a Decl. - for (auto &I : DG) + for (auto &I : DG) { + if (I->isInvalidDecl()) { + continue; + } Builder->EmitTopLevelDecl(I); + } return true; } diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp index 56608e990fd50..d20c921d05fdd 100644 --- a/clang/lib/Sema/Sema.cpp +++ b/clang/lib/Sema/Sema.cpp @@ -2092,21 +2092,34 @@ Sema::SemaDiagnosticBuilder::~SemaDiagnosticBuilder() { } Sema::SemaDiagnosticBuilder -Sema::targetDiag(SourceLocation Loc, unsigned DiagID, const FunctionDecl *FD) { +Sema::targetDiag(SourceLocation Loc, unsigned DiagID, FunctionDecl *FD) { FD = FD ? FD : getCurFunctionDecl(); - if (LangOpts.OpenMP) - return LangOpts.OpenMPIsTargetDevice - ? OpenMP().diagIfOpenMPDeviceCode(Loc, DiagID, FD) - : OpenMP().diagIfOpenMPHostCode(Loc, DiagID, FD); - if (getLangOpts().CUDA) - return getLangOpts().CUDAIsDevice ? CUDA().DiagIfDeviceCode(Loc, DiagID) - : CUDA().DiagIfHostCode(Loc, DiagID); - if (getLangOpts().SYCLIsDevice) - return SYCL().DiagIfDeviceCode(Loc, DiagID); + SemaDiagnosticBuilder SDB = [&]() -> SemaDiagnosticBuilder { + if (LangOpts.OpenMP) { + return LangOpts.OpenMPIsTargetDevice + ? OpenMP().diagIfOpenMPDeviceCode(Loc, DiagID, FD) + : OpenMP().diagIfOpenMPHostCode(Loc, DiagID, FD); + } + + if (getLangOpts().CUDA) { + return getLangOpts().CUDAIsDevice ? CUDA().DiagIfDeviceCode(Loc, DiagID) + : CUDA().DiagIfHostCode(Loc, DiagID); + } + + if (getLangOpts().SYCLIsDevice) { + return SYCL().DiagIfDeviceCode(Loc, DiagID); + } + + return SemaDiagnosticBuilder(SemaDiagnosticBuilder::K_Immediate, Loc, + DiagID, FD, *this); + }(); + + if (SDB.isDeferred()) { + FD->setInvalidDecl(); + } - return SemaDiagnosticBuilder(SemaDiagnosticBuilder::K_Immediate, Loc, DiagID, - FD, *this); + return SDB; } void Sema::checkTypeSupport(QualType Ty, SourceLocation Loc, ValueDecl *D) { @@ -2138,9 +2151,8 @@ void Sema::checkTypeSupport(QualType Ty, SourceLocation Loc, ValueDecl *D) { // Try to associate errors with the lexical context, if that is a function, or // the value declaration otherwise. - const FunctionDecl *FD = isa(C) - ? cast(C) - : dyn_cast_or_null(D); + FunctionDecl *FD = isa(C) ? cast(C) + : dyn_cast_or_null(D); auto CheckDeviceType = [&](QualType Ty) { if (Ty->isDependentType()) diff --git a/clang/lib/Sema/SemaStmtAsm.cpp b/clang/lib/Sema/SemaStmtAsm.cpp index b949178f6a938..724c53af16540 100644 --- a/clang/lib/Sema/SemaStmtAsm.cpp +++ b/clang/lib/Sema/SemaStmtAsm.cpp @@ -309,9 +309,10 @@ StmtResult Sema::ActOnGCCAsmStmt(SourceLocation AsmLoc, bool IsSimple, TargetInfo::ConstraintInfo Info(ConstraintStr, OutputName); if (!Context.getTargetInfo().validateOutputConstraint(Info) && !(LangOpts.HIPStdPar && LangOpts.CUDAIsDevice)) { - return StmtError(targetDiag(Constraint->getBeginLoc(), - diag::err_asm_invalid_output_constraint) - << Info.getConstraintStr()); + targetDiag(Constraint->getBeginLoc(), + diag::err_asm_invalid_output_constraint) + << Info.getConstraintStr(); + return CreateGCCAsmStmt(); } ExprResult ER = CheckPlaceholderExpr(Exprs[i]); @@ -377,9 +378,9 @@ StmtResult Sema::ActOnGCCAsmStmt(SourceLocation AsmLoc, bool IsSimple, FeatureMap, GCCAsmStmt::ExtractStringFromGCCAsmStmtComponent(Constraint), Size)) { - return StmtError(targetDiag(OutputExpr->getBeginLoc(), - diag::err_asm_invalid_output_size) - << Info.getConstraintStr()); + targetDiag(OutputExpr->getBeginLoc(), diag::err_asm_invalid_output_size) + << Info.getConstraintStr(); + return CreateGCCAsmStmt(); } } @@ -398,9 +399,10 @@ StmtResult Sema::ActOnGCCAsmStmt(SourceLocation AsmLoc, bool IsSimple, TargetInfo::ConstraintInfo Info(ConstraintStr, InputName); if (!Context.getTargetInfo().validateInputConstraint(OutputConstraintInfos, Info)) { - return StmtError(targetDiag(Constraint->getBeginLoc(), - diag::err_asm_invalid_input_constraint) - << Info.getConstraintStr()); + targetDiag(Constraint->getBeginLoc(), + diag::err_asm_invalid_input_constraint) + << Info.getConstraintStr(); + return CreateGCCAsmStmt(); } ExprResult ER = CheckPlaceholderExpr(Exprs[i]); @@ -502,9 +504,13 @@ StmtResult Sema::ActOnGCCAsmStmt(SourceLocation AsmLoc, bool IsSimple, GCCAsmStmt::ExtractStringFromGCCAsmStmtComponent(ClobberExpr); if (!Context.getTargetInfo().isValidClobber(Clobber)) { - return StmtError(targetDiag(ClobberExpr->getBeginLoc(), - diag::err_asm_unknown_register_name) - << Clobber); + targetDiag(ClobberExpr->getBeginLoc(), + diag::err_asm_unknown_register_name) + << Clobber; + return new (Context) GCCAsmStmt( + Context, AsmLoc, IsSimple, IsVolatile, NumOutputs, NumInputs, Names, + constraints.data(), Exprs.data(), asmString, NumClobbers, + clobbers.data(), NumLabels, RParenLoc); } if (Clobber == "unwind") { From dec2f5d7b3a8f0fdea6c2a190f3411b23607bb0e Mon Sep 17 00:00:00 2001 From: Oleksandr Tarasiuk Date: Fri, 11 Jul 2025 16:58:45 +0300 Subject: [PATCH 3/5] mark functions as invalid for deferred diagnostics, restricted to host code only --- clang/lib/Sema/Sema.cpp | 37 +++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp index d20c921d05fdd..a1843bcb4b59a 100644 --- a/clang/lib/Sema/Sema.cpp +++ b/clang/lib/Sema/Sema.cpp @@ -2095,31 +2095,36 @@ Sema::SemaDiagnosticBuilder Sema::targetDiag(SourceLocation Loc, unsigned DiagID, FunctionDecl *FD) { FD = FD ? FD : getCurFunctionDecl(); - SemaDiagnosticBuilder SDB = [&]() -> SemaDiagnosticBuilder { - if (LangOpts.OpenMP) { - return LangOpts.OpenMPIsTargetDevice - ? OpenMP().diagIfOpenMPDeviceCode(Loc, DiagID, FD) - : OpenMP().diagIfOpenMPHostCode(Loc, DiagID, FD); + if (LangOpts.OpenMP) { + if (LangOpts.OpenMPIsTargetDevice) { + return OpenMP().diagIfOpenMPDeviceCode(Loc, DiagID, FD); } - if (getLangOpts().CUDA) { - return getLangOpts().CUDAIsDevice ? CUDA().DiagIfDeviceCode(Loc, DiagID) - : CUDA().DiagIfHostCode(Loc, DiagID); + SemaDiagnosticBuilder SDB = OpenMP().diagIfOpenMPHostCode(Loc, DiagID, FD); + if (SDB.isDeferred()) { + FD->setInvalidDecl(); } + return SDB; + } - if (getLangOpts().SYCLIsDevice) { - return SYCL().DiagIfDeviceCode(Loc, DiagID); + if (getLangOpts().CUDA) { + if (getLangOpts().CUDAIsDevice) { + return CUDA().DiagIfDeviceCode(Loc, DiagID); } - return SemaDiagnosticBuilder(SemaDiagnosticBuilder::K_Immediate, Loc, - DiagID, FD, *this); - }(); + SemaDiagnosticBuilder SDB = CUDA().DiagIfHostCode(Loc, DiagID); + if (SDB.isDeferred()) { + FD->setInvalidDecl(); + } + return SDB; + } - if (SDB.isDeferred()) { - FD->setInvalidDecl(); + if (getLangOpts().SYCLIsDevice) { + return SYCL().DiagIfDeviceCode(Loc, DiagID); } - return SDB; + return SemaDiagnosticBuilder(SemaDiagnosticBuilder::K_Immediate, Loc, DiagID, + FD, *this); } void Sema::checkTypeSupport(QualType Ty, SourceLocation Loc, ValueDecl *D) { From b25f56fa220c58133a8db3610c7241e615326710 Mon Sep 17 00:00:00 2001 From: Oleksandr Tarasiuk Date: Fri, 11 Jul 2025 16:59:50 +0300 Subject: [PATCH 4/5] cleanup --- clang/lib/Sema/SemaStmtAsm.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/lib/Sema/SemaStmtAsm.cpp b/clang/lib/Sema/SemaStmtAsm.cpp index 724c53af16540..4507a21a4c111 100644 --- a/clang/lib/Sema/SemaStmtAsm.cpp +++ b/clang/lib/Sema/SemaStmtAsm.cpp @@ -520,8 +520,8 @@ StmtResult Sema::ActOnGCCAsmStmt(SourceLocation AsmLoc, bool IsSimple, // Using unwind clobber and asm-goto together is not supported right now. if (UnwindClobberLoc && NumLabels > 0) { - return StmtError( - targetDiag(*UnwindClobberLoc, diag::err_asm_unwind_and_goto)); + targetDiag(*UnwindClobberLoc, diag::err_asm_unwind_and_goto); + return CreateGCCAsmStmt(); } GCCAsmStmt *NS = CreateGCCAsmStmt(); From f64d8a1610494873dfe5f7f3b25da56625c1b88d Mon Sep 17 00:00:00 2001 From: Oleksandr Tarasiuk Date: Fri, 11 Jul 2025 17:57:45 +0300 Subject: [PATCH 5/5] cleanup --- clang/lib/Sema/Sema.cpp | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp index a1843bcb4b59a..3789af0d3d943 100644 --- a/clang/lib/Sema/Sema.cpp +++ b/clang/lib/Sema/Sema.cpp @@ -2096,32 +2096,29 @@ Sema::targetDiag(SourceLocation Loc, unsigned DiagID, FunctionDecl *FD) { FD = FD ? FD : getCurFunctionDecl(); if (LangOpts.OpenMP) { - if (LangOpts.OpenMPIsTargetDevice) { + if (LangOpts.OpenMPIsTargetDevice) return OpenMP().diagIfOpenMPDeviceCode(Loc, DiagID, FD); - } SemaDiagnosticBuilder SDB = OpenMP().diagIfOpenMPHostCode(Loc, DiagID, FD); - if (SDB.isDeferred()) { + if (SDB.isDeferred()) FD->setInvalidDecl(); - } + return SDB; } if (getLangOpts().CUDA) { - if (getLangOpts().CUDAIsDevice) { + if (getLangOpts().CUDAIsDevice) return CUDA().DiagIfDeviceCode(Loc, DiagID); - } SemaDiagnosticBuilder SDB = CUDA().DiagIfHostCode(Loc, DiagID); - if (SDB.isDeferred()) { + if (SDB.isDeferred()) FD->setInvalidDecl(); - } + return SDB; } - if (getLangOpts().SYCLIsDevice) { + if (getLangOpts().SYCLIsDevice) return SYCL().DiagIfDeviceCode(Loc, DiagID); - } return SemaDiagnosticBuilder(SemaDiagnosticBuilder::K_Immediate, Loc, DiagID, FD, *this);