On 11 December 2015 at 12:57, Richard Smith <rich...@metafoo.co.uk> wrote:
> On Fri, Dec 11, 2015 at 12:43 PM, Gao, Yunzhong via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> gcc 4.8.2 accepts the following case silently without error or warning: >> >> void f(); >> >> void g() __asm__(“real_g”); >> >> void f() { g(); } // gcc emits a call to real_g() here >> >> void real_g() __asm__(“gold”); >> >> void real_g() { } // gcc generates a body for gold() here >> >> >> >> gcc 4.8.2 generates a warning for the following test case: >> >> void g() __asm__(“func1”); >> >> void g() __asm__(“func2”); // gcc generates a warning and ignores this >> asm label >> >> void g() { } >> >> >> >> Comparing to gcc behavior, I think it is better to emit error in both >> cases. >> > > Why? I don't see anything wrong with the first example. We have a function > whose source-level name is "g()" and whose asm name is "real_g", and we > have a function whose source-level name is "real_g()" and whose asm name is > "gold". What's wrong with that? > Hah, I completely misread the example and fixed something else. What I added an error for is: void f() __asm__("something"); void f() __asm__("somethingelse"); With my patch, we don't issue an error on the case Yunzhong actually supplied. Joerg has pointed out over IRC that this is a really useful construct and that NetBSD's stack smashing protection uses it. We shouldn't break that. I've added a testcase to make sure it continues working. Please review! Nick Looks good to me. Thanks! >> >> - Gao >> >> >> >> >> >> *From:* Nick Lewycky [mailto:nlewy...@google.com] >> *Sent:* Friday, December 11, 2015 12:44 AM >> *To:* Gao, Yunzhong >> *Cc:* Clang Commits >> *Subject:* Re: PATCH: error on code that redeclares with an __asm__ >> label after the first ODR use >> >> >> >> On 10 December 2015 at 17:42, Gao, Yunzhong < >> yunzhong_...@playstation.sony.com> wrote: >> >> Out of curiosity, is the following test case possible too? >> >> >> >> void f(); >> >> void g() __asm__(“real_g”); // rename g into real_g. >> >> >> >> void f() { >> >> g(); // this would actually be calling real_g() >> >> } >> >> void real_g() __asm__("gold"); // re-declaring real_g() into gold <-- >> should this be an error too? >> >> >> >> I can't see any reason why not. Both clang and gcc will emit "real_g" >> here instead of "gold", but changing the asm label in a program is highly >> dubious. I added an error for this too. >> >> >> >> Thanks! >> >> >> >> *From:* cfe-commits [mailto:cfe-commits-boun...@lists.llvm.org] *On >> Behalf Of *Nick Lewycky via cfe-commits >> *Sent:* Thursday, December 10, 2015 3:15 PM >> *To:* Clang Commits >> *Subject:* PATCH: error on code that redeclares with an __asm__ label >> after the first ODR use >> >> >> >> PR22830 shows a case where some code was adding an __asm__ label after >> the first use of a function. GCC and Clang diverged on this code, GCC >> emitting the name in the last __asm__ label for all uses while clang would >> switch in the middle of the TU as the redeclaration was parsed. The >> attached patch detects this case and issues an error on such a >> redeclaration. If this breaks real code, we can turn it down to a warning. >> >> >> >> Please review! >> >> >> >> Nick >> >> >> >> >> >> _______________________________________________ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> >> >
Index: include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- include/clang/Basic/DiagnosticSemaKinds.td (revision 255303) +++ include/clang/Basic/DiagnosticSemaKinds.td (working copy) @@ -4256,6 +4256,9 @@ def err_conflicting_types : Error<"conflicting types for %0">; def err_different_pass_object_size_params : Error< "conflicting pass_object_size attributes on parameters">; +def err_late_asm_label_name : Error< + "cannot apply asm label to %select{variable|function}0 after its first use">; +def err_different_asm_label : Error<"conflicting asm label">; def err_nested_redefinition : Error<"nested redefinition of %0">; def err_use_with_wrong_tag : Error< "use of %0 with tag type that does not match previous declaration">; Index: lib/Sema/SemaDecl.cpp =================================================================== --- lib/Sema/SemaDecl.cpp (revision 255303) +++ lib/Sema/SemaDecl.cpp (working copy) @@ -2379,9 +2379,24 @@ if (!Old->hasAttrs() && !New->hasAttrs()) return; - // attributes declared post-definition are currently ignored + // Attributes declared post-definition are currently ignored. checkNewAttributesAfterDef(*this, New, Old); + if (AsmLabelAttr *NewA = New->getAttr<AsmLabelAttr>()) { + if (AsmLabelAttr *OldA = Old->getAttr<AsmLabelAttr>()) { + if (OldA->getLabel() != NewA->getLabel()) { + // This redeclaration changes __asm__ label. + Diag(New->getLocation(), diag::err_different_asm_label); + Diag(OldA->getLocation(), diag::note_previous_declaration); + } + } else if (Old->isUsed()) { + // This redeclaration adds an __asm__ label to a declaration that has + // already been ODR-used. + Diag(New->getLocation(), diag::err_late_asm_label_name) + << isa<FunctionDecl>(Old) << New->getAttr<AsmLabelAttr>()->getRange(); + } + } + if (!Old->hasAttrs()) return; Index: test/Sema/asm-label.c =================================================================== --- test/Sema/asm-label.c (revision 0) +++ test/Sema/asm-label.c (working copy) @@ -0,0 +1,30 @@ +// RUN: %clang_cc1 -verify %s + +void f(); +void f() __asm__("fish"); +void g(); + +void f() { + g(); +} +void g() __asm__("gold"); // expected-error{{cannot apply asm label to function after its first use}} + +void h() __asm__("hose"); // expected-note{{previous declaration is here}} +void h() __asm__("hair"); // expected-error{{conflicting asm label}} + +int x; +int x __asm__("xenon"); +int y; + +int test() { return y; } + +int y __asm__("yacht"); // expected-error{{cannot apply asm label to variable after its first use}} + +int z __asm__("zebra"); // expected-note{{previous declaration is here}} +int z __asm__("zooms"); // expected-error{{conflicting asm label}} + + +// No diagnostics on the following. +void __real_readlink() __asm("readlink"); +void readlink() __asm("__protected_readlink"); +void readlink() { __real_readlink(); }
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits