[llvm-commits] [compiler-rt] r159132 - /compiler-rt/trunk/lib/asan/asan_malloc_linux.cc

[Kostya Serebryany]

On Mon, Jun 25, 2012 at 2:41 PM, Chandler Carruth <chandlerc at google.com>wrote:

> On Mon, Jun 25, 2012 at 3:37 AM, Kostya Serebryany <kcc at google.com> wrote:
>
>>
>>
>> On Mon, Jun 25, 2012 at 2:28 PM, Chandler Carruth <chandlerc at google.com>wrote:
>>
>>> On Mon, Jun 25, 2012 at 3:18 AM, Kostya Serebryany <kcc at google.com>wrote:
>>>
>>>>
>>>>
>>>> On Mon, Jun 25, 2012 at 2:15 PM, Chandler Carruth <chandlerc at google.com
>>>> > wrote:
>>>>
>>>>> On Mon, Jun 25, 2012 at 2:58 AM, Kostya Serebryany <kcc at google.com>wrote:
>>>>>
>>>>>> Author: kcc
>>>>>> Date: Mon Jun 25 04:58:29 2012
>>>>>> New Revision: 159132
>>>>>>
>>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=159132&view=rev
>>>>>> Log:
>>>>>> [asan] get rid of '#include <malloc.h>' in the implementation of
>>>>>> malloc interceptors
>>>>>>
>>>>>> Modified:
>>>>>>    compiler-rt/trunk/lib/asan/asan_malloc_linux.cc
>>>>>>
>>>>>> Modified: compiler-rt/trunk/lib/asan/asan_malloc_linux.cc
>>>>>> URL:
>>>>>> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/asan/asan_malloc_linux.cc?rev=159132&r1=159131&r2=159132&view=diff
>>>>>>
>>>>>> ==============================================================================
>>>>>> --- compiler-rt/trunk/lib/asan/asan_malloc_linux.cc (original)
>>>>>> +++ compiler-rt/trunk/lib/asan/asan_malloc_linux.cc Mon Jun 25
>>>>>> 04:58:29 2012
>>>>>> @@ -20,15 +20,13 @@
>>>>>>  #include "asan_internal.h"
>>>>>>  #include "asan_stack.h"
>>>>>>
>>>>>> -#include <malloc.h>
>>>>>> -
>>>>>>  #ifdef ANDROID
>>>>>>  struct MallocDebug {
>>>>>> -  void* (*malloc)(size_t bytes);
>>>>>> +  void* (*malloc)(uptr bytes);
>>>>>>
>>>>>
>>>>> This seems really wrong to me... There are definitely platforms where
>>>>> sizeof(size_t) != sizeof(void*)...
>>>>>
>>>>
>>>> There are such platforms indeed, but asan will be broken on such
>>>> systems in all other possible ways, not just this.
>>>>
>>>
>>> Why not define these according to the standard?
>>>
>>> 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'.
>>>
>>> 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?
>>>
>>>
>>>> Otoh, this change protects us from problems like this one, where
>>>> malloc.h has some unexpected features.
>>>>
>>>
>>> My comment here isn't about including 'malloc.h', but about using 'uptr'
>>> instead of 'size_t'.
>>>
>>
>> It is indeed very pitty that we can not use the standard size_t. I'd love
>> to be able to.
>> This has something to do with Windows.
>> We can not build asan run-time with clang on Windows (at least yet), so
>> we have to build it with MSVC.
>> 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).
>>
>
> Wait, what?
>
> I can't really believe this:
>
> #include <stddef.h>
>

> size_t x = 42u;
>

It's actually the other way around: stddef.h on windows defines way too
much.
The pre-processed output for this test is 33K!
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.

We could define our own __sanitizer::size_t, use it for all interceptors
that have size_t in the interface,
and in a separate file (where we can include stddef.h) make sure that
sizeof(::size_t)==sizeof(__sanitizer::size_t).
WDYT?

This is worth doing only for the interceptors and not for the rest of the
code.

--kcc


>
> 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.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120625/f36e5d8c/attachment.html>