https://github.com/a-tarasyuk updated https://github.com/llvm/llvm-project/pull/147163
>From 4e0cf4e00d4cfd837e9dfd9e6aed88aca1de295a Mon Sep 17 00:00:00 2001 From: Oleksandr Tarasiuk <oleksandr.taras...@outlook.com> Date: Sun, 6 Jul 2025 00:35:48 +0300 Subject: [PATCH 1/2] [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 <oleksandr.taras...@outlook.com> Date: Fri, 11 Jul 2025 15:06:17 +0300 Subject: [PATCH 2/2] 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<FunctionDecl>(C) - ? cast<FunctionDecl>(C) - : dyn_cast_or_null<FunctionDecl>(D); + FunctionDecl *FD = isa<FunctionDecl>(C) ? cast<FunctionDecl>(C) + : dyn_cast_or_null<FunctionDecl>(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") { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits