llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Chen Miao (ChenMiaoi) <details> <summary>Changes</summary> Clang previously accepted both `weak` and `ifunc` attributes on the same function, but silently ignored `weak` when emitting the IFUNC. Since `ifunc` functions must not have weak linkage, the new mutual exclusion rule diagnoses the conflict during attribute processing and redeclaration merging. Using `#pragma weak` after an `ifunc` declaration adds an implicit `weak` attribute directly, bypassing the normal mutual exclusion checks. The pragma handler now checks attribute compatibility as well. If the pragma follows a redeclaration, the check uses the original IFUNC definition, because the redeclaration does not inherit the `ifunc` attribute. Link: https://github.com/llvm/llvm-project/issues/220923#issuecomment-5993507925 Fixes #<!-- -->220923 --- Full diff: https://github.com/llvm/llvm-project/pull/229393.diff 6 Files Affected: - (modified) clang/docs/ReleaseNotes.md (+3) - (modified) clang/include/clang/Basic/Attr.td (+2) - (modified) clang/lib/Sema/SemaDecl.cpp (+20-2) - (modified) clang/test/Sema/attr-weak.c (+17-1) - (modified) clang/test/Sema/pragma-weak.c (+21) - (modified) clang/test/SemaCXX/attr-weak.cpp (+7) ``````````diff diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index 7ec126a065ae5..c1b92cec96f91 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -626,6 +626,9 @@ features cannot lower the translation-unit ABI level; #### Bug Fixes to Attribute Support +- Clang now diagnoses incompatible `weak` and `ifunc` attributes, including + weak linkage introduced through redeclarations or `#pragma weak`. (#GH220923) + - Fixed crash (assertion) when the `alloc_align` attribute was applied to a declaration whose type has a `FunctionProtoType` but which is not itself a `FunctionDecl`, such as a function-pointer variable. (#GH122058) - Fixed a crash on `bool` vectors declared with `ext_vector_type` and more than diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index 4ef0bd2b5ca7c..db85029682cf8 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -4042,6 +4042,8 @@ def Weak : InheritableAttr { let SimpleHandler = 1; } +def : MutualExclusions<[IFunc, Weak]>; + def WeakImport : InheritableAttr { let Spellings = [Clang<"weak_import">]; let Documentation = [Undocumented]; diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 094601a58d508..10633a0c266ae 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -7167,12 +7167,27 @@ void Sema::deduceOpenCLAddressSpace(VarDecl *Var) { Var->assignAddressSpace(Context, ImplAS); } +static bool checkWeakAttrCompatibility(Sema &S, const NamedDecl &ND, + const WeakAttr &Attr) { + const NamedDecl *D = &ND; + // IFuncAttr is not inherited, so a redeclaration may need to check the + // attributes on the definition instead. + if (const auto *FD = dyn_cast<FunctionDecl>(&ND)) + if (const FunctionDecl *Def = FD->getDefinition()) + D = Def; + return DiagnoseMutualExclusions(S, D, &Attr); +} + static void checkWeakAttr(Sema &S, NamedDecl &ND) { // 'weak' only applies to declarations with external linkage. if (WeakAttr *Attr = ND.getAttr<WeakAttr>()) { if (!ND.isExternallyVisible()) { S.Diag(Attr->getLocation(), diag::err_attribute_weak_static); ND.dropAttr<WeakAttr>(); + } else if (!checkWeakAttrCompatibility(S, ND, *Attr)) { + // A forward #pragma weak adds the attribute without checking mutual + // exclusions during attribute processing. + ND.dropAttr<WeakAttr>(); } } } @@ -21435,10 +21450,13 @@ void Sema::ActOnPragmaRedefineExtname(IdentifierInfo* Name, void Sema::ActOnPragmaWeakID(IdentifierInfo* Name, SourceLocation PragmaLoc, SourceLocation NameLoc) { - Decl *PrevDecl = LookupSingleName(TUScope, Name, NameLoc, LookupOrdinaryName); + NamedDecl *PrevDecl = + LookupSingleName(TUScope, Name, NameLoc, LookupOrdinaryName); if (PrevDecl) { - PrevDecl->addAttr(WeakAttr::CreateImplicit(Context, PragmaLoc)); + auto *Attr = WeakAttr::CreateImplicit(Context, PragmaLoc); + if (checkWeakAttrCompatibility(*this, *PrevDecl, *Attr)) + PrevDecl->addAttr(Attr); } else { (void)WeakUndeclaredIdentifiers[Name].insert(WeakInfo(nullptr, NameLoc)); } diff --git a/clang/test/Sema/attr-weak.c b/clang/test/Sema/attr-weak.c index f6482109bc9f6..8f6c632028f2d 100644 --- a/clang/test/Sema/attr-weak.c +++ b/clang/test/Sema/attr-weak.c @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -verify -fsyntax-only %s +// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -verify -fsyntax-only %s extern int f0(void) __attribute__((weak)); extern int g0 __attribute__((weak)); @@ -28,3 +28,19 @@ extern int pr14946_x __attribute__((weak)); // expected-error {{weak declaratio static void pr14946_f(void); void pr14946_f(void) __attribute__((weak)); // expected-error {{weak declaration cannot have internal linkage}} + +// An ifunc declaration is valid on its own, but cannot also be weak. +void *resolver(void) { return 0; } +void ifunc_function(void) __attribute__((ifunc("resolver"))); +void ifunc_weak(void) __attribute__((ifunc("resolver"), weak)); +// expected-error@-1 {{'weak' and 'ifunc' attributes are not compatible}} +// expected-note@-2 {{conflicting attribute is here}} + +// A weak attribute inherited from an earlier declaration also conflicts. +void inherited_weak(void) __attribute__((weak)); // expected-note {{conflicting attribute is here}} +void inherited_weak(void) __attribute__((ifunc("resolver"))); +// expected-error@-1 {{'ifunc' and 'weak' attributes are not compatible}} + +// Adding weak after an ifunc definition is already diagnosed and ignored. +void late_weak(void) __attribute__((ifunc("resolver"))); // expected-note {{previous definition is here}} +void late_weak(void) __attribute__((weak)); // expected-warning {{attribute declaration must precede definition}} diff --git a/clang/test/Sema/pragma-weak.c b/clang/test/Sema/pragma-weak.c index c14125eac9f7c..56a2f44af109c 100644 --- a/clang/test/Sema/pragma-weak.c +++ b/clang/test/Sema/pragma-weak.c @@ -9,3 +9,24 @@ void __a3(void) __attribute((noinline)); #pragma weak a3 = __a3 // expected-note {{previous definition}} void a3(void) __attribute((alias("__a3"))); // expected-error {{redefinition of 'a3'}} void __a3(void) {} + +void *resolver(void) { return 0; } + +// Pragma weak can be applied before or after the ifunc declaration. +#pragma weak pragma_before // expected-note {{conflicting attribute is here}} +void pragma_before(void) __attribute__((ifunc("resolver"))); +// expected-error@-1 {{'ifunc' and 'weak' attributes are not compatible}} + +void pragma_after(void) __attribute__((ifunc("resolver"))); +// expected-error@-1 {{'ifunc' and 'weak' attributes are not compatible}} +#pragma weak pragma_after // expected-note {{conflicting attribute is here}} + +// The ifunc attribute is not inherited by later declarations. +void pragma_after_redecl(void) __attribute__((ifunc("resolver"))); +// expected-error@-1 {{'ifunc' and 'weak' attributes are not compatible}} +void pragma_after_redecl(void); +#pragma weak pragma_after_redecl // expected-note {{conflicting attribute is here}} + +// A weak alias of an ifunc does not make the ifunc itself weak. +#pragma weak pragma_alias = ifunc_alias_target +void ifunc_alias_target(void) __attribute__((ifunc("resolver"))); diff --git a/clang/test/SemaCXX/attr-weak.cpp b/clang/test/SemaCXX/attr-weak.cpp index c6272ef5786ef..ebca563efc30f 100644 --- a/clang/test/SemaCXX/attr-weak.cpp +++ b/clang/test/SemaCXX/attr-weak.cpp @@ -63,3 +63,10 @@ extern int g0 __attribute__((weak_import)); extern "C" int g1 = 0; // expected-note {{previous definition is here}} extern int g1 __attribute__((weak_import)); // expected-warning {{attribute declaration must precede definition}} + +extern "C" void *resolver() { return nullptr; } + +// Weak and ifunc attributes are incompatible with C++11 attribute syntax. +[[gnu::weak, gnu::ifunc("resolver")]] void weak_ifunc(); +// expected-error@-1 {{'gnu::ifunc' and 'gnu::weak' attributes are not compatible}} +// expected-note@-2 {{conflicting attribute is here}} `````````` </details> https://github.com/llvm/llvm-project/pull/229393 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
