rsmith created this revision.
rsmith added reviewers: rjmccall, aaron.ballman.
Herald added a project: clang.
rsmith requested review of this revision.
Such code will not work in general; we might have already used the
non-weakness, for example when constant-evaluating an address comparison
Thisor when generating code for the declaration.
Modern versions of GCC reject some such cases, but silently accept
others (based, it appears, on whether the weakness of the declaration
was already inspected). It doesn't seem feasible for us to exactly
follow their behavior, and it might be problematic to start rejecting
cases that GCC accepts and that work in practice, so only warn on this
rather than rejecting, at least for now.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D89212
Files:
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/AST/DeclBase.cpp
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/test/Sema/attr-weak.c
clang/test/Sema/init.c
Index: clang/test/Sema/init.c
===================================================================
--- clang/test/Sema/init.c
+++ clang/test/Sema/init.c
@@ -155,7 +155,7 @@
int PR4386_a = ((void *) PR4386_bar) != 0;
int PR4386_b = ((void *) PR4386_foo) != 0; // expected-error{{initializer element is not a compile-time constant}}
int PR4386_c = ((void *) PR4386_zed) != 0;
-int PR4386_zed() __attribute((weak));
+int PR4386_zed() __attribute((weak)); // expected-warning{{'PR4386_zed' declared weak after its first use}} expected-note {{attribute}}
// <rdar://problem/10185490> (derived from SPEC vortex benchmark)
typedef char strty[10];
Index: clang/test/Sema/attr-weak.c
===================================================================
--- clang/test/Sema/attr-weak.c
+++ clang/test/Sema/attr-weak.c
@@ -1,4 +1,5 @@
-// RUN: %clang_cc1 -verify -fsyntax-only %s
+// RUN: %clang_cc1 -verify -fsyntax-only %s -triple x86_64-unknown-macosx10.3.0 -DMACOS
+// RUN: %clang_cc1 -verify -fsyntax-only %s -triple x86_64-linux-gnu -DLINUX
extern int f0() __attribute__((weak));
extern int g0 __attribute__((weak));
@@ -25,3 +26,47 @@
static void pr14946_f();
void pr14946_f() __attribute__((weak)); // expected-error {{weak declaration cannot have internal linkage}}
+
+void some_function();
+
+void pr47663_a();
+void pr47663_b();
+static void pr47663_c();
+void pr47663_d();
+void pr47663_e(); // expected-warning {{declared weak after its first use}}
+void pr47663_f(); // expected-note {{possible target}}
+void pr47663_g();
+int pr47663_h;
+void pr47663_z() __attribute__((weak));
+void use() {
+ int arr_a[&pr47663_a ? 1 : -1];
+ int arr_b[&pr47663_b ? 1 : -1];
+ int arr_c[&pr47663_c ? 1 : -1];
+ int arr_d[&pr47663_d ? 1 : -1];
+ int arr_e[&pr47663_e ? 1 : -1];
+ int arr_f[&pr47663_f ? 1 : -1];
+ int arr_g[&pr47663_g ? 1 : -1];
+ int arr_h[&pr47663_h ? 1 : -1]; // expected-warning {{will always evaluate to 'true'}}
+ int arr_z[&pr47663_z ? -1 : 1];
+}
+void pr47663_a() __attribute__((weak)); // expected-warning {{declared weak after its first use}} expected-note {{'weak' attribute here}}
+void pr47663_b() __attribute__((weak_import)); // expected-warning {{declared weak after its first use}} expected-note {{'weak_import' attribute here}}
+#ifdef LINUX
+static void pr47663_c() __attribute__((weakref, alias("might_not_exist"))); // expected-warning {{declared weak after its first use}} expected-note {{'weakref' attribute here}}
+#endif
+#ifdef MACOS
+void pr47663_d() __attribute__((availability(macos, introduced=10.4))); // expected-warning {{declared weak after its first use}} expected-note {{'availability' attribute here}}
+#endif
+#pragma weak pr47663_e // expected-note {{pragma 'weak' here}}
+
+// FIXME: This should warn; see PR47796. But it actually creates a bogus
+// overload set. When this is fixed, ensure we produce the 'declared weak after
+// its first use' warning.
+#pragma weak pr47663_f = some_function // expected-note {{possible target}}
+void q() { pr47663_f; } // expected-error {{overloaded}}
+
+#pragma clang attribute push (__attribute__((weak)), apply_to = function) // expected-note {{'weak' attribute here}}
+void pr47663_g(); // expected-warning {{declared weak after its first use}}
+#pragma clang attribute pop
+int pr47663_h __attribute__((weak)); // expected-warning {{declared weak after its first use}} expected-note {{'weak' attribute here}}
+void pr47663_z() __attribute__((weak_import));
Index: clang/lib/Sema/SemaDeclAttr.cpp
===================================================================
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -8151,6 +8151,9 @@
W.setUsed(true);
if (W.getAlias()) { // clone decl, impersonate __attribute(weak,alias(...))
IdentifierInfo *NDId = ND->getIdentifier();
+ // FIXME (PR47796): Check for a previous declaration with the target name
+ // here, and build a redeclaration of it. Check whether the previous
+ // declaration is used and warn if so.
NamedDecl *NewD = DeclClonePragmaWeak(ND, W.getAlias(), W.getLocation());
NewD->addAttr(
AliasAttr::CreateImplicit(Context, NDId->getName(), W.getLocation()));
@@ -8166,6 +8169,13 @@
PushOnScopeChains(NewD, S);
CurContext = SavedContext;
} else { // just add weak to existing
+ if (auto *VD = dyn_cast<ValueDecl>(ND)) {
+ if (!VD->isWeak() && VD->isUsed(false)) {
+ Diag(VD->getLocation(), diag::warn_attribute_weak_after_use) << VD;
+ Diag(W.getLocation(), diag::note_declared_weak_here)
+ << /*IsPragma*/ true << "weak";
+ }
+ }
ND->addAttr(WeakAttr::CreateImplicit(Context, W.getLocation(),
AttributeCommonInfo::AS_Pragma));
}
Index: clang/lib/Sema/SemaDecl.cpp
===================================================================
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -2769,6 +2769,41 @@
}
}
+/// Some attributes can't be applied consistently if they're introduced after
+/// the first use. Check for this case.
+static void checkNewAttributesAfterUse(Sema &S, Decl *New, const Decl *Old) {
+ if (!New->hasAttrs() || !Old->isUsed(false))
+ return;
+
+ // If the new declaration is weak and the old was already used, we might have
+ // used the non-weakness of the old declaration in one of various ways
+ // (constant evaluation, code generation, diagnostic emission).
+ auto *OldVD = dyn_cast<ValueDecl>(Old);
+ auto *NewVD = dyn_cast<ValueDecl>(New);
+ if (NewVD && OldVD && NewVD->isWeak() && !OldVD->isWeak()) {
+ Attr *WeakA = nullptr;
+ for (Attr *A : NewVD->getAttrs()) {
+ if (isa<WeakAttr, WeakRefAttr, WeakImportAttr>(A)) {
+ WeakA = A;
+ break;
+ }
+ auto *Avail = dyn_cast<AvailabilityAttr>(A);
+ if (Avail && Avail->getAvailability(S.Context) == AR_NotYetIntroduced) {
+ WeakA = A;
+ break;
+ }
+ }
+
+ if (WeakA) {
+ S.Diag(NewVD->getLocation(), diag::warn_attribute_weak_after_use)
+ << NewVD;
+ S.Diag(WeakA->getLocation(), diag::note_declared_weak_here)
+ << (WeakA->getSyntax() == AttributeCommonInfo::AS_Pragma)
+ << WeakA->getSpelling();
+ }
+ }
+}
+
static void diagnoseMissingConstinit(Sema &S, const VarDecl *InitDecl,
const ConstInitAttr *CIAttr,
bool AttrBeforeInit) {
@@ -2871,6 +2906,9 @@
// Attributes declared post-definition are currently ignored.
checkNewAttributesAfterDef(*this, New, Old);
+ // Some attributes declared after the first use might be ignored.
+ checkNewAttributesAfterUse(*this, New, Old);
+
if (AsmLabelAttr *NewA = New->getAttr<AsmLabelAttr>()) {
if (AsmLabelAttr *OldA = Old->getAttr<AsmLabelAttr>()) {
if (!OldA->isEquivalent(NewA)) {
@@ -18243,36 +18281,34 @@
(void)ExtnameUndeclaredIdentifiers.insert(std::make_pair(Name, Attr));
}
-void Sema::ActOnPragmaWeakID(IdentifierInfo* Name,
+void Sema::ActOnPragmaWeakID(IdentifierInfo *Name,
SourceLocation PragmaLoc,
SourceLocation NameLoc) {
- Decl *PrevDecl = LookupSingleName(TUScope, Name, NameLoc, LookupOrdinaryName);
+ WeakInfo W((IdentifierInfo*)nullptr, NameLoc);
- if (PrevDecl) {
- PrevDecl->addAttr(WeakAttr::CreateImplicit(Context, PragmaLoc, AttributeCommonInfo::AS_Pragma));
- } else {
+ if (NamedDecl *PrevDecl =
+ LookupSingleName(TUScope, Name, NameLoc, LookupOrdinaryName))
+ DeclApplyPragmaWeak(TUScope, PrevDecl, W);
+ else
(void)WeakUndeclaredIdentifiers.insert(
- std::pair<IdentifierInfo*,WeakInfo>
- (Name, WeakInfo((IdentifierInfo*)nullptr, NameLoc)));
- }
+ std::pair<IdentifierInfo *, WeakInfo>(Name, W));
}
-void Sema::ActOnPragmaWeakAlias(IdentifierInfo* Name,
- IdentifierInfo* AliasName,
+void Sema::ActOnPragmaWeakAlias(IdentifierInfo *Name,
+ IdentifierInfo *AliasName,
SourceLocation PragmaLoc,
SourceLocation NameLoc,
SourceLocation AliasNameLoc) {
- Decl *PrevDecl = LookupSingleName(TUScope, AliasName, AliasNameLoc,
- LookupOrdinaryName);
+ NamedDecl *PrevDecl =
+ LookupSingleName(TUScope, AliasName, AliasNameLoc, LookupOrdinaryName);
WeakInfo W = WeakInfo(Name, NameLoc);
if (PrevDecl && (isa<FunctionDecl>(PrevDecl) || isa<VarDecl>(PrevDecl))) {
if (!PrevDecl->hasAttr<AliasAttr>())
- if (NamedDecl *ND = dyn_cast<NamedDecl>(PrevDecl))
- DeclApplyPragmaWeak(TUScope, ND, W);
+ DeclApplyPragmaWeak(TUScope, PrevDecl, W);
} else {
(void)WeakUndeclaredIdentifiers.insert(
- std::pair<IdentifierInfo*,WeakInfo>(AliasName, W));
+ std::pair<IdentifierInfo *, WeakInfo>(AliasName, W));
}
}
Index: clang/lib/AST/DeclBase.cpp
===================================================================
--- clang/lib/AST/DeclBase.cpp
+++ clang/lib/AST/DeclBase.cpp
@@ -619,6 +619,10 @@
return AR_Available;
}
+AvailabilityResult AvailabilityAttr::getAvailability(ASTContext &C) const {
+ return CheckAvailability(C, this, nullptr, VersionTuple());
+}
+
AvailabilityResult Decl::getAvailability(std::string *Message,
VersionTuple EnclosingVersion,
StringRef *RealizedPlatform) const {
@@ -725,8 +729,7 @@
return true;
if (const auto *Availability = dyn_cast<AvailabilityAttr>(A)) {
- if (CheckAvailability(getASTContext(), Availability, nullptr,
- VersionTuple()) == AR_NotYetIntroduced)
+ if (Availability->getAvailability(getASTContext()) == AR_NotYetIntroduced)
return true;
}
}
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3247,6 +3247,11 @@
InGroup<DiagGroup<"unsupported-dll-base-class-template">>, DefaultIgnore;
def err_attribute_dll_ambiguous_default_ctor : Error<
"'__declspec(dllexport)' cannot be applied to more than one default constructor in %0">;
+def warn_attribute_weak_after_use : Warning<
+ "%0 declared weak after its first use; weakness may not be respected in all contexts">,
+ InGroup<DiagGroup<"weak-after-use">>;
+def note_declared_weak_here : Note<
+ "declared weak by %select{'%1' attribute|pragma '%1'}0 here">;
def err_attribute_weakref_not_static : Error<
"weakref declaration must have internal linkage">;
def err_attribute_weakref_not_global_context : Error<
Index: clang/include/clang/Basic/Attr.td
===================================================================
--- clang/include/clang/Basic/Attr.td
+++ clang/include/clang/Basic/Attr.td
@@ -839,7 +839,10 @@
.Case("tvOSApplicationExtension", "tvos_app_extension")
.Case("watchOSApplicationExtension", "watchos_app_extension")
.Default(Platform);
-} }];
+}
+// Defined in lib/AST/DeclBase.cpp.
+AvailabilityResult getAvailability(ASTContext &C) const;
+}];
let HasCustomParsing = 1;
let InheritEvenIfAlreadyPresent = 1;
let Subjects = SubjectList<[Named]>;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
