<div style="font-family: arial, helvetica, sans-serif"><font size="2"><div class="gmail_quote">On Mon, Jun 25, 2012 at 4:38 AM, Kostya Serebryany <span dir="ltr"><<a href="mailto:kcc@google.com" target="_blank">kcc@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="font-family:arial,helvetica,sans-serif"><font size="2"><br><br><div class="gmail_quote"><div><div class="h5">
On Mon, Jun 25, 2012 at 2:41 PM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@google.com" target="_blank">chandlerc@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="font-family:arial,helvetica,sans-serif"><font size="2"><div class="gmail_quote"><div><div>On Mon, Jun 25, 2012 at 3:37 AM, Kostya Serebryany <span dir="ltr"><<a href="mailto:kcc@google.com" target="_blank">kcc@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="font-family:arial,helvetica,sans-serif"><font size="2"><br><br><div class="gmail_quote"><div><div>
On Mon, Jun 25, 2012 at 2:28 PM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@google.com" target="_blank">chandlerc@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="font-family:arial,helvetica,sans-serif"><font size="2"><div class="gmail_quote"><div><div>On Mon, Jun 25, 2012 at 3:18 AM, Kostya Serebryany <span dir="ltr"><<a href="mailto:kcc@google.com" target="_blank">kcc@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="font-family:arial,helvetica,sans-serif"><font size="2"><br><br><div class="gmail_quote"><div><div>
On Mon, Jun 25, 2012 at 2:15 PM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@google.com" target="_blank">chandlerc@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="font-family:arial,helvetica,sans-serif"><font size="2"><div class="gmail_quote"><div>On Mon, Jun 25, 2012 at 2:58 AM, Kostya Serebryany <span dir="ltr"><<a href="mailto:kcc@google.com" target="_blank">kcc@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: kcc<br>
Date: Mon Jun 25 04:58:29 2012<br>
New Revision: 159132<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=159132&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=159132&view=rev</a><br>
Log:<br>
[asan] get rid of '#include <malloc.h>' in the implementation of malloc interceptors<br>
<br>
Modified:<br>
compiler-rt/trunk/lib/asan/asan_malloc_linux.cc<br>
<br>
Modified: compiler-rt/trunk/lib/asan/asan_malloc_linux.cc<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/asan/asan_malloc_linux.cc?rev=159132&r1=159131&r2=159132&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/asan/asan_malloc_linux.cc?rev=159132&r1=159131&r2=159132&view=diff</a><br>
==============================================================================<br>
--- compiler-rt/trunk/lib/asan/asan_malloc_linux.cc (original)<br>
+++ compiler-rt/trunk/lib/asan/asan_malloc_linux.cc Mon Jun 25 04:58:29 2012<br>
@@ -20,15 +20,13 @@<br>
#include "asan_internal.h"<br>
#include "asan_stack.h"<br>
<br>
-#include <malloc.h><br>
-<br>
#ifdef ANDROID<br>
struct MallocDebug {<br>
- void* (*malloc)(size_t bytes);<br>
+ void* (*malloc)(uptr bytes);<br></blockquote><div><br></div></div><div>This seems really wrong to me... There are definitely platforms where sizeof(size_t) != sizeof(void*)...</div></div></font></div></blockquote><div>
<br></div></div></div><div>There are such platforms indeed, but asan will be broken on such systems in all other possible ways, not just this. </div></div></font></div></blockquote><div><br></div></div></div><div>Why not define these according to the standard?</div>
<div><br></div><div>In particular, I don't understand an aversion to 'stddef.h' and 'size_t'. These don't come from the OS, they come from the standard and from the compiler. The 'stddef.h' that ships with Clang doesn't include any other headers. It cannot be simulated by your code because it leverages implementation-details to expose standard interfaces such as 'size_t'.</div>
<div><br></div><div>For example, 'stddef.h' can be included into code that is compiled with -ffreestanding, and thus without any system libraries or headers. It seems like that should be a sufficiently strict bar for portability for ASan?</div>
<div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="font-family:arial,helvetica,sans-serif"><font size="2"><div class="gmail_quote"><div>Otoh, this change protects us from problems like this one, where malloc.h has some unexpected features.</div>
</div></font></div></blockquote><div><br></div></div><div>My comment here isn't about including 'malloc.h', but about using 'uptr' instead of 'size_t'.</div></div></font></div></blockquote><div>
<br></div></div></div><div>It is indeed very pitty that we can not use the standard size_t. I'd love to be able to. </div><div>This has something to do with Windows. </div><div>We can not build asan run-time with clang on Windows (at least yet), so we have to build it with MSVC. </div>
<div>There, stddef.h is not nice at all. (Frankly, I don't remember exactly, but I think it just doesn't define size_t at all).</div></div></font></div></blockquote><div><br></div></div></div><div>Wait, what?</div>
<div><br></div>
<div>I can't really believe this:</div><div><br></div><div>#include <stddef.h></div></div></font></div></blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div style="font-family:arial,helvetica,sans-serif"><font size="2"><div class="gmail_quote"><div><br></div><div>size_t x = 42u;</div></div></font></div></blockquote><div><br></div></div></div><div>It's actually the other way around: stddef.h on windows defines way too much. </div>
<div>The pre-processed output for this test is 33K! </div><div>So, if we include stddef.h on windows, we end up with tons of stuff that conflicts more or less with everything we do in asan. </div><div><br></div><div>We could define our own __sanitizer::size_t, use it for all interceptors that have size_t in the interface,</div>
<div>and in a separate file (where we can include stddef.h) make sure that sizeof(::size_t)==sizeof(__sanitizer::size_t). </div><div>WDYT?</div></div></font></div></blockquote><div><br></div><div>Yuck. ;]</div><div><br></div>
<div>How about a slightly different approach, in sanitizer_stddef.h or some other common header:</div><div><br></div><div>#ifndef _WIN32</div><div># include <stddef.h></div><div>#else</div><div><br></div><div>// Provide custom typedefs emulating the minimal interface that</div>
<div>// should be exported by stddef.h -- The header on</div><div>// Windows is too bloated to use.</div><div>typedef uptr size_t;</div><div>typedef sptr ptrdiff_t;</div><div>...</div><div><br></div><div>#endif // _WIN32</div>
<div><br></div><div><br></div><div>My thought is that you really only need a few things from stddef, and you can get them directly when reasonable, and provide them yourself when the MSVC header gets in the way...</div><div>
</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="font-family:arial,helvetica,sans-serif"><font size="2"><div class="gmail_quote"><div>This is worth doing only for the interceptors and not for the rest of the code. </div>
</div></font></div></blockquote><div><br></div><div>Clearly. It's about matching interfaces not about these types being superior in some way. =]</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div style="font-family:arial,helvetica,sans-serif"><font size="2"><div class="gmail_quote">
<div><br></div><div>--kcc </div><div class="im"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="font-family:arial,helvetica,sans-serif"><font size="2"><div class="gmail_quote">
<div><br></div><div>Fails to compile on windows. If it does, I would suggest a more targeted fix rather than avoiding stddef.h / size_t... Providing a tiny stub of code to include stddef.h, and forcibly typedef size_t to something reasonable if MSVC has left us hanging seems much better than avoiding size_t in standard spec'ed interfaces. </div>
</div></font></div>
</blockquote></div></div><br></font></div>
</blockquote></div><br></font></div>