On Wed, Jan 22, 2014 at 3:41 PM, Alp Toker <[email protected]> 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 >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> > > -- > http://www.nuanti.com > the browser experts > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
