[patch][pr17535] Reject alias attributes that point to undefined names.
Alp Toker
alp at nuanti.com
Mon Oct 21 08:11:33 PDT 2013
On 21/10/2013 15:25, Rafael Espíndola wrote:
> On 19 October 2013 23:55, Alp Toker <alp at nuanti.com> wrote:
>> 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?
> We chatted a bit on IRC and I agree.
>
> I was undecided as to what is the best way of handling pr17535, but
> given the above argument I now think that this is one of the
> exceptional cases where producing an error from codegen is reasonable.
>
> I also agree that solution is reasonable only because this is a really
> terrible attribute. Designing a more user friendly one that uses
> identifiers and works with c++ features would be really nice.
>
> I will try to implement the codegen fix for pr17535 and start a new
> thread with it.
>
> Anyone interested in implementing a c++ and user friendly alias
> attribute should probably start from Alp's patch.
>
> Thanks,
> Rafael
Great, I think diagnosing at CodeGen is the better choice here.
Rafael, in the new patch could you swap around one of the tests to prove
that checking isn't dependent on parse order?
-void alias1() {}
void alias2 [[gnu::alias("_Z6alias1v")]] ();
+void alias1() {}
For posterity I'm attaching the combined patch in case someone wants to
pick it up.
Alp.
--
http://www.nuanti.com
the browser experts
-------------- next part --------------
diff --git a/include/clang/Basic/Attr.td b/include/clang/Basic/Attr.td
index 381eaeb..37d8a4d 100644
--- a/include/clang/Basic/Attr.td
+++ b/include/clang/Basic/Attr.td
@@ -176,6 +176,7 @@ def AddressSpace : TypeAttr {
def Alias : InheritableAttr {
let Spellings = [GNU<"alias">, CXX11<"gnu", "alias">];
let Args = [StringArgument<"Aliasee">];
+ let AdditionalMembers = [{ NamedDecl *AliaseeDecl; }];
}
def Aligned : InheritableAttr {
diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index fa5a311..99e8547 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2015,6 +2015,8 @@ def err_attribute_weakref_without_alias : Error<
"weakref declaration of '%0' must also have an alias attribute">;
def err_alias_not_supported_on_darwin : Error <
"only weak aliases are supported on darwin">;
+def err_alias_to_undefined : Error<
+ "alias must point to a defined variable or function">;
def warn_attribute_wrong_decl_type : Warning<
"%0 attribute only applies to %select{functions|unions|"
"variables and functions|functions and methods|parameters|"
diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index b1906ab..35b15cf 100644
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -372,6 +372,8 @@ public:
/// cycle detection at the end of the TU.
DelegatingCtorDeclsType DelegatingCtorDecls;
+ SmallVector<Decl*, 2> Aliases;
+
/// \brief All the overriding destructors seen during a class definition
/// (there could be multiple due to nested classes) that had their exception
/// spec checks delayed, plus the overridden destructor.
@@ -954,6 +956,8 @@ public:
void ActOnEndOfTranslationUnit();
+ void CheckAliases();
+
void CheckDelegatingCtorCycles();
Scope *getScopeForContext(DeclContext *Ctx);
diff --git a/lib/CodeGen/CGExprCXX.cpp b/lib/CodeGen/CGExprCXX.cpp
index 5674442..2311e08 100644
--- a/lib/CodeGen/CGExprCXX.cpp
+++ b/lib/CodeGen/CGExprCXX.cpp
@@ -929,9 +929,8 @@ static RValue EmitNewDeleteCall(CodeGenFunction &CGF,
/// to a replaceable global allocation function.
///
/// We model such elidable calls with the 'builtin' attribute.
- llvm::Function *Fn = dyn_cast<llvm::Function>(CalleeAddr);
if (Callee->isReplaceableGlobalAllocationFunction() &&
- Fn && Fn->hasFnAttribute(llvm::Attribute::NoBuiltin)) {
+ !Callee->hasAttr<AliasAttr>()) {
// FIXME: Add addAttribute to CallSite.
if (llvm::CallInst *CI = dyn_cast<llvm::CallInst>(CallOrInvoke))
CI->addAttribute(llvm::AttributeSet::FunctionIndex,
diff --git a/lib/CodeGen/CodeGenModule.cpp b/lib/CodeGen/CodeGenModule.cpp
index 96ae437..1ce6285 100644
--- a/lib/CodeGen/CodeGenModule.cpp
+++ b/lib/CodeGen/CodeGenModule.cpp
@@ -933,6 +933,12 @@ void CodeGenModule::EmitDeferred() {
GlobalDecl D = DeferredDeclsToEmit.back();
DeferredDeclsToEmit.pop_back();
+ const ValueDecl *Global = cast<ValueDecl>(D.getDecl());
+ if (Global->hasAttr<AliasAttr>()) {
+ EmitAliasDefinition(D);
+ continue;
+ }
+
// Check to see if we've already emitted this. This is necessary
// for a couple of reasons: first, decls can end up in the
// deferred-decls queue multiple times, and second, decls can end
@@ -1098,7 +1104,7 @@ void CodeGenModule::EmitGlobal(GlobalDecl GD) {
// If this is an alias definition (which otherwise looks like a declaration)
// emit it now.
if (Global->hasAttr<AliasAttr>())
- return EmitAliasDefinition(GD);
+ DeferredDeclsToEmit.push_back(GD);
// If this is CUDA, be selective about which declarations we emit.
if (LangOpts.CUDA) {
@@ -2093,12 +2099,14 @@ void CodeGenModule::EmitAliasDefinition(GlobalDecl GD) {
// Create a reference to the named value. This ensures that it is emitted
// if a deferred decl.
llvm::Constant *Aliasee;
- if (isa<llvm::FunctionType>(DeclTy))
- Aliasee = GetOrCreateLLVMFunction(AA->getAliasee(), DeclTy, GD,
- /*ForVTable=*/false);
- else
- Aliasee = GetOrCreateLLVMGlobal(AA->getAliasee(),
- llvm::PointerType::getUnqual(DeclTy), 0);
+ if (FunctionDecl *F = dyn_cast<FunctionDecl>(AA->AliaseeDecl)) {
+ GlobalDecl GDA(F);
+ Aliasee = GetAddrOfFunction(GDA, DeclTy, /*ForVTable=*/false);
+ }
+ else {
+ VarDecl *Var = cast<VarDecl>(AA->AliaseeDecl);
+ Aliasee = GetAddrOfGlobalVar(Var,DeclTy);
+ }
// Create the new alias itself, but don't set a name yet.
llvm::GlobalValue *GA =
diff --git a/lib/Sema/Sema.cpp b/lib/Sema/Sema.cpp
index 41f72a6..6763d63 100644
--- a/lib/Sema/Sema.cpp
+++ b/lib/Sema/Sema.cpp
@@ -638,6 +638,8 @@ void Sema::ActOnEndOfTranslationUnit() {
<< I->first;
}
+ CheckAliases();
+
if (LangOpts.CPlusPlus11 &&
Diags.getDiagnosticLevel(diag::warn_delegating_ctor_cycle,
SourceLocation())
diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp
index e01c032..dd4c1fb 100644
--- a/lib/Sema/SemaDecl.cpp
+++ b/lib/Sema/SemaDecl.cpp
@@ -1977,7 +1977,10 @@ static bool mergeDeclAttribute(Sema &S, NamedDecl *D, InheritableAttr *Attr,
else if (SectionAttr *SA = dyn_cast<SectionAttr>(Attr))
NewAttr = S.mergeSectionAttr(D, SA->getRange(), SA->getName(),
AttrSpellingListIndex);
- else if (isa<AlignedAttr>(Attr))
+ else if (isa<AliasAttr>(Attr)) {
+ NewAttr = cast<InheritableAttr>(Attr->clone(S.Context));
+ S.Aliases.push_back(D);
+ } else if (isa<AlignedAttr>(Attr))
// AlignedAttrs are handled separately, because we need to handle all
// such attributes on a declaration at the same time.
NewAttr = 0;
diff --git a/lib/Sema/SemaDeclAttr.cpp b/lib/Sema/SemaDeclAttr.cpp
index a71d3c0..4031adf 100644
--- a/lib/Sema/SemaDeclAttr.cpp
+++ b/lib/Sema/SemaDeclAttr.cpp
@@ -1681,10 +1681,48 @@ 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()));
+ 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) {
@@ -5020,10 +5058,17 @@ void Sema::DeclApplyPragmaWeak(Scope *S, NamedDecl *ND, WeakInfo &W) {
W.setUsed(true);
if (W.getAlias()) { // clone decl, impersonate __attribute(weak,alias(...))
IdentifierInfo *NDId = ND->getIdentifier();
+
+ NamedDecl *Aliasee = LookupSingleName(TUScope, W.getAlias(),
+ W.getLocation(), LookupOrdinaryName);
+ if (Aliasee && Aliasee->hasAttr<AliasAttr>())
+ return;
+
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));
+ Aliases.push_back(NewD);
WeakTopLevelDecl.push_back(NewD);
// FIXME: "hideous" code from Sema::LazilyCreateBuiltin
// to insert Decl at TU scope, sorry.
diff --git a/test/CodeGen/alias.c b/test/CodeGen/alias.c
index efa94b3..31ba9a9 100644
--- a/test/CodeGen/alias.c
+++ b/test/CodeGen/alias.c
@@ -6,6 +6,10 @@ int g0;
static int bar1 = 42;
// CHECKBASIC: @bar1 = internal global i32 42
+static int bar2() {
+ return 0;
+}
+
extern int g1;
extern int g1 __attribute((alias("g0")));
// CHECKBASIC-DAG: @g1 = alias i32* @g0
@@ -34,11 +38,11 @@ static int inner_weak(int a) { return 0; }
extern __typeof(inner) inner_a __attribute__((alias("inner")));
static __typeof(inner_weak) inner_weak_a __attribute__((weakref, alias("inner_weak")));
// CHECKCC: @inner_a = alias i32 (i32)* @inner
-// CHECKCC: define internal arm_aapcs_vfpcc i32 @inner(i32 %a) [[NUW:#[0-9]+]] {
int outer(int a) { return inner(a); }
-// CHECKCC: define arm_aapcs_vfpcc i32 @outer(i32 %a) [[NUW]] {
+// CHECKCC: define arm_aapcs_vfpcc i32 @outer(i32 %a) [[NUW:#[0-9]+]] {
// CHECKCC: call arm_aapcs_vfpcc i32 @inner(i32 %{{.*}})
+// CHECKCC: define internal arm_aapcs_vfpcc i32 @inner(i32 %a) [[NUW]] {
int outer_weak(int a) { return inner_weak_a(a); }
// CHECKCC: define arm_aapcs_vfpcc i32 @outer_weak(i32 %a) [[NUW]] {
diff --git a/test/CodeGenCXX/attr.cpp b/test/CodeGenCXX/attr.cpp
index 8bcff36..4a78054 100644
--- a/test/CodeGenCXX/attr.cpp
+++ b/test/CodeGenCXX/attr.cpp
@@ -29,6 +29,6 @@ void C::bar4() { }
// CHECK-LABEL: define i32 @_Z5test1v()
int test1() { return 10; }
// CHECK at top of file
-extern "C" int test2() __attribute__((alias("_Z5test1v")));
+extern "C" int test2() __attribute__((alias("test1")));
// CHECK: attributes [[NUW]] = { nounwind{{.*}} }
diff --git a/test/Misc/ast-dump-attr.cpp b/test/Misc/ast-dump-attr.cpp
index 3efcd09..2ea1f9e 100644
--- a/test/Misc/ast-dump-attr.cpp
+++ b/test/Misc/ast-dump-attr.cpp
@@ -79,6 +79,7 @@ void TestInt(void) __attribute__((constructor(123)));
// CHECK: FunctionDecl{{.*}}TestInt
// CHECK-NEXT: ConstructorAttr{{.*}} 123
+int alias1;
int TestString __attribute__((alias("alias1")));
// CHECK: VarDecl{{.*}}TestString
// CHECK-NEXT: AliasAttr{{.*}} "alias1"
diff --git a/test/Sema/attr-alias-elf.c b/test/Sema/attr-alias-elf.c
new file mode 100644
index 0000000..55cae9b
--- /dev/null
+++ b/test/Sema/attr-alias-elf.c
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -triple x86_64-pc-linux -fsyntax-only -verify %s
+
+void f1(void) __attribute__((alias("g1")));
+void g1(void) {
+}
+
+void f2(void) __attribute__((alias("g2"))); // expected-error {{alias must point to a defined variable or function}}
+
+
+void f3(void) __attribute__((alias("g3"))); // expected-error {{alias must point to a defined variable or function}}
+void g3(void);
+
+extern int a1 __attribute__((alias("b1")));
+int b1 = 42;
+
+extern int a2 __attribute__((alias("b2"))); // expected-error {{alias must point to a defined variable or function}}
+
+extern int a3 __attribute__((alias("b3"))); // expected-error {{alias must point to a defined variable or function}}
+extern int b3;
+
+extern int a4 __attribute__((alias("b4"))); // expected-error {{alias must point to a defined variable or function}}
+typedef int b4;
diff --git a/test/SemaCXX/cxx11-gnu-attrs.cpp b/test/SemaCXX/cxx11-gnu-attrs.cpp
index def83a9..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 alias2 [[gnu::alias("alias1")]] ();
+void alias1(int) {}
void alias1() {}
-void alias2 [[gnu::alias("_Z6alias1v")]] ();
[[gnu::aligned(8)]] int aligned;
void aligned_fn [[gnu::aligned(32)]] ();
More information about the cfe-commits
mailing list