r199850 - Don't forget about a builtin if we're about to redeclare it and we couldn't

[Richard Smith]

On Wed, Jan 22, 2014 at 3:41 PM, Alp Toker <alp at nuanti.com> wrote:

>
> On 22/01/2014 23:07, Richard Smith wrote:
>
>> Author: rsmith
>> Date: Wed Jan 22 17:07:19 2014
>> New Revision: 199850
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=199850&view=rev
>> Log:
>> Don't forget about a builtin if we're about to redeclare it and we
>> couldn't
>> create an implicit declaration of it (because some type it depends on is
>> unavailable). This had the effect of causing us to not implicitly give it
>> the
>> right attributes. It turns out that glibc's __sigsetjmp is declared before
>> sigjmp_buf is declared, and this resulted in us not implicitly giving it
>> __attribute__((returns_twice)), which in turn resulted in miscompiles in
>> any C
>> code calling glibc's sigsetjmp.
>>
>
> Do you suppose we should provide a DefaultIgnore warning for ForgetBuiltin
> to assist system header developers? It looks a bit hit-and-miss at the
> moment.
>

We were already emitting an (enabled by default) warning for this case in
this system header -- building with -Wsystem-headers was enough to reveal
it. The warning isn't great, though -- it claims that <setjmp.h> should
#include <setjmp.h>. That said, there's nothing really wrong with the
system header, it just uses a strategy that defeats our mechanism for
implicitly declaring builtin library functions -- I'm inclined to think we
should suppress the warning in this case.


> Alp.
>
>
>
>
>> (See also the vaguely-related sourceware.org/PR4662.)
>>
>> Modified:
>>      cfe/trunk/lib/Sema/SemaLookup.cpp
>>      cfe/trunk/test/Sema/implicit-builtin-decl.c
>>
>> Modified: cfe/trunk/lib/Sema/SemaLookup.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/
>> SemaLookup.cpp?rev=199850&r1=199849&r2=199850&view=diff
>> ============================================================
>> ==================
>> --- cfe/trunk/lib/Sema/SemaLookup.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaLookup.cpp Wed Jan 22 17:07:19 2014
>> @@ -545,14 +545,6 @@ static bool LookupBuiltin(Sema &S, Looku
>>             R.addDecl(D);
>>             return true;
>>           }
>> -
>> -        if (R.isForRedeclaration()) {
>> -          // If we're redeclaring this function anyway, forget that
>> -          // this was a builtin at all.
>> -          S.Context.BuiltinInfo.ForgetBuiltin(BuiltinID,
>> S.Context.Idents);
>> -        }
>> -
>> -        return false;
>>         }
>>       }
>>     }
>>
>> Modified: cfe/trunk/test/Sema/implicit-builtin-decl.c
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/
>> implicit-builtin-decl.c?rev=199850&r1=199849&r2=199850&view=diff
>> ============================================================
>> ==================
>> --- cfe/trunk/test/Sema/implicit-builtin-decl.c (original)
>> +++ cfe/trunk/test/Sema/implicit-builtin-decl.c Wed Jan 22 17:07:19 2014
>> @@ -1,4 +1,6 @@
>>   // RUN: %clang_cc1 -fsyntax-only -verify %s
>> +// RUN: not %clang_cc1 -fsyntax-only -ast-dump %s | FileCheck %s
>> +
>>   void f() {
>>     int *ptr = malloc(sizeof(int) * 10); // expected-warning{{implicitly
>> declaring library function 'malloc' with type}} \
>>     // expected-note{{please include the header <stdlib.h> or explicitly
>> provide a declaration for 'malloc'}} \
>> @@ -57,3 +59,10 @@ void snprintf() { }
>>   void longjmp(); // expected-warning{{declaration of built-in function
>> 'longjmp' requires inclusion of the header <setjmp.h>}}
>>     extern float fmaxf(float, float);
>> +
>> +struct __jmp_buf_tag {};
>> +void sigsetjmp(struct __jmp_buf_tag[1], int); //
>> expected-warning{{declaration of built-in function 'sigsetjmp' requires
>> inclusion of the header <setjmp.h>}}
>> +
>> +// CHECK:     FunctionDecl {{.*}} <line:[[@LINE-2]]:1, col:44> sigsetjmp
>> '
>> +// CHECK-NOT: FunctionDecl
>> +// CHECK:     ReturnsTwiceAttr {{.*}} <{{.*}}> Implicit
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>
> --
> http://www.nuanti.com
> the browser experts
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140122/71aa30da/attachment.html>