[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;
>
> 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.
>

Let me refresh my windows knowledge and get back to you.

--kcc



>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120625/edce5bc8/attachment.html>