On 11 December 2015 at 12:57, Richard Smith <[email protected]> wrote:
> On Fri, Dec 11, 2015 at 12:43 PM, Gao, Yunzhong via cfe-commits <
> [email protected]> 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:[email protected]]
>> *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 <
>> [email protected]> 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:[email protected]] *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
>> [email protected]
>> 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
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
