<div style="font-family: arial, helvetica, sans-serif"><font size="2"><div class="gmail_quote">On Mon, Jun 25, 2012 at 5:01 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 3:48 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 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>
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></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> </div></div></div></font></div></blockquote><div><br></div></div></div><div>We actually had something similar. </div><div>But then, there are Linux, Mac and Android which we support now; more systems may be added in future. </div>
<div>Who knows what else is lurking in someone's stddef.h!?</div><div>E.g. is NULL defined there and shell we use it then (currently, we don't)? </div><div>Once a system header is included into every file in asan it will be very hard to get rid of it, so I prefer to not even try.</div>
</div></font></div></blockquote><div><br></div><div>While I'm generally sympathetic to this, I think we should distinguish between *compiler* provided headers, and *system* provided headers...</div><div><br></div><div>
I think the best way to achieve this distinction is to leverage the formal requirements from C for a freestanding implementation. This mode only provides a limited set af headers which often expose language / standard interfaces which are not achievable directly through the language. IE, they encapsulate compiler implementation details. These include 'stdarg.h' and 'stddef.h'; they are expected to be usable without linking *any* object code into the binary, so they shouldn't be problematic for a runtime library.</div>
<div><br></div><div>One way to ensure that you're abiding by these rules is to actually build the runtime using '-ffreestanding', which should constrain you to a much more limited set of headers and logic within those headers. For example, 'stdint.h' in C99 freestanding mode will *only* expose those types spelled out in the C standard. Without the '-ffreestanding', you get all the POSIX types, tons of other stuff you don't want.</div>
</div></font></div>