[patch][pr17535] Reject alias attributes that point to undefined names.

[Alp Toker]

Hi Rafael,

The old gcc syntax, ugly as it is, allows aliasing of overloaded functions:

void alias2() __attribute__((alias("_Z6alias1v")));
void alias1() {}
void alias1(int) {}

Your patch precludes that because it's using simple identifier lookup.

I've attached a patch against yours that uses overload resolution to
find an exact match, otherwise the appropriate overload as long it can
be found unambiguously, so this continues to work as:

void alias2() __attribute__((alias("alias1")));
void alias1() {}
void alias1(int) {}

My second thought is that, instead of overloading the gcc alias
attribute in an incompatible way, I'd prefer to preserve the current
behaviour, at least in the quoted form. It doesn't feel right to change
the attribute semantics given that it's already specified to work with
symbol names, complete with working tests.

Some options might be to have a new clang syntax accepting an unquoted
IdentifierArgument / NNS, or even to make this a new attribute as a
clang extension. What do you think?

Alp.


On 19/10/2013 23:54, Rafael Espíndola wrote:
> ping
>
>> On 14 October 2013 12:31, Rafael Espíndola <rafael.espindola at gmail.com> 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")));
>>>
>>> if g1 is not defined in this file. This matches "modern" gcc (>= 4.2
>>> at least) behavior. What is different in this implementation is that
>>> we use the C/C++ level names, not the assembly names like gcc does.
>>> This is consistent with our implementation of #pragma weak, with has
>>> the same difference to gcc's implementation.
>>>
>>> Other options include
>>> * Using the mangled name from sema. This would be very expensive and
>>> we would have to mangle each decl to see if we got it.
>>> * Produce a terse error from CodeGen. We normally avoid this, but it
>>> is probably the right thing to do here if we decide to use the mangled
>>> names.
> Cheers,
> Rafael
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

-- 
http://www.nuanti.com
the browser experts

-------------- next part --------------
diff --git a/lib/Sema/Sema.cpp b/lib/Sema/Sema.cpp
index 96f0a3e..6763d63 100644
--- a/lib/Sema/Sema.cpp
+++ b/lib/Sema/Sema.cpp
@@ -557,36 +557,6 @@ static bool IsRecordFullyDefined(const CXXRecordDecl *RD,
   return Complete;
 }
 
-static bool isValidAliasee(NamedDecl *ND) {
-  if (!ND)
-    return false;
-  FunctionDecl *FD = dyn_cast<FunctionDecl>(ND);
-  if (FD)
-    return FD->isDefined();
-  VarDecl *VD = dyn_cast<VarDecl>(ND);
-  if (!VD)
-    return false;
-  return VD->hasDefinition() != VarDecl::DeclarationOnly;
-}
-
-void Sema::CheckAliases() {
-  for (SmallVectorImpl<Decl*>::iterator I = Aliases.begin(), E = Aliases.end();
-       I != E; ++I) {
-    Decl *D = *I;
-    if (D->hasAttr<WeakRefAttr>())
-      continue;
-    AliasAttr *AA = D->getAttr<AliasAttr>();
-    StringRef Name = AA->getAliasee();
-    const IdentifierInfo *NameI = &Context.Idents.get(Name);
-    SourceLocation NameLoc = AA->getLocation();
-    NamedDecl *Aliasee =
-        LookupSingleName(TUScope, NameI, NameLoc, LookupOrdinaryName);
-    if (!isValidAliasee(Aliasee))
-      Diag(NameLoc, diag::err_alias_to_undefined);
-    AA->AliaseeDecl = Aliasee;
-  }
-}
-
 /// ActOnEndOfTranslationUnit - This is called at the very end of the
 /// translation unit when EOF is reached and all but the top-level scope is
 /// popped.
diff --git a/lib/Sema/SemaDeclAttr.cpp b/lib/Sema/SemaDeclAttr.cpp
index a0bcace..9c4fe57 100644
--- a/lib/Sema/SemaDeclAttr.cpp
+++ b/lib/Sema/SemaDeclAttr.cpp
@@ -1687,6 +1687,44 @@ static void handleAliasAttr(Sema &S, Decl *D, const AttributeList &Attr) {
   S.Aliases.push_back(D);
 }
 
+static bool isValidAliasee(NamedDecl *ND) {
+  if (!ND)
+    return false;
+  FunctionDecl *FD = dyn_cast<FunctionDecl>(ND);
+  if (FD)
+    return FD->isDefined();
+  VarDecl *VD = dyn_cast<VarDecl>(ND);
+  if (!VD)
+    return false;
+  return VD->hasDefinition() != VarDecl::DeclarationOnly;
+}
+
+void Sema::CheckAliases() {
+  for (SmallVectorImpl<Decl *>::iterator I = Aliases.begin(), E = Aliases.end();
+       I != E; ++I) {
+    Decl *D = *I;
+    if (D->hasAttr<WeakRefAttr>())
+      continue;
+    AliasAttr *AA = D->getAttr<AliasAttr>();
+    StringRef Name = AA->getAliasee();
+    const IdentifierInfo *NameI = &Context.Idents.get(Name);
+    SourceLocation NameLoc = AA->getLocation();
+
+    LookupResult R(*this, NameI, NameLoc, LookupOrdinaryName);
+    LookupName(R, TUScope, /*CreateBuiltins*/ false);
+
+    NamedDecl *Aliasee = 0;
+    if (FunctionDecl *FD = dyn_cast<FunctionDecl>(D))
+      CheckOverload(TUScope, FD, R, Aliasee, false);
+    if (!Aliasee)
+      Aliasee = R.getAsSingle<NamedDecl>();
+
+    if (!isValidAliasee(Aliasee))
+      Diag(NameLoc, diag::err_alias_to_undefined);
+    AA->AliaseeDecl = Aliasee;
+  }
+}
+
 static void handleMinSizeAttr(Sema &S, Decl *D, const AttributeList &Attr) {
   if (!isa<FunctionDecl>(D) && !isa<ObjCMethodDecl>(D)) {
     S.Diag(Attr.getLoc(), diag::err_attribute_wrong_decl_type)
diff --git a/test/SemaCXX/cxx11-gnu-attrs.cpp b/test/SemaCXX/cxx11-gnu-attrs.cpp
index f587805..3c4adf2 100644
--- a/test/SemaCXX/cxx11-gnu-attrs.cpp
+++ b/test/SemaCXX/cxx11-gnu-attrs.cpp
@@ -11,8 +11,9 @@ int *[[gnu::unused]] attr_on_ptr;
 
 // Valid cases.
 
-void alias1() {}
 void alias2 [[gnu::alias("alias1")]] ();
+void alias1(int) {}
+void alias1() {}
 
 [[gnu::aligned(8)]] int aligned;
 void aligned_fn [[gnu::aligned(32)]] ();