[patch][pr17535] Reject alias attributes that point to undefined name (V2).
Alp Toker
alp at nuanti.com
Tue Oct 22 06:29:22 PDT 2013
Hello Rafael,
To get an idea of whether this is the right direction I've pushed
forward a little, once again derived from your work.
The attached patch implements the full set of alias and weak attribute
checking to reject some accepts-invalid that existed before, along with
nice diagnostics.
The errors match gcc but we diagnose them more logically, for example:
|void f2() {}|
|void fun2(void) __attribute((alias("f2")));|
|void fun2() {}|
|
||test/Sema/alias-redefinition.c:12:6: error: redefinition of alias 'fun2'|
|void fun2() {}|
| ^|
|test/Sema/alias-redefinition.c:11:6: note: previous definition is here|
|void fun2(void) __attribute((alias("f2")));|
| ^ ~~~~~~~~~~~|
Some of the main changes:
* Mark declarations generated by pragma weak as implicit.
* Fix for FIXME: we should reject this (pr17640).
* Fix for FIXME: Missing call to CheckFunctionDeclaration().
* Treat aliases similarly to definitions.
* Diagnose as error all explicit redefinitions and redefinitions-by-alias
* Diagnose as extension warning all implicit compiler redefinitions
and redefinitions-by-alias
* Tests for each variation (some of these look similar but exercise
different code paths)
I'm still not 100% comfortable with the lookup and checking order for
pragma weak in SemaDeclAttr.cpp, so will continue working on this a
little longer while your initial patch lands.
But I'm encouraged that these two language features are nearing
completion :-)
Alp.
On 21/10/2013 21:25, Rafael Espíndola wrote:
> The attached patch causes clang to reject alias attributes that point
> to undefined names. For example, with this patch we now reject
>
> void f1(void) __attribute__((alias("g1")));
>
> Unlike the previous patch this one is implemented in CodeGen. It is
> quiet a bit simpler. The downside is that the errors only fire during
> -emit-llvm.
>
> Cheers,
> Rafael
--
http://www.nuanti.com
the browser experts
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131022/fc523500/attachment.html>
-------------- next part --------------
diff --git a/include/clang/Basic/Attr.td b/include/clang/Basic/Attr.td
index 381eaeb..d8ba91d 100644
--- a/include/clang/Basic/Attr.td
+++ b/include/clang/Basic/Attr.td
@@ -173,7 +173,7 @@ def AddressSpace : TypeAttr {
let Args = [IntArgument<"AddressSpace">];
}
-def Alias : InheritableAttr {
+def Alias : Attr {
let Spellings = [GNU<"alias">, CXX11<"gnu", "alias">];
let Args = [StringArgument<"Aliasee">];
}
diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 08240d1..6258bf7 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3601,6 +3601,11 @@ def err_redefinition_different_type : Error<
"redefinition of %0 with a different type%diff{: $ vs $|}1,2">;
def err_redefinition_different_kind : Error<
"redefinition of %0 as different kind of symbol">;
+def err_redefinition_alias : Error<
+ "redefinition of alias %0">;
+def warn_redefinition_alias : Warning<
+ "redefinition of alias %0">,
+ InGroup<IgnoredAttributes>;
def warn_forward_class_redefinition : Warning<
"redefinition of forward class %0 of a typedef name of an object type is ignored">,
InGroup<DiagGroup<"objc-forward-class-redefinition">>;
diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp
index 89eeb5e..6a7d874 100644
--- a/lib/AST/Decl.cpp
+++ b/lib/AST/Decl.cpp
@@ -2223,8 +2223,12 @@ bool FunctionDecl::hasTrivialBody() const
bool FunctionDecl::isDefined(const FunctionDecl *&Definition) const {
for (redecl_iterator I = redecls_begin(), E = redecls_end(); I != E; ++I) {
- if (I->IsDeleted || I->IsDefaulted || I->Body || I->IsLateTemplateParsed) {
- Definition = I->IsDeleted ? I->getCanonicalDecl() : *I;
+ if (I->IsDeleted || I->hasAttr<AliasAttr>()) {
+ Definition = I->getCanonicalDecl();
+ return true;
+ }
+ if (I->IsDefaulted || I->Body || I->IsLateTemplateParsed) {
+ Definition = *I;
return true;
}
}
diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index 5c4b59f..794846a 100644
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -2034,6 +2034,13 @@ static void checkNewAttributesAfterDef(Sema &S, Decl *New, const Decl *Old) {
continue; // regular attr merging will take care of validating this.
}
+ if (isa<AliasAttr>(NewAttribute) || isa<WeakAttr>(NewAttribute)) {
+ if (FunctionDecl* FD = dyn_cast<FunctionDecl>(New))
+ S.CheckForFunctionRedefinition(FD);
+ ++I;
+ continue;
+ }
+
if (isa<C11NoReturnAttr>(NewAttribute)) {
// C's _Noreturn is allowed to be added to a function after it is defined.
++I;
@@ -2288,6 +2295,18 @@ bool Sema::MergeFunctionDecl(FunctionDecl *New, Decl *OldD, Scope *S,
else
PrevDiag = diag::note_previous_declaration;
+ if (Old->hasAttr<AliasAttr>() && New->hasAttr<AliasAttr>()) {
+ bool BothImplicit = Old->isImplicit() && New->isImplicit();
+ Diag(New->getLocation(), BothImplicit ? diag::warn_redefinition_alias
+ : diag::err_redefinition_alias)
+ << New->getDeclName();
+ Diag(Old->getLocation(), diag::note_previous_definition)
+ << (Old->hasAttr<AliasAttr>() ? Old->getAttr<AliasAttr>()->getRange()
+ : SourceRange());
+ if (!BothImplicit)
+ return true;
+ }
+
// Don't complain about this if we're in GNU89 mode and the old function
// is an extern inline function.
// Don't complain about specializations. They are not supposed to have
@@ -9329,6 +9348,17 @@ void Sema::CheckForFunctionRedefinition(FunctionDecl *FD) {
const FunctionDecl *Definition;
if (FD->isDefined(Definition) &&
!canRedefineFunction(Definition, getLangOpts())) {
+ if (Definition->hasAttr<AliasAttr>()) {
+ Diag(FD->getLocation(), diag::err_redefinition_alias)
+ << FD->getDeclName();
+ if (FD != Definition)
+ Diag(Definition->getLocation(), diag::note_previous_definition)
+ << (Definition->hasAttr<AliasAttr>()
+ ? Definition->getAttr<AliasAttr>()->getRange()
+ : SourceRange());
+ FD->setInvalidDecl();
+ return;
+ }
if (getLangOpts().GNUMode && Definition->isInlineSpecified() &&
Definition->getStorageClass() == SC_Extern)
Diag(FD->getLocation(), diag::err_redefinition_extern_inline)
diff --git a/lib/Sema/SemaDeclAttr.cpp b/lib/Sema/SemaDeclAttr.cpp
index a08abd9..a1e06da 100644
--- a/lib/Sema/SemaDeclAttr.cpp
+++ b/lib/Sema/SemaDeclAttr.cpp
@@ -1681,8 +1681,6 @@ static void handleAliasAttr(Sema &S, Decl *D, const AttributeList &Attr) {
return;
}
- // FIXME: check if target symbol exists in current file
-
D->addAttr(::new (S.Context) AliasAttr(Attr.getRange(), S.Context, Str,
Attr.getAttributeSpellingListIndex()));
}
@@ -4966,13 +4964,12 @@ void Sema::checkUnusedDeclAttributes(Declarator &D) {
/// DeclClonePragmaWeak - clone existing decl (maybe definition),
/// \#pragma weak needs a non-definition decl and source may not have one.
-NamedDecl * Sema::DeclClonePragmaWeak(NamedDecl *ND, IdentifierInfo *II,
- SourceLocation Loc) {
+NamedDecl *Sema::DeclClonePragmaWeak(NamedDecl *ND, IdentifierInfo *II,
+ SourceLocation Loc) {
assert(isa<FunctionDecl>(ND) || isa<VarDecl>(ND));
NamedDecl *NewD = 0;
if (FunctionDecl *FD = dyn_cast<FunctionDecl>(ND)) {
FunctionDecl *NewFD;
- // FIXME: Missing call to CheckFunctionDeclaration().
// FIXME: Mangling?
// FIXME: Is the qualifier info correct?
// FIXME: Is the DeclContext correct?
@@ -5010,6 +5007,8 @@ NamedDecl * Sema::DeclClonePragmaWeak(NamedDecl *ND, IdentifierInfo *II,
NewVD->setQualifierInfo(VD->getQualifierLoc());
}
}
+
+ NewD->setImplicit();
return NewD;
}
@@ -5021,16 +5020,19 @@ void Sema::DeclApplyPragmaWeak(Scope *S, NamedDecl *ND, WeakInfo &W) {
if (W.getAlias()) { // clone decl, impersonate __attribute(weak,alias(...))
IdentifierInfo *NDId = ND->getIdentifier();
- // FIXME: we should reject this (pr17640).
- NamedDecl *Aliasee = LookupSingleName(TUScope, W.getAlias(),
- W.getLocation(), LookupOrdinaryName);
- if (Aliasee && Aliasee->hasAttr<AliasAttr>())
- return;
+ LookupResult Previous(*this, W.getAlias(), W.getLocation(),
+ LookupOrdinaryName, ForRedeclaration);
+ LookupName(Previous, S, /*CreateBuiltins*/ false);
NamedDecl *NewD = DeclClonePragmaWeak(ND, W.getAlias(), W.getLocation());
NewD->addAttr(::new (Context) AliasAttr(W.getLocation(), Context,
NDId->getName()));
NewD->addAttr(::new (Context) WeakAttr(W.getLocation(), Context));
+
+ if (FunctionDecl *NewFD = dyn_cast<FunctionDecl>(NewD))
+ if (CheckFunctionDeclaration(S, NewFD, Previous, false))
+ return;
+
WeakTopLevelDecl.push_back(NewD);
// FIXME: "hideous" code from Sema::LazilyCreateBuiltin
// to insert Decl at TU scope, sorry.
diff --git a/test/CodeGen/pragma-weak.c b/test/CodeGen/pragma-weak.c
index 40eb525..857ddcf 100644
--- a/test/CodeGen/pragma-weak.c
+++ b/test/CodeGen/pragma-weak.c
@@ -5,10 +5,6 @@
// CHECK: @correct_linkage = weak global
-// CHECK-DAG: @both = alias void ()* @__both
-// CHECK-DAG: @both2 = alias void ()* @__both2
-// CHECK-DAG: @both3 = alias weak void ()* @__both3
-// CHECK-DAG: @a3 = alias weak void ()* @__a3
// CHECK-DAG: @weakvar_alias = alias weak i32* @__weakvar_alias
// CHECK-DAG: @foo = alias weak void ()* @__foo
// CHECK-DAG: @foo2 = alias weak void ()* @__foo2
@@ -72,10 +68,15 @@ void __stutter(void) {}
// CHECK-LABEL: define void @__stutter()
void __stutter2(void) {}
-#pragma weak stutter2 = __stutter2
-#pragma weak stutter2 = __stutter2
+#pragma weak stutter2 = __stutter2 // expected-note {{previous definition}}
+#pragma weak stutter2 = __stutter2 // expected-warning {{redefinition of alias}}
// CHECK-LABEL: define void @__stutter2()
+void __stutter3(void) {}
+void __stutter4(void) {}
+#pragma weak stutter3 = __stutter3 // expected-note {{previous definition}}
+#pragma weak stutter3 = __stutter4 // expected-warning {{redefinition of alias}}
+// CHECK-LABEL: define void @__stutter3()
// test decl/pragma weak order
@@ -106,31 +107,6 @@ void __mix2(void) __attribute((noinline));
void __mix2(void) {}
// CHECK-LABEL: define void @__mix2()
-////////////// test #pragma weak/__attribute combinations
-
-// if the SAME ALIAS is already declared then it overrides #pragma weak
-// resulting in a non-weak alias in this case
-void both(void) __attribute((alias("__both")));
-#pragma weak both = __both
-void __both(void) {}
-// CHECK-LABEL: define void @__both()
-
-// if the TARGET is previously declared then whichever aliasing method
-// comes first applies and subsequent aliases are discarded.
-// TODO: warn about this
-
-void __both2(void);
-void both2(void) __attribute((alias("__both2"))); // first, wins
-#pragma weak both2 = __both2
-void __both2(void) {}
-// CHECK-LABEL: define void @__both2()
-
-void __both3(void);
-#pragma weak both3 = __both3 // first, wins
-void both3(void) __attribute((alias("__both3")));
-void __both3(void) {}
-// CHECK-LABEL: define void @__both3()
-
///////////// ensure that #pragma weak does not alter existing __attributes()
void __a1(void) __attribute((noinline));
@@ -138,14 +114,6 @@ void __a1(void) __attribute((noinline));
void __a1(void) {}
// CHECK: define void @__a1() [[NI:#[0-9]+]]
-// attributes introduced BEFORE a combination of #pragma weak and alias()
-// hold...
-void __a3(void) __attribute((noinline));
-#pragma weak a3 = __a3
-void a3(void) __attribute((alias("__a3")));
-void __a3(void) {}
-// CHECK: define void @__a3() [[NI]]
-
#pragma weak xxx = __xxx
__attribute((pure,noinline,const)) void __xxx(void) { }
// CHECK: void @__xxx() [[RN:#[0-9]+]]
diff --git a/test/Sema/alias-redefinition.c b/test/Sema/alias-redefinition.c
new file mode 100644
index 0000000..712ffc6
--- /dev/null
+++ b/test/Sema/alias-redefinition.c
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only -verify %s
+
+void f0() {}
+void fun0(void) __attribute((alias("f0")));
+
+void f1() {}
+void fun1() {}
+void fun1(void) __attribute((alias("f1"))); // expected-error {{redefinition of alias}}
+
+void f2() {}
+void fun2(void) __attribute((alias("f2"))); // expected-note {{previous definition}}
+void fun2() {} // expected-error {{redefinition of alias}}
+
+void f3() {}
+void fun3(void) __attribute((alias("f3"))); // expected-note {{previous definition}}
+void fun3(void) __attribute((alias("f3"))); // expected-error {{redefinition of alias}}
+
+void f4() {}
+void fun4(void) __attribute((alias("f4")));
+void fun4(void);
+
+void f5() {}
+void __attribute((alias("f5"))) fun5(void) {} // expected-error {{redefinition of alias}}
diff --git a/test/Sema/pragma-weak.c b/test/Sema/pragma-weak.c
new file mode 100644
index 0000000..f767b1a
--- /dev/null
+++ b/test/Sema/pragma-weak.c
@@ -0,0 +1,20 @@
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only -verify %s
+
+void both(void) __attribute((alias("__both"))); // expected-note {{previous definition}}
+#pragma weak both = __both // expected-error {{redefinition of alias}}
+void __both(void) {}
+
+void __both2(void);
+void both2(void) __attribute((alias("__both2"))); // expected-note {{previous definition}}
+#pragma weak both2 = __both2 // expected-error {{redefinition of alias}}
+void __both2(void) {}
+
+void __both3(void);
+#pragma weak both3 = __both3 // expected-note {{previous definition}}
+void both3(void) __attribute((alias("__both3"))); // expected-error {{redefinition of alias}}
+void __both3(void) {}
+
+void __a3(void) __attribute((noinline));
+#pragma weak a3 = __a3 // expected-note {{previous definition}}
+void a3(void) __attribute((alias("__a3"))); // expected-error {{redefinition of alias}}
+void __a3(void) {}
More information about the cfe-commits
mailing list