PATCH: error on code that redeclares with an __asm__ label after the first ODR use

[Nick Lewycky via cfe-commits]

On 11 December 2015 at 12:57, Richard Smith <richard at metafoo.co.uk> wrote:

> On Fri, Dec 11, 2015 at 12:43 PM, Gao, Yunzhong via cfe-commits <
> cfe-commits at 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:nlewycky at 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_gao at 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-bounces at 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 at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20151211/caa6c461/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: pr22830-3.patch
Type: text/x-patch
Size: 2998 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20151211/caa6c461/attachment-0001.bin>