[PATCH] D16171: Warning on redeclaring with a conflicting asm label

[Nick Lewycky via cfe-commits]

On 14 January 2016 at 15:05, Zhao, Weiming <weimingz at codeaurora.org> wrote:

> I agree what you said about different code generated with clang and GCC
> generates. In this case, we should throw an error (err_late_asm_label).
>
> But in this example, there is no use of the function. They are just
> redundant declarations and there is no actual code generated.
> So I suggest we just give warnings for this case. Otherwise, it will break
> existing code like some SPEC benchmarks.
>
> Please review my 2nd patch.
>

I think your second patch checks whether it's used at the time of the
redeclaration, which is too early. It may be used between there and the end
of the program. I expect it not to warn but not error on the testcase I
posted in my previous email?

To fix, you'd need to store a list of different-asm-label declarations in
Sema, check it each time something is ODR-used (see the
Sema::Mark<...>Referenced family of calls) to emit the error and remove it
from the list. Also, when emitting a PCH or a Module, you need to serialize
and deserialize this list.

Nick

Weiming
>
>
> On 1/14/2016 2:28 PM, Nick Lewycky wrote:
>
> On 14 January 2016 at 10:38, Weiming Zhao via cfe-commits <
> <cfe-commits at lists.llvm.org>cfe-commits at lists.llvm.org> wrote:
>
>> weimingz added a comment.
>>
>> Hi Nick,
>>
>> Below is a reduced code:
>> t.c:
>>
>>   static long double acoshl (long double __x) __asm__ ("" "acosh") ;  //
>> this is from <gcc4.9>/arm-linux-gnueabi/libc/usr/include/bits/mathcalls.h
>>   extern long double acoshl (long double) __asm__ ("" "__acoshl_finite")
>> ; // this is from existing code
>>
>> GCC gives warning like:
>> /tmp/t.c:2:1: warning: asm declaration ignored due to conflict with
>> previous rename [-Wpragmas]
>>  extern long double acoshl (long double) __asm__ ("" "__acoshl_finite") ;
>>
>
> That's the same case as in this testcase:
>
>   void foo() __asm__("one");
>   void foo() __asm__("two");
>   void test() { foo(); }
>
> GCC emits a call to 'one' while Clang emits a call to 'two'. This is a
> real bug. Please don't downgrade this to a warning.
>
> As an alternative, I would accept a patch which changes how clang
> generates code so that it also produces a call to 'one', with a warning. It
> looks like what we need to do is drop subsequent asm label declarations on
> functions that already have one.
>
> Nick
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160114/db0931d1/attachment.html>