https://github.com/keinflue created https://github.com/llvm/llvm-project/pull/173747
For X86 and RISCV checking of target_clones attribute arguments attempted to use the location of the first argument for diagnosing a missing default argument. However, if the argument list is empty, then this location doesn't exist and causes an assertion. This commit passes the location of the attribute itself to the target-specific validation function in the case of X86 and RISCV in order to provide a usable location for this diagnostic. Fixes #173684 --- I am not sure whether this is intentional, but for AArch64 the validation does not emit a diagnostic for missing `"default"` argument. Therefore the issue did not appear there and I did not make any changes to it. This is the only other target besides X86 and RISCV that supports `target_clones`. >From a80cab38ad1e1738bd5bad55a2d2ccfbdeff39f2 Mon Sep 17 00:00:00 2001 From: keinflue <[email protected]> Date: Sun, 28 Dec 2025 00:56:21 +0100 Subject: [PATCH] [Clang] Use valid source loc for empty target_clones diagnostic For X86 and RISCV checking of target_clones attribute arguments attempted to use the location of the first argument for diagnosing a missing default argument. However, if the argument list is empty, then this location doesn't exist and causes an assertion. This commit passes the location of the attribute itself to the target-specific validation function in the case of X86 and RISCV in order to provide a usable location for this diagnostic. Fixes #173684 --- clang/include/clang/Sema/SemaRISCV.h | 7 ++++--- clang/include/clang/Sema/SemaX86.h | 7 ++++--- clang/lib/Sema/SemaDeclAttr.cpp | 6 ++++-- clang/lib/Sema/SemaRISCV.cpp | 8 +++++--- clang/lib/Sema/SemaX86.cpp | 9 +++++---- clang/test/Sema/attr-target-clones.c | 8 ++++++++ clang/test/SemaCXX/attr-target-clones-riscv.cpp | 7 +++++++ 7 files changed, 37 insertions(+), 15 deletions(-) diff --git a/clang/include/clang/Sema/SemaRISCV.h b/clang/include/clang/Sema/SemaRISCV.h index 844cc3ce4a440..ccf38d6517554 100644 --- a/clang/include/clang/Sema/SemaRISCV.h +++ b/clang/include/clang/Sema/SemaRISCV.h @@ -57,9 +57,10 @@ class SemaRISCV : public SemaBase { std::unique_ptr<sema::RISCVIntrinsicManager> IntrinsicManager; bool checkTargetVersionAttr(const StringRef Param, const SourceLocation Loc); - bool checkTargetClonesAttr(SmallVectorImpl<StringRef> &Params, - SmallVectorImpl<SourceLocation> &Locs, - SmallVectorImpl<SmallString<64>> &NewParams); + bool checkTargetClonesAttr(const SmallVectorImpl<StringRef> &Params, + const SmallVectorImpl<SourceLocation> &Locs, + SmallVectorImpl<SmallString<64>> &NewParams, + const SourceLocation &AttrLoc); }; std::unique_ptr<sema::RISCVIntrinsicManager> diff --git a/clang/include/clang/Sema/SemaX86.h b/clang/include/clang/Sema/SemaX86.h index 20783e344c02f..3c636305cdf29 100644 --- a/clang/include/clang/Sema/SemaX86.h +++ b/clang/include/clang/Sema/SemaX86.h @@ -38,9 +38,10 @@ class SemaX86 : public SemaBase { void handleAnyInterruptAttr(Decl *D, const ParsedAttr &AL); void handleForceAlignArgPointerAttr(Decl *D, const ParsedAttr &AL); - bool checkTargetClonesAttr(SmallVectorImpl<StringRef> &Params, - SmallVectorImpl<SourceLocation> &Locs, - SmallVectorImpl<SmallString<64>> &NewParams); + bool checkTargetClonesAttr(const SmallVectorImpl<StringRef> &Params, + const SmallVectorImpl<SourceLocation> &Locs, + SmallVectorImpl<SmallString<64>> &NewParams, + const SourceLocation &AttrLoc); }; } // namespace clang diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 3107876565e8e..d1b766bd1d65d 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -3438,10 +3438,12 @@ static void handleTargetClonesAttr(Sema &S, Decl *D, const ParsedAttr &AL) { if (S.ARM().checkTargetClonesAttr(Params, Locations, NewParams)) return; } else if (S.Context.getTargetInfo().getTriple().isRISCV()) { - if (S.RISCV().checkTargetClonesAttr(Params, Locations, NewParams)) + if (S.RISCV().checkTargetClonesAttr(Params, Locations, NewParams, + AL.getLoc())) return; } else if (S.Context.getTargetInfo().getTriple().isX86()) { - if (S.X86().checkTargetClonesAttr(Params, Locations, NewParams)) + if (S.X86().checkTargetClonesAttr(Params, Locations, NewParams, + AL.getLoc())) return; } Params.clear(); diff --git a/clang/lib/Sema/SemaRISCV.cpp b/clang/lib/Sema/SemaRISCV.cpp index 3ba93ff98898b..1e6280c04e346 100644 --- a/clang/lib/Sema/SemaRISCV.cpp +++ b/clang/lib/Sema/SemaRISCV.cpp @@ -1714,8 +1714,10 @@ bool SemaRISCV::checkTargetVersionAttr(const StringRef Param, } bool SemaRISCV::checkTargetClonesAttr( - SmallVectorImpl<StringRef> &Params, SmallVectorImpl<SourceLocation> &Locs, - SmallVectorImpl<SmallString<64>> &NewParams) { + const SmallVectorImpl<StringRef> &Params, + const SmallVectorImpl<SourceLocation> &Locs, + SmallVectorImpl<SmallString<64>> &NewParams, + const SourceLocation &AttrLoc) { using namespace DiagAttrParams; assert(Params.size() == Locs.size() && @@ -1768,7 +1770,7 @@ bool SemaRISCV::checkTargetClonesAttr( NewParams.push_back(Param); } if (!HasDefault) - return Diag(Locs[0], diag::err_target_clone_must_have_default); + return Diag(AttrLoc, diag::err_target_clone_must_have_default); return false; } diff --git a/clang/lib/Sema/SemaX86.cpp b/clang/lib/Sema/SemaX86.cpp index 850bcb17bece1..7bce88c0976a2 100644 --- a/clang/lib/Sema/SemaX86.cpp +++ b/clang/lib/Sema/SemaX86.cpp @@ -1061,9 +1061,10 @@ void SemaX86::handleForceAlignArgPointerAttr(Decl *D, const ParsedAttr &AL) { X86ForceAlignArgPointerAttr(getASTContext(), AL)); } -bool SemaX86::checkTargetClonesAttr( - SmallVectorImpl<StringRef> &Params, SmallVectorImpl<SourceLocation> &Locs, - SmallVectorImpl<SmallString<64>> &NewParams) { +bool SemaX86::checkTargetClonesAttr(const SmallVectorImpl<StringRef> &Params, + const SmallVectorImpl<SourceLocation> &Locs, + SmallVectorImpl<SmallString<64>> &NewParams, + const SourceLocation &AttrLoc) { using namespace DiagAttrParams; assert(Params.size() == Locs.size() && @@ -1113,7 +1114,7 @@ bool SemaX86::checkTargetClonesAttr( Diag(Locs[0], diag::warn_target_clone_mixed_values); if (!HasDefault) - return Diag(Locs[0], diag::err_target_clone_must_have_default); + return Diag(AttrLoc, diag::err_target_clone_must_have_default); return false; } diff --git a/clang/test/Sema/attr-target-clones.c b/clang/test/Sema/attr-target-clones.c index 4597ea54d02bf..4d720e1d93ff4 100644 --- a/clang/test/Sema/attr-target-clones.c +++ b/clang/test/Sema/attr-target-clones.c @@ -125,3 +125,11 @@ void bad_isa_level(int) __attribute__((target_clones("default", "arch=x86-64-v5" // expected-warning@+1 {{unsupported 'sha' in the 'target_clones' attribute string; 'target_clones' attribute ignored}} void bad_feature(void) __attribute__((target_clones("default", "sse4.2", "sha"))); + +// expected-error@+1 {{'target_clones' multiversioning requires a default target}} +void __attribute__((target_clones())) +gh173684_empty_attribute_args(void); + +// expected-error@+1 {{'target_clones' multiversioning requires a default target}} +void __attribute__((target_clones)) +gh173684_empty_attribute_args_2(void); diff --git a/clang/test/SemaCXX/attr-target-clones-riscv.cpp b/clang/test/SemaCXX/attr-target-clones-riscv.cpp index 7648284f80c48..f75ef8c8aad50 100644 --- a/clang/test/SemaCXX/attr-target-clones-riscv.cpp +++ b/clang/test/SemaCXX/attr-target-clones-riscv.cpp @@ -51,3 +51,10 @@ void lambda() { auto y = []() __attribute__((target_clones("arch=+v", "default"))){}; y(); } + +namespace GH173684 { + // expected-error@+1 {{'target_clones' multiversioning requires a default target}} + void __attribute__((target_clones())) withoutDefault() {} + // expected-error@+1 {{'target_clones' multiversioning requires a default target}} + void __attribute__((target_clones)) withoutDefault2() {} +} _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
