<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Jan 22, 2014 at 3:41 PM, Alp Toker <span dir="ltr"><<a href="mailto:alp@nuanti.com" target="_blank">alp@nuanti.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im"><br>
On 22/01/2014 23:07, Richard Smith wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Author: rsmith<br>
Date: Wed Jan 22 17:07:19 2014<br>
New Revision: 199850<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=199850&view=rev" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project?rev=199850&view=rev</a><br>
Log:<br>
Don't forget about a builtin if we're about to redeclare it and we couldn't<br>
create an implicit declaration of it (because some type it depends on is<br>
unavailable). This had the effect of causing us to not implicitly give it the<br>
right attributes. It turns out that glibc's __sigsetjmp is declared before<br>
sigjmp_buf is declared, and this resulted in us not implicitly giving it<br>
__attribute__((returns_twice))<u></u>, which in turn resulted in miscompiles in any C<br>
code calling glibc's sigsetjmp.<br>
</blockquote>
<br></div>
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.<br></blockquote><div><br></div><div>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.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Alp.<div class="HOEnZb"><div class="h5"><br>
<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
(See also the vaguely-related <a href="http://sourceware.org/PR4662" target="_blank">sourceware.org/PR4662</a>.)<br>
<br>
Modified:<br>
cfe/trunk/lib/Sema/SemaLookup.<u></u>cpp<br>
cfe/trunk/test/Sema/implicit-<u></u>builtin-decl.c<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaLookup.<u></u>cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=199850&r1=199849&r2=199850&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/cfe/trunk/lib/Sema/<u></u>SemaLookup.cpp?rev=199850&r1=<u></u>199849&r2=199850&view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- cfe/trunk/lib/Sema/SemaLookup.<u></u>cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaLookup.<u></u>cpp Wed Jan 22 17:07:19 2014<br>
@@ -545,14 +545,6 @@ static bool LookupBuiltin(Sema &S, Looku<br>
R.addDecl(D);<br>
return true;<br>
}<br>
-<br>
- if (R.isForRedeclaration()) {<br>
- // If we're redeclaring this function anyway, forget that<br>
- // this was a builtin at all.<br>
- S.Context.BuiltinInfo.<u></u>ForgetBuiltin(BuiltinID, S.Context.Idents);<br>
- }<br>
-<br>
- return false;<br>
}<br>
}<br>
}<br>
<br>
Modified: cfe/trunk/test/Sema/implicit-<u></u>builtin-decl.c<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/implicit-builtin-decl.c?rev=199850&r1=199849&r2=199850&view=diff" target="_blank">http://llvm.org/viewvc/llvm-<u></u>project/cfe/trunk/test/Sema/<u></u>implicit-builtin-decl.c?rev=<u></u>199850&r1=199849&r2=199850&<u></u>view=diff</a><br>
==============================<u></u>==============================<u></u>==================<br>
--- cfe/trunk/test/Sema/implicit-<u></u>builtin-decl.c (original)<br>
+++ cfe/trunk/test/Sema/implicit-<u></u>builtin-decl.c Wed Jan 22 17:07:19 2014<br>
@@ -1,4 +1,6 @@<br>
// RUN: %clang_cc1 -fsyntax-only -verify %s<br>
+// RUN: not %clang_cc1 -fsyntax-only -ast-dump %s | FileCheck %s<br>
+<br>
void f() {<br>
int *ptr = malloc(sizeof(int) * 10); // expected-warning{{implicitly declaring library function 'malloc' with type}} \<br>
// expected-note{{please include the header <stdlib.h> or explicitly provide a declaration for 'malloc'}} \<br>
@@ -57,3 +59,10 @@ void snprintf() { }<br>
void longjmp(); // expected-warning{{declaration of built-in function 'longjmp' requires inclusion of the header <setjmp.h>}}<br>
extern float fmaxf(float, float);<br>
+<br>
+struct __jmp_buf_tag {};<br>
+void sigsetjmp(struct __jmp_buf_tag[1], int); // expected-warning{{declaration of built-in function 'sigsetjmp' requires inclusion of the header <setjmp.h>}}<br>
+<br>
+// CHECK: FunctionDecl {{.*}} <line:[[@LINE-2]]:1, col:44> sigsetjmp '<br>
+// CHECK-NOT: FunctionDecl<br>
+// CHECK: ReturnsTwiceAttr {{.*}} <{{.*}}> Implicit<br>
<br>
<br>
______________________________<u></u>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/cfe-commits</a><br>
</blockquote>
<br></div></div><span class="HOEnZb"><font color="#888888">
-- <br>
<a href="http://www.nuanti.com" target="_blank">http://www.nuanti.com</a><br>
the browser experts<br>
<br>
</font></span></blockquote></div><br></div></div>