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

[Kostya Serebryany]

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).

--kcc



>
>
>>
>> --kcc
>>
>>
>>>
>>>
>>>>   void  (*free)(void* mem);
>>>> -  void* (*calloc)(size_t n_elements, size_t elem_size);
>>>> -  void* (*realloc)(void* oldMem, size_t bytes);
>>>> -  void* (*memalign)(size_t alignment, size_t bytes);
>>>> +  void* (*calloc)(uptr n_elements, uptr elem_size);
>>>> +  void* (*realloc)(void* oldMem, uptr bytes);
>>>> +  void* (*memalign)(uptr alignment, uptr bytes);
>>>>  };
>>>>
>>>>  const MallocDebug asan_malloc_dispatch ALIGNED(32) = {
>>>> @@ -64,18 +62,18 @@
>>>>   asan_free(ptr, &stack);
>>>>  }
>>>>
>>>> -INTERCEPTOR(void*, malloc, size_t size) {
>>>> +INTERCEPTOR(void*, malloc, uptr size) {
>>>>   GET_STACK_TRACE_HERE_FOR_MALLOC;
>>>>   return asan_malloc(size, &stack);
>>>>  }
>>>>
>>>> -INTERCEPTOR(void*, calloc, size_t nmemb, size_t size) {
>>>> +INTERCEPTOR(void*, calloc, uptr nmemb, uptr size) {
>>>>   if (!asan_inited) {
>>>>     // Hack: dlsym calls calloc before REAL(calloc) is retrieved from
>>>> dlsym.
>>>> -    const size_t kCallocPoolSize = 1024;
>>>> +    const uptr kCallocPoolSize = 1024;
>>>>     static uptr calloc_memory_for_dlsym[kCallocPoolSize];
>>>> -    static size_t allocated;
>>>> -    size_t size_in_words = ((nmemb * size) + kWordSize - 1) /
>>>> kWordSize;
>>>> +    static uptr allocated;
>>>> +    uptr size_in_words = ((nmemb * size) + kWordSize - 1) / kWordSize;
>>>>     void *mem = (void*)&calloc_memory_for_dlsym[allocated];
>>>>     allocated += size_in_words;
>>>>     CHECK(allocated < kCallocPoolSize);
>>>> @@ -85,26 +83,34 @@
>>>>   return asan_calloc(nmemb, size, &stack);
>>>>  }
>>>>
>>>> -INTERCEPTOR(void*, realloc, void *ptr, size_t size) {
>>>> +INTERCEPTOR(void*, realloc, void *ptr, uptr size) {
>>>>   GET_STACK_TRACE_HERE_FOR_MALLOC;
>>>>   return asan_realloc(ptr, size, &stack);
>>>>  }
>>>>
>>>> -INTERCEPTOR(void*, memalign, size_t boundary, size_t size) {
>>>> +INTERCEPTOR(void*, memalign, uptr boundary, uptr size) {
>>>>   GET_STACK_TRACE_HERE_FOR_MALLOC;
>>>>   return asan_memalign(boundary, size, &stack);
>>>>  }
>>>>
>>>> -INTERCEPTOR(void*, __libc_memalign, size_t align, size_t s)
>>>> +INTERCEPTOR(void*, __libc_memalign, uptr align, uptr s)
>>>>   ALIAS("memalign");
>>>>
>>>> -INTERCEPTOR(size_t, malloc_usable_size, void *ptr) {
>>>> +INTERCEPTOR(uptr, malloc_usable_size, void *ptr) {
>>>>   GET_STACK_TRACE_HERE_FOR_MALLOC;
>>>>   return asan_malloc_usable_size(ptr, &stack);
>>>>  }
>>>>
>>>> -INTERCEPTOR(struct mallinfo, mallinfo, void) {
>>>> -  struct mallinfo res;
>>>> +// We avoid including malloc.h for portability reasons.
>>>> +// man mallinfo says the fields are "long", but the implementation
>>>> uses int.
>>>> +// It doesn't matter much -- we just need to make sure that the libc's
>>>> mallinfo
>>>> +// is not called.
>>>> +struct fake_mallinfo {
>>>> +  int x[10];
>>>> +};
>>>> +
>>>> +INTERCEPTOR(struct fake_mallinfo, mallinfo, void) {
>>>> +  struct fake_mallinfo res;
>>>>   REAL(memset)(&res, 0, sizeof(res));
>>>>   return res;
>>>>  }
>>>> @@ -113,18 +119,18 @@
>>>>   return -1;
>>>>  }
>>>>
>>>> -INTERCEPTOR(int, posix_memalign, void **memptr, size_t alignment,
>>>> size_t size) {
>>>> +INTERCEPTOR(int, posix_memalign, void **memptr, uptr alignment, uptr
>>>> size) {
>>>>   GET_STACK_TRACE_HERE_FOR_MALLOC;
>>>>   // Printf("posix_memalign: %zx %zu\n", alignment, size);
>>>>   return asan_posix_memalign(memptr, alignment, size, &stack);
>>>>  }
>>>>
>>>> -INTERCEPTOR(void*, valloc, size_t size) {
>>>> +INTERCEPTOR(void*, valloc, uptr size) {
>>>>   GET_STACK_TRACE_HERE_FOR_MALLOC;
>>>>   return asan_valloc(size, &stack);
>>>>  }
>>>>
>>>> -INTERCEPTOR(void*, pvalloc, size_t size) {
>>>> +INTERCEPTOR(void*, pvalloc, uptr size) {
>>>>   GET_STACK_TRACE_HERE_FOR_MALLOC;
>>>>   return asan_pvalloc(size, &stack);
>>>>  }
>>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>>
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120625/6eeb938b/attachment.html>